Skip to content

Adding Hydroelastic and SDF collision configuration to IsaacLab#5160

Open
vidurv-nvidia wants to merge 11 commits intoisaac-sim:developfrom
vidurv-nvidia:vidur/feature/sdf-collision
Open

Adding Hydroelastic and SDF collision configuration to IsaacLab#5160
vidurv-nvidia wants to merge 11 commits intoisaac-sim:developfrom
vidurv-nvidia:vidur/feature/sdf-collision

Conversation

@vidurv-nvidia
Copy link
Copy Markdown

@vidurv-nvidia vidurv-nvidia commented Apr 2, 2026

Description

Adds SDF (Signed Distance Field) collision and hydroelastic contact support to Newton's IsaacLab integration. Since Newton cannot read SDF designation from USD schemas, SDF is configured entirely through runtime config and applied at the ModelBuilder level before model finalization.

New config classes

SDFCfg — configures SDF mesh collision via Newton's mesh.build_sdf() API:

  • max_resolution / target_voxel_size — SDF grid resolution
  • body_patterns / shape_patterns — regex patterns to select which bodies/shapes get SDF (matching against body_label and shape_label respectively)
  • pattern_resolutions — per-pattern resolution overrides
  • use_visual_meshes — optionally create collision shapes from visual meshes for bodies lacking collision geometry
  • hydroelastic_cfg — nested hydroelastic contact configuration

HydroelasticCfg — configures distributed surface contacts via SDF overlap:

  • k_hydro, reduce_contacts, normal_matching, moment_matching, buffer multipliers, etc.

NewtonCollisionPipelineCfg — configures Newton's collision pipeline parameters:

  • broad_phase, reduce_contacts, rigid_contact_max, max_triangle_pairs, etc.

Usage on NewtonCfg:

NewtonCfg(
    solver_cfg=MJWarpSolverCfg(use_mujoco_contacts=False),
    collision_cfg=NewtonCollisionPipelineCfg(broad_phase="explicit"),
    sdf_cfg=SDFCfg(
        max_resolution=256,
        body_patterns=[".*robot.*"],
        hydroelastic_cfg=HydroelasticCfg(k_hydro=1e10),
    ),
)

Implementation

  • _build_sdf_on_mesh — helper that builds SDF on a mesh with per-pattern resolution overrides
  • _apply_sdf_config — collects matching shapes via body/shape patterns, patches collision meshes with SDF, optionally creates collision from visual meshes
  • _create_sdf_collision_from_visual — creates collision shapes from visual meshes for matched bodies that lack collision geometry
  • _create_collision_pipeline — constructs CollisionPipeline from collision_cfg + hydroelastic config from sdf_cfg
  • SDF is applied on prototypes in the cloner (before add_builder replication) so it is built once and shared across all environments
  • For non-replicate path, SDF is applied in instantiate_builder_from_stage
  • initialize_solver forces Newton collision pipeline when sdf_cfg or collision_cfg is configured

Cloner integration

Shapes matching SDF patterns are excluded from convex hull approximation in _build_newton_builder_from_mapping, preserving original triangle meshes for mesh.build_sdf().

Type of change

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

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 updated the changelog and the corresponding version in the extension's config/extension.toml file
  • I have added my name to the CONTRIBUTORS.md or my name already exists there

@github-actions github-actions bot added enhancement New feature or request isaac-lab Related to Isaac Lab team labels Apr 2, 2026
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot bot left a comment

Choose a reason for hiding this comment

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

🤖 Isaac Lab Review Bot

Summary

This PR adds SDF (Signed Distance Field) and hydroelastic collision configuration to the Newton physics backend. It introduces three new config dataclasses (SDFCfg, HydroelasticCfg, NewtonCollisionPipelineCfg), integrates SDF mesh building into the model builder pipeline, modifies the cloner to preserve triangle meshes for SDF-targeted shapes, and creates a configurable collision pipeline factory. The overall design is sound but there is one bug that will crash at runtime when collision_cfg is provided.

Design Assessment

Problem: Newton backend needs SDF-based mesh collision and hydroelastic contacts, requiring pre-built SDF on mesh geometry and a configurable collision pipeline.

The PR places SDF config application at the right lifecycle phase — NewtonManager._apply_sdf_config() operates on the ModelBuilder before finalize(), which is correct. Pattern-based matching (regex on body/shape labels) is consistent with how other Newton features configure per-shape properties. The cloner integration to skip convex hull approximation for SDF shapes is architecturally necessary since approximation destroys source geometry needed for mesh.build_sdf().

The coupling between the cloner and PhysicsManager._cfg is a pragmatic tradeoff — threading the SDF config as an explicit parameter would be cleaner but would require API changes to newton_physics_replicate() and all its callers.

Design is sound.

Architecture Impact

New fields collision_cfg and sdf_cfg on NewtonCfg default to None. All access uses getattr(cfg, "sdf_cfg", None), so existing configs without these attributes are safe. The collision pipeline creation path changes from hardcoded to configurable, but default behavior ("explicit" broad phase) is preserved. No cross-module breakage.

Implementation Verdict

Significant concerns — 1 bug that will crash at runtime when collision_cfg is used with default None fields, plus a misleading docstring and minor code quality issues.

Test Coverage

The test file (test_sdf_config.py) has 12 well-structured tests covering _build_sdf_on_mesh and _apply_sdf_config. Good use of mocking and clear assertions. Missing coverage:

  • _create_collision_pipeline is untested (where the bug is)
  • _create_sdf_collision_from_visual is untested
  • Hydroelastic flag application path in _apply_sdf_config is untested
  • Cloner SDF skip logic in newton_replicate.py is untested

A test for _create_collision_pipeline with default config would have caught the None kwargs bug.

CI Status

Only labeler has run (✅). No pre-commit, linter, or test CI checks have executed yet.

Findings

🔴 Critical: newton_manager.py:570 — to_dict() passes None values to CollisionPipeline, will crash at runtime

🟡 Warning: newton_manager_cfg.py:111 — target_voxel_size docstring claims precedence that code doesn't enforce

🟡 Warning: newton_replicate.py:70 — import re belongs at module level

🔵 Improvement: test_sdf_config.py — Missing tests for _create_collision_pipeline, _create_sdf_collision_from_visual, hydroelastic flags, and cloner skip logic

@vidurv-nvidia vidurv-nvidia marked this pull request as ready for review April 3, 2026 01:18
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 3, 2026

Greptile Summary

This PR adds three new configuration classes (SDFCfg, HydroelasticCfg, NewtonCollisionPipelineCfg) to isaaclab_newton that enable SDF-based mesh collision, hydroelastic distributed contacts, and fine-grained control of Newton's collision pipeline. The implementation adds per-body and per-shape regex pattern matching for selective SDF application, per-pattern resolution overrides, and optional creation of collision shapes from visual meshes. The Newton cloner is updated to preserve original triangle meshes (skipping convex-hull approximation) for shapes that will receive SDF, and SDF is built on prototypes before replication so all environment copies share the same SDF data.

Key changes:

  • SDFCfg drives mesh.build_sdf() on matched shapes at builder finalisation time via _apply_sdf_config, called on each prototype in the replicate path and at the end of instantiate_builder_from_stage for the non-replicate path.
  • HydroelasticCfg maps onto HydroelasticSDF.Config and sets HYDROELASTIC shape flags on matched shapes.
  • NewtonCollisionPipelineCfg replaces the hardcoded CollisionPipeline(model, broad_phase="explicit") call with a configurable factory method.
  • Previous review concerns (double _apply_sdf_config call, O(B·S) shape scan, collision shape visibility) are all addressed in this revision.
  • One P1 defect remains: _apply_sdf_config accesses PhysicsManager._cfg without a None guard, but is called unconditionally from newton_replicate.py which does guard against None — this inconsistency creates a crash path if _cfg is ever None in that context.

Confidence Score: 4/5

Safe to merge after addressing the PhysicsManager._cfg None guard inconsistency in _apply_sdf_config.

One P1 defect: _apply_sdf_config is called unconditionally from the replicate path but lacks the same if cfg is None: return guard that the surrounding replicate code uses, creating an AttributeError crash path. All prior review concerns (double-apply, O(B·S) scan, collision shape visibility) are correctly resolved. The rest of the implementation is clean, well-tested, and well-documented.

newton_manager.py (_apply_sdf_config) and newton_replicate.py (unconditional call site) — the None guard inconsistency spans both files.

Important Files Changed

Filename Overview
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py Adds _apply_sdf_config, _build_sdf_on_mesh, _create_sdf_collision_from_visual, and _create_collision_pipeline class methods; _apply_sdf_config lacks a None guard on PhysicsManager._cfg (present inconsistently in the calling replicate code), which can cause an AttributeError crash.
source/isaaclab_newton/isaaclab_newton/cloner/newton_replicate.py Adds SDF-aware mesh approximation filtering per prototype; guards PhysicsManager._cfg for None before reading patterns, but then calls _apply_sdf_config unconditionally, which does not have the same guard, creating a potential crash path.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager_cfg.py Adds HydroelasticCfg, SDFCfg, and NewtonCollisionPipelineCfg dataclasses with comprehensive docstrings; target_voxel_size docstring has misleading "takes precedence" language.
source/isaaclab_newton/test/physics/test_sdf_config.py New unit tests for SDF config logic using mocks; comprehensive coverage of pattern matching, resolution overrides, and edge cases.
source/isaaclab_newton/isaaclab_newton/physics/init.pyi Public API stub updated to export new config classes; clean and complete.
source/isaaclab_newton/docs/CHANGELOG.rst Changelog entry for 0.5.10 added correctly with new version heading, proper RST formatting, and accurate description of new classes.
source/isaaclab_newton/config/extension.toml Version bumped from 0.5.9 to 0.5.10 to match changelog.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[NewtonCfg with sdf_cfg / collision_cfg] --> B{Replicate path?}

    B -- Yes --> C[_build_newton_builder_from_mapping]
    C --> D{has_sdf_patterns?}
    D -- Yes --> E[Filter SDF shapes from convex-hull approx]
    D -- No --> F[approximate_meshes all MESH shapes]
    E --> G[approximate_meshes non-SDF shapes only]
    F --> H[_apply_sdf_config per prototype]
    G --> H
    H --> I[set_builder global_builder]

    B -- No --> J[start_simulation]
    J --> K{_builder is None?}
    K -- Yes --> L[instantiate_builder_from_stage]
    L --> M[_apply_sdf_config builder]
    M --> N[set_builder]
    K -- No --> O[builder already set - SDF already applied]

    N --> P[builder.finalize Model]
    O --> P
    I --> J

    P --> Q[initialize_solver]
    Q --> R{needs_collision_pipeline?}
    R -- Yes --> S[_create_collision_pipeline]
    S --> T{hydro_cfg?}
    T -- Yes --> U[CollisionPipeline with HydroelasticSDF.Config]
    T -- No --> V[CollisionPipeline with pipeline_kwargs]
    R -- No --> W[MuJoCo native contacts]
Loading

Reviews (2): Last reviewed commit: "Raise TypeError for invalid collision_cf..." | Re-trigger Greptile

# Pipeline parameters from collision_cfg (or defaults)
collision_cfg = getattr(cfg, "collision_cfg", None)
if collision_cfg is not None:
pipeline_kwargs = {k: v for k, v in collision_cfg.to_dict().items() if v is not None} if hasattr(collision_cfg, "to_dict") else {}
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 drop of collision_cfg parameters when to_dict() is absent

When collision_cfg is not None but the hasattr(collision_cfg, "to_dict") guard returns False, pipeline_kwargs is set to {} and CollisionPipeline(model) is instantiated with no parameters. The user's NewtonCollisionPipelineCfg settings (broad_phase, reduce_contacts, max_triangle_pairs, etc.) are silently ignored.

configclass in IsaacLab should always expose to_dict(), so this case is unlikely in practice. However, the defensive else: {} fallback creates a confusing failure mode: instead of raising a clear AttributeError, the pipeline silently reverts to Newton defaults, which may differ from what was configured.

Consider either removing the guard (and letting an AttributeError surface a missing method) or explicitly raising:

if collision_cfg is not None:
    if not hasattr(collision_cfg, "to_dict"):
        raise TypeError(
            f"collision_cfg {type(collision_cfg)} does not implement to_dict(). "
            "Only @configclass instances are supported."
        )
    pipeline_kwargs = {k: v for k, v in collision_cfg.to_dict().items() if v is not None}
else:
    pipeline_kwargs = {"broad_phase": "explicit"}

Comment on lines +434 to +444
shape_cfg_kwargs = dict(
density=0.0,
has_shape_collision=True,
has_particle_collision=True,
is_visible=True,
)
if sdf_cfg.margin is not None:
shape_cfg_kwargs["margin"] = sdf_cfg.margin
if hydro_cfg is not None:
shape_cfg_kwargs["is_hydroelastic"] = True
shape_cfg_kwargs["kh"] = hydro_cfg.k_hydro
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 Newly-created collision shape is also marked visible, rendering collision geometry

When use_visual_meshes=True, a new collision shape is added with is_visible=True. This means the collision mesh will be rendered on top of the existing visual mesh, resulting in duplicated (and likely z-fighting) geometry in the viewport.

Unless the intent is to explicitly show collision shapes for debugging purposes, the collision shape should not be visible:

Suggested change
shape_cfg_kwargs = dict(
density=0.0,
has_shape_collision=True,
has_particle_collision=True,
is_visible=True,
)
if sdf_cfg.margin is not None:
shape_cfg_kwargs["margin"] = sdf_cfg.margin
if hydro_cfg is not None:
shape_cfg_kwargs["is_hydroelastic"] = True
shape_cfg_kwargs["kh"] = hydro_cfg.k_hydro
shape_cfg_kwargs = dict(
density=0.0,
has_shape_collision=True,
has_particle_collision=True,
is_visible=False,
)

If visualisation of SDF collision shapes is a desired feature, it should be opt-in and documented in SDFCfg.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Addressed in caca3c3 — now correctly set to .

Comment on lines 607 to 613
# Create builder from USD stage if not provided
if cls._builder is None:
cls.instantiate_builder_from_stage()
else:
# Builder was set externally (e.g. by newton_replicate) — still apply SDF
cls._apply_sdf_config(cls._builder)

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 Double _apply_sdf_config call when instantiate_builder_from_stage is invoked directly

instantiate_builder_from_stage is a public class method that already calls _apply_sdf_config(builder) before set_builder(builder). If a caller invokes instantiate_builder_from_stage() first (setting cls._builder), and start_simulation is then called, cls._builder is not None triggers the else branch and _apply_sdf_config runs a second time on the already-patched builder.

The consequences:

  • Every matching collision mesh has clear_sdf() + build_sdf() called again (wasted GPU/CPU work).
  • With use_visual_meshes=True, _create_sdf_collision_from_visual is re-entered; the second pass gracefully skips adding duplicate shapes (bodies already have collision shapes), but the SDF is rebuilt on the newly-created shapes from the first pass.

The cleanest fix is to make _apply_sdf_config idempotency-safe by tracking whether it has already run, or to guard the call in start_simulation by verifying it has not been applied yet.

A simpler option: move _apply_sdf_config out of instantiate_builder_from_stage and call it only from start_simulation (in both branches), or mark instantiate_builder_from_stage as internal with a leading underscore.

Comment on lines +509 to +514
if body_patterns is not None:
for body_idx in range(len(builder.body_label)):
if any(p.search(builder.body_label[body_idx]) for p in body_patterns):
for si in range(builder.shape_count):
if builder.shape_body[si] == body_idx and builder.shape_type[si] == GeoType.MESH:
sdf_shape_indices.add(si)
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.

P2 O(bodies × shapes) nested scan when collecting SDF shape indices by body pattern

For each matched body, the code performs a full linear scan over all shapes (range(builder.shape_count)). For a large model with many bodies and shapes this is O(B·S) at startup. Building a reverse index (body_idx → [shape_indices]) once before the loop would reduce this to O(S) total:

# Build reverse map once
body_to_shapes: dict[int, list[int]] = {}
for si in range(builder.shape_count):
    if builder.shape_type[si] == GeoType.MESH:
        body_to_shapes.setdefault(builder.shape_body[si], []).append(si)

if body_patterns is not None:
    for body_idx in range(len(builder.body_label)):
        if any(p.search(builder.body_label[body_idx]) for p in body_patterns):
            sdf_shape_indices.update(body_to_shapes.get(body_idx, []))

This also avoids the redundant shape_type check inside the inner loop.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Addressed in caca3c3 — reverse map body_to_shapes implemented as suggested, eliminating the O(B*S) scan for both body and shape pattern matching.

@vidurv-nvidia vidurv-nvidia marked this pull request as draft April 3, 2026 01:37
@vidurv-nvidia vidurv-nvidia marked this pull request as ready for review April 3, 2026 20:42
# Build SDF on prototype before add_builder copies it N times.
# Mesh objects are shared by reference, so SDF is built once and
# all environments inherit it.
NewtonManager._apply_sdf_config(p)
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 _apply_sdf_config crashes when PhysicsManager._cfg is None

_apply_sdf_config(p) is called unconditionally for every prototype here, but inside _apply_sdf_config, the first two lines are:

cfg = PhysicsManager._cfg
sdf_cfg = cfg.sdf_cfg   # AttributeError if cfg is None

There is no None guard on cfg. Meanwhile, the code added just above in this same function (lines 77–78) explicitly guards against cfg being None:

cfg = PhysicsManager._cfg
sdf_cfg = getattr(cfg, "sdf_cfg", None) if cfg is not None else None

This inconsistency means: in the exact scenario the replicate code considers valid (where PhysicsManager._cfg is None), calling _apply_sdf_config(p) unconditionally will raise AttributeError: 'NoneType' object has no attribute 'sdf_cfg'.

The simplest fix is to add the same guard inside _apply_sdf_config:

@classmethod
def _apply_sdf_config(cls, builder: ModelBuilder):
    """Apply SDF collision and optional hydroelastic flags to matching mesh shapes."""
    from newton import GeoType, ShapeFlags

    cfg = PhysicsManager._cfg
    if cfg is None:
        return
    sdf_cfg = cfg.sdf_cfg
    if sdf_cfg is None:
        return
    ...

vidurv-nvidia and others added 11 commits April 3, 2026 17:07
Co-authored-by: isaaclab-review-bot[bot] <270793704+isaaclab-review-bot[bot]@users.noreply.github.com>
Signed-off-by: vidurv-nvidia <vidurv@nvidia.com>
When collision_cfg is not None but lacks to_dict(), the previous code
silently fell back to empty kwargs, ignoring all user settings. Now
raises a clear TypeError so misconfiguration surfaces immediately.
The cloner calls _apply_sdf_config unconditionally on each prototype.
If PhysicsManager._cfg is None at that point, cfg.sdf_cfg would raise
AttributeError. Add an early return when cfg is None, matching the
guard already used in the cloner's pattern compilation code.
@myurasov-nv myurasov-nv force-pushed the vidur/feature/sdf-collision branch from 22d6673 to b7c2c76 Compare April 4, 2026 00:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant