From 21a852504cd4a70c6362a4a9e4dc3e777d6437a3 Mon Sep 17 00:00:00 2001 From: Robin Alazard Date: Fri, 4 Jul 2025 07:34:39 -0700 Subject: [PATCH] Make sure we never apply the ArticulationRoot API more than once. PiperOrigin-RevId: 779185190 Change-Id: If441d0e2ba24191883839977b46864db2b4b8204 --- .../usd/plugins/mjcf/mujoco_to_usd.cc | 9 +++- .../usd/plugins/mjcf/mjcf_file_format_test.cc | 42 +++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc b/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc index 44ee6ac0..8a8a0467 100644 --- a/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc +++ b/src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include @@ -231,6 +232,8 @@ class ModelWriter { std::vector joint_paths_; // Mapping from mesh names to Mesh prim path. std::unordered_map mesh_paths_; + // Set of body ids that have had the articulation root API applied. + std::unordered_set articulation_roots_; // Whether to write physics data. bool write_physics_ = false; @@ -1998,9 +2001,13 @@ class ModelWriter { // then we need to apply the articulation root API. if (parent_id != kWorldIndex) { int parent_parent_id = mjs_getId(mjs_getParent(parent->element)->element); - if (parent_parent_id == kWorldIndex) { + // We guard against applying the API more than once, which can happen when + // there are multiple children. + if (parent_parent_id == kWorldIndex && + articulation_roots_.find(parent_id) == articulation_roots_.end()) { ApplyApiSchema(data_, parent_path, pxr::UsdPhysicsTokens->PhysicsArticulationRootAPI); + articulation_roots_.insert(parent_id); } } diff --git a/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc b/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc index 2fc83460..e8e97590 100644 --- a/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc +++ b/test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include @@ -41,6 +42,7 @@ #include #include #include +#include #include #include #include @@ -49,6 +51,7 @@ #include // IWYU pragma: keep, used for TraverseAll #include #include +#include #include #include #include @@ -1131,6 +1134,45 @@ TEST_F(MjcfSdfFileFormatPluginTest, TestPhysicsToggleSdfFormatArg) { pxr::UsdPhysicsRigidBodyAPI); } +TEST_F(MjcfSdfFileFormatPluginTest, TestArticulationRootAppliedOnce) { + static constexpr char kXml[] = R"( + + + + + + + + + + + + + + )"; + + pxr::SdfFileFormat::FileFormatArguments args; + args["usdMjcfToggleUsdPhysics"] = "true"; + pxr::SdfLayerRefPtr layer = LoadLayer(kXml, args); + + // This test is particular in the sense that the authoring mistake, which is + // made on the SdfLayer level, would disappear when we access the COMPOSED + // stage because duplicates are removed. So we need to check the SdfLayer + // directly to see the problem. + auto primSpec = layer->GetPrimAtPath(pxr::SdfPath("/physics_test/parent")); + EXPECT_TRUE(primSpec); + + pxr::VtValue apiSchemasValue = primSpec->GetInfo(pxr::UsdTokens->apiSchemas); + const pxr::SdfTokenListOp& listOp = + apiSchemasValue.UncheckedGet(); + const pxr::SdfTokenListOp::ItemVector& prependedItems = + listOp.GetPrependedItems(); + + int count = std::count(prependedItems.begin(), prependedItems.end(), + pxr::UsdPhysicsTokens->PhysicsArticulationRootAPI); + EXPECT_EQ(count, 1); +} + TEST_F(MjcfSdfFileFormatPluginTest, TestPhysicsRigidBody) { static constexpr char kXml[] = R"(