Shimmed Ovstage integration into OVRTX Renderer#6602
Conversation
641fd3b to
62f299e
Compare
Greptile SummaryThis PR introduces a shimmed ovstage integration into the OVRTX renderer, adding a parallel code path where ovstage owns scene data and OVRTX consumes it as a rendering-only consumer. The new path is gated by
Confidence Score: 3/5The ovstage code path itself is well-structured and the cleanup ordering is correct, but the mandatory ovstage dep in isaaclab_ov/pyproject.toml forces it on all users even though the PR presents it as opt-in. The mandatory
Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Sim as Simulation
participant Renderer as OVRTXRenderer
participant OvStage as ovstage.Stage
participant OVRTX as ovrtx.Renderer
Note over Renderer: _use_ovstage = True path
Sim->>Renderer: prepare_stage(stage, num_envs)
Renderer->>Renderer: export_stage_to_string_ovstage()
Sim->>Renderer: create_render_data(spec)
Renderer->>OvStage: Stage(isaaclab.ovrtx)
Renderer->>OvStage: "population.open_usd_from_string(ordinal=1)"
Renderer->>OvStage: "advance_write_floor(ordinal=1)"
Renderer->>OVRTX: attach_ovstage(stage)
Renderer->>OvStage: stage.clone(source, targets, ordinal)
Renderer->>OvStage: advance_write_floor(clone_ordinal)
Renderer->>OvStage: write_attribute(env_query, omni:xform)
loop Each Frame
Sim->>Renderer: update_transforms()
Renderer->>Renderer: wp.synchronize_device() GPU to CPU
Renderer->>OvStage: "write_attribute(object_query, omni:xform, ordinal=N)"
Renderer->>OvStage: advance_write_floor(N)
Sim->>Renderer: update_camera()
Renderer->>Renderer: wp.synchronize_device()
Renderer->>OvStage: "write_attribute(camera_query, omni:xform, ordinal=N+1)"
Renderer->>OvStage: advance_write_floor(N+1)
Sim->>Renderer: render(render_data)
Renderer->>OVRTX: "step(ordinal=N+1)"
OVRTX-->>Renderer: frames[]
end
Sim->>Renderer: cleanup()
Renderer->>OvStage: release_query for camera, object, deformable
Renderer->>OVRTX: detach_ovstage()
Renderer->>OvStage: ExitStack.close()
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Sim as Simulation
participant Renderer as OVRTXRenderer
participant OvStage as ovstage.Stage
participant OVRTX as ovrtx.Renderer
Note over Renderer: _use_ovstage = True path
Sim->>Renderer: prepare_stage(stage, num_envs)
Renderer->>Renderer: export_stage_to_string_ovstage()
Sim->>Renderer: create_render_data(spec)
Renderer->>OvStage: Stage(isaaclab.ovrtx)
Renderer->>OvStage: "population.open_usd_from_string(ordinal=1)"
Renderer->>OvStage: "advance_write_floor(ordinal=1)"
Renderer->>OVRTX: attach_ovstage(stage)
Renderer->>OvStage: stage.clone(source, targets, ordinal)
Renderer->>OvStage: advance_write_floor(clone_ordinal)
Renderer->>OvStage: write_attribute(env_query, omni:xform)
loop Each Frame
Sim->>Renderer: update_transforms()
Renderer->>Renderer: wp.synchronize_device() GPU to CPU
Renderer->>OvStage: "write_attribute(object_query, omni:xform, ordinal=N)"
Renderer->>OvStage: advance_write_floor(N)
Sim->>Renderer: update_camera()
Renderer->>Renderer: wp.synchronize_device()
Renderer->>OvStage: "write_attribute(camera_query, omni:xform, ordinal=N+1)"
Renderer->>OvStage: advance_write_floor(N+1)
Sim->>Renderer: render(render_data)
Renderer->>OVRTX: "step(ordinal=N+1)"
OVRTX-->>Renderer: frames[]
end
Sim->>Renderer: cleanup()
Renderer->>OvStage: release_query for camera, object, deformable
Renderer->>OVRTX: detach_ovstage()
Renderer->>OvStage: ExitStack.close()
|
| # Only interpackage dependencies should go here. | ||
| dependencies = [ | ||
| "isaaclab", | ||
| "ovstage>=0.1.0,<0.2.0", |
There was a problem hiding this comment.
ovstage is a hard dependency but is intended to be optional
ovstage is listed as a required dependency here, but the PR description explicitly states it is "disabled by default" and the ovrtx_renderer.py treats it as purely optional (try: import ovstage … except ModuleNotFoundError: pass). The root pyproject.toml correctly places it under an optional extra (ovstage = ["ovstage>=0.1.0,<0.2.0"]). Listing it here as a hard dep forces it onto every user who installs isaaclab_ov, including those who only want the legacy renderer — contradicting the intent and potentially blocking environments where ovstage cannot be installed.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
There was a problem hiding this comment.
Yeah good call. Before we actually merge this PR we may want to back out that this change and also remove it from the "all" selector, so that this can go in and folks can install ovstage separately
| # therefore relies on every ancestor holding a correct xform in the ovstage data. In | ||
| # particular, if an env root has no valid xform, these prims collapse toward env_0 frame | ||
| # (e.g. tables ending up at env_0 and missing from the other tiles). |
There was a problem hiding this comment.
Null check after
write_attribute use — dead guard
write_attribute is called on line 692 with self._object_xform_query before the if self._object_xform_query is None guard on line 701. If query_from_path_list ever returned None, the write call on line 692 would already raise, making the guard unreachable. The same pattern repeats in _setup_deformable_bindings_ovstage (for _deformable_points_query). Either move the guard before the first use, or remove it if the ovstage API guarantees a non-None return.
| def ovrtx_use_ovstage_enabled() -> bool: | ||
| """Return whether the ovstage scene-ownership path should be used. | ||
|
|
||
| Requires both ``ISAAC_LAB_OVRTX_USE_OVSTAGE=1`` and ovstage being importable | ||
| (``_OVSTAGE_AVAILABLE``). Defaults to ``0`` so existing deployments are unaffected | ||
| until ovstage is explicitly opted into. | ||
| """ | ||
| value = os.environ.get(_USE_OVSTAGE_ENV, "0").strip() | ||
| if value not in {"0", "1"}: | ||
| raise ValueError(f"Invalid value for environment variable `{_USE_OVSTAGE_ENV}`: {value}. Expected 0 or 1.") |
There was a problem hiding this comment.
Silent no-op when ovstage unavailable but explicitly requested
When a user sets ISAAC_LAB_OVRTX_USE_OVSTAGE=1 but _OVSTAGE_AVAILABLE is False (e.g., installed without the extra), this function silently returns False and falls back to the legacy path with no log message. The user gets no indication that their environment variable was ignored. Since ovstage is hard-required in isaaclab_ov/pyproject.toml, this is currently an edge case, but if that dep is made optional (as the PR description implies it should be), a warning here would prevent silent misconfiguration.
c733fe8 to
58b70e2
Compare
Add ovstage>=0.1.0,<0.2.0 as a new OV runtime extra alongside the existing ovphysx and ovrtx selectors. Users can now install it via ./isaaclab.sh -i "ov[ovstage]" or include it with ov[all]. The version pin lives in [tool.isaaclab.versions] as the single source of truth; the extra literal and the test assertion mirror it.
The new code path is enabled if ovstage can be imported, AND if ISAAC_LAB_OVRTX_USE_OVSTAGE=1. By default it is off
…cloning as a work around for tiled rendering issue in ovrtx 0.4 / ovstage 0.1
58b70e2 to
2332ab8
Compare
Description
A shimmed ovstage implementation that we can optionally merge into develop to make it easier to collectively test and iterate, at the cost of supporting 2 separate code paths in the meantime (with agents it is not too much work). Once ovstage is considered ready for IsaacLab, it should be very straightforward to remove the legacy codepaths. This work was mostly agentic - this skills and context in https://github.com/NVIDIA-Omniverse/ovrtx made it straightforward.
ovstage code path is disabled by default and can be enabled via
ISAAC_LAB_OVRTX_USE_OVSTAGE=1.Example of running the cartpole rendering correctness tests with ovrtx + ovstage:
The ovstage implementation still suffers from the tiled rendering bug introduced in ovrtx-0.4 that @nvsekkin was looking at:

In order to further test ovstage without being bitten by the tiled rendering bug, I introduced a separate
ISAAC_LAB_OVRTX_ENABLE_CLONE=0env var to disable OVRTX cloning (ovrtx.clone_usd/ovstage.clone) which allows us to at least have working renders we can benchmark. (@r-schmitt @pbarejko)Summarized differences
Renderer.open_usd_from_string(usd)— renderer owns scene data internallyovstage.Stage+PathDictionarycreated first, USD loaded viapopulation.open_usd_from_string, renderer attached viaattach_ovstage. Scene data lives in ovstage; renderer is a consumerSdf.Layercopy; non-source env roots removed entirely viaSdf.BatchNamespaceEdit. Required becausestage.clonerejects pre-existing targetsRenderer.clone_usd(source, targets)— single call per row, renderer handles internally, stubs may already existUsdGeom.XformCache(_capture_env_root_xforms_ovstage) — export removes non-source env roots, so they can't be read back from ovstage. Thenstage.clone(source, targets, ordinal)per row (shared ordinal for atomicity), and the captured xforms are written back withwrite_attribute("omni:xform", semantic=MATRIX)so hierarchy-positioned static prims stay correctly placed per envbind_attributereturns persistent binding; each frame maps into GPU memory viabinding.map(Device.CUDA)and Warp kernel writes directly — zero-copy, GPU-residentquery_from_path_list; each frame: kernel →wp.synchronize_device→.numpy()CPU copy →stage.write_attributewith_xform_tensor_from_numpybind_array_attribute+binding.write— GPU-to-GPU, cross-stream dependency, no host stallquery_from_path_list+wp.synchronize_device(host stall) +particle_q.numpy()+stage.write_attribute— host copy required to rebuild the warpvec3fslices as lanes=3 DLTensors (semanticPOINT) matching thepoint3f[]column;make_dltensorwon't override dtype on GPU DLPack producers, so no zero-copy path_current_ordinal; all init writes share one ordinal committed viaadvance_write_floorbeforeattach_ovstage; all perordinal, committed in render beforestep(ordinal=N), then incremented for the next framebinding.unbind()per binding,Renderer.reset_stage()stage.release_queryper query,PathDictionary.destroy_path_listper path list,renderer.detach_ovstage()before `ExitStack.cmake_dltensordoesn't support lane-packed dtype :xformmatrices, lanes=3 forpoints`) on GPU DLPackproducersNotable detailed differences
Env-root transform snapshot: legacy vs ovstage
After cloning, we need to restore each env root's original transform so that static prims (tables, fixtures, etc.) that aren't driven by physics remain correctly placed per environment.
The two paths diverge in when that env root xform snapshot can be taken:
read_attribute("omni:xform", env_prim_paths)can be called on the loaded stage immediately before cloning.We'd need to do some profiling around this comparing the legacy way of traversing the env_1...N-1 descendent hierarchies and deactivating all the prims vs flatten + the batch namespace edit prune approach.
Newton GPU transform/points write path: zero-copy vs host round-trip
When the physics engine is Newton - each frame, Isaac Lab computes rigid body and camera world transforms on the GPU using Warp kernels that write into wp.array(dtype=wp.mat44d) buffers — one 4×4 float64 matrix per prim. These buffers stay GPU-resident and need to be handed to the renderer so it can update prim placements before the next step().
Legacy - Renderer.bind_attribute maps a persistent GPU buffers for the camera & object xforms; the Warp kernel writes directly into it every frame and is picked up by the renderer.
Ovstage path writes transforms via stage.write_attribute("omni:xform", ...). The omni:xform Fabric column is typed as float64, lanes=16 — a single 16-element vector per prim representing the flattened 4×4 matrix. The problem is that wp.mat44d exports through DLPack as shape (N, 4, 4), dtype=float64, lanes=1, which mismatches the existing column type and causes ovstage to reject the write with INVALID_ARGUMENT: existing attribute 'omni:xform' has a different type.
make_dltensor can reinterpret a buffer as lanes=16 via its dtype and shape override kwargs, but those overrides only work when the input is a numpy array. For any DLPack producer (warp, torch, cupy) it calls DLTensor.from_dlpack(array) directly and the overrides are rejected:
The current implementation takes the numpy path: wp.synchronize_device + .numpy().reshape(-1, 4, 4) + _xform_tensor_from_numpy. This is a per-frame GPU→CPU copy for every camera and rigid body transform write — the main performance regression relative to the legacy binding path. Once ovstage supports dtype/shape overrides for GPU DLPack producers, the copy can be eliminated and the warp array passed directly.
Similarly for deformable points, Newton particle_q (warp vec3f) dlpack-exports as lanes=1, but the USD points column is lanes=3 so we must copy to host and rebuild it as a lanes=3 DLTensor (semantic POINT), since make_dltensor only honors the lanes override on numpy arrays, not GPU DLPack arrays (@huidongc):
Type of change
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there