Honor selfCollisionEnabled on imported cables

[mmichelis]

Description

Newton's USD cable importer builds rod articulations via add_rod / add_rod_graph, but it never resolved the per-articulation self-collision flag. As a result, authoring newton:selfCollisionEnabled (or the physxArticulation:enabledSelfCollisions fallback) on a cable had no effect, even though the general rigid importer in parse_usd already honors it. Non-adjacent segments of the same cable kept colliding even when self-collision was disabled.

This PR resolves self_collision_enabled per cable articulation and, when it is disabled, filters collisions between all of the cable's segment shape pairs, mirroring the rigid importer's behavior. Adjacent segments are already filtered by the CABLE joints' collision_filter_parent, so this only adds the non-adjacent pairs, and cables imported without collision add no pairs at all. The enable_self_collisions argument of add_usd() / parse_usd() supplies the default when a cable authors nothing, while an authored attribute takes precedence. Welded rod graphs are one articulation, so they resolve self-collision from the values their member curves actually author: any member opting out disables the whole graph, and the importer warns only when authored values genuinely disagree. With the default enable_self_collisions=True and nothing authored, no filter pairs are added, so existing cable behavior is unchanged.

Checklist

• New or existing tests cover these changes
• The documentation is up to date with these changes
• CHANGELOG.md has been updated (if user-facing change)

Test plan

uv run --extra dev -m newton.tests -k test_import_usd_deformable_cable

Regression tests were added and fail on main without the importer change:

• test_cable_self_collision_disabled_filters_non_adjacent_pairs (single-curve cable, covering the authored attribute, the enable_self_collisions argument, authored-beats-default, and the non-colliding skip)
• test_welded_graph_self_collision_disabled_filters_non_adjacent_pairs (welded rod graph, covering disabled, all-disabled, mixed authoring warnings, and authored-beats-default)
• test_self_collision_disabled_does_not_filter_across_curves_in_one_prim (multi-curve prim: filters within a curve, not across sibling curves)
• test_cable_self_collision_physx_fallback_needs_resolver (the physxArticulation:enabledSelfCollisions fallback applies only when SchemaResolverPhysx is passed)

Bug fix

Steps to reproduce:

1. Author a linear UsdGeom.BasisCurves cable with PhysicsCurvesDeformableSimAPI and collision enabled.
2. Author newton:selfCollisionEnabled = False on the cable prim.
3. Import the stage with ModelBuilder.add_usd() and inspect builder.shape_collision_filter_pairs.
4. Observe that only adjacent segment pairs are filtered, so non-adjacent segments of the same cable still collide.

Minimal reproduction:

import newton

# stage: a linear BasisCurves cable (>= 3 points) with PhysicsCurvesDeformableSimAPI,
# collision enabled, and newton:selfCollisionEnabled = False authored on the curve prim.
builder = newton.ModelBuilder()
builder.add_usd(stage)

# Before this PR: non-adjacent pairs such as (0, 2) are absent, so those segments collide.
# After this PR: all segment shape pairs of the cable are filtered.
assert (0, 2) in set(builder.shape_collision_filter_pairs)

Summary by CodeRabbit

• Bug Fixes

  • USD-imported and ModelBuilder.add_usd()-created cables now correctly apply newton:selfCollisionEnabled and the import enable_self_collisions default, filtering only non-adjacent self-collisions when disabled.
  • Mixed authored values follow documented conflict behavior: self-collision is disabled for the affected import scope with warnings as applicable.

• Tests

  • Expanded regression tests for standalone vs welded cable graphs, multi-curve prim separation, and resolver-dependent PhysX fallback behavior.

• Documentation

  • Updated USD parsing docs clarifying cable self-collision resolution, warning rules, and cable remapping details.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

parse_usd() now propagates self-collision defaults to deformable imports. Cable importers resolve newton:selfCollisionEnabled and filter non-adjacent segment collisions for standalone and welded cables when disabled, with regression tests and documentation updates.

Changes

Cable self-collision handling

Layer / File(s)	Summary
Propagate self-collision defaults newton/_src/utils/import_usd.py, newton/_src/utils/import_usd_deformable_utils.py	The deformable import context receives and stores the default self-collision setting.
Apply cable collision filtering newton/_src/utils/import_usd_deformable_cable.py	Standalone and welded cable imports resolve articulation settings, warn on mixed welded-member values, and filter non-adjacent body-shape pairs when disabled.
Validate and document imported cable behavior newton/tests/test_import_usd_deformable_cable.py, docs/concepts/usd_parsing.rst, CHANGELOG.md	Tests cover resolver behavior, collision participation, standalone, welded, and multi-curve cables; documentation and the changelog describe the updated behavior.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• newton-physics/newton#3192: Both changes update deformable USD import plumbing and _DeformableImportContext used by cable importing.

Sequence Diagram(s)

sequenceDiagram
  participant USDParser
  participant CableImporter
  participant SchemaResolver
  participant ModelBuilder
  USDParser->>CableImporter: pass enable_self_collisions
  CableImporter->>SchemaResolver: resolve cable self-collision setting
  SchemaResolver-->>CableImporter: authored value or default
  CableImporter->>ModelBuilder: register non-adjacent shape filter pairs

Loading

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: imported cables now honor selfCollisionEnabled.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

newton/tests/test_import_usd_deformable_cable.py (1)

979-1015: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Cover the remaining self-collision resolution paths.

Add cases for physxArticulation:enabledSelfCollisions and unauthored cables imported with enable_self_collisions=False. Current coverage would not catch a fallback-priority or context-default propagation regression.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@newton/tests/test_import_usd_deformable_cable.py` around lines 979 - 1015,
Add coverage to
test_welded_graph_self_collision_disabled_filters_non_adjacent_pairs for both
physxArticulation:enabledSelfCollisions and unauthored cables imported with
enable_self_collisions=False. Exercise each resolution path on the welded cable
graph and assert the same within-curve and cross-curve pair filtering, including
any expected warning behavior, so fallback priority and context-default
propagation are validated.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@newton/_src/utils/import_usd_deformable_cable.py`:
- Around line 786-797: Update the curve-articulation import loop so the
self-collision setting is resolved once, then call
_apply_cable_self_collision_filter immediately after each add_rod(...,
wrap_in_articulation=True) using that loop’s local bodies list. Remove the
aggregate cable_bodies filtering so independent curve articulations are never
filtered against one another.

---

Nitpick comments:
In `@newton/tests/test_import_usd_deformable_cable.py`:
- Around line 979-1015: Add coverage to
test_welded_graph_self_collision_disabled_filters_non_adjacent_pairs for both
physxArticulation:enabledSelfCollisions and unauthored cables imported with
enable_self_collisions=False. Exercise each resolution path on the welded cable
graph and assert the same within-curve and cross-curve pair filtering, including
any expected warning behavior, so fallback priority and context-default
propagation are validated.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: 89ebab8b-9727-4b44-b8b1-1a58a440f46b

📥 Commits

Reviewing files that changed from the base of the PR and between d124e5b and 889c144.

📒 Files selected for processing (5)

• CHANGELOG.md
• newton/_src/utils/import_usd.py
• newton/_src/utils/import_usd_deformable_cable.py
• newton/_src/utils/import_usd_deformable_utils.py
• newton/tests/test_import_usd_deformable_cable.py

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@newton/tests/test_import_usd_deformable_cable.py`:
- Around line 988-1002: Update the test around _author_attached_cable_pair and
its case parameterization to exercise both CableA and CableB as the authored
self-collision-disabled member. Apply the attribute to the selected member,
retain the warning assertion for the authored-disabled case, and capture
warnings in the default case to assert that no mixed-settings warning is
emitted.
- Around line 957-964: Extend the parameterized cases in the test around
_deformable_stage and _add_cable_curve to cover PhysX-only self-collision set to
False and conflicting Newton/PhysX authored values. Author
physxArticulation:enabledSelfCollisions for those cases, and assert the resolved
behavior verifies PhysX fallback when Newton is absent and Newton’s
newton:selfCollisionEnabled value takes precedence when both are authored.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: c0792c2d-a200-4502-b69d-4a45015a6088

📥 Commits

Reviewing files that changed from the base of the PR and between 889c144 and 35869ff.

📒 Files selected for processing (5)

• CHANGELOG.md
• newton/_src/utils/import_usd.py
• newton/_src/utils/import_usd_deformable_cable.py
• newton/_src/utils/import_usd_deformable_utils.py
• newton/tests/test_import_usd_deformable_cable.py

🚧 Files skipped from review as they are similar to previous changes (4)

• CHANGELOG.md
• newton/_src/utils/import_usd.py
• newton/_src/utils/import_usd_deformable_utils.py
• newton/_src/utils/import_usd_deformable_cable.py

[codecov]

Codecov Report

❌ Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
newton/_src/utils/import_usd_deformable_cable.py	95.83%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/concepts/usd_parsing.rst`:
- Around line 147-150: Update the self-collision resolution descriptions at both
referenced passages to mention the PhysX fallback attribute
physxArticulation:enabledSelfCollisions alongside newton:selfCollisionEnabled,
or link to the complete priority rule at the later documented section. Ensure
the text explains that the fallback is considered when resolving authored values
and that enable_self_collisions no longer applies once either supported
attribute authors a value.
- Around line 147-150: Update CHANGELOG.md with an imperative [Unreleased] entry
in the appropriate category documenting the self-collision behavior change: any
member-authored value controls the graph, conflicting values warn, and the
enable_self_collisions argument is ignored thereafter.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro Plus

Run ID: df5e7953-d766-4718-afaf-953167e0c274

📥 Commits

Reviewing files that changed from the base of the PR and between 28a22d5 and a5ee16a.

📒 Files selected for processing (5)

• CHANGELOG.md
• docs/concepts/usd_parsing.rst
• newton/_src/utils/import_usd_deformable_cable.py
• newton/_src/utils/import_usd_deformable_utils.py
• newton/tests/test_import_usd_deformable_cable.py

🚧 Files skipped from review as they are similar to previous changes (4)

• newton/_src/utils/import_usd_deformable_utils.py
• CHANGELOG.md
• newton/_src/utils/import_usd_deformable_cable.py
• newton/tests/test_import_usd_deformable_cable.py

[eric-heiden]

🔵 Could you collect all active resolver attributes for each accepted cable prim before this collision check? get_value_with_resolver() only records the resolver that supplies the winning value, and this whole block is skipped for a non-colliding welded graph. That means result["schema_attrs"] can omit authored attributes from the other active resolver, or omit the cable prim entirely. Would something like this work here, with the same collection added in the standalone cable path?

Suggested change

	if collision_enabled:
	for key in comp_paths:
	if ctx.collect_schema_attrs:
	ctx.resolver.collect_prim_attrs(curve_recs[key].prim)
	if collision_enabled:

A regression test with both Newton and PhysX resolvers, plus a non-colliding welded graph, would help pin this down.