[Fix] Apply rigid-body and mass properties to every link of nested-hierarchy assets#6377
Conversation
The apply_nested decorator stops descending into a prim's children once the wrapped function succeeds, assuming physics schemas cannot nest. The URDF importer in Isaac Sim 6.0+ authors child link prims under their parent link prim, so rigid-body and mass properties were only applied to the first link of such assets. Add a stop_on_success flag to the decorator (default True, keeping the existing behavior) and disable it for the rigid-body and mass modifiers so every nested link is visited. Closes isaac-sim/IsaacLab issue 5918 together with the earlier contact-sensor fix (PR 6259).
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5Safe to merge — a focused, well-contained bug fix that adds backward-compatible behavior to a single decorator utility. The dual-use decorator pattern (func=None sentinel + keyword-only stop_on_success) is the standard Python idiom and is implemented correctly. All existing @apply_nested call-sites receive stop_on_success=True with no behavioral change. The two opt-out functions are the only ones where nested schemas legitimately occur, and the test directly validates the fix on a 3-deep chain. No files require special attention; all changes are tightly scoped to the decorator utility and the two schema functions that need nested traversal. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["apply_nested called"] --> B{func is None?}
B -- "No (bare @apply_nested)" --> C["decorator(func) → wrapper\nstop_on_success=True"]
B -- "Yes (@apply_nested(stop_on_success=...))" --> D["return decorator"]
D --> E["decorator(func) → wrapper\nstop_on_success=False or True"]
C --> W
E --> W
subgraph W["wrapper execution"]
W1["BFS over prim subtree"] --> W2["Call func on each prim"]
W2 --> W3{success?}
W3 -- "False" --> W4["Add children to queue"]
W3 -- "True" --> W5{stop_on_success?}
W5 -- "True (default)" --> W6["Skip children"]
W5 -- "False (rigid/mass props)" --> W4
W4 --> W1
end
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A["apply_nested called"] --> B{func is None?}
B -- "No (bare @apply_nested)" --> C["decorator(func) → wrapper\nstop_on_success=True"]
B -- "Yes (@apply_nested(stop_on_success=...))" --> D["return decorator"]
D --> E["decorator(func) → wrapper\nstop_on_success=False or True"]
C --> W
E --> W
subgraph W["wrapper execution"]
W1["BFS over prim subtree"] --> W2["Call func on each prim"]
W2 --> W3{success?}
W3 -- "False" --> W4["Add children to queue"]
W3 -- "True" --> W5{stop_on_success?}
W5 -- "True (default)" --> W6["Skip children"]
W5 -- "False (rigid/mass props)" --> W4
W4 --> W1
end
Reviews (1): Last reviewed commit: "Fix apply_nested skipping nested rigid b..." | Re-trigger Greptile |
|
AI-generated review comment: Important — |
AntoineRichard
left a comment
There was a problem hiding this comment.
There is the AI comment otherwise LGTM
The configured decorator form returned a bare Callable, losing the decorated function's signature and dropping the required boolean return contract of wrapped functions. Add ParamSpec-based overloads for the bare and configured forms so decorated functions keep their exact signature and type as returning None, matching the wrapper's actual return. Enforce the bool contract on wrapped functions and make bind_visual_material conform: it returned None when Kit is absent and an untyped value otherwise.
Description
Spawning an asset whose rigid bodies are nested (child link prims authored under their parent link prim, as produced by the URDF importer in Isaac Sim 6.0 and later) only applied
rigid_propsandmass_propsto the first link: theapply_nesteddecorator stops descending into a prim's children once the wrapped function succeeds, assuming physics schemas cannot nest. On a 29-link arm,disable_gravity=Truereached 1/29 bodies with no warning.This completes the nested-hierarchy fix started in #6259 (which covered contact-sensor activation):
apply_nestedgains astop_on_successflag (defaultTrue; the dual-form decorator keeps every existing bare@apply_nestedusage working unchanged).modify_rigid_body_propertiesandmodify_mass_propertiesopt out of the early stop, since their anchor prims are exactly the link prims that nest.Scope: why only the rigid-body and mass writers
An audit of the 40 asset stages referenced by
isaaclab_assets/isaaclab_tasksinformed the scoping:ShadowHandNewtonauthors 24 of its 25 links body-under-body and is spawned withrigid_propsby the shadow-hand task — this fix is required for those values to reach every link.CollisionAPIwith SDF approximation on two ancestor Xforms of the same meshes, via layer composition). Those anchors describe the same geometry rather than independent colliders — the USD Physics spec tiesCollisionAPIto leaf gprims, so nested Xform-level anchors have no defined precedence. Widening the traversal there would paper over an asset-level duplication instead of fixing schema application, so the collision writers keep the early stop; the asset duplication is being followed up separately.MassAPIon both link prims and mesh prims beneath them. With this changemass_propsnow also authors the nestedMassAPIprims; per the USD Physics spec, a parent's explicit mass overrides mass properties authored further down the subtree, so effective per-body mass is unchanged.Behavior note
The Newton shadow-hand task cfg sets
disable_gravity=True(and related rigid-body properties) that previously reached only 1 of 25 links. With this fix the cfg applies to all 25 links as intended. This is the bug being fixed, but baselines tuned against the old (partial) behavior may shift for tasks whose assets have nested links.Verification
UsdFileCfg→ the same property-application pathUrdfFileCfguses) on a nested Geometry-scoped 3-link chain:disable_gravityandmassauthor on 1/3 bodies before the fix and 3/3 after; contact-report API 3/3 on both (via Fix contact sensor activation for nested rigid bodies #6259).test_schemas.py(nested chain, asserts both properties on every link).Fixes #5918
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there