From 53130fcbc94c0e56b49d4400ec08ea381cc68d07 Mon Sep 17 00:00:00 2001 From: Alan Blevins Date: Tue, 21 Sep 2021 12:41:50 -0700 Subject: [PATCH] [usdGeom] avoid crash from UsdGeomCamera::SetFromCamera with EditTarget specifying a weaker layer and a stronger layer defining xformOpList --- pxr/usd/usdGeom/CMakeLists.txt | 5 ++++ pxr/usd/usdGeom/camera.cpp | 10 ++++++- pxr/usd/usdGeom/camera.h | 18 +++++++++++ pxr/usd/usdGeom/testenv/testUsdGeomCamera.py | 30 ++++++++++++++++++- .../usdGeom/testenv/testUsdGeomCamera/a.usda | 13 ++++++++ .../usdGeom/testenv/testUsdGeomCamera/b.usda | 9 ++++++ .../testenv/testUsdGeomCamera/layers_a_b.usda | 9 ++++++ pxr/usd/usdGeom/xformable.cpp | 6 ++++ 8 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 pxr/usd/usdGeom/testenv/testUsdGeomCamera/a.usda create mode 100644 pxr/usd/usdGeom/testenv/testUsdGeomCamera/b.usda create mode 100644 pxr/usd/usdGeom/testenv/testUsdGeomCamera/layers_a_b.usda diff --git a/pxr/usd/usdGeom/CMakeLists.txt b/pxr/usd/usdGeom/CMakeLists.txt index 82f8a4cb1b..49e4932347 100644 --- a/pxr/usd/usdGeom/CMakeLists.txt +++ b/pxr/usd/usdGeom/CMakeLists.txt @@ -186,6 +186,11 @@ pxr_install_test_dir( DEST testUsdGeomBBoxCache ) +pxr_install_test_dir( + SRC testenv/testUsdGeomCamera + DEST testUsdGeomCamera +) + pxr_install_test_dir( SRC testenv/testUsdGeomConsts DEST testUsdGeomConsts diff --git a/pxr/usd/usdGeom/camera.cpp b/pxr/usd/usdGeom/camera.cpp index e8aab2aa2f..13fe251f13 100644 --- a/pxr/usd/usdGeom/camera.cpp +++ b/pxr/usd/usdGeom/camera.cpp @@ -552,7 +552,15 @@ UsdGeomCamera::SetFromCamera(const GfCamera &camera, const UsdTimeCode &time) const GfMatrix4d camMatrix = camera.GetTransform() * parentToWorldInverse; - MakeMatrixXform().Set(camMatrix, time); + UsdGeomXformOp xformOp = MakeMatrixXform(); + if (!xformOp) { + // The returned xformOp may be invalid if there are xform op + // opinions in the composed layer stack stronger than that of + // the current edit target. + return; + } + xformOp.Set(camMatrix, time); + GetProjectionAttr().Set(_ProjectionToToken(camera.GetProjection()), time); GetHorizontalApertureAttr().Set(camera.GetHorizontalAperture(), time); GetVerticalApertureAttr().Set(camera.GetVerticalAperture(), time); diff --git a/pxr/usd/usdGeom/camera.h b/pxr/usd/usdGeom/camera.h index 99d4ea4ba9..fe868f0e07 100644 --- a/pxr/usd/usdGeom/camera.h +++ b/pxr/usd/usdGeom/camera.h @@ -552,6 +552,24 @@ class UsdGeomCamera : public UsdGeomXformable GfCamera GetCamera(const UsdTimeCode &time) const; /// Write attribute values from \p camera for \p time. + /// These attributes will be updated: + /// - projection + /// - horizontalAperture + /// - horizontalApertureOffset + /// - verticalAperture + /// - verticalApertureOffset + /// - focalLength + /// - clippingRange + /// - clippingPlanes + /// - fStop + /// - focalDistance + /// - xformOpOrder and xformOp:transform ** + /// **NOTE: This will clear any existing xformOpOrder and replace + /// it with a single xformOp:transform entry. The xformOp:transform + /// property is created or updated here to match the transform + /// on \p camera . This operation will fail if there are stronger xform op + /// opinions in the composed layer stack that are stronger than that of + /// the current edit target. /// USDGEOM_API void SetFromCamera(const GfCamera &camera, const UsdTimeCode &time); diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomCamera.py b/pxr/usd/usdGeom/testenv/testUsdGeomCamera.py index c71b93bb8c..75a864ef26 100644 --- a/pxr/usd/usdGeom/testenv/testUsdGeomCamera.py +++ b/pxr/usd/usdGeom/testenv/testUsdGeomCamera.py @@ -22,7 +22,7 @@ # KIND, either express or implied. See the Apache License for the specific # language governing permissions and limitations under the Apache License. -from pxr import Gf, Usd, UsdGeom +from pxr import Gf, Usd, UsdGeom, Sdf, Tf import unittest, math class TestUsdGeomCamera(unittest.TestCase): @@ -128,5 +128,33 @@ def test_SetFromCamera(self): for a, e in zip(actual, expected): self.assertTrue(Gf.IsClose(a, e, 1e-2)) + def test_SetFromCameraWithComposition(self): + stage = Usd.Stage.Open("layers_a_b.usda") + layerA = Sdf.Layer.FindOrOpen("a.usda") + layerB = Sdf.Layer.FindOrOpen("b.usda") + stage.SetEditTarget(layerB) + + usdCamera = UsdGeom.Camera.Define(stage, '/camera') + + camera = Gf.Camera() + newXform = Gf.Matrix4d().SetTranslate(Gf.Vec3d(100, 200, 300)) + camera.transform = newXform + camera.horizontalAperture = 500.0 + + # Verify that trying to SetFromCamera from a weaker edit target does not crash, + # and does not modify any existing camera attributes. + usdCamera.SetFromCamera(camera, 1.0) + self.assertEqual(usdCamera.GetHorizontalApertureAttr().Get(1.0), 1.0) + + # Now use the stronger layer + stage.SetEditTarget(layerA) + + # This should succeed + usdCamera.SetFromCamera(camera, 1.0) + + self.assertEqual(usdCamera.GetHorizontalApertureAttr().Get(1.0), 500.0) + self.assertEqual(usdCamera.ComputeLocalToWorldTransform(1.0), newXform) + + if __name__ == '__main__': unittest.main() diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomCamera/a.usda b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/a.usda new file mode 100644 index 0000000000..0e2ab41ca9 --- /dev/null +++ b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/a.usda @@ -0,0 +1,13 @@ +#usda 1.0 +( + defaultPrim = "camera" +) + +over "camera" +{ + matrix4d xformOp:transform = ((1, 0, 0, 0), + (0, 1, 0, 0), + (0, 0, 1, 0), + (0, 0, 0, 1)) + uniform token[] xformOpOrder = ["xformOp:transform"] +} diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomCamera/b.usda b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/b.usda new file mode 100644 index 0000000000..500c4dc949 --- /dev/null +++ b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/b.usda @@ -0,0 +1,9 @@ +#usda 1.0 +( + defaultPrim = "camera" +) + +def Camera "camera" +{ + float horizontalAperture = 1.0 +} diff --git a/pxr/usd/usdGeom/testenv/testUsdGeomCamera/layers_a_b.usda b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/layers_a_b.usda new file mode 100644 index 0000000000..b3cdf22fd6 --- /dev/null +++ b/pxr/usd/usdGeom/testenv/testUsdGeomCamera/layers_a_b.usda @@ -0,0 +1,9 @@ +#usda 1.0 +( + defaultPrim = "camera" + subLayers = [ + @./a.usda@, + @./b.usda@ + ] +) + diff --git a/pxr/usd/usdGeom/xformable.cpp b/pxr/usd/usdGeom/xformable.cpp index 2094a7caf1..d94b2e727c 100644 --- a/pxr/usd/usdGeom/xformable.cpp +++ b/pxr/usd/usdGeom/xformable.cpp @@ -428,6 +428,12 @@ UsdGeomXformOp UsdGeomXformable::MakeMatrixXform() const { ClearXformOpOrder(); + bool unused = false; + if (!GetOrderedXformOps(&unused).empty()) { + TF_WARN("Could not clear xformOps for <%s>", + GetPrim().GetPath().GetText()); + return UsdGeomXformOp(); + } return AddTransformOp(); }