Skip to content

usdPhysics: treat disabled rigid bodies as unapplied in all ancestor traversals - #4187

Closed
andrewkaufman wants to merge 9 commits into
PixarAnimationStudios:devfrom
andrewkaufman:unify-disabled-body-ancestor-traversal
Closed

andrewkaufman wants to merge 9 commits into
PixarAnimationStudios:devfrom
andrewkaufman:unify-disabled-body-ancestor-traversal

Conversation

@andrewkaufman

@andrewkaufman andrewkaufman commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Description of Change(s)

Per the schema docs, physics:rigidBodyEnabled = false means the body takes no part in simulation, which is equivalent to not applying UsdPhysicsRigidBodyAPI at all. Every ancestor traversal that resolves which body owns a prim should therefore continue past a disabled body to the nearest enabled one.

This supersedes #4164 and fixes two more traversals that were missed there.

  • ComputeMassProperties counted colliders with physics:collisionEnabled = false toward mass, center of mass and inertia, even though it skipped them as shapes.
  • A disabled nested body pruned its subtree, so the colliders below it contributed mass to no body at all.
  • _HasDynamicBodyParent stopped at a disabled body, so colliders below it were treated as static instead of belonging to the enabled body above.
  • _GetBodyPrim, which resolves a joint's body relationship, returned the disabled body and never looked at physics:rigidBodyEnabled at all.
  • PhysicsJointChecker only looked for RigidBodyAPI on the prim a body relationship targets. A relationship may point at any UsdGeomXformable, and joint parsing resolves it to the closest ancestor body, so a joint anchored to a collider under a body was flagged as bodyless while parsing resolved it to a real enabled body.

Before this change, a collider under a disabled inner body under an enabled outer body got three different answers: collider attribution said the outer body, the joint relationship resolved to the disabled inner body, and validation reported an error. All three now resolve to the outer body.

Enabled bodies and colliders relying on the true fallback are unaffected. JointNoEnabledRigidBody is evaluated per joint rather than per relationship, and is unchanged here: one of the two relationships reaching an enabled body is sufficient, either directly or through an ancestor, so an empty relationship or a non-body target opposite a body does not error.

Fixes Issue(s)

Supersedes and includes #4164.

AI Assistance

Developed with AI assistance (Anthropic Claude, primarily Opus 5): the
code, tests, and this description were AI-drafted and then reviewed by
me before submission.

Checklist

UsdPhysicsRigidBodyAPI::ComputeMassProperties gathered every descendant
prim with PhysicsCollisionAPI applied, regardless of whether the collider
was enabled. A collider with physics:collisionEnabled = false was still
contributing its mass, center of mass, and inertia to the body.

physics:collisionEnabled determines whether the PhysicsCollisionAPI is
enabled; a collider whose collision is disabled takes no part in
simulation and so should not contribute to a body's aggregated mass
properties. Skip colliders whose resolved physics:collisionEnabled is
false when gathering shapes for the mass computer.

Also clarifies in the PhysicsMassAPI documentation and the usdPhysics
overview that a disabled collider or disabled rigid body takes no part in
simulation, so it is excluded from mass computation and any MassAPI
applied to it has no effect.

Adds a regression test: a rigid body with one enabled unit cube and one
disabled cube offset along z now reports the mass, center of mass, and
inertia of the single enabled cube only.
A nested UsdPhysicsRigidBodyAPI forms the root of its own subtree only while
it is enabled. ComputeMassProperties pruned the subtree of every nested prim
with RigidBodyAPI applied without consulting physics:rigidBodyEnabled, so the
enabled colliders beneath a disabled nested body were excluded from that body's
mass, while no mass was computed for the disabled body itself. Their mass was
therefore attributed to no body at all and silently lost.

Prune only at enabled nested bodies, so those colliders continue to belong to
the nearest enabled rigid body above them. Documentation is updated to state
that disabling a prim ignores MassAPI on that prim without discarding the mass
of the enabled colliders beneath it.
The physics parsing utilities stopped walking the ancestors at the first prim
with RigidBodyAPI applied, whether or not it was enabled, and reported that
prim as the collider's body. A collider below a disabled nested body was
therefore assigned to a body that takes no part in simulation, even when an
enabled body existed further up the hierarchy, and disagreed with the subtree
that ComputeMassProperties aggregates.

Continue searching the ancestors past a disabled body so the collider is
assigned to the nearest enabled rigid body. When no enabled body exists above
it, the nearest disabled body is still reported, as before.
A disabled rigid body takes no part in simulation and is equivalent to the API
not being applied at all, so every ancestor traversal must look past it.

Two traversals still stopped at the nearest body regardless of whether it was
enabled, disagreeing with the collider attribution the preceding commits
established:

- _GetBodyPrim, which resolves a joint body relationship, returned a disabled
  body and never consulted physics:rigidBodyEnabled at all.
- PhysicsJointChecker's HasDynamicBodyParent reported a joint below a disabled
  body as having no enabled rigid body, even when an enabled body sat above it.

Both now continue past a disabled body to the nearest enabled one, so joint
relationship resolution, collider attribution, and validation agree on which
body owns a given prim.
@sunyab

sunyab commented Aug 20, 2026

Copy link
Copy Markdown
Member

Filed as internal issue #USD-12568

(This is an automated message. See here for more information.)

Comment thread pxr/usd/usdPhysics/parseUtils.cpp Outdated
Comment thread pxr/usd/usdPhysics/parseUtils.cpp Outdated
Comment thread pxr/usdValidation/usdPhysicsValidators/validators.cpp Outdated
Comment thread pxr/usdValidation/usdPhysicsValidators/validators.cpp Outdated
Comment thread pxr/usdValidation/usdPhysicsValidators/validators.cpp Outdated
andrewkaufman and others added 2 commits September 4, 2026 09:07
Co-authored-by: nvmkuruc <122411138+nvmkuruc@users.noreply.github.com>
…d case

The suggestions applied in the previous commit inverted the polarity at both
the recording and the reporting site of the nearest disabled body, in
_HasDynamicBodyParent and in the validators' HasDynamicBodyParent:

- The recording site only assigned disabledBodyPrim when it was already
  non-null, so it was never assigned at all.
- The reporting site wrote to *outBodyPrimPath only when disabledBodyPrim was
  null, clobbering the out parameter with a null prim.

Restore the intended conditions. A collider below a disabled body with no
enabled body above it is once again reported as belonging to that disabled
body, rather than degrading to a static collision.

Also apply the UsdPrim default construction cleanup to parseUtils.cpp, which
only landed in validators.cpp.

No existing test covered a disabled body without an enabled body above it, so
the whole physics and validation suite passed with the polarity inverted. Add
that case to testUsdPhysicsParsing; it fails without this change.

@andrewkaufman andrewkaufman left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @nvmkuruc, the UsdPrim disabledBodyPrim; default construction was right, and I've applied it to parseUtils.cpp as well, which the suggestion only covered in validators.cpp.

The other four inverted the polarity at both sites, so I've corrected them in b33797415fc5aa5b791982f86eaa15b1a9ffa9ea rather than reverting. The recording site needs !disabledBodyPrim, so the nearest disabled body is remembered the first time one is seen; the reporting site needs disabledBodyPrim, so the out parameter is only written when one was actually found. As applied in 74907c7431d666a6eb5289b013bf379081a3c48b, disabledBodyPrim was never assigned and the reporting site wrote a null prim to *outBodyPrimPath.

No existing test covered a disabled body with no enabled body above it, so the entire physics and validation suite passed with the polarity inverted. That case is the one that regressed (the collider degraded to a static collision instead of being reported against the disabled body) so I've added it to testUsdPhysicsParsing, and it fails without this change.

@tallytalwar tallytalwar left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, some minor notes. @andrewkaufman could you please address these and I can get this in.

Comment thread pxr/usd/usdPhysics/overview.dox Outdated
Comment thread pxr/usd/usdPhysics/overview.dox Outdated
Comment thread pxr/usdValidation/usdPhysicsValidators/validators.cpp Outdated
Comment thread pxr/usdValidation/usdPhysicsValidators/testenv/testUsdPhysicsValidation.py Outdated
- overview.dox: rephrase the nested-body exception in terms of the prim's
  physics:rigidBodyEnabled state, and reword the implicit-mass note to apply
  to any rigid body (not only nested ones), naming physics:rigidBodyEnabled.
- validators.cpp: default isAPISchemaEnabled to true to match the schema's
  rigidBodyEnabled fallback.
- testUsdPhysicsValidation.py: use assertEqual(len(errors), N) instead of
  assertTrue(len(errors) == N) for better failure diagnostics.
A collider below a disabled rigid body with no enabled body above it was
reported as belonging to that disabled body. The schema says a disabled
rigid body takes no part in simulation and owns no colliders: such a
collider is a static collision, and its body path is empty.

_HasDynamicBodyParent no longer records a disabled body as a fallback
owner. It continues walking past disabled bodies to find an enabled body,
and returns no body when none is found. _GetRigidBody then yields an empty
path so the collider is treated as static.

Behavior change: a collider under a disabled body with no enabled ancestor
now resolves to an empty rigid body path instead of the disabled body's
path. In the common case with a default simulation owner the collider is
still parsed, now as a static collision; with an explicit non-default
simulation owner filter it is excluded, matching static collision handling.

The nested case is unchanged: a collider below a disabled body that has an
enabled body above it still belongs to the enabled body.
@andrewkaufman

Copy link
Copy Markdown
Contributor Author

Pushed a follow-up commit (aa29b9a01) correcting the disabled-body collider case.

The earlier commits, when a collider sat below a disabled rigid body with no enabled body above it, reported the collider as belonging to that disabled body. That contradicts the schema: a rigid body with physics:rigidBodyEnabled = false takes no part in simulation and owns no colliders (see the MassAPI and GetCollisionPrims documentation). Such a collider is a static collision, and its body path should be empty.

_HasDynamicBodyParent no longer records a disabled body as a fallback owner; it walks past disabled bodies to the nearest enabled one and returns no body if there is none. test_rigidbody_disabled_body_only_collision_parse is updated to assert the collider resolves to an empty body path.

The nested case is unchanged: a collider below a disabled body that has an enabled body above it still belongs to the enabled body.

@pixar-oss pixar-oss closed this in 997b46e Sep 18, 2026
@andrewkaufman
andrewkaufman deleted the unify-disabled-body-ancestor-traversal branch September 19, 2026 01:15
andrewkaufman added a commit to andrewkaufman/OpenUSD that referenced this pull request Sep 21, 2026
…g resolution

The disabled-body ancestor-traversal-skip that landed on dev (997b46e,
squash of PixarAnimationStudios#4187) decides a question still under discussion: whether a
collider owned by a disabled rigid body becomes a static collision /
propagates to an enabled ancestor, or remains attributed to the disabled
body. Until that is settled, revert only that behavior so every ancestor
walk again stops at the first rigid body regardless of
physics:rigidBodyEnabled, matching pre-PixarAnimationStudios#4187 attribution.

Reverted (contested collider/joint-body ownership):
- parseUtils.cpp _GetBodyPrim: no longer skips a disabled body when
  resolving a joint body relationship.
- parseUtils.cpp _HasDynamicBodyParent: no longer walks past a disabled
  body or reports a nearest-disabled-body fallback.
- validators.cpp HasDynamicBodyParent: same revert.
- Drops the three parsing tests and the one validation test that assert
  the disabled-body skip.

Kept (bug fixes independent of the debate, all still on dev):
- rigidBodyAPI.cpp: exclude physics:collisionEnabled=false colliders from
  mass, and do not prune a disabled nested body's subtree (its enabled
  colliders' mass is no longer lost).
- validators.cpp: isAPISchemaEnabled defaults to true, matching the
  rigidBodyEnabled fallback.
- validators.cpp HasEnabledRigidBody: searches ancestors so a joint
  anchored to a collider below a body is no longer flagged bodyless.

This introduces a deliberate discrepancy. On dev every path treats a
disabled body consistently: colliders below it belong to the nearest
enabled ancestor (or the disabled body itself when none exists), and mass,
attribution, joint-body resolution and validation all agree. With this
revert the mass computer still walks past a disabled nested body (that
fix is kept, so its enabled colliders' mass is not lost), while collider
attribution, joint-body resolution and validation once again stop at the
disabled body. So a collider under a disabled nested body contributes its
mass to the enabled outer body but is attributed to the disabled inner
body. This inconsistency is the cost of parking the ownership question and
is expected to be resolved, in one direction or the other, once the
discussion settles.
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.

4 participants