MJCF Import: fix duplicate transform application by adenzler-nvidia · Pull Request #1379 · newton-physics/newton

adenzler-nvidia · 2026-01-15T15:49:32Z

Static geoms had their incoming xform applied twice.

Newton Migration Guide

Please ensure the migration guide for warp.sim users is up-to-date with the changes made in this PR.

The migration guide in docs/migration.rst is up-to date

Before your PR is "Ready for review"

Necessary tests have been added and new examples are tested (see newton/tests/test_examples.py)
Documentation is up-to-date
Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

Bug Fixes
- Corrected geometric transform application to prevent double-application of transforms to static geometry and capsule shapes when importing MJCF files.
- Improved transform handling consistency for various geometry types during import.
Tests
- Added verification tests for correct single transform application to static geometry and capsule shapes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

coderabbitai · 2026-01-15T15:49:43Z

📝 Walkthrough

Walkthrough

This change fixes a double-application issue in transform handling during MJCF shape parsing. The incoming transform (incoming_xform) is now applied once to the local transform without subsequently mutating geometry positions or reapplying the transform. Capsule and plane geometries are updated to derive their properties from the already-transformed frame.

Changes

Cohort / File(s)	Summary
Transform Application Fix `newton/_src/utils/import_mjcf.py`	Refactored incoming transform application logic to eliminate double-application: removed redundant geom_pos/geom_rot mutation after incoming_xform, relaxed conditional gate from "link == -1 and incoming_xform is not None" to "incoming_xform is not None", explicitly transform capsule start/end points and plane properties using the transformed frame.
Test Coverage `newton/tests/test_import_mjcf.py`	Added two test methods: `test_static_geom_xform_not_applied_twice` verifies single application of xform to static geometry; `test_static_fromto_capsule_xform` validates correct xform application to capsule with fromto attribute.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

eric-heiden
preist-nvidia

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'MJCF Import: fix duplicate transform application' is clear, concise, and directly describes the main change—fixing a bug where transforms were applied twice during MJCF import.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

📜 Recent review details

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b64b2b and 8d9a725.

📒 Files selected for processing (2)

newton/_src/utils/import_mjcf.py
newton/tests/test_import_mjcf.py

🧰 Additional context used

📓 Path-based instructions (4)

newton/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Any user-facing class/function/object added under _src must be exposed via the public Newton API through re-exports

Files:

newton/tests/test_import_mjcf.py
newton/_src/utils/import_mjcf.py

**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use prefix-first naming for classes: ActuatorPD, ActuatorPID (not PDActuator, PIDActuator)
Use prefix-first naming for methods: add_shape_sphere() (not add_sphere_shape())
Method names must use snake_case
Prefer nested classes when self-contained: if a helper type or enum is only meaningful inside one parent class and doesn't need a public identity, define it as a nested class instead of a top-level class/module
Follow PEP 8 for Python code
Use Google-style docstrings with clear, concise explanations of what functions do, their parameters, and return values

Files:

newton/tests/test_import_mjcf.py
newton/_src/utils/import_mjcf.py

**/*

📄 CodeRabbit inference engine (AGENTS.md)

CLI arguments must use kebab-case (e.g., --use-cuda-graph, not --use_cuda_graph)

Files:

newton/tests/test_import_mjcf.py
newton/_src/utils/import_mjcf.py

newton/_src/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

The newton/_src/ directory is internal library implementation only and must not be imported by user code (examples, documentation, or external users)

Files:

newton/_src/utils/import_mjcf.py

🧠 Learnings (1)

📚 Learning: 2026-01-13T03:11:40.556Z

Learnt from: jumyungc
Repo: newton-physics/newton PR: 1333
File: newton/_src/sim/builder.py:0-0
Timestamp: 2026-01-13T03:11:40.556Z
Learning: Ensure all VBD damping semantics follow Rayleigh-style damping and are unitless. This convention, demonstrated in SolverVBD, should be applied consistently across all VBD-related constraints in the codebase. In newton/_src/sim/builder.py, validate that any damping terms use the same normalization and are treated as dimensionless; add comments and unit tests to enforce consistency across VBD components.

Applied to files:

newton/tests/test_import_mjcf.py
newton/_src/utils/import_mjcf.py

🧬 Code graph analysis (1)

newton/tests/test_import_mjcf.py (2)

newton/_src/sim/builder.py (2)

ModelBuilder (70-6843)

add_mjcf (1533-1627)

newton/_src/utils/import_utils.py (1)

transform (139-140)

🔇 Additional comments (6)

newton/tests/test_import_mjcf.py (2)

1733-1762: Well-designed regression test for the double-transform bug.

The test clearly verifies that a static geom at pos=(1,0,0) with an xform translation of (0,2,0) results in (1,2,0), not (1,4,0) from double application. The inline comments documenting the expected vs buggy behavior are helpful for future maintenance.

1764-1788: Good coverage for fromto capsule transform handling.

This test validates that capsules defined with fromto coordinates are correctly transformed by the incoming xform. The test verifies the capsule centered at (0.5,0,0) with xform translation (0,5,0) results in (0.5,5,0).

newton/_src/utils/import_mjcf.py (4)

310-311: Correct fix: uniform application of incoming_xform.

The condition change from if link == -1 and incoming_xform is not None: to if incoming_xform is not None: properly applies the transform uniformly. The previous condition was causing the transform to be applied twice for static geoms through a separate code path that has been removed (per the AI summary).

423-439: Proper handling of fromto coordinates with incoming_xform.

Transforming the start and end points before computing the capsule axis and orientation is the correct approach. Since tf is now computed from these transformed points (line 435), the capsule will be positioned correctly without double-applying the transform.

466-476: Plane geometry correctly derives normal/distance from transformed frame.

Using tf.q and tf.p (which already incorporates incoming_xform) to compute the plane normal and distance is correct. This ensures the plane is positioned consistently with other geometry types.

441-447: Non-fromto capsule/cylinder path also handles transform correctly.

For the non-fromto case, tf was already transformed at line 311, and only an axis rotation adjustment is applied at line 447. This ensures consistent single application of incoming_xform.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

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

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

codecov · 2026-01-15T15:59:26Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

eric-heiden

LGTM!

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

adenzler-nvidia added 3 commits January 15, 2026 16:13

add test

8b14fa6

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

add more regression tests

56a534b

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

fix incoming xform application

8d9a725

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

adenzler-nvidia requested a review from jvonmuralt January 15, 2026 15:50

eric-heiden approved these changes Jan 16, 2026

View reviewed changes

eric-heiden added this pull request to the merge queue Jan 16, 2026

Merged via the queue into newton-physics:main with commit bbbbdd4 Jan 16, 2026
22 checks passed

coderabbitai Bot mentioned this pull request Jan 16, 2026

MJCF import: handle <frame> tags #1390

Merged

3 tasks

jnskkmhr pushed a commit to jnskkmhr/newton that referenced this pull request Jan 22, 2026

MJCF Import: fix duplicate transform application (newton-physics#1379)

875312a

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

eric-heiden pushed a commit to eric-heiden/newton that referenced this pull request Jan 28, 2026

MJCF Import: fix duplicate transform application (newton-physics#1379)

e533afc

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

coderabbitai Bot mentioned this pull request Feb 23, 2026

Fix fromto capsule/cylinder orientation in MJCF parser #1741

Merged

5 tasks

mmacklin pushed a commit to mmacklin/newton that referenced this pull request Apr 7, 2026

MJCF Import: fix duplicate transform application (newton-physics#1379)

a288898

Signed-off-by: Alain Denzler <adenzler@nvidia.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MJCF Import: fix duplicate transform application#1379

MJCF Import: fix duplicate transform application#1379
eric-heiden merged 3 commits into
newton-physics:mainfrom
adenzler-nvidia:dev/adenzler/mjcf-static-double-transform

adenzler-nvidia commented Jan 15, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jan 15, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

codecov Bot commented Jan 15, 2026

Uh oh!

eric-heiden left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

adenzler-nvidia commented Jan 15, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Newton Migration Guide

Before your PR is "Ready for review"

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Suggested reviewers

Uh oh!

codecov Bot commented Jan 15, 2026

Codecov Report

Uh oh!

eric-heiden left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adenzler-nvidia commented Jan 15, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jan 15, 2026 •

edited

Loading