Defensive check for UsdSkel_SkelAnimationQueryImp to avoid crash

Author: roggiezhang-nv

pxr/usd/usdSkel/animQueryImpl.cpp

@@ -73,6 +73,9 @@ class UsdSkel_SkelAnimationQueryImpl : public UsdSkel_AnimQueryImpl
     bool BlendShapeWeightsMightBeTimeVarying() const override;
 
 private:
+    bool _VerifyAnimation() const {
+        return TF_VERIFY(_anim, "PackedJointAnimation schema object is invalid.");
+    }
     template <typename Matrix4>
     bool _ComputeJointLocalTransforms(VtArray<Matrix4>* xforms,
                                       UsdTimeCode time) const;
@@ -170,6 +173,10 @@ UsdSkel_SkelAnimationQueryImpl::GetJointTransformTimeSamples(
     const GfInterval& interval,
     std::vector<double>* times) const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
+    
     return UsdAttribute::GetUnionedTimeSamplesInInterval(
         {_translations.GetAttribute(),
          _rotations.GetAttribute(),
@@ -180,6 +187,10 @@ bool
 UsdSkel_SkelAnimationQueryImpl::GetJointTransformAttributes(
     std::vector<UsdAttribute>* attrs) const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
+
     attrs->push_back(_translations.GetAttribute());
     attrs->push_back(_rotations.GetAttribute());
     attrs->push_back(_scales.GetAttribute());
@@ -190,6 +201,10 @@ UsdSkel_SkelAnimationQueryImpl::GetJointTransformAttributes(
 bool
 UsdSkel_SkelAnimationQueryImpl::JointTransformsMightBeTimeVarying() const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
+    
     return _translations.ValueMightBeTimeVarying() ||
            _rotations.ValueMightBeTimeVarying() ||
            _scales.ValueMightBeTimeVarying();
@@ -201,10 +216,10 @@ UsdSkel_SkelAnimationQueryImpl::ComputeBlendShapeWeights(
     VtFloatArray* weights,
     UsdTimeCode time) const
 {
-    if (TF_VERIFY(_anim, "PackedJointAnimation schema object is invalid.")) {
-        return _blendShapeWeights.Get(weights, time);
+    if (!_VerifyAnimation()) {
+        return false;
     }
-    return false;
+    return _blendShapeWeights.Get(weights, time);
 }
 
 
@@ -213,6 +228,9 @@ UsdSkel_SkelAnimationQueryImpl::GetBlendShapeWeightTimeSamples(
     const GfInterval& interval,
     std::vector<double>* times) const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
     return _blendShapeWeights.GetTimeSamplesInInterval(interval, times);
 }
 
@@ -221,13 +239,19 @@ bool
 UsdSkel_SkelAnimationQueryImpl::GetBlendShapeWeightAttributes(
     std::vector<UsdAttribute>* attrs) const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
     attrs->push_back(_blendShapeWeights.GetAttribute());
     return true;
 }
 
 bool
 UsdSkel_SkelAnimationQueryImpl::BlendShapeWeightsMightBeTimeVarying() const
 {
+    if (!_VerifyAnimation()) {
+        return false;
+    }
     return _blendShapeWeights.ValueMightBeTimeVarying();
 }
 

pxr/usdImaging/usdSkelImaging/CMakeLists.txt

@@ -35,3 +35,32 @@ pxr_library(usdSkelImaging
         plugInfo.json
         shaders/skinning.glslfx
 )
+
+# Build tests
+pxr_build_test(testUsdSkelImagingSkeletonAdapter
+    LIBRARIES
+        usdImaging
+        usdShade
+        usdGeom
+        usdSkel
+        usd
+        sdf
+        hd
+        tf
+        gf
+        arch
+    CPPFILES
+        testenv/testUsdSkelImagingSkeletonAdapter.cpp
+)
+
+# Install (copy test assets and baselines)
+pxr_install_test_dir(
+    SRC testenv/testUsdSkelImagingSkeletonAdapter
+    DEST testUsdSkelImagingSkeletonAdapter
+)
+
+# Register tests
+pxr_register_test(testUsdSkelImagingSkeletonAdapter
+    COMMAND "${CMAKE_INSTALL_PREFIX}/tests/testUsdSkelImagingSkeletonAdapter"
+    EXPECTED_RETURN_CODE 0
+)

pxr/usdImaging/usdSkelImaging/testEnv/testUsdSkelImagingSkeletonAdapter.cpp

@@ -0,0 +1,81 @@
+//
+// Copyright 2025 Pixar
+//
+// Licensed under the terms set forth in the LICENSE.txt file available at
+// https://openusd.org/license.
+//
+
+#include "pxr/usdImaging/usdImaging/unitTestHelper.h"
+
+#include "pxr/imaging/hd/material.h"
+#include "pxr/imaging/hd/renderIndex.h"
+#include "pxr/imaging/hd/unitTestNullRenderDelegate.h"
+
+#include "pxr/usd/usd/stage.h"
+#include "pxr/usd/usd/editContext.h"
+#include "pxr/usd/usdSkel/animation.h"
+#include "pxr/usd/usdSkel/bindingAPI.h"
+#include "pxr/usd/usdSkel/root.h"
+
+#include <iostream>
+
+PXR_NAMESPACE_USING_DIRECTIVE
+
+static void
+TestSkelAnimUpdateCrash()
+{
+    std::cout << "-------------------------------------------------------\n";
+    std::cout << "TestSkelAnimUpdate\n";
+    std::cout << "-------------------------------------------------------\n";
+
+    const std::string usdPath = "model.usda";
+    UsdStageRefPtr stage = UsdStage::Open(usdPath);
+    TF_AXIOM(stage);
+    
+    // Bring up Hydra
+    Hd_UnitTestNullRenderDelegate renderDelegate;
+    std::unique_ptr<HdRenderIndex>
+        renderIndex(HdRenderIndex::New(&renderDelegate, HdDriverVector()));
+    auto delegate = std::make_unique<UsdImagingDelegate>(renderIndex.get(),
+                               SdfPath::AbsoluteRootPath());
+    delegate->Populate(stage->GetPseudoRoot());
+    delegate->SetTime(0);
+    delegate->SyncAll(true);
+    
+    {
+        UsdEditContext editContext(stage, stage->GetSessionLayer());
+        // Create new animation
+        SdfPath animation_new_path("/Animation");
+        UsdSkelAnimation animation_new = UsdSkelAnimation::Define(stage, animation_new_path);
+        UsdPrim animation_new_prim = animation_new.GetPrim();
+        TF_AXIOM(animation_new_prim);
+
+        // Update skeleton binding
+        UsdPrim skeleton_prim = stage->GetPrimAtPath(SdfPath("/Root/Skeleton"));
+        UsdSkelBindingAPI skeleton_bindingAPI = UsdSkelBindingAPI(skeleton_prim);
+        skeleton_bindingAPI.GetAnimationSourceRel().SetTargets({animation_new_path});
+        delegate->SetTime(0);
+        delegate->SyncAll(true);
+
+        // Remove animation and update skelroot's visibility
+        {
+            SdfChangeBlock changeBlock;
+            stage->RemovePrim(animation_new_path);
+            skeleton_bindingAPI.GetAnimationSourceRel().ClearTargets(false); 
+            UsdPrim skel_root_prim = stage->GetPrimAtPath(SdfPath("/Root"));
+            UsdSkelRoot skel_root = UsdSkelRoot(skel_root_prim);
+            skel_root.GetVisibilityAttr().Set(UsdGeomTokens->inherited);
+        }
+
+        // Test crash due to resync by updating visibility in skelroot.
+        // NOTE: This is a test for a crash that happened in the
+        //       UsdSkelImagingSkeletonAdapter::_IsAffectedByTimeVaryingSkelAnim() method.
+        delegate->SetTime(0);
+        delegate->SyncAll(true);
+    }
+}
+
+int main()
+{
+    TestSkelAnimUpdateCrash();
+}

pxr/usdImaging/usdSkelImaging/testEnv/testUsdSkelImagingSkeletonAdapter/model.usda

@@ -0,0 +1,46 @@
+#usda 1.0
+(
+    defaultPrim = "Root"
+    endTimeCode = 120
+    metersPerUnit = 0.01
+    startTimeCode = 0
+    timeCodesPerSecond = 24
+    upAxis = "Z"
+)
+
+def SkelRoot "Root" (
+    prepend apiSchemas = ["SkelBindingAPI"]
+)
+{
+    matrix4d xformOp:transform = ( (1, 0, 0, 0), (0, 1, 0, 0), (0, 0, 1, 0), (0, 0, 0, 1) )
+    uniform token[] xformOpOrder = ["xformOp:transform"]
+
+    def Mesh "box" (
+        prepend apiSchemas = ["SkelBindingAPI"]
+    )
+    {
+        int[] faceVertexCounts = [4, 4, 4, 4, 4, 4]
+        int[] faceVertexIndices = [0, 1, 3, 2, 4, 5, 6, 7, 1, 2, 5, 6, 0, 4, 7, 3, 0, 1, 5, 4, 3, 7, 6, 2]
+        token orientation = "rightHanded"
+        point3f[] points = [(-0.6, -0.6, 0), (-0.6, 0.6, 0), (0.6, 0.6, 0), (0.6, -0.6, 0), (-0.6, -0.6, 0.14), (-0.6, 0.6, 0.14), (0.6, 0.6, 0.14), (0.6, -0.6, 0.14)]
+        
+        int[] primvars:skel:jointIndices = [0, 0, 0, 0] (interpolation = "vertex")
+        float[] primvars:skel:jointWeights = [1., 1., 1., 1.] (interpolation = "vertex")
+        prepend rel skel:skeleton = </Root/Skeleton>
+        bool doubleSided = 1
+    }
+
+    def Skeleton "Skeleton" (
+        prepend apiSchemas = ["SkelBindingAPI"]
+    )
+    {
+        uniform token[] joints = ["Root"]
+        uniform matrix4d[] bindTransforms = [
+            ((1., 0., 0., 0.), (0., 1., 0., 0.), (0., 0., 1., 0.), (0.0, 0.0, 0.0, 1.)),    # Root
+        ]
+        uniform matrix4d[] restTransforms = [
+            ((1., 0., 0., 0.), (0., 1., 0., 0.), (0., 0., 1., 0.), (0., 0., 0., 1.)),    # Root
+        ]
+    }
+}
+