Skip to content

Fix Newton Kit visualization for articulated body visual children#5501

Draft
mmichelis wants to merge 2 commits intoisaac-sim:developfrom
mmichelis:mym/newton-kit-fix
Draft

Fix Newton Kit visualization for articulated body visual children#5501
mmichelis wants to merge 2 commits intoisaac-sim:developfrom
mmichelis:mym/newton-kit-fix

Conversation

@mmichelis
Copy link
Copy Markdown

Description

Fixed Newton Kit visualization for articulated bodies whose render geometry lives under child visual prims.

Previously, Newton synced only the body prim omni:fabric:worldMatrix. The simulated Franka body transforms updated correctly, but Kit did not propagate those updates to child visual prims such as fr3_link7_visual, so the Franka appeared static while the simulation continued to run.

Changes

  • Updated Newton Fabric transform sync to write both:

    • omni:fabric:worldMatrix
    • omni:fabric:localMatrix
  • Added cached Newton body parent metadata:

    • nearest Newton body parent index
    • inverse world transform of non-Newton USD parent prims for root bodies
  • Computes local matrices correctly for:

    • nested Newton articulated links, relative to their Newton parent body
    • root Newton bodies, relative to their actual USD/Fabric parent
  • Keeps local transform tracking enabled during Fabric sync so Kit can propagate body-local changes into visual child prims.

  • Uses shared NewtonManager._usdrt_stage state consistently instead of subclass-local cls._usdrt_stage, avoiding cases where active solver subclasses see None while the base manager owns the Fabric stage.

Fixes #5500

Type of change

  • Bug fix (non-breaking change which fixes an issue)

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 bug Something isn't working isaac-lab Related to Isaac Lab team labels May 5, 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 fixes Newton's Fabric transform synchronization so that visual child prims (e.g., fr3_link7_visual under articulated body links) receive proper transform updates. The fix adds omni:fabric:localMatrix writes alongside the existing worldMatrix writes, computing local matrices relative to each body's nearest Newton body ancestor. This allows Kit's hierarchy system to propagate transforms to visual children correctly.

Architecture Impact

The changes are well-contained within NewtonManager's transform sync system. The new _body_parent_indices and _body_parent_inv_matrices arrays are computed once during start_simulation() and used per-frame in the Warp kernel. The kernel modification adds 3 new inputs but maintains the same launch pattern. No external APIs are changed; this is purely an internal fix to the rendering pipeline.

Potential callers affected: Any code relying on sync_transforms_to_usd() now gets correct local matrix propagation. This is a pure fix with no breaking changes to the simulation or sensor APIs.

Implementation Verdict

Minor fixes needed — The core algorithm is correct, but there are a few issues around silent fallback behavior and a removed API call that may cause problems.

Test Coverage

The PR description claims "I have added tests that prove my fix is effective" but no test files are included in the diff. This is a bug fix that lacks a regression test. The fix affects visual rendering which is harder to unit test, but at minimum there should be a test verifying:

  1. _body_parent_indices is computed correctly for a known articulation hierarchy
  2. _body_parent_inv_matrices contains correct inverse transforms
  3. The kernel produces expected local matrices for nested bodies

CI Status

No CI checks available yet — cannot assess pass/fail status.

Findings

🟡 Warning: newton_manager.py:327-328 — Removed track_local_xform_changes(False) may cause performance regression
The original code paused both world and local transform tracking. The fix removes track_local_xform_changes(False/True) calls but now writes to localMatrix. If the hierarchy system tracks local changes separately, this could trigger unnecessary dirty-flagging or hierarchy rebuilds. Verify that track_world_xform_changes alone is sufficient when writing both matrices, or restore the local tracking pause.

🟡 Warning: newton_manager.py:294-296 — Silent early return when parent metadata is missing

if cls._body_parent_indices is None or cls._body_parent_inv_matrices is None:
    NewtonManager._transforms_dirty = False
    return

This silently skips transform sync if metadata is missing, which could happen if start_simulation() fails partway. Consider logging a warning on the first occurrence to help debug rendering issues:

if cls._body_parent_indices is None or cls._body_parent_inv_matrices is None:
    logger.warning_once("Skipping transform sync: parent metadata not initialized")
    cls._transforms_dirty = False
    return

🟡 Warning: newton_manager.py:843-853 — Exception swallowed during parent matrix read with fallback to identity
The try/except Exception block catches all errors and falls back to identity matrix. This is reasonable for robustness, but the logger.debug level may be too quiet for important failures. If a Franka's base link has a non-identity parent transform that fails to read, the robot will render in the wrong position. Consider logger.warning for the first occurrence per prim path.

🔵 Improvement: newton_manager.py:820 — Redundant import of numpy

import numpy as np

numpy is already imported at module level (line 18). This inner import is redundant but harmless.

🔵 Improvement: newton_manager.py:66-67 — Kernel parameter naming could be clearer
The parameter newton_parent_inv_matrices stores inverse matrices for non-Newton USD parents of root bodies, but the name suggests it's for Newton parents. Consider renaming to root_body_parent_inv_matrices or adding a comment clarifying its purpose:

newton_parent_inv_matrices: wp.array(dtype=wp.mat44f),  # Inverse world matrix of USD parent for root bodies (parent_idx < 0)

🔵 Improvement: newton_manager.py:821-822 — Path conversion may fail for non-string body labels

body_paths = [str(path) for path in body_paths]

This handles potential Sdf.Path objects, which is good. However, if body_label contains unexpected types, this could produce nonsense paths. The original code didn't do this conversion — verify this doesn't change behavior for existing use cases.

🟡 Warning: newton_manager.py:367 — update_world_xforms() now called unconditionally with cubric
Previously the cubric path called cls._cubric.compute() without update_world_xforms(). Now it calls both. This may be intentional (the fix needs hierarchy propagation), but verify this doesn't cause double-processing or performance issues with the cubric path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant