usdPhysics: add public getters for joint/collider body resolution and refactor the joint parser onto them - #4165
andrewkaufman wants to merge 13 commits into
Conversation
|
Filed as internal issue #USD-12474 (This is an automated message. See here for more information.) |
fbce7bc to
5969977
Compare
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.
5969977 to
7faa4fb
Compare
|
Restacked onto #4187, which supersedes #4164. Only the @AlesBorovicka on your question about how far to take this: you are right that it invites symmetric accessors, a get rigid body on a collider and get body 0/1 on a joint. #4187 is relevant to that now, because it makes all of those the same traversal. It resolves which body owns a prim consistently across mass computation, collider attribution, joint relationship resolution and validation, so symmetric accessors would be thin wrappers over one shared walk rather than four separate ones. We would like to discuss the scope before adding them, so this PR stays limited to the collider getter. |
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.
- 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.
|
@andrewkaufman could you rebase this on top of #4187 to remove commits which are already in 4187? Thanks Or if its just 7faa4fb which needs to be included, I can extract that out internaly, lmk. Thanks |
7faa4fb to
8fdd386
Compare
|
@tallytalwar I've rebased on #4187 to incorporate the code review changes there. The commits still show here though... I can't seem to stack the PRs for some reason, perhaps because the source branch lives on my fork, not quite sure.... Its just the last commit that is unique here. I have one addition planned before this is ready: alongside |
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.
Exposes the traversal that ComputeMassProperties uses to gather the colliders belonging to a rigid body as a public method. Starting at the body prim, the subtree is traversed (including instance proxies) and every enabled UsdPhysicsCollisionAPI prim is collected; subtrees rooted at nested rigid bodies are pruned since those colliders belong to a different body, and colliders whose physics:collisionEnabled is false are skipped. ComputeMassProperties is refactored to call GetCollisionPrims so the body-ownership and enabled-collider semantics live in one place, and clients (e.g. physics engines consuming USD) can reproduce them without re-implementing the traversal. Adds a Python wrapping returning the list of collider prims and a regression test covering the enabled/disabled/nested-body cases.
89347b3 to
8b5b5e9
Compare
Replace the parser's private _GetBodyPrim / _GetLocalPose helpers with calls to UsdPhysicsJoint::GetBody0/1 and GetLocalPose0/1, so the body resolution and local-pose computation have a single implementation shared by the parser and the public API. Two intended behavior changes fall out of adopting the public semantics, both for malformed or body-less joint relationships: - A joint side whose relationship resolves to no enclosing enabled body now reports an empty body path instead of falling back to the nearest collider. A collider is not a body and was never a correct answer here. - A joint side whose relationship target does not resolve to a prim on the stage (a dangling relationship) now contributes no anchor pose (identity) instead of silently passing the authored local pose through. Dangling relationships are malformed authoring and are no longer honored.
8b5b5e9 to
e63f00c
Compare
|
@AlesBorovicka @tallytalwar, pushed the new public getters and refactored the joint parser onto them. New API:
The joint parser's private One behavior change worth a check: a joint side with no enclosing enabled body now gives an empty @AlesBorovicka, does this match your expectation for the body-resolution semantics, and do you agree the joint parser is more correct now? PS. |
andrewkaufman
changed the title
Well a standalone collider is a static body, it boils down to the transformation used in the end of the day that you compute from the joint local pose. So its going to be different based on what you return. This will create different joints in PhysX for sure. So changes like this will have some consequences. |
5 participants
Description of Change(s)
Adds a set of public getters that expose the body-resolution and mass-collider traversals that previously lived only inside usdPhysics internals, and refactors the joint parser to use them so the resolution logic has a single implementation.
New API:
UsdPhysicsRigidBodyAPI::GetCollisionPrims()returns the collider prims belonging to a rigid body, the same traversalComputeMassProperties()uses internally to decide which colliders' mass is accumulated into the body. Starting at the rigid body's prim the subtree is traversed (including instance proxies) and every prim withUsdPhysicsCollisionAPIapplied is collected, with two exceptions: a collider whosephysics:collisionEnabledresolves tofalseis skipped, and the subtree rooted at a nested prim withUsdPhysicsRigidBodyAPIis pruned (those colliders belong to that other body).ComputeMassProperties()is refactored to call it.UsdPhysicsCollisionAPI::GetBody()resolves a collider to its owning enabled rigid body via the ancestor walk, returning an invalid prim when there is none.UsdPhysicsJoint::GetBody0()/GetBody1()return the enabled rigid body each side of the joint attaches to, or an invalid prim.UsdPhysicsJoint::GetLocalPose0()/GetLocalPose1()return the joint-local anchor pose in that body's frame, with the frame scale baked into the translation (physics is scale-free) and an optionalUsdGeomXformCachefor nested hierarchies.Refactor: the joint parser's private
_GetBodyPrim/_GetLocalPoseare deleted and_FinalizeJointnow calls the public getters, so joint body resolution has a single implementation shared between the parser and clients.Behavior change: a joint side with no enclosing enabled body now gives an empty
body0/body1, where the previous parser fell back to the nearest collider. This is called out for review in the PR discussion.Each getter has Python wrapping and regression tests.
Motivation: the traversals that map a rigid body to its colliders, and a joint or collider to its owning body, previously existed only inside
ComputeMassPropertiesand the joint parser. Clients that need this had to re-implement it (instance-proxy traversal and enabled-body resolution are easy to get subtly wrong). Exposing the canonical logic lets clients reuse it and lets the parser drop its private copy.Link to proposal
N/A, additive public API, no schema change.
Dependency
Based on top of #4187 (treat disabled rigid bodies as unapplied in all ancestor traversals); these getters share that enabled-body filter. Restacked from #4164, which #4187 supersedes.
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