Render Newton cables on all three renderers - #7016
Conversation
Greptile SummaryThe PR adds animated Newton cable bindings for OVRTX, ovstage, and Isaac RTX, moves Kit cable synchronization onto the simulation device, and adds cross-renderer motion tests.
Confidence Score: 3/5The PR should not merge until PhysX camera forwarding and complete late Fabric-stage recovery are restored. The changed camera pump leaves PhysX Fabric transforms stale, while the late-stage Newton branch can permanently disable body and particle rendering synchronization. Files Needing Attention: source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py; source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py Important Files Changed
Sequence DiagramsequenceDiagram
participant Physics as Newton/PhysX
participant Camera
participant Renderer
participant Fabric
Physics->>Physics: Step simulation
Camera->>Renderer: Request frame
Renderer->>Physics: pre_render()
Physics->>Fabric: Sync transforms and cable points
Renderer->>Fabric: Read current scene
Renderer-->>Camera: RGB output
Reviews (1): Last reviewed commit: "OMPE-103001: Render and animate Newton c..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| # sync_*_to_usd calls that only pre_render() makes. A sensor read that skips them draws bodies at | ||
| # the current pose and cables frozen at their spawn pose, which still yields a stable, plausible | ||
| # image. | ||
| sim.physics_manager.pre_render() |
There was a problem hiding this comment.
When an Isaac RTX camera reads after sim.step(render=False) without a visualizer that performs forwarding, this now calls PhysX's no-op pre_render() instead of forward(), so current transforms are not written to Fabric and the camera renders stale body poses.
Knowledge Base Used: IsaacLab Core Simulation Layer
| if not cls._clone_physics_only and get_current_stage(fabric=True) is None: | ||
| logger.warning( | ||
| "[NewtonManager] Fabric stage unavailable at start_simulation; the cable sync will" | ||
| " re-acquire it lazily on first use." | ||
| ) | ||
| elif not cls._clone_physics_only: |
There was a problem hiding this comment.
When the Fabric stage is unavailable during start_simulation, this branch skips body, cable, and particle initialization, while the later recovery restores only cable bindings. Because body and Fabric-mesh particle synchronization still require _usdrt_stage, those objects remain frozen in Kit rendering after the stage becomes available.
Knowledge Base Used: IsaacLab Core Simulation Layer
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The device-side cable rendering paths are coherently integrated across Newton Fabric, OVRTX, and Isaac RTX, but two concrete issues remain: late Fabric-stage recovery is blocked by the dirty-flag guard, and the demo silently accepts --frames_dir without enabling a camera, producing no frames.
- Design and architecture: The cable-point setup, per-frame update, and teardown follow the existing geometry-binding architecture across both OVRTX paths. However, the lazy Fabric-stage recovery cannot fulfill its intended role because cable synchronization is not marked dirty when
_usdrt_stagewas unavailable at startup. - API:
CameraRenderSpecnow consistently enforces the documented camera-path/view-count invariant. The demo’s new--frames_diroption has an ambiguous invalid combination: without--camera, it creates a directory but writes nothing, so argument validation should reject that usage. - Implementation: The device kernels, OVRTX ASYNC stream handoff, ovstage host-copy path, Fabric invalidation, and Isaac RTX
pre_render()pump are internally consistent. The remaining implementation defect is thatsync_cables_to_usd()checks_cables_dirtybefore attempting recovery, while per-step dirtying depends on_usdrt_stage; therefore the targeted late-stage case never reaches_recover_cable_fabric_path().
Minor fixes needed. Posted 2 actionable findings inline.
Automated review; human maintainers own approval decisions.
| if not cls._cables_dirty: | ||
| return | ||
| if cls._usdrt_stage is None or cls._cable_shape_ids is None: | ||
| if cls._cable_fabric_stage() is None: |
There was a problem hiding this comment.
🟡 Warning · Implementation — Lazy Fabric cable recovery is unreachable
sync_cables_to_usd returns at the _cables_dirty guard before reaching _recover_cable_fabric_path. Per-frame dirtying in step() is gated on cls._usdrt_stage is not None, and recovery deliberately stores the stage in _cable_usdrt_stage only, so in the exact late-stage case this targets the flag never becomes true and cables keep rendering frozen. Mark cables dirty (or attempt recovery) independently of _usdrt_stage.
| help="Attach a camera that renders the cable pile with the given renderer.", | ||
| ) | ||
| parser.add_argument( | ||
| "--frames_dir", |
There was a problem hiding this comment.
🔵 Suggestion · Api — frames_dir accepted without a camera
--frames_dir documents writing each rendered frame, but save_frame is only reached inside the camera is not None branch of run_simulator. Passing --frames_dir without --camera creates the directory and then writes nothing, with no diagnostic. Add a parser.error for this combination alongside the existing argument checks.
e7d00a4 to
05dc2cf
Compare
2892b40 to
e8b227f
Compare
huidongc
changed the title
8da16c3 to
86752a2
Compare
Cable curve points are now computed on device from the Newton segment bodies and written GPU/ASYNC each frame, so cables follow their simulated pose instead of drawing at their spawn pose and never moving. Drops the CPU mirror in sync_cables_to_usd, which copied the whole model's body_q device-to-host on every dirty render frame. Adds the first rendering coverage for cables, gated on motion rather than presence, and fixes three defects found alongside: the view_count / camera-prim mismatch, the Isaac RTX sensor pump refreshing transforms only, and the Fabric stage being resolved exactly once at startup. Requires Kit !47946, !48359 and !48511, and OVRTX >= 0.4.1. The rendering cells skip below those floors rather than fail.
…dering Keep the cable PR focused on sync/binding; peel the view_count contract and Isaac RTX empty-annotator skip for a follow-up.
Register Isaac-Lift-Cable-Franka(+Camera) with a CouplerProxy MJWarp+VBD four-cable pile and kit/kitless golden-image tests so Newton cable rendering is exercised across AOVs and renderers.
…capture. Spawn cables above the table so free-fall is visible under gravity, and gate multi-step camera capture behind ISAAC_LAB_SAVE_RENDERING_GIF while keeping golden validation as the default path.
Validate clone destinations against source USD prototypes in collect_cable_segment_shapes and pass ClonePlan from OVRTX so env_1+ cables animate instead of staying frozen at spawn.
…able spawn heights and increasing thickness of cables.
Switch to one camera per env with wider spacing and clipping so lit-mask metrics cover every tile without neighbor leakage, thicken the cable for reliable segmentation, and add optional GIF capture for debugging.
Reuse shared golden scene overrides for the production Franka cable camera env, refresh mismatched AOV baselines across Isaac RTX, Newton, and OVRTX, and add a skip changelog fragment for the test-only changes.
Newton cable/particle Fabric sync belongs on SimulationContext.render() via physics_manager.pre_render(). ensure_isaac_rtx_render_update() is the Kit pump path and should keep forward() for PhysX Fabric refresh; using pre_render() here papers over callers that skip sim.render() and regresses PhysX on paths that rely on this util alone.
The changelog and isaac_rtx cable test assumed camera.update() after sim.step(render=False) must itself sync Newton cable/particle Fabric state. That is not the production contract: SimulationContext.render() already calls physics_manager.pre_render() before sensors/visualizers, and ensure_isaac_rtx_render_update() is the Kit pump path that should keep forward() for PhysX Fabric refresh. The test therefore imposed an invalid premise and pushed a layer-crossing "fix" that papers over skipping sim.render() while regressing PhysX on util-only pump paths. Remove the fragment and test rather than reshape production around them.
Align cable dirty-retry and stage handling with particle Fabric sync, register cables from CableObject for Fabric/OVRTX, and drop clone_plan discovery.
2b5a7be to
342581d
Compare
2 participants
Description
Cable curve points are now computed on device from the Newton segment bodies and written GPU/ASYNC each frame, so cables follow their simulated pose instead of drawing at their spawn pose and never moving. Drops the CPU mirror in sync_cables_to_usd, which copied the whole model's body_q device-to-host on every dirty render frame.
Fixes OMPE-103001
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there