Skip to content

Adding Mesh Collision USD data classes and writers - #6264

Merged
vidurv-nvidia merged 14 commits into
isaac-sim:developfrom
vidurv-nvidia:vidurv/schema-frag-meshcollision
Jun 27, 2026
Merged

Adding Mesh Collision USD data classes and writers#6264
vidurv-nvidia merged 14 commits into
isaac-sim:developfrom
vidurv-nvidia:vidurv/schema-frag-meshcollision

Conversation

@vidurv-nvidia

@vidurv-nvidia vidurv-nvidia commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the mesh-collision schema-fragment API (collision-mesh cooking approximations).

  • MeshCollisionFragment marker + UsdPhysicsMeshCollisionCfg (physics:approximation token; UsdPhysics.MeshCollisionAPI anchor) in isaaclab.
  • PhysX cooking fragments in isaaclab_physxPhysxConvexHullCfg, PhysxConvexDecompositionCfg, PhysxTriangleMeshCfg, PhysxSDFMeshCfg (each its own physx*Collision:* namespace + applied schema).
  • NewtonMeshCollisionCfg / NewtonSDFCollisionCfg in isaaclab_newton.
  • apply_mesh_collision — the default func every MeshCollisionFragment carries. It writes the fragment's backend cooking namespace (via apply_namespaced) and the shared physics:approximation token its mesh_approximation_name implies (skipping "none" so it does not clobber). This keeps the approximation-token coupling with the fragment, matching the tendon fragments where the backend application logic lives in a func rather than being special-cased by the caller.
  • apply_mesh_collision_properties family writer — applies the MeshCollisionAPI anchor, then dispatches each fragment via its func and aggregates the results. It is the same generic shape as the sibling family writers (apply_collision_properties, apply_rigid_body_properties); the shared-token "last non-none wins" behavior emerges from dispatch order rather than a special reduction pass in the writer body. Core imports no backend.
  • The spawner mesh_collision_props slot now also accepts a fragment list.

This PR is purely additive and self-contained: it builds only on the single-namespace schema-fragment base (SchemaFragment + apply_namespaced) already in develop, existing call sites are untouched (a transition bridge routes legacy single cfgs to the existing define_/modify_ writers), and it does not depend on any other open PR.

Type of change

  • New feature (non-breaking change which adds functionality)

Screenshots

N/A — non-visual API change.

Checklist

  • I have read and understood the contribution guidelines
  • I have run the pre-commit checks with ./isaaclab.sh --format
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump extension.toml — CI handles that)
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

Add the additive mesh-collision schema-fragment family, mirroring the
rigid-body pilot. Core gains the MeshCollisionFragment marker, the
UsdPhysicsMeshCollisionCfg fragment carrying the physics:approximation
token, and the apply_mesh_collision_properties family writer (applies the
MeshCollisionAPI anchor, resolves and validates the approximation token
from whichever cooking fragment is present, then dispatches each fragment
via its func).

PhysX cooking fragments (PhysxConvexHullCfg, PhysxConvexDecompositionCfg,
PhysxTriangleMeshCfg, PhysxTriangleMeshSimplificationCfg, PhysxSDFMeshCfg)
and Newton cooking fragments (NewtonMeshCollisionCfg, NewtonSDFCollisionCfg)
each own a single namespace and applied schema, dispatched through the
generic apply_namespaced applier.

Widen the MeshConverterCfg.mesh_collision_props slot to accept a fragment
list and add the transition bridge in mesh_converter.py without breaking
the legacy single-cfg path. Legacy cfgs and modify_mesh_collision_properties
stay intact.
No behavior change; collapse over-explained inline comments to terse intent.
@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label Jun 25, 2026
The rigid transition shim referenced schemas.SchemaFragment, but the
schemas module does not define SchemaFragment (it lives in schemas_cfg).
SchemaFragment is already imported directly in this file; use it, matching
the mesh-collision shim and fixing the AttributeError at convert time.
# Conflicts:
#	source/isaaclab/isaaclab/sim/__init__.pyi
#	source/isaaclab/isaaclab/sim/converters/mesh_converter.py
#	source/isaaclab/isaaclab/sim/schemas/__init__.pyi
The develop merge brought in the mass shim using schemas.SchemaFragment
(package form), but this branch imports the schemas module (which lacks
SchemaFragment) and references the directly-imported SchemaFragment.
Make the mass-shim checks use the bare SchemaFragment too, matching the
mesh-collision and rigid shims and fixing the AttributeError.
Bring apply_mesh_collision_properties in line with the sibling apply_*
writers (already done for rigid body and mass): add the prim-validity
ValueError guard, aggregate per-fragment results instead of unconditionally
returning True, and annotate the fragments param as
Iterable[MeshCollisionFragment]. Add invalid-prim and aggregation tests.
@vidurv-nvidia
vidurv-nvidia marked this pull request as ready for review June 25, 2026 21:15
The newton:sdf*/hydroelastic* attributes (and detection of the unregistered
NewtonSDFCollisionAPI token via the raw apiSchemas list-op) are consumed by
Newton's USD importer starting in Newton 1.3.0; on older builds they are
authored but inert. Document this so the attributes' effect is not a surprise.
@greptile-apps

greptile-apps Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces a new mesh-collision schema-fragment API: MeshCollisionFragment / UsdPhysicsMeshCollisionCfg in isaaclab, PhysX cooking fragments (PhysxConvexHullCfg, PhysxConvexDecompositionCfg, PhysxTriangleMeshCfg, PhysxTriangleMeshSimplificationCfg, PhysxSDFMeshCfg) in isaaclab_physx, and Newton fragments (NewtonMeshCollisionCfg, NewtonSDFCollisionCfg) in isaaclab_newton, together with the apply_mesh_collision/apply_mesh_collision_properties family writers.

  • New fragment hierarchy: MeshCollisionFragment marker → per-backend cooking fragment classes, each owning one physx*Collision:* or newton:* namespace and its applied schema. The physics:approximation token is resolved from whichever cooking fragment carries a non-"none" mesh_approximation_name (last-wins composition).
  • Transition bridge in MeshConverter: routes SchemaFragment lists to apply_mesh_collision_properties and falls back to the legacy define_mesh_collision_properties for existing MeshCollisionBaseCfg callers — existing call sites are untouched.
  • Generator safety: apply_mesh_collision_properties now iterates fragments exactly once (fixing the previously-flagged double-iteration bug), confirmed by a dedicated generator test.

Confidence Score: 5/5

Purely additive, no existing call sites changed; the transition bridge correctly routes legacy cfgs and all new fragment paths are exercised by the new test suite.

The change is self-contained and additive. The only issues found are documentation-level: the generic apply_namespaced now contains a hardcoded field-name skip that creates implicit coupling, and the mesh_approximation_name docstring on UsdPhysicsMeshCollisionCfg doesn't clarify that 'none' is a composition skip-sentinel rather than a token that gets written. Neither affects runtime correctness.

The apply_namespaced skip in schemas.py and the mesh_approximation_name field docstring in schemas_cfg.py are worth a second look for long-term maintainability.

Important Files Changed

Filename Overview
source/isaaclab/isaaclab/sim/schemas/schemas.py Adds apply_mesh_collision and apply_mesh_collision_properties writers; also patches the generic apply_namespaced to skip mesh_approximation_name globally. Single-loop dispatch correctly handles generators.
source/isaaclab/isaaclab/sim/schemas/schemas_cfg.py Adds MeshCollisionFragment marker and UsdPhysicsMeshCollisionCfg; _usd_namespace="physics" is set but no physics-namespaced attrs are written via apply_namespaced (all fields are skipped). Logic is correct but the namespace declaration is cosmetically misleading.
source/isaaclab/isaaclab/sim/converters/mesh_converter.py Transition bridge correctly routes SchemaFragment lists to the new apply_mesh_collision_properties and falls back to legacy define_mesh_collision_properties for MeshCollisionBaseCfg instances.
source/isaaclab_physx/isaaclab_physx/sim/schemas/schemas_cfg.py Adds five PhysX cooking fragments (ConvexHull, ConvexDecomposition, TriangleMesh, TriangleMeshSimplification, SDFMesh); each sets mesh_approximation_name to the appropriate token, with PhysxTriangleMeshCfg correctly using "none" as the skip sentinel.
source/isaaclab_newton/isaaclab_newton/sim/schemas/schemas_cfg.py Adds NewtonMeshCollisionCfg and NewtonSDFCollisionCfg fragments; neither declares mesh_approximation_name (getattr returns None → token writing skipped), correctly deferring the physics:approximation token to the PhysX/USD fragment in the same list.
source/isaaclab/test/sim/test_mesh_collision_fragments.py Comprehensive tests covering fragment metadata, token writing, namespace isolation, composition, generator consumption, failure aggregation, and public imports. Covers the previously-flagged generator double-iteration issue with a dedicated test.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[MeshConverterCfg.mesh_collision_props] --> B{Type check}
    B -- "list/tuple" --> C[mesh_collision_frags = as-is]
    B -- "single value" --> D[mesh_collision_frags = wrapped in list]
    C --> E{all SchemaFragment?}
    D --> E
    E -- "Yes" --> F[apply_mesh_collision_properties]
    E -- "No" --> G[define_mesh_collision_properties - legacy path]
    F --> H[Apply UsdPhysics.MeshCollisionAPI anchor]
    H --> I[For each MeshCollisionFragment]
    I --> J{cfg.func callable?}
    J -- "Yes" --> K[call func directly]
    J -- "No" --> L[string_to_callable - default: apply_mesh_collision]
    K --> M[apply_mesh_collision]
    L --> M
    M --> N[Validate prim]
    N --> O[apply_namespaced - writes namespace attrs, skips func + mesh_approximation_name]
    O --> P{mesh_approximation_name != 'none'?}
    P -- "Yes" --> Q[Validate token in MESH_APPROXIMATION_TOKENS]
    Q --> R[Write physics:approximation token]
    P -- "No" --> S[Skip - leave token unchanged]
    R --> T[return success]
    S --> T
Loading
%%{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[MeshConverterCfg.mesh_collision_props] --> B{Type check}
    B -- "list/tuple" --> C[mesh_collision_frags = as-is]
    B -- "single value" --> D[mesh_collision_frags = wrapped in list]
    C --> E{all SchemaFragment?}
    D --> E
    E -- "Yes" --> F[apply_mesh_collision_properties]
    E -- "No" --> G[define_mesh_collision_properties - legacy path]
    F --> H[Apply UsdPhysics.MeshCollisionAPI anchor]
    H --> I[For each MeshCollisionFragment]
    I --> J{cfg.func callable?}
    J -- "Yes" --> K[call func directly]
    J -- "No" --> L[string_to_callable - default: apply_mesh_collision]
    K --> M[apply_mesh_collision]
    L --> M
    M --> N[Validate prim]
    N --> O[apply_namespaced - writes namespace attrs, skips func + mesh_approximation_name]
    O --> P{mesh_approximation_name != 'none'?}
    P -- "Yes" --> Q[Validate token in MESH_APPROXIMATION_TOKENS]
    Q --> R[Write physics:approximation token]
    P -- "No" --> S[Skip - leave token unchanged]
    R --> T[return success]
    S --> T
Loading

Reviews (3): Last reviewed commit: "Route mesh-collision fragments through a..." | Re-trigger Greptile

Comment on lines +524 to +545
# resolve the shared approximation token: last fragment with a non-"none" name wins
approximation_name = "none"
for cfg in fragments:
name = getattr(cfg, "mesh_approximation_name", None)
if name is not None and name != "none":
approximation_name = name
if approximation_name not in MESH_APPROXIMATION_TOKENS:
raise ValueError(
f"Invalid mesh approximation name: '{approximation_name}'. "
f"Valid options are: {list(MESH_APPROXIMATION_TOKENS.keys())}"
)
approximation_token = MESH_APPROXIMATION_TOKENS[approximation_name]
safe_set_attribute_on_usd_schema(
UsdPhysics.MeshCollisionAPI(prim), "Approximation", approximation_token, camel_case=False
)

# dispatch each fragment via its ``func`` (cooking-schema application + namespaced tuning attrs),
# aggregating per-fragment results so a reported failure is not masked by the always-applied anchor
success = True
for cfg in fragments:
func = cfg.func if callable(cfg.func) else string_to_callable(cfg.func)
success = bool(func(cfg, prim_path, stage)) and success

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.

P1 Silent no-op when a generator is passed as fragments

apply_mesh_collision_properties iterates over fragments twice — first to resolve approximation_name (lines 526-529) and again to dispatch each fragment's func (lines 543-545). When the caller supplies a one-shot iterable (generator, map(...), filter(...), etc.), the first loop exhausts it; the second loop produces no iterations, so no fragment attributes are ever written. Yet success is returned as True because it was never set to False, making the failure completely silent.

The type annotation explicitly advertises Iterable, which includes generators. Materializing to a list at the top of the function (fragments = list(fragments)) is the standard fix and aligns with the pattern already used in apply_rigid_body_properties (which only iterates once and never encounters this issue).

Suggested change
# resolve the shared approximation token: last fragment with a non-"none" name wins
approximation_name = "none"
for cfg in fragments:
name = getattr(cfg, "mesh_approximation_name", None)
if name is not None and name != "none":
approximation_name = name
if approximation_name not in MESH_APPROXIMATION_TOKENS:
raise ValueError(
f"Invalid mesh approximation name: '{approximation_name}'. "
f"Valid options are: {list(MESH_APPROXIMATION_TOKENS.keys())}"
)
approximation_token = MESH_APPROXIMATION_TOKENS[approximation_name]
safe_set_attribute_on_usd_schema(
UsdPhysics.MeshCollisionAPI(prim), "Approximation", approximation_token, camel_case=False
)
# dispatch each fragment via its ``func`` (cooking-schema application + namespaced tuning attrs),
# aggregating per-fragment results so a reported failure is not masked by the always-applied anchor
success = True
for cfg in fragments:
func = cfg.func if callable(cfg.func) else string_to_callable(cfg.func)
success = bool(func(cfg, prim_path, stage)) and success
if stage is None:
stage = get_current_stage()
prim = stage.GetPrimAtPath(prim_path)
# fail loudly on an invalid path (matches the sibling apply_* writers)
if not prim.IsValid():
raise ValueError(f"Prim path '{prim_path}' is not valid.")
# materialize so the list can be iterated twice (approximation scan + dispatch)
fragments = list(fragments)
# apply the standard MeshCollisionAPI anchor (carrier of ``physics:approximation``)
if not UsdPhysics.MeshCollisionAPI(prim):
UsdPhysics.MeshCollisionAPI.Apply(prim)
# resolve the shared approximation token: last fragment with a non-"none" name wins
approximation_name = "none"
for cfg in fragments:

apply_mesh_collision_properties iterates fragments twice (approximation-token
resolution, then dispatch), so a generator/map/filter passed as the Iterable
was exhausted by the first pass -- the dispatch loop wrote nothing yet still
returned True (silent no-op). Materialize to a list once at the top. Add a
generator regression test.
…-meshcollision

# Conflicts:
#	source/isaaclab/isaaclab/sim/__init__.pyi
#	source/isaaclab/isaaclab/sim/schemas/__init__.pyi
#	source/isaaclab/isaaclab/sim/schemas/schemas_cfg.py
Move the physics:approximation token coupling out of the
apply_mesh_collision_properties writer and into apply_mesh_collision,
the default func every MeshCollisionFragment now carries. The writer
collapses to the same generic shape as the sibling family writers
(apply the MeshCollisionAPI anchor, then dispatch each fragment's
func), and the shared-token last-non-none-wins behavior now emerges
from dispatch order rather than a special reduction pass in the
writer body.

This makes mesh collision consistent with the tendon fragments, where
the backend application logic lives in a func referenced by the
fragment rather than being special-cased by the caller.
mesh_converter.py imports the schemas module (from isaaclab.sim.schemas
import schemas), which does not export SchemaFragment (it lives in
schemas_cfg). The collider transition shim used schemas.SchemaFragment,
raising AttributeError on any collision_props. Use the bare SchemaFragment
already imported in this file, matching the other shims here.
…-meshcollision

# Conflicts:
#	source/isaaclab/isaaclab/sim/schemas/schemas_cfg.py
@vidurv-nvidia
vidurv-nvidia merged commit 25b30c1 into isaac-sim:develop Jun 27, 2026
60 of 61 checks passed
@vidurv-nvidia
vidurv-nvidia deleted the vidurv/schema-frag-meshcollision branch June 27, 2026 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants