Fix Newton Kit visualization for articulated body visual children#5501
Fix Newton Kit visualization for articulated body visual children#5501mmichelis wants to merge 2 commits intoisaac-sim:developfrom
Conversation
There was a problem hiding this comment.
🤖 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:
_body_parent_indicesis computed correctly for a known articulation hierarchy_body_parent_inv_matricescontains correct inverse transforms- 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
returnThis 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 npnumpy 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.
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 asfr3_link7_visual, so the Franka appeared static while the simulation continued to run.Changes
Updated Newton Fabric transform sync to write both:
omni:fabric:worldMatrixomni:fabric:localMatrixAdded cached Newton body parent metadata:
Computes local matrices correctly for:
Keeps local transform tracking enabled during Fabric sync so Kit can propagate body-local changes into visual child prims.
Uses shared
NewtonManager._usdrt_stagestate consistently instead of subclass-localcls._usdrt_stage, avoiding cases where active solver subclasses seeNonewhile the base manager owns the Fabric stage.Fixes #5500
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there