Adding Rigid Body Material USD data classes and writers - #6287
Conversation
Convert rigid-body physics materials to the single-namespace "fragment" model used by the other schema families. Add RigidBodyMaterialFragment and the solver-common UsdPhysicsRigidBodyMaterialCfg (physics:* friction / restitution) in core, and PhysxMaterialCfg (physxMaterial:* compliant contact + combine modes) in the PhysX extension. A material is spawned as a separate UsdShade.Material prim and bound, so the family writer spawn_rigid_body_material_from_fragments spawns the prim, applies the UsdPhysics.MaterialAPI anchor, and dispatches each fragment. Spawner physics_material slots now accept a list of fragments alongside the legacy material cfg, dispatched through spawn_physics_material; the legacy single-cfg path is unchanged.
Rename test prim paths MatA/MatB to MaterialA/MaterialB so codespell no longer flags the truncated tokens, and drop a stray blank line per ruff.
Greptile SummaryThis PR introduces rigid-body physics-material "fragments" — a single-namespace config pattern already used across other schema families — letting spawners accept either a list of
Confidence Score: 5/5Safe to merge. The change is purely additive: legacy callers are unchanged, the new dispatch path is gated behind isinstance checks, and invalid inputs (empty list, mixed-type list) surface clear errors rather than opaque AttributeErrors. The dispatch logic in spawn_physics_material is straightforward and handles all three input forms correctly. apply_namespaced already skips the func field (confirmed in schemas.py), so no spurious physics:func attribute is written. The UsdPhysics.MaterialAPI anchor is applied before fragment dispatch, satisfying the schema-presence requirement for physics:* attribute writes. Tests cover multi-namespace composition, single-fragment form, the slot dispatcher for both forms, and partial-update (None-field) semantics. No correctness issues were found beyond concerns already surfaced in prior review threads. No files require special attention. Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Caller as Spawner (shapes/meshes/from_files)
participant SPM as spawn_physics_material
participant SRBMFF as spawn_rigid_body_material_from_fragments
participant AN as apply_namespaced
participant Stage as Usd.Stage
Caller->>SPM: (prim_path, material, stage)
alt material is list/tuple
SPM->>SPM: validate: non-empty, all RigidBodyMaterialFragment
SPM->>SRBMFF: (prim_path, list(material), stage)
else material is RigidBodyMaterialFragment
SPM->>SRBMFF: (prim_path, [material], stage)
else legacy PhysicsMaterialCfg
SPM->>Caller: material.func(prim_path, material)
end
SRBMFF->>Stage: GetPrimAtPath / Material.Define
SRBMFF->>Stage: UsdPhysics.MaterialAPI.Apply (anchor)
loop for each fragment
SRBMFF->>AN: func(cfg, prim_path, stage)
AN->>Stage: AddAppliedSchema (if _usd_applied_schema set)
AN->>Stage: "set physics:* / physxMaterial:* attrs"
end
SRBMFF-->>Caller: Usd.Prim
%%{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"}}}%%
sequenceDiagram
participant Caller as Spawner (shapes/meshes/from_files)
participant SPM as spawn_physics_material
participant SRBMFF as spawn_rigid_body_material_from_fragments
participant AN as apply_namespaced
participant Stage as Usd.Stage
Caller->>SPM: (prim_path, material, stage)
alt material is list/tuple
SPM->>SPM: validate: non-empty, all RigidBodyMaterialFragment
SPM->>SRBMFF: (prim_path, list(material), stage)
else material is RigidBodyMaterialFragment
SPM->>SRBMFF: (prim_path, [material], stage)
else legacy PhysicsMaterialCfg
SPM->>Caller: material.func(prim_path, material)
end
SRBMFF->>Stage: GetPrimAtPath / Material.Define
SRBMFF->>Stage: UsdPhysics.MaterialAPI.Apply (anchor)
loop for each fragment
SRBMFF->>AN: func(cfg, prim_path, stage)
AN->>Stage: AddAppliedSchema (if _usd_applied_schema set)
AN->>Stage: "set physics:* / physxMaterial:* attrs"
end
SRBMFF-->>Caller: Usd.Prim
Reviews (3): Last reviewed commit: "Validate physics-material fragment lists..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``) | ||
| return material.func(prim_path, material) |
There was a problem hiding this comment.
stage is silently dropped on the legacy path. Callers in meshes.py and from_files.py pass an explicit stage, expecting it to be used for both the fragment and legacy code paths. On the legacy branch, material.func(prim_path, material) ignores stage entirely and whatever the legacy func does internally (typically get_current_stage()) takes over instead.
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``) | |
| return material.func(prim_path, material) | |
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``) | |
| # NOTE: legacy funcs do not accept a ``stage`` kwarg; they call get_current_stage() internally. | |
| return material.func(prim_path, material) |
There was a problem hiding this comment.
Confirmed on current head 2d70ad4a37d with a two-stage reproduction: spawn_physics_material(..., legacy_cfg, stage=other_stage) creates the material on the global current stage, while other_stage remains empty. The explanatory comment documents the limitation but the public stage contract is still branch-dependent. Please either forward stage through the built-in legacy material writers or explicitly remove/narrow that contract.
There was a problem hiding this comment.
The legacy path comment in the current code says stage is "intentionally not forwarded" because legacy funcs have signature (prim_path, cfg). That's accurate — but the public API still accepts and silently discards stage, which is the contract violation.
The cleanest fix without touching the legacy writers themselves is to add a stage keyword guard: if a caller passes an explicit non-None stage on the legacy path, raise early rather than silently ignoring it. That makes the contract explicitly narrow instead of deceptively broad.
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``) | |
| return material.func(prim_path, material) | |
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``). | |
| # Legacy material funcs take only ``(prim_path, cfg)`` and resolve the stage internally via | |
| # ``get_current_stage()``; they have no ``stage`` parameter. | |
| if stage is not None: | |
| raise ValueError( | |
| f"Legacy material cfg '{type(material).__name__}' does not support an explicit stage. " | |
| "Pass stage=None or switch to the fragment-based API." | |
| ) | |
| return material.func(prim_path, material) |
Alternatively, if you want zero-breakage for callers that pass stage=None (the common case) but still want to honour an explicit stage, the minimal change is to push stage through get_current_stage before handing off to the legacy path and temporarily set it as the active stage — but that's invasive and relies on internal USD stage-management APIs.
The guard approach is the most honest fix: it converts a silent contract violation into a loud, debuggable one and leaves the door open to properly forwarding stage later once the legacy writers are updated.
Would you like me to also add a DeprecationWarning note on spawn_physics_material suggesting callers migrate to the fragment API for full stage control?
There was a problem hiding this comment.
Resolved by narrowing the contract in ffa19f3: the legacy path now raises ValueError when given an explicit stage that is not the current stage (your two-stage repro is the regression test, incl. asserting nothing leaks onto the current stage). Passing the current stage explicitly remains valid — the in-tree spawners pass stage=stage unconditionally with legacy cfgs, so raising on any explicit stage would have broken them. Truly forwarding the stage would require making the @clone path-resolution stage-aware, which is out of scope for the transition bridge; the fragment path honors explicit stages, and the docstring now states the legacy path is current-stage-bound. Flagged as Breaking: in the changelog with migration guidance.
| def spawn_physics_material( | ||
| prim_path: str, | ||
| material, | ||
| stage: Usd.Stage | None = None, | ||
| ) -> Usd.Prim: |
There was a problem hiding this comment.
material parameter has no type annotation. Every other public function in this file is fully annotated, and this function sits at the dispatch boundary between the fragment and legacy interfaces. Adding the union type makes the contract explicit for type checkers and readers.
| def spawn_physics_material( | |
| prim_path: str, | |
| material, | |
| stage: Usd.Stage | None = None, | |
| ) -> Usd.Prim: | |
| def spawn_physics_material( | |
| prim_path: str, | |
| material: physics_materials_cfg.PhysicsMaterialCfg | |
| | physics_materials_cfg.RigidBodyMaterialFragment | |
| | list[physics_materials_cfg.RigidBodyMaterialFragment], | |
| stage: Usd.Stage | None = None, | |
| ) -> Usd.Prim: |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| def spawn_rigid_body_material_from_fragments( | ||
| prim_path: str, | ||
| fragments: physics_materials_cfg.RigidBodyMaterialFragment | list[physics_materials_cfg.RigidBodyMaterialFragment], | ||
| stage: Usd.Stage | None = None, | ||
| ) -> Usd.Prim: | ||
| """Spawn a rigid-body physics material from a list of single-namespace fragments. | ||
|
|
||
| Creates (or reuses) the ``UsdShade.Material`` prim at ``prim_path``, applies the standard | ||
| ``UsdPhysics.MaterialAPI`` anchor, then dispatches each fragment via its | ||
| :attr:`~isaaclab.sim.schemas.SchemaFragment.func` to author its namespace onto the material prim. | ||
| Backend fragments carry backend-specific namespaces (e.g. PhysX ``physxMaterial:*``) without core | ||
| importing a backend. | ||
|
|
||
| Args: | ||
| prim_path: The prim path to spawn the material at. | ||
| fragments: A single :class:`~isaaclab.sim.spawners.materials.RigidBodyMaterialFragment` or a list | ||
| of them. | ||
| stage: The stage to spawn on. Defaults to None, in which case the current stage is used. | ||
|
|
||
| Returns: | ||
| The spawned rigid body material prim. | ||
|
|
||
| Raises: | ||
| ValueError: When a prim already exists at the path and is not a material. | ||
| """ | ||
| if stage is None: | ||
| stage = get_current_stage() | ||
| if not isinstance(fragments, (list, tuple)): | ||
| fragments = [fragments] | ||
|
|
||
| # create the material prim if none exists yet | ||
| if not stage.GetPrimAtPath(prim_path).IsValid(): | ||
| UsdShade.Material.Define(stage, prim_path) | ||
| prim = stage.GetPrimAtPath(prim_path) | ||
| if not prim.IsA(UsdShade.Material): | ||
| raise ValueError(f"A prim already exists at path: '{prim_path}' but is not a material.") | ||
|
|
||
| # apply the standard UsdPhysics MaterialAPI anchor (the defining schema for a physics material) | ||
| if not UsdPhysics.MaterialAPI(prim): | ||
| UsdPhysics.MaterialAPI.Apply(prim) | ||
|
|
||
| # dispatch each fragment's applier (writes its single namespace onto the material prim) | ||
| for cfg in fragments: | ||
| func = cfg.func if callable(cfg.func) else string_to_callable(cfg.func) | ||
| func(cfg, prim_path, stage) | ||
| return prim |
There was a problem hiding this comment.
@clone decorator
spawn_rigid_body_material and spawn_deformable_body_material are both decorated with @clone, which resolves regex prim-path patterns (e.g. /World/Robot_.*/body) to concrete paths before spawning. spawn_rigid_body_material_from_fragments skips the decorator, so callers who pass a regex path directly will silently get a single prim at the literal pattern string rather than one prim per match. The docstring does not note this limitation, which diverges from the documented behaviour of the rest of the spawner family.
Annotate the spawn_physics_material material parameter with its accepted union, document that the legacy dispatch path intentionally does not forward stage (legacy funcs resolve the stage internally), and note that the fragment writer expects a concrete prim path rather than a regex pattern.
| fragments = material if isinstance(material, (list, tuple)) else [material] | ||
| if fragments and all(isinstance(f, physics_materials_cfg.RigidBodyMaterialFragment) for f in fragments): | ||
| return spawn_rigid_body_material_from_fragments(prim_path, list(fragments), stage) | ||
| # legacy single-cfg path (rigid or deformable material cfg with its own spawner ``func``). | ||
| # NOTE: legacy material funcs take only ``(prim_path, cfg)`` and resolve the stage internally via | ||
| # ``get_current_stage()``; they have no ``stage`` parameter, so ``stage`` is intentionally not | ||
| # forwarded here. This is invisible in single-stage workflows (the only ones materials are used in). | ||
| return material.func(prim_path, material) |
There was a problem hiding this comment.
AttributeError on empty or mixed-type list input
fragments = material if isinstance(material, (list, tuple)) else [material] keeps material as the original list, then the if fragments and all(...) guard short-circuits on an empty list (if [] is falsy), which falls through to material.func(prim_path, material) where material is an empty list. That raises AttributeError: 'list' object has no attribute 'func' instead of a clear validation error. The same failure occurs if a user mistakenly passes a list containing a non-fragment element (the all() check fails, legacy branch runs with a list as material).
An early guard that validates and rejects a list on the legacy branch would make the error actionable.
Reject an empty list (ValueError) and a list containing non-fragment entries (TypeError) in spawn_physics_material, so a malformed slot value surfaces a clear error instead of an opaque AttributeError on the legacy path. Route a lone fragment through the fragment writer explicitly. Add a regression test for the empty and mixed-type cases.
| dynamic_friction: float | None = None | ||
| """The dynamic friction coefficient. Writes ``physics:dynamicFriction``.""" | ||
|
|
||
| restitution: float | None = None |
There was a problem hiding this comment.
Blocking: this fragment omits physics:density. The configured Kit 110.1.2 schema registry reports four UsdPhysics.MaterialAPI attributes: dynamic friction, static friction, restitution, and density. Material density participates in mass computation through material binding, so it is not equivalent to placing density on MassCfg. Please add density: float | None and a round-trip test; a schema-parity assertion would also prevent future drift.
| """How friction is combined during collisions. Writes ``physxMaterial:frictionCombineMode``.""" | ||
|
|
||
| restitution_combine_mode: Literal["average", "min", "multiply", "max"] | None = None | ||
| """How restitution is combined during collisions. Writes ``physxMaterial:restitutionCombineMode``.""" |
There was a problem hiding this comment.
Blocking: the target PhysxMaterialAPI has six attributes, while this fragment exposes four. It is missing damping_combine_mode and compliant_contact_acceleration_spring; both affect compliant-contact behavior. Please add both fields and test them, and update the 104.2 schema link because it no longer represents the Isaac Sim 6 / Kit 110 surface.
| A rigid-body physics material is a single ``UsdShade.Material`` prim that carries one or more | ||
| physics-material schemas. The fragments author single namespaces onto that prim: the solver-common | ||
| ``physics:*`` friction/restitution (:class:`UsdPhysicsRigidBodyMaterialCfg`) and any backend-specific | ||
| namespace (e.g. PhysX ``physxMaterial:*`` via :class:`~isaaclab_physx.sim.spawners.materials.PhysxMaterialCfg`). |
There was a problem hiding this comment.
Blocking: #5276 already added NewtonMaterialPropertiesCfg with newton:torsionalFriction and newton:rollingFriction, but this family adds only the PhysX backend fragment. Because the new dispatcher rejects a legacy cfg inside a fragment list, Newton properties cannot compose with UsdPhysicsRigidBodyMaterialCfg. That diverges from the backend-symmetric pattern in #5976, #6254, #6264, and #6273. Please add a Newton material fragment under isaaclab_newton.sim.spawners.materials, retaining the existing class as the legacy path.
| physics_material: ( | ||
| materials.PhysicsMaterialCfg | ||
| | materials.RigidBodyMaterialFragment | ||
| | list[materials.RigidBodyMaterialFragment] |
There was a problem hiding this comment.
Blocking: this newly advertised fragment-list form is still rejected by _spawn_mesh_geom_from_mesh whenever rigid_props is set. Its guard checks isinstance(cfg.physics_material, RigidBodyMaterialCfg), so a lone fragment or fragment list raises before reaching spawn_physics_material. Please make that validation backend-neutral and add a real rigid-mesh spawner test; direct dispatcher tests do not exercise this path.
| # Create physics material | ||
| if cfg.physics_material is not None: | ||
| cfg.physics_material.func(f"{prim_path}/physicsMaterial", cfg.physics_material) | ||
| spawn_physics_material(f"{prim_path}/physicsMaterial", cfg.physics_material) |
There was a problem hiding this comment.
This call now supports fragments at runtime, but GroundPlaneCfg.physics_material remains annotated and documented only as materials.RigidBodyMaterialCfg. Please widen that field to the same legacy-or-fragment union and add a ground-plane binding test so the public type contract matches this consumer.
Address maintainer review on the physics-material fragment PR: - Add physics:density to UsdPhysicsRigidBodyMaterialCfg, completing its coverage of UsdPhysics.MaterialAPI's four attributes. - Add the two missing PhysxMaterialCfg fields (damping combine mode, compliant-contact acceleration spring) so the fragment covers all six PhysxMaterialAPI attributes; refresh the stale Kit 104.2 schema doc link to the current one. - Add a Newton rigid-body material fragment (NewtonMaterialCfg) for backend symmetry with the PhysX fragment, so torsional/rolling friction can compose in a fragment list alongside other backends. - Widen the mesh spawner's rigid-vs-deformable physics_material guard to accept fragments and fragment lists, not just the legacy cfg. - Widen GroundPlaneCfg.physics_material to the same fragment-aware union FileCfg already uses; its spawner already routed through the shared dispatcher, so only the annotation was stale.
|
Thanks @ooctipus — all five addressed in a8bc6aa (schema facts verified against the installed Kit 110 schemas, not guessed):
Tests: |
| rolling_friction: float | None = None | ||
| """Rolling friction coefficient (resistance to rolling motion) [dimensionless]. | ||
|
|
||
| Writes ``newton:rollingFriction``. Range: [0, inf). |
There was a problem hiding this comment.
Blocking: the registered-schema audit is 2/2, but this fragment is only 2/6 against the Newton runtime that IsaacLab actually pins. At commit 79e95bf from isaaclab_newton/pyproject.toml, SchemaResolverNewton.mapping[PrimType.MATERIAL] also consumes newton:contactStiffness, newton:contactDamping, newton:contactFrictionGain, and newton:contactAdhesion. Newton uses these as the canonical material-side replacements for the deprecated per-shape contact_ke/kd/kf/ka fields, and its pinned import tests exercise them on a bound NewtonMaterialAPI material.
Please add contact_stiffness, contact_damping, contact_friction_gain, and contact_adhesion as float | None fields, with an authoring/parser regression test. apply_namespaced can create these float attrs while the generated schema plugin catches up. This follows the #5276 precedent of auditing the actual backend reader, rather than treating a lagging generated schema as the whole property contract.
| physics_material_frags = ( | ||
| cfg.physics_material if isinstance(cfg.physics_material, (list, tuple)) else [cfg.physics_material] | ||
| ) | ||
| is_rigid_material = isinstance(cfg.physics_material, RigidBodyMaterialCfg) or all( |
There was a problem hiding this comment.
This only partially addresses the earlier mesh-spawner finding. RigidBodyMaterialCfg is the deprecated PhysX alias and is a subclass of the recommended PhysxRigidBodyMaterialCfg, so the canonical PhysX cfg is not an instance of it. The same check rejects #5276 NewtonMaterialPropertiesCfg, even though both derive from RigidBodyMaterialBaseCfg and both are accepted by spawn_physics_material. Thus a rigid MeshCfg still fails before the supposedly backend-neutral dispatcher for valid legacy cfgs.
Please import/check the core RigidBodyMaterialBaseCfg here and add a real mesh regression using at least PhysxRigidBodyMaterialCfg (ideally Newton legacy too). The new fragment-list test cannot catch this transition path, and retaining this alias check keeps a PhysX-specific dependency in core.
| physics_material: materials.RigidBodyMaterialCfg = materials.RigidBodyMaterialCfg() | ||
| """Physics material properties. Defaults to the default rigid body material.""" | ||
| physics_material: ( | ||
| materials.PhysicsMaterialCfg | materials.RigidBodyMaterialFragment | list[materials.RigidBodyMaterialFragment] |
There was a problem hiding this comment.
The fragment case and binding test are fixed, but this legacy side is now broader than the old rigid-only contract: PhysicsMaterialCfg also includes deformable and surface-deformable material cfgs. spawn_physics_material will happily spawn one and the ground-plane path will bind it to a rigid collision plane.
Please use the backend-neutral rigid base here: RigidBodyMaterialBaseCfg | RigidBodyMaterialFragment | list[RigidBodyMaterialFragment]. That preserves #5275 backend subclassing (including Newton) without advertising deformable materials on a rigid-only spawner.
| :attr:`~isaaclab_physx.sim.spawners.materials.PhysxMaterialCfg.compliant_contact_acceleration_spring` | ||
| (writes ``physxMaterial:compliantContactAccelerationSpring``), completing the fragment's coverage | ||
| of ``PhysxMaterialAPI``. Also added the same two fields to the legacy | ||
| :class:`~isaaclab_physx.sim.spawners.materials.PhysxRigidBodyMaterialCfg`. |
There was a problem hiding this comment.
This compatibility claim is not implemented: PhysxRigidBodyMaterialCfg still stops at compliant_contact_stiffness, compliant_contact_damping, friction_combine_mode, and restitution_combine_mode; the two new fields exist only on PhysxMaterialCfg. Please either add them to the supported legacy cfg with coverage, as stated here, or remove/correct this changelog sentence. Adding them is the cleaner transition because the existing metadata writer already handles them and it keeps legacy/new property parity during the deprecation window.
ooctipus
left a comment
ooctipus
left a comment
There was a problem hiding this comment.
Follow-up review of a8bc6aab4f2 against my prior review at bd0f0632790.
The response materially improves the PR:
UsdPhysicsRigidBodyMaterialCfgnow covers all 4 registeredMaterialAPIattributes, with a useful exact-parity test.PhysxMaterialCfgnow covers all 6 registered Kit 110PhysxMaterialAPIattributes.- A Newton fragment was added in the correct backend package and composes with the USD fragment.
- The fragment-list mesh path and ground-plane path now have real spawner/binding tests.
- Current CI is fully green and the diff passes whitespace validation.
I still need changes for two substantive gaps:
- The new Newton fragment matches the lagging generated schema (2 fields), but not the actual Newton reader pinned by this repository (6 material fields). The pinned resolver additionally consumes contact stiffness, damping, friction gain, and adhesion from the bound Newton material. Per the runtime-reader audit precedent in #5276, those four canonical material attributes need to be represented and tested.
- The mesh integration fix still checks the deprecated PhysX
RigidBodyMaterialCfgalias. It therefore rejects the recommendedPhysxRigidBodyMaterialCfgandNewtonMaterialPropertiesCfg, despite both deriving from the core rigid-material base and being accepted by the dispatcher. UsingRigidBodyMaterialBaseCfgrestores the clean #5275 core/backend boundary.
Two narrower inconsistencies are inline as well: GroundPlaneCfg now advertises generic/deformable material cfgs despite being rigid-only, and the PhysX changelog claims two fields were added to the legacy cfg when they were only added to the fragment.
So the original five comments are largely addressed, but property completeness must include the active backend consumer, and the remaining legacy transition guard is not yet backend-neutral.
The mesh spawner's rigid-vs-deformable guard checked isinstance against the deprecated RigidBodyMaterialCfg leaf alias instead of the neutral RigidBodyMaterialBaseCfg, wrongly rejecting the canonical PhysxRigidBodyMaterialCfg and Newton's legacy NewtonMaterialPropertiesCfg. GroundPlaneCfg.physics_material was widened to the common PhysicsMaterialCfg base, which also advertises deformable materials on a rigid-only ground plane. Narrow both to RigidBodyMaterialBaseCfg. Also close two gaps flagged in review: add the four newton:* contact attributes (contactStiffness/contactDamping/contactFrictionGain/ contactAdhesion) that Newton's pinned-commit USD schema resolver reads in place of the deprecated per-shape ke/kd/kf/ka parameters, and add damping_combine_mode / compliant_contact_acceleration_spring to the legacy PhysxRigidBodyMaterialCfg so it matches the changelog's existing claim of parity with the PhysxMaterialCfg fragment.
|
Thanks @ooctipus — round 2 addressed in 2d70ad4. Two of these were real bugs in my prior fix; good catches.
Tests: |
ooctipus
left a comment
ooctipus
left a comment
There was a problem hiding this comment.
Re-reviewed the latest head. The previously reported schema-coverage issues are fixed: the USD, PhysX, and Newton fragments now cover their full active property surfaces, and the core/backend separation is clean. I am keeping my existing changes-requested state because four integration/transition gaps remain in the inline comments below. I also confirmed the existing explicit-stage thread with a two-stage local reproduction and replied there rather than duplicating it. Nonblocking test note: the Newton six-attribute test proves USD authoring, but still does not exercise SchemaResolverNewton parsing as previously requested.
| """ | ||
| physics_material: materials.PhysicsMaterialCfg | None = None | ||
| physics_material: ( | ||
| materials.PhysicsMaterialCfg |
There was a problem hiding this comment.
Blocking: ShapeCfg is a rigid-object spawner, but this legacy arm remains PhysicsMaterialCfg, which also admits volume and surface deformable material cfgs. spawn_physics_material() will spawn and bind those rather than reject them. Please mirror the now-correct GroundPlaneCfg contract here: RigidBodyMaterialBaseCfg | RigidBodyMaterialFragment | list[RigidBodyMaterialFragment] | None, and add acceptance/rejection coverage. This keeps backend legacy subclasses admissible without advertising deformable materials on a rigid-only spawner.
There was a problem hiding this comment.
Fixed in c84801e. ShapeCfg.physics_material is now RigidBodyMaterialBaseCfg | RigidBodyMaterialFragment | list[RigidBodyMaterialFragment] | None — backend legacy subclasses stay admissible, deformables are no longer advertised. For the record, the broad PhysicsMaterialCfg arm predates this PR (develop has it today); tightened here since this PR defines the fragment-era slot contract. Coverage is structural rather than per-slot: test_material_slot_unions_match_fragment_kind-style enforcement in 1983a59 asserts every rigid-only slot (ShapeCfg, GroundPlaneCfg, TerrainImporterCfg, SimulationCfg) excludes PhysicsMaterialCfg and admits base+fragments, and that the mixed slots (FileCfg, MeshCfg) keep it — so a future slot with the wrong union fails CI.
| ``physics:*`` friction/restitution), plus the family writer | ||
| :func:`~isaaclab.sim.spawners.materials.spawn_rigid_body_material_from_fragments` and the slot | ||
| dispatcher :func:`~isaaclab.sim.spawners.materials.spawn_physics_material`. Spawner | ||
| ``physics_material`` slots now accept a list of single-namespace fragments in addition to the |
There was a problem hiding this comment.
Blocking completeness gap: the public terrain material slot still bypasses this dispatcher. Plane terrain now reaches GroundPlaneCfg, but generated terrain reaches terrains/utils.py:create_prim_from_mesh, which still calls physics_material_cfg.func(...). On this head I reproduced a fragment list failing there with AttributeError: 'list' object has no attribute 'func'. Please widen TerrainImporterCfg.physics_material, route the generated-mesh utility through spawn_physics_material, and add a generator-terrain regression test.
There was a problem hiding this comment.
Fixed in 435240e. create_prim_from_mesh now routes through spawn_physics_material (your AttributeError repro is the regression test — verified failing before the fix with exactly that error), and TerrainImporterCfg.physics_material is widened to RigidBodyMaterialBaseCfg | RigidBodyMaterialFragment | list[RigidBodyMaterialFragment]. The generated-terrain test asserts both physics:* and newton:* attrs authored end-to-end through the fragment list.
| restitution: float | None = None | ||
| """The restitution coefficient. Writes ``physics:restitution``.""" | ||
|
|
||
| density: float | None = None |
There was a problem hiding this comment.
Blocking under the transition-parity convention: this makes the fragment complete at 4/4 UsdPhysics.MaterialAPI properties, but RigidBodyMaterialBaseCfg above remains 3/4 and cannot author material density. Please also add density: float | None = None to the legacy base and cover its authoring path. That is backward-compatible because None is unauthored, and it matches the fragment/legacy parity established by #6263 and the PhysX legacy parity fix in this revision.
There was a problem hiding this comment.
Fixed in bb3226f — density: float | None = None on RigidBodyMaterialBaseCfg (None default = unauthored, backward compatible; the metadata-driven writer picks it up under physics:*). Authoring test covers the set and unset cases. 1983a59 additionally adds an introspective parity test (fragment fields == legacy fields per backend, minus func) so this class of asymmetry fails CI instead of waiting for review.
| list passed to | ||
| :func:`~isaaclab.sim.spawners.materials.spawn_rigid_body_material_from_fragments`. For the | ||
| legacy (non-fragment) equivalent, see | ||
| :class:`~isaaclab_newton.sim.schemas.NewtonMaterialPropertiesCfg`. |
There was a problem hiding this comment.
Blocking: this calls NewtonMaterialPropertiesCfg the legacy equivalent, but the fragment and pinned runtime reader now have six properties while that legacy cfg still has only torsional/rolling friction. Please add contact_stiffness, contact_damping, contact_friction_gain, and contact_adhesion as optional fields to the legacy cfg and test them. This is the same transition parity used by #6254/#6273 and already applied to the PhysX material cfg in this revision.
There was a problem hiding this comment.
| """ | ||
|
|
||
| contact_friction_gain: float | None = None | ||
| """Friction-force stiffness gain used by the tangential (friction) contact response [N/m]. |
There was a problem hiding this comment.
Nonblocking documentation correction: the pinned Newton runtime documents Model.shape_material_kf as a tangential friction response gain in [N·s/m], not [N/m]. Please update this unit so the newly exposed property's contract matches its consumer.
There was a problem hiding this comment.
Fixed in 85592af — [N·s/m] in both the fragment and the (new) legacy field docstrings, matching the pinned runtime doc for Model.shape_material_kf.
RigidBodyMaterialBaseCfg previously exposed only friction and restitution, while the newer UsdPhysicsRigidBodyMaterialCfg fragment also authors density. This left the legacy and fragment paths non-interchangeable and meant Newton's importer, which reads material density, silently ignored it when configs used the legacy base class.
Give NewtonMaterialPropertiesCfg the same four contact-model attributes (stiffness, damping, friction gain, adhesion) already authored by the NewtonMaterialCfg fragment, keeping the legacy and fragment configs in parity. Also corrects the contact_friction_gain docstring unit from N/m to N*s/m in both classes.
Generated-terrain mesh spawning called the physics material cfg's func directly, crashing with AttributeError when given a fragment list. Route it through spawn_physics_material instead, and widen TerrainImporterCfg.physics_material to accept a legacy cfg, a single fragment, or a fragment list.
Widen SimulationCfg.physics_material to accept the fragment forms (a single rigid-body fragment or list of them) in addition to a legacy material cfg, and have the PhysX manager spawn it through spawn_physics_material instead of calling the legacy .func directly. Also swap the default instance from the deprecated RigidBodyMaterialCfg to its base RigidBodyMaterialBaseCfg, verified to author identical USD.
Route the compliant-contact USD spawner's material creation through spawn_physics_material instead of calling the legacy cfg's .func directly, keeping it consistent with the other USD spawner call sites. Also pass the stage to the ground-plane physics-material call, matching its sibling call sites in the same file.
Shapes are rigid-only spawners, so the physics_material slot now accepts the rigid material base class or rigid-material fragments, and no longer advertises deformable material configurations.
Legacy material cfgs resolve the stage internally via get_current_stage() and ignore the explicit stage argument, so an explicit non-current stage was silently authoring on the wrong stage instead of on the one the caller asked for. Raise ValueError in that case; passing None or the current stage explicitly still works, which keeps the in-tree spawners that pass stage=stage unconditionally working unchanged.
Adds two enforcement tests: one asserts each backend's fragment dataclass authors exactly the same fields as its legacy cfg counterpart, and one asserts each material slot's type union matches whether its spawner is rigid-only or also spawns deformables. These catch field or slot-typing drift structurally instead of relying on manual review.
Backfill changelog fragments for the density parity, Newton contact attribute parity, terrain/default-material/compliant-contact routing, ShapeCfg narrowing, and stage-rejection changes, and correct the mesh-spawner rigid-material guard entry from a widening to a fix (the deprecated-leaf-class check was rejecting valid rigid materials).
Document the behavior change where PhysX's default physics-material spawn now goes through the unified spawn_physics_material function, enabling list-of-fragments support in SimulationCfg.
The physics_material defaults on SimulationCfg, TerrainImporterCfg, and GroundPlaneCfg were switched from the deprecated PhysX leaf RigidBodyMaterialCfg to the core RigidBodyMaterialBaseCfg. The authored USD is unchanged, but downstream code that mutates the default material in place to set PhysX-only fields (e.g. combine modes) would now silently author nothing since those fields no longer exist on the base class. Call this out as a breaking change in the changelog, with migration guidance to assign a PhysxRigidBodyMaterialCfg instance instead. Also fix GroundPlaneCfg itself, which still constructed the deprecated leaf class as its default despite already accepting the base class in its type union. Also clean up stale docstrings that predate the density field, and tighten changelog wording: spell out the three accepted forms for physics_material slots, mark the stage-rejection change as breaking, and reword the Newton material parity entry to be self-contained instead of pointing at another bullet.
|
While addressing the latest review I audited the full material-slot surface (every
To keep these from recurring, 1983a59 adds two structural tests: an introspective fragment↔legacy field-parity check per backend, and a slot-union check asserting each Test state: |
github-actions
Bot
added
the
documentation
## Description Converts **rigid-body physics materials** to the single-namespace "fragment" model already used by the rigid-body / collision / mass / mesh / tendon / joint-drive / articulation families. Additive and backward-compatible — the legacy inheritance cfgs remain as deprecated shims. The one material-specific twist vs. the schema families: a physics material is **spawned as its own `UsdShade.Material` prim and bound** (not applied onto the body prim). So the family writer both spawns the prim + applies the anchor *and* dispatches the fragment list. ### Added - **Core** (`isaaclab.sim.spawners.materials`): - `RigidBodyMaterialFragment` — marker base typing the `physics_material` slot. - `UsdPhysicsRigidBodyMaterialCfg` — solver-common `physics:*` friction/restitution (anchor `UsdPhysics.MaterialAPI`). - `spawn_rigid_body_material_from_fragments(prim_path, fragments, stage)` — spawns the `UsdShade.Material` prim, applies the `MaterialAPI` anchor, dispatches each fragment's `func` (default `apply_namespaced`). - `spawn_physics_material(prim_path, material, stage)` — slot dispatcher: fragment list → fragment writer, else legacy cfg via its own `func`. - **PhysX** (`isaaclab_physx.sim.spawners.materials`): - `PhysxMaterialCfg` — single-namespace `physxMaterial:*` (`PhysxMaterialAPI`): compliant-contact spring + combine-mode tokens. ### Changed - Spawner `physics_material` slots (`shapes`, `meshes`, `from_files`) now accept `RigidBodyMaterialFragment | list[...]` in addition to the legacy material cfg; consume sites route through `spawn_physics_material`. **Legacy single-cfg path unchanged.** ### Scope - **Rigid-body materials only.** Deformable-body materials are deferred — they pair with the deferred deformable-body family (multi-inherited OmniPhysics + PhysX material APIs). ## Tests - New `test_material_fragments.py` (6): fragment metadata; spawn-from-fragments composes `physics:*` + `physxMaterial:*` with the `MaterialAPI` anchor + `PhysxMaterialAPI`; single-fragment; partial-update (None left unauthored); slot dispatcher handles both fragment and legacy forms. - Regression: existing `test_spawn_materials.py` (6) and `test_spawn_shapes.py` (12) pass — legacy path intact. ## Checklist - [x] Ran `./isaaclab.sh --format` - [x] Added changelog fragments (core + physx, minor) --------- Co-authored-by: Octi Zhang <zhengyuz@nvidia.com>
2 participants
Description
Converts rigid-body physics materials to the single-namespace "fragment" model already used by the rigid-body / collision / mass / mesh / tendon / joint-drive / articulation families. Additive and backward-compatible — the legacy inheritance cfgs remain as deprecated shims.
The one material-specific twist vs. the schema families: a physics material is spawned as its own
UsdShade.Materialprim and bound (not applied onto the body prim). So the family writer both spawns the prim + applies the anchor and dispatches the fragment list.Added
isaaclab.sim.spawners.materials):RigidBodyMaterialFragment— marker base typing thephysics_materialslot.UsdPhysicsRigidBodyMaterialCfg— solver-commonphysics:*friction/restitution (anchorUsdPhysics.MaterialAPI).spawn_rigid_body_material_from_fragments(prim_path, fragments, stage)— spawns theUsdShade.Materialprim, applies theMaterialAPIanchor, dispatches each fragment'sfunc(defaultapply_namespaced).spawn_physics_material(prim_path, material, stage)— slot dispatcher: fragment list → fragment writer, else legacy cfg via its ownfunc.isaaclab_physx.sim.spawners.materials):PhysxMaterialCfg— single-namespacephysxMaterial:*(PhysxMaterialAPI): compliant-contact spring + combine-mode tokens.Changed
physics_materialslots (shapes,meshes,from_files) now acceptRigidBodyMaterialFragment | list[...]in addition to the legacy material cfg; consume sites route throughspawn_physics_material. Legacy single-cfg path unchanged.Scope
Tests
test_material_fragments.py(6): fragment metadata; spawn-from-fragments composesphysics:*+physxMaterial:*with theMaterialAPIanchor +PhysxMaterialAPI; single-fragment; partial-update (None left unauthored); slot dispatcher handles both fragment and legacy forms.test_spawn_materials.py(6) andtest_spawn_shapes.py(12) pass — legacy path intact.Checklist
./isaaclab.sh --format