Add nested gprim check to validation framework and additional test cases. This addresses the issue where usdchecker does not emit errors on nested gprims.

pixar-oss · pixar-oss · commit efe40e2c2aa4 · 2025-10-07T10:34:54.000-07:00
Also enabled badGprims usdchecker test now that the validation framework supports this check and also updated baselines. Fixes #3835 (Internal change: 2381216)
diff --git a/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/arkitRules_validationFramework.txt b/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/arkitRules_validationFramework.txt
@@ -3,6 +3,11 @@
 	Keywords: RmanUsdValidators
 	SchemaTypes: UsdRiPxrRenderTerminalsAPI
 	isSuite: False
+[usdGeomValidators:EncapsulationChecker]:
+	Doc: Validates there are no nested Gprims.
+	Keywords: UsdGeomValidators
+	SchemaTypes: UsdGeomBoundable
+	isSuite: False
 [usdGeomValidators:StageMetadataChecker]:
 	Doc: All stages must declare their 'upAxis' and 'metersPerUnit'.
 	Keywords: UsdGeomValidators
diff --git a/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/arkitRules_validationFramework_withRootPackageValidator.txt b/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/arkitRules_validationFramework_withRootPackageValidator.txt
@@ -3,6 +3,11 @@
 	Keywords: RmanUsdValidators
 	SchemaTypes: UsdRiPxrRenderTerminalsAPI
 	isSuite: False
+[usdGeomValidators:EncapsulationChecker]:
+	Doc: Validates there are no nested Gprims.
+	Keywords: UsdGeomValidators
+	SchemaTypes: UsdGeomBoundable
+	isSuite: False
 [usdGeomValidators:StageMetadataChecker]:
 	Doc: All stages must declare their 'upAxis' and 'metersPerUnit'.
 	Keywords: UsdGeomValidators
diff --git a/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/baseRules_validationFramework.txt b/pxr/usdValidation/bin/usdchecker/testenv/testUsdChecker/baseline/baseRules_validationFramework.txt
@@ -3,6 +3,11 @@
 	Keywords: RmanUsdValidators
 	SchemaTypes: UsdRiPxrRenderTerminalsAPI
 	isSuite: False
+[usdGeomValidators:EncapsulationChecker]:
+	Doc: Validates there are no nested Gprims.
+	Keywords: UsdGeomValidators
+	SchemaTypes: UsdGeomBoundable
+	isSuite: False
 [usdGeomValidators:StageMetadataChecker]:
 	Doc: All stages must declare their 'upAxis' and 'metersPerUnit'.
 	Keywords: UsdGeomValidators
diff --git a/pxr/usdValidation/usdGeomValidators/plugInfo.json b/pxr/usdValidation/usdGeomValidators/plugInfo.json
@@ -3,6 +3,12 @@
         {
             "Info": {
                 "Validators": {
+                    "EncapsulationChecker": {
+                        "doc": "Validates there are no nested Gprims.", 
+                        "schemaTypes": [
+                            "UsdGeomBoundable"
+                        ]
+                    }, 
                     "StageMetadataChecker": {
                         "doc": "All stages must declare their 'upAxis' and 'metersPerUnit'."
                     }, 
diff --git a/pxr/usdValidation/usdGeomValidators/testenv/testUsdGeomValidators.cpp b/pxr/usdValidation/usdGeomValidators/testenv/testUsdGeomValidators.cpp
@@ -29,6 +29,7 @@ TestUsdGeomValidators()
         UsdGeomValidatorNameTokens->subsetFamilies,
         UsdGeomValidatorNameTokens->subsetParentIsImageable,
         UsdGeomValidatorNameTokens->stageMetadataChecker,
+        UsdGeomValidatorNameTokens->encapsulationChecker,
     };
 
     const UsdValidationRegistry &registry
@@ -42,7 +43,7 @@ TestUsdGeomValidators()
     UsdValidationValidatorMetadataVector metadata
         = registry.GetValidatorMetadataForPlugin(
             _tokens->usdGeomValidatorsPlugin);
-    TF_AXIOM(metadata.size() == 3);
+    TF_AXIOM(metadata.size() == 4);
     for (const UsdValidationValidatorMetadata &m: metadata) {
         validatorMetadataNameSet.insert(m.name);
     }
@@ -166,6 +167,21 @@ def Xform "SubsetsTest" (
 }
 )usda";
 
+static const std::string nestedLayerContents =
+    R"usda(#usda 1.0
+(
+    defaultPrim = "NestedGprimTest"
+    upAxis = "Z"
+)
+def Mesh "NestedGprimTest"
+{
+    def Sphere "NestedPrim"
+    {
+    
+    }
+}
+)usda";
+
 void
 TestUsdGeomSubsetFamilies()
 {
@@ -364,10 +380,52 @@ TestUsdStageMetadata()
     TF_AXIOM(errors.empty());
 }
 
+void
+TestUsdGeomEncapsulationChecker()
+{
+    UsdValidationRegistry &registry = UsdValidationRegistry::GetInstance();
+    const UsdValidationValidator *validator = registry.GetOrLoadValidatorByName(
+        UsdGeomValidatorNameTokens->encapsulationChecker);
+    TF_AXIOM(validator);
+
+    SdfLayerRefPtr layer = SdfLayer::CreateAnonymous(".usda");
+    layer->ImportFromString(nestedLayerContents);
+    UsdStageRefPtr usdStage = UsdStage::Open(layer);
+    TF_AXIOM(usdStage);
+
+    const TfToken expectedErrorIdentifier = TfToken(
+        "usdGeomValidators:EncapsulationChecker.InvalidNestedGprims");
+
+    {
+        const UsdPrim usdPrim = usdStage->GetPrimAtPath(
+            SdfPath("/NestedGprimTest/NestedPrim"));
+
+        const UsdValidationErrorVector errors = validator->Validate(usdPrim);
+
+        TF_AXIOM(errors.size() == 1u);
+        const UsdValidationError &error = errors[0u];
+        TF_AXIOM(error.GetIdentifier() == expectedErrorIdentifier);
+        TF_AXIOM(error.GetType() == UsdValidationErrorType::Error);
+        TF_AXIOM(error.GetSites().size() == 1u);
+        const UsdValidationErrorSite &errorSite = error.GetSites()[0u];
+        TF_AXIOM(errorSite.IsValid());
+        TF_AXIOM(errorSite.IsPrim());
+        TF_AXIOM(errorSite.GetPrim().GetPath() == usdPrim.GetPath());
+        const std::string expectedErrorMsg
+            = "Gprim "
+              "</NestedGprimTest/NestedPrim> has an ancestor prim that is also "
+              "a Gprim, which is not allowed.";
+ 
+        TF_AXIOM(error.GetMessage() == expectedErrorMsg);
+    }
+}
+
+
 int
 main()
 {
     TestUsdGeomValidators();
+    TestUsdGeomEncapsulationChecker();
     TestUsdGeomSubsetFamilies();
     TestUsdGeomSubsetParentIsImageable();
     TestUsdStageMetadata();
diff --git a/pxr/usdValidation/usdGeomValidators/validatorTokens.h b/pxr/usdValidation/usdGeomValidators/validatorTokens.h
@@ -19,7 +19,8 @@ PXR_NAMESPACE_OPEN_SCOPE
 #define USD_GEOM_VALIDATOR_NAME_TOKENS                                         \
     ((stageMetadataChecker, "usdGeomValidators:StageMetadataChecker"))         \
     ((subsetFamilies, "usdGeomValidators:SubsetFamilies"))                     \
-    ((subsetParentIsImageable, "usdGeomValidators:SubsetParentIsImageable"))
+    ((subsetParentIsImageable, "usdGeomValidators:SubsetParentIsImageable"))   \
+    ((encapsulationChecker, "usdGeomValidators:EncapsulationChecker"))         
 
 #define USD_GEOM_VALIDATOR_KEYWORD_TOKENS                                      \
     (UsdGeomSubset)                                                            \
@@ -29,7 +30,8 @@ PXR_NAMESPACE_OPEN_SCOPE
     ((missingMetersPerUnitMetadata, "MissingMetersPerUnitMetadata"))           \
     ((missingUpAxisMetadata, "MissingUpAxisMetadata"))                         \
     ((invalidSubsetFamily, "InvalidSubsetFamily"))                             \
-    ((notImageableSubsetParent, "NotImageableSubsetParent"))
+    ((notImageableSubsetParent, "NotImageableSubsetParent"))                   \
+    ((invalidNestedGprims, "InvalidNestedGprims"))                              
 
 /// \def USD_GEOM_VALIDATOR_NAME_TOKENS
 /// Tokens representing validator names. Note that for plugin provided
diff --git a/pxr/usdValidation/usdGeomValidators/validators.cpp b/pxr/usdValidation/usdGeomValidators/validators.cpp
@@ -146,6 +146,50 @@ _SubsetParentIsImageable(const UsdPrim &usdPrim,
             usdPrim.GetPath().GetText(), parentPrim.GetPath().GetText())) };
 }
 
+static UsdValidationErrorVector
+_HasValidEncapsulation(const UsdPrim &usdPrim, 
+                       const UsdValidationTimeRange &/*timeRange*/)
+{
+    UsdValidationErrorVector errors;
+    const UsdValidationErrorSites primErrorSites
+        = { UsdValidationErrorSite(usdPrim.GetStage(), usdPrim.GetPath()) };
+    
+    const UsdPrim parentPrim = usdPrim.GetParent();
+
+    if (usdPrim.IsA<UsdGeomBoundable>()){
+        if (parentPrim) {
+            // Of course we must allow Boundables under other Boundables, so that
+            // schemas like UsdGeom.Pointinstancer can nest their prototypes.  
+            // But we disallow a PointInstancer under a Mesh just as we disallow 
+            // a Mesh under a Mesh, for the same reason: we cannot then 
+            // independently adjust visibility for the two objects, nor can we 
+            // reasonably compute the parent Mesh's extent.
+            std::function<void(const UsdPrim &)> _VerifyGprimAncestor = 
+                [&](const UsdPrim &currentAncestor) {
+                if (!currentAncestor || currentAncestor.IsPseudoRoot()) {
+                    return;
+                }
+
+                if (currentAncestor.IsA<UsdGeomGprim>()) {
+                    errors.emplace_back(
+                        UsdGeomValidationErrorNameTokens
+                            ->invalidNestedGprims,
+                        UsdValidationErrorType::Error,
+                        primErrorSites,
+                        TfStringPrintf(
+                            "Gprim <%s> has an ancestor prim that is also a "
+                            "Gprim, which is not allowed.",
+                            usdPrim.GetPath().GetText()));
+                    return;
+                }
+                _VerifyGprimAncestor(currentAncestor.GetParent());
+            };
+            _VerifyGprimAncestor(parentPrim);
+        }
+    }
+    return errors;
+}
+
 TF_REGISTRY_FUNCTION(UsdValidationRegistry)
 {
     UsdValidationRegistry &registry = UsdValidationRegistry::GetInstance();
@@ -160,6 +204,10 @@ TF_REGISTRY_FUNCTION(UsdValidationRegistry)
     registry.RegisterPluginValidator(
         UsdGeomValidatorNameTokens->subsetParentIsImageable,
         _SubsetParentIsImageable);
+
+    registry.RegisterPluginValidator(
+        UsdGeomValidatorNameTokens->encapsulationChecker,
+        _HasValidEncapsulation);
 }
 
 PXR_NAMESPACE_CLOSE_SCOPE
