Hydra SDG rendering fix#5384
Conversation
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR fixes a CUDA memory error (cudaErrorIllegalAddress) in headless SDG rendering by ensuring the RTX Hydra engine is properly attached to the USD context before Replicator render products are created. The fix adds a new dependency (omni.kit.hydra_texture) to the headless rendering kit file and introduces an ensure_rtx_hydra_engine_attached() utility function called during renderer initialization.
Architecture Impact
The change is well-contained within the Isaac RTX renderer subsystem:
IsaacRtxRenderer.__init__()now callsensure_rtx_hydra_engine_attached()at construction time- Any code path that creates an
IsaacRtxRenderer(primarilyTiledCamerasensors) will trigger this initialization - The function is idempotent, so GUI workflows with
omni.kit.viewport.windowalready attached will see no change - No breaking API changes
Implementation Verdict
Minor fixes needed — The core logic is sound but there's an inconsistency in error handling and a potential issue with import timing.
Test Coverage
No tests added. This is a bug fix for a headless rendering crash that would be difficult to unit test (requires specific GPU behavior). However, the PR description doesn't reference the original issue number or provide reproduction steps, making it hard to verify the fix works. The checklist item for tests is unchecked.
CI Status
No CI checks available yet — cannot verify this doesn't regress existing rendering tests.
Findings
🔴 Critical: source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py:14 — Top-level import omni.usd will fail in non-Kit environments
The module now unconditionally imports omni.usd at the top level (line 14). This will cause ImportError when this module is imported in environments without Omniverse (e.g., some unit tests, pure-Python tooling). The other Omniverse imports in this file (omni.kit.app) are done inside functions specifically to avoid this issue.
# Line 14 - problematic top-level import
import omni.usdShould be moved inside ensure_rtx_hydra_engine_attached() or guarded:
def ensure_rtx_hydra_engine_attached() -> None:
try:
import omni.usd
ctx = omni.usd.get_context()
# ...🟡 Warning: source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py:91 — Docstring says "warnings" but implementation uses logger.error
The docstring at line 78 states "Failures are logged as warnings and do not propagate" but line 91 uses logger.error(). This inconsistency could mislead readers and affects log filtering in production:
# Line 78 docstring says:
# "Failures are logged as warnings and do not propagate"
# Line 91 uses error:
except Exception as e: # noqa: BLE001
logger.error("RTX Hydra engine attach failed: %s", e)Either update the docstring to say "errors" or change to logger.warning() as documented.
🟡 Warning: source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py:75 — ensure_rtx_hydra_engine_attached() called before stage is available
IsaacRtxRenderer.__init__() calls ensure_rtx_hydra_engine_attached() which queries omni.usd.get_context(). If the renderer is constructed before a Kit app is fully initialized or a USD context exists, get_context() returns None and the function silently returns without attaching the engine. Later, create_render_data() will still fail.
Consider whether this should be called in create_render_data() instead (line 78), where we know the stage/context must exist since the sensor view is being accessed:
def create_render_data(self, sensor: SensorBase) -> IsaacRtxRenderData:
ensure_rtx_hydra_engine_attached() # Context guaranteed to exist here
import omni.replicator.core as rep
# ...🔵 Improvement: apps/isaaclab.python.headless.rendering.kit:25 — Extra blank line after dependency
Minor: There's a double blank line after the new omni.kit.hydra_texture dependency which is inconsistent with the rest of the file's formatting:
+"omni.kit.hydra_texture" = {} # required for headless rendering and rtx instantiation
+
+
[settings.isaaclab]
🔵 Improvement: source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py:60-92 — Function placement breaks logical grouping
The new ensure_rtx_hydra_engine_attached() function is inserted between _wait_for_streaming_complete() (a private helper) and ensure_isaac_rtx_render_update() (a public function). For better code organization, consider placing it either:
- Before all the streaming-related private helpers (after the module-level constants), or
- After
ensure_isaac_rtx_render_update()since they're both public API functions
This is minor but affects readability when navigating the module.
Greptile SummaryThis PR fixes headless RTX rendering by ensuring the RTX Hydra engine is attached to the USD context at Confidence Score: 5/5Safe to merge; only P2 findings remain (log level mismatch in docstring, redundant local import). The fix is minimal and well-scoped: the new helper is idempotent, errors are swallowed rather than propagated, and the .kit change adds only one targeted extension. Both remaining findings are style/documentation level (P2) and do not affect correctness or runtime behaviour. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant App as Headless Kit App
participant Renderer as IsaacRtxRenderer.__init__
participant Utils as isaac_rtx_renderer_utils
participant USD as omni.usd.UsdContext
App->>Renderer: instantiate(cfg)
Renderer->>Utils: ensure_rtx_hydra_engine_attached()
Utils->>USD: get_context()
USD-->>Utils: ctx
Utils->>USD: ctx.get_attached_hydra_engine_names()
USD-->>Utils: engine names
alt "rtx" not already attached
Utils->>USD: create_hydra_engine("rtx", ctx)
USD-->>Utils: engine attached
else already attached
Utils-->>Renderer: no-op
end
Renderer-->>App: ready
|
# Description This MR is a continuation of isaac-sim#5375 without `omni.ui` as a dependency, only minimal dependencies are added and required code was pushed to the render backend that requires it. Fixes # (issue) ``` cudaErrorIllegalAddress ``` ``` ./isaaclab.sh -p -m pytest source/isaaclab/test/sensors/test_multi_tiled_camera.py -v ``` ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) 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 <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Piotr Barejko <piotrbarejko@protonmail.com>
# Description This MR is a continuation of isaac-sim#5375 without `omni.ui` as a dependency, only minimal dependencies are added and required code was pushed to the render backend that requires it. Fixes # (issue) ``` cudaErrorIllegalAddress ``` ``` ./isaaclab.sh -p -m pytest source/isaaclab/test/sensors/test_multi_tiled_camera.py -v ``` ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Screenshots Please attach before and after screenshots of the change if applicable. <!-- Example: | Before | After | | ------ | ----- | | _gif/png before_ | _gif/png after_ | To upload images to a PR -- simply drag and drop an image while in edit mode and it should upload the image directly. You can then paste that source into the above before/after sections. --> ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) 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 <!-- As you go through the checklist above, you can mark something as done by putting an x character in it For example, - [x] I have done this task - [ ] I have not done this task --> --------- Co-authored-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Piotr Barejko <piotrbarejko@protonmail.com>
Description
This MR is a continuation of #5375 without
omni.uias a dependency, only minimal dependencies are added and required code was pushed to the render backend that requires it.Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there