Skip to content

Check physics-purpose material bindings in USD decoder for contact properties - #3394

Open
anishesg wants to merge 2 commits into
google-deepmind:mainfrom
proudhare:fix/ph-issue-3393
Open

Check physics-purpose material bindings in USD decoder for contact properties#3394
anishesg wants to merge 2 commits into
google-deepmind:mainfrom
proudhare:fix/ph-issue-3393

Conversation

@anishesg

Copy link
Copy Markdown

The USD decoder previously only checked default/all-purpose material bindings when importing colliders, missing physics-purpose material bindings that carry contact properties like friction, solref, solimp, margin, and gap. In USD, a collider can bind one material for visual appearance (default purpose) and a separate material for physics simulation (physics purpose).

The fix modifies ParseUsdPhysicsCollider in plugin/usd_decoder/usd_decoder.cc to query physics-purpose material bindings before falling back to default bindings. When a physics-purpose binding exists with physics material APIs, the decoder now correctly extracts contact properties from that material. The default binding continues to provide visual material assignment. This ensures MJCF→USD→MJCF roundtrips preserve authored contact properties.

Fixes #3393

…operties

The USD decoder previously only checked default/all-purpose material bindings when importing colliders, missing physics-purpose material bindings that carry contact properties like friction, solref, solimp, margin, and gap. In USD, a collider can bind one material for visual appearance (default purpose) and a separate material for physics simulation (physics purpose).

Signed-off-by: anish <anishesg@users.noreply.github.com>
@LouRohanNV

Copy link
Copy Markdown
Contributor

I cloned this PR locally and built the USD decoder target with USD enabled:

cmake --build /tmp/mujoco-pr3394-build --target usd_decoder_plugin --parallel 8

It currently does not compile against the OpenUSD headers used by MuJoCo. The failure is in plugin/usd_decoder/usd_decoder.cc, where the physics-purpose ComputeBoundMaterial call passes the material purpose token before the cache pointers. For the cached overload, the cache pointers come first and the material purpose token comes after them.

The relevant compiler error is:

error: no matching function for call to
UsdShadeMaterialBindingAPI::ComputeBoundMaterial(
    TfToken&, BindingsCache*, CollectionQueryCache*)

I also think the behavior should be a little more careful than only checking whether a physics-purpose material binding exists. A physics-purpose material can exist without actually authoring the physics/contact APIs, and in that case the decoder should still be able to fall back to the default/all-purpose material for physics/contact fields.

I pushed a branch with the version I have been testing here:

https://github.com/LouRohanNV/mujoco/tree/roundtrip-pr/usd-decoder-physics-materials

The main differences in that branch are:

  • use the cached ComputeBoundMaterial overload with the correct argument order;
  • prefer physics-purpose material bindings for contact properties;
  • track whether physics/contact APIs were actually parsed, rather than only whether a physics-purpose material was bound;
  • fall back to the default/all-purpose material when the physics-purpose material does not provide physics/contact APIs;
  • keep separate caches for default material lookup and physics-purpose material lookup.

Maybe this PR could be updated in that direction, or we could use that branch as the basis for the final fix.

… parsed physics APIs

Signed-off-by: anish <anishesg@users.noreply.github.com>
@anishesg

Copy link
Copy Markdown
Author

ah good catch on the arg order, fixed. also switched to tracking whether physics APIs were actually parsed rather than just checking if the binding exists, and added separate caches like your branch. should compile now

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

USD decoder misses physics-purpose material bindings on colliders

3 participants