chenlin/Mujoco_WASM
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix authoring type mismatch for geoms sizes attributes.

Browse Source 
Fixes wrong sizes being displayed in usdview.
Also introduce a new test to ensure we catch as many of those kinds of errors as possible in the future.

PiperOrigin-RevId: 764388588
Change-Id: Id3c878edd054fe5272c42c9f960c65c850655559
This commit is contained in:
Robin Alazard
2025-05-28 13:19:06 -07:00
committed by Copybara-Service
parent 287f46b220
commit fc69ef1084
4 changed files with 91 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/experimental/usd/plugins/mjcf/mujoco_to_usd.cc
+13 -13
View File
@@ -820,7 +820,7 @@ class ModelWriter {
// MuJoCo uses half sizes.
pxr::SdfPath size_attr_path =
CreateAttributeSpec(data_, box_path, pxr::UsdGeomTokens->size,
pxr::SdfValueTypeNames->Float);
pxr::SdfValueTypeNames->Double);
pxr::GfVec3f scale(static_cast<float>(size[0]), static_cast<float>(size[1]),
static_cast<float>(size[2]));
SetAttributeDefault(data_, size_attr_path, 2.0);
@@ -857,14 +857,14 @@ class ModelWriter {
pxr::SdfPath radius_attr_path =
CreateAttributeSpec(data_, capsule_path, pxr::UsdGeomTokens->radius,
pxr::SdfValueTypeNames->Float);
SetAttributeDefault(data_, radius_attr_path, size[0]);
pxr::SdfValueTypeNames->Double);
SetAttributeDefault(data_, radius_attr_path, (double)size[0]);
pxr::SdfPath height_attr_path =
CreateAttributeSpec(data_, capsule_path, pxr::UsdGeomTokens->height,
pxr::SdfValueTypeNames->Float);
pxr::SdfValueTypeNames->Double);
// MuJoCo uses half sizes.
SetAttributeDefault(data_, height_attr_path, size[1] * 2);
SetAttributeDefault(data_, height_attr_path, (double)(size[1] * 2));
return capsule_path;
}
@@ -885,14 +885,14 @@ class ModelWriter {
pxr::SdfPath radius_attr_path =
CreateAttributeSpec(data_, cylinder_path, pxr::UsdGeomTokens->radius,
pxr::SdfValueTypeNames->Float);
SetAttributeDefault(data_, radius_attr_path, size[0]);
pxr::SdfValueTypeNames->Double);
SetAttributeDefault(data_, radius_attr_path, (double)size[0]);
pxr::SdfPath height_attr_path =
CreateAttributeSpec(data_, cylinder_path, pxr::UsdGeomTokens->height,
pxr::SdfValueTypeNames->Float);
pxr::SdfValueTypeNames->Double);
// MuJoCo uses half sizes.
SetAttributeDefault(data_, height_attr_path, size[1] * 2);
SetAttributeDefault(data_, height_attr_path, (double)(size[1] * 2));
return cylinder_path;
}
@@ -917,8 +917,8 @@ class ModelWriter {
pxr::SdfPath radius_attr_path =
CreateAttributeSpec(data_, ellipsoid_path, pxr::UsdGeomTokens->radius,
pxr::SdfValueTypeNames->Float);
SetAttributeDefault(data_, radius_attr_path, 1.0f);
pxr::SdfValueTypeNames->Double);
SetAttributeDefault(data_, radius_attr_path, 1.0);
WriteScaleXformOp(ellipsoid_path, scale);
WriteXformOpOrder(ellipsoid_path,
@@ -943,8 +943,8 @@ class ModelWriter {
pxr::SdfPath radius_attr_path =
CreateAttributeSpec(data_, sphere_path, pxr::UsdGeomTokens->radius,
pxr::SdfValueTypeNames->Float);
SetAttributeDefault(data_, radius_attr_path, size[0]);
pxr::SdfValueTypeNames->Double);
SetAttributeDefault(data_, radius_attr_path, (double)size[0]);
return sphere_path;
}
test/experimental/usd/plugins/mjcf/mjcf_file_format_test.cc
+26
View File
@@ -37,9 +37,11 @@
#include <pxr/usd/sdf/declareHandles.h>
#include <pxr/usd/sdf/fileFormat.h>
#include <pxr/usd/sdf/path.h>
#include <pxr/usd/sdf/schema.h>
#include <pxr/usd/usd/common.h>
#include <pxr/usd/usd/modelAPI.h>
#include <pxr/usd/usd/prim.h>
#include <pxr/usd/usd/primRange.h> // IWYU pragma: keep, used for TraverseAll
#include <pxr/usd/usd/stage.h>
#include <pxr/usd/usdGeom/capsule.h>
#include <pxr/usd/usdGeom/cube.h>
@@ -450,6 +452,30 @@ TEST_F(MjcfSdfFileFormatPluginTest, TestKindAuthoring) {
pxr::KindTokens->subcomponent);
}
TEST_F(MjcfSdfFileFormatPluginTest, TestAttributesMatchSchemaTypes) {
// TODO(robinalazard): Make the scene much more comprehensive. We ideally want
// to test all the prims that the plugin can generate.
static constexpr char kXml[] = R"(
<mujoco model="test">
<worldbody>
<geom type="plane" name="plane_geom" size="10 20 0.1"/>
<geom type="box" name="box_geom" size="10 20 30"/>
<geom type="sphere" name="sphere_geom" size="10 20 30"/>
<geom type="capsule" name="capsule_geom" size="10 20 30"/>
<geom type="cylinder" name="cylinder_geom" size="10 20 30"/>
<geom type="ellipsoid" name="ellipsoid_geom" size="10 20 30"/>
</worldbody>
</mujoco>
)";
pxr::SdfLayerRefPtr layer = LoadLayer(kXml);
auto stage = pxr::UsdStage::Open(layer);
for (const auto& prim : stage->TraverseAll()) {
ExpectAllAuthoredAttributesMatchSchemaTypes(prim);
}
}
TEST_F(MjcfSdfFileFormatPluginTest, TestGeomsPrims) {
static constexpr char kXml[] = R"(
<mujoco model="test">
test/experimental/usd/test_utils.cc
+48
View File
@@ -18,10 +18,13 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>
#include <pxr/base/tf/token.h>
#include <pxr/usd/sdf/assetPath.h>
#include <pxr/usd/sdf/childrenPolicies.h>
#include <pxr/usd/sdf/declareHandles.h>
#include <pxr/usd/sdf/fileFormat.h>
#include <pxr/usd/sdf/path.h>
#include <pxr/usd/sdf/schema.h>
#include <pxr/usd/usd/common.h>
#include <pxr/usd/usd/modelAPI.h>
#include <pxr/usd/usd/stage.h>
@@ -71,5 +74,50 @@ void ExpectAttributeHasConnection(pxr::UsdStageRefPtr stage, const char* path,
EXPECT_EQ(sources.size(), 1);
EXPECT_EQ(sources[0], SdfPath(connection_path));
}
void ExpectAllAuthoredAttributesMatchSchemaTypes(const pxr::UsdPrim& prim) {
// Get all properties on the prim that have authored opinions.
for (const pxr::UsdProperty& prop : prim.GetAuthoredProperties()) {
// We only care about attributes, as they are the ones with a typeName.
if (pxr::UsdAttribute attr = prop.As<pxr::UsdAttribute>()) {
// 1. Get the official, composed schema type name for the attribute.
const pxr::TfToken schemaTypeName = attr.GetTypeName().GetAsToken();
// An empty schema type name means the attribute is not defined by
// a schema, or is of a dynamically-determined type. We can't
// check for a mismatch in this case.
if (schemaTypeName.IsEmpty()) {
continue;
}
// 2. Get the property stack to check for authored opinions.
// The stack is ordered from strongest to weakest.
const pxr::SdfPropertySpecHandleVector propStack =
attr.GetPropertyStack();
for (const pxr::SdfPropertySpecHandle& spec : propStack) {
// We only care about attribute specs.
if (auto attrSpec = TfDynamic_cast<pxr::SdfAttributeSpecHandle>(spec)) {
// 3. Check if this spec has an authored `typeName`.
if (attrSpec->HasField(pxr::SdfFieldKeys->TypeName)) {
const pxr::TfToken authoredTypeName =
attrSpec->GetTypeName().GetAsToken();
EXPECT_EQ(authoredTypeName, schemaTypeName)
<< "Type mismatch for attribute <" << attr.GetPath()
<< ">: expected schema-defined type '"
<< schemaTypeName.GetString() << "', got authored type '"
<< authoredTypeName.GetString() << "' in layer @"
<< attrSpec->GetLayer()->GetIdentifier() << "@";
// We've found the strongest authored opinion for `typeName`,
// so we can stop checking the stack for this attribute.
break;
}
}
}
}
}
}
} // namespace usd
} // namespace mujoco
test/experimental/usd/test_utils.h
+4
View File
@@ -103,6 +103,10 @@ void ExpectAttributeEqual<pxr::SdfAssetPath>(pxr::UsdStageRefPtr stage,
void ExpectAttributeHasConnection(pxr::UsdStageRefPtr stage, const char* path,
const char* connection_path);
// Checks that all authored attributes on the given prim have types that match
// the schema types.
void ExpectAllAuthoredAttributesMatchSchemaTypes(const pxr::UsdPrim& prim);
} // namespace usd
} // namespace mujoco
#endif // MUJOCO_TEST_EXPERIMENTAL_USD_PLUGINS_MJCF_FIXTURE_H_