Unit tests for newton warp, refactor camera replicator functionality to rtx_renderer

[bareya]

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.

Fixes # (issue)

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (existing functionality will not work without user modification)
• Documentation update

Screenshots

Please attach before and after screenshots of the change if applicable.

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

[greptile-apps]

Greptile Summary

This PR extracts duplicated camera-renderer logic from Camera, TiledCamera, and IsaacRtxRenderer into a shared isaac_rtx_renderer_utils module, and adds a comprehensive unit-test suite for the extracted helpers. The key changes are:

• New utilities (apply_depth_clipping, slice_output_channels, configure_isaac_rtx_settings, resolve_simple_shading_mode, DEPTH_DATA_TYPES, ANNOTATOR_CHANNEL_COUNTS, SEGMENTATION_COLORIZE_FIELDS) replace three separate copies of the same logic across Camera, IsaacRtxRenderer, and old inline code.
• Camera._initialize_replicator cleanly isolates the Replicator annotator pipeline from the rest of _initialize_impl.
• _check_supported_data_types is consolidated into Camera and removed from TiledCamera; the error message now uses type(self).__name__ for accuracy.
• renderer / render_data attributes are lifted to Camera.__init__ so they always exist at the base-class level.
• Three new test files cover the utilities in depth: test_depth_clipping.py, test_channel_slicing.py, and test_isaac_rtx_renderer_utils.py, plus test_newton_warp_renderer.py for the Newton backend.

Issues already raised in prior review threads (not repeated here):

1. The isaaclab_physx import inside Camera._initialize_replicator / _update_buffers_impl is unconditional, breaking base isaaclab installs without isaaclab_physx.
2. The GUI override in configure_isaac_rtx_settings sets disableColorRender = False unconditionally whenever a GUI is active, even when the fast-path flag was never written.

New concern found in this review:

• Camera.SIMPLE_SHADING_MODES (class attribute) and isaac_rtx_renderer_utils.SIMPLE_SHADING_MODES (module constant) are now two independent copies of the same dictionary. If a future shading mode is added to one but not the other, annotator registration, version-checking warnings, and channel-count slicing will silently diverge. See inline comment for details.

Confidence Score: 3/5

• The refactoring is structurally sound and well-tested, but two open issues from prior review rounds (unguarded cross-package import in the base Camera class, and the unconditional GUI override in configure_isaac_rtx_settings) should be resolved before merging.
• The extracted utilities are cleanly designed and backed by a thorough test suite. However, the two issues flagged in prior threads represent real breaking-change and logic concerns: any deployment of isaaclab without isaaclab_physx will fail at Camera initialization, and the GUI override path writes a settings flag even when the fast-path was never enabled. These are not newly introduced by this PR but are directly surfaced by it. Additionally, the duplicated SIMPLE_SHADING_MODES constant is a new maintenance risk introduced here. Together these keep the score below 4.
• source/isaaclab/isaaclab/sensors/camera/camera.py (unguarded isaaclab_physx import, duplicated SIMPLE_SHADING_MODES) and source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py (GUI override logic in configure_isaac_rtx_settings).

Important Files Changed

Filename	Overview
source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer_utils.py	New utility module consolidating RTX renderer helpers (configure_isaac_rtx_settings, apply_depth_clipping, slice_output_channels, DEPTH_DATA_TYPES, ANNOTATOR_CHANNEL_COUNTS, SEGMENTATION_COLORIZE_FIELDS). GUI override logic still runs unconditionally even when fast-path was never set (flagged in prior thread).
source/isaaclab/isaaclab/sensors/camera/camera.py	Refactored _initialize_impl into _initialize_replicator helper and moved depth-clipping/channel-slicing logic to utils. Unconditional isaaclab_physx import in both _initialize_replicator and _update_buffers_impl (flagged in prior thread). SIMPLE_SHADING_MODES class attribute now duplicates the utils module constant.
source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py	Cleaned up inline depth-clipping and channel-slicing in favour of shared utils; removed duplicate _resolve_simple_shading_mode and SIMPLE_SHADING_MODES/SIMPLE_SHADING_MODE_SETTING definitions. Well-structured refactoring.
source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_utils.py	Good unit tests for configure_isaac_rtx_settings and resolve_simple_shading_mode with boundary-value versioning fixtures. Missing a test for the case where rgb/rgba is requested AND a GUI is active (unconditional False override).
source/isaaclab_newton/test/renderers/test_newton_warp_renderer.py	New test file with good coverage of RenderData lifecycle, weak-reference GC, set_outputs routing, _from_torch memory aliasing, and renderer integration. test_noop_when_output_not_set has no assertion (is a "does not raise" test), which is acceptable but implicit.

Sequence Diagram

sequenceDiagram
    participant C as Camera / TiledCamera
    participant IR as _initialize_replicator
    participant IU as isaac_rtx_renderer_utils
    participant Rep as omni.replicator.core
    participant UB as _update_buffers_impl

    Note over C: Camera._initialize_impl()
    C->>IR: _initialize_replicator()
    IR->>IU: configure_isaac_rtx_settings(data_types)
    IU-->>IR: RTX settings applied (fast-path / warnings)
    IR->>IU: resolve_simple_shading_mode(data_types)
    IU-->>IR: mode int or None
    IR->>Rep: create.render_product(cam_path, resolution)
    Rep-->>IR: render_prod_path
    IR->>Rep: AnnotatorRegistry.get_annotator(name)
    Rep-->>IR: rep_annotator
    IR->>Rep: rep_annotator.attach(render_prod_path)
    IR-->>C: _rep_registry + _render_product_paths populated
    C->>C: _create_buffers()
    C->>C: _update_intrinsic_matrices()

    Note over C: Per-step update
    C->>UB: _update_buffers_impl(env_mask)
    UB->>IU: ensure_isaac_rtx_render_update()
    IU-->>UB: RTX renderer pumped (once per step)
    UB->>Rep: annotator.get_data() per env_id
    Rep-->>UB: raw output
    UB->>C: _process_annotator_output(name, output)
    Note over C: slice_output_channels / colorize / depth reshape
    UB->>IU: apply_depth_clipping(output, name, range, behavior)
    IU-->>UB: tensor clipped in-place

Loading

Comments Outside Diff (1)

1. source/isaaclab/isaaclab/sensors/camera/camera.py, line 595-601 (link)

   Duplicated SIMPLE_SHADING_MODES constant creates a maintenance risk

   Camera.SIMPLE_SHADING_MODES (class attribute, line 99) is used here to register annotators and build simple_shading_cases, while isaac_rtx_renderer_utils.SIMPLE_SHADING_MODES (the new module constant) is used by configure_isaac_rtx_settings, resolve_simple_shading_mode, and ANNOTATOR_CHANNEL_COUNTS.

   Both dictionaries currently contain the same three entries. However, if a new shading mode is added to one but not the other, several things would silently break:

   • configure_isaac_rtx_settings wouldn't emit the "unsupported on old Isaac Sim" warning for the new mode
   • resolve_simple_shading_mode wouldn't recognise it
   • ANNOTATOR_CHANNEL_COUNTS wouldn't include the expected 3-channel slice for it
   • Camera._initialize_replicator might not register the corresponding AOV annotator

   Consider having Camera.SIMPLE_SHADING_MODES delegate to (or simply re-export) the constant from isaac_rtx_renderer_utils so there is a single source of truth:

   from isaaclab_physx.renderers.isaac_rtx_renderer_utils import (
       SIMPLE_SHADING_MODES as _SIMPLE_SHADING_MODES,
   )
   # … inside the Camera class body:
   SIMPLE_SHADING_MODES: dict[str, int] = _SIMPLE_SHADING_MODES

   If the cross-package import at class-definition time is undesirable, the duplicate definition should at least be accompanied by a comment linking it to isaac_rtx_renderer_utils.SIMPLE_SHADING_MODES so future editors know both places must stay in sync.

_{Last reviewed commit: 2362f49}

[greptile-apps]

Unguarded import of optional isaaclab_physx in core Camera class

The import at lines 417–420 is executed unconditionally every time _initialize_impl() is called, with no try/except or conditional guard. Since isaaclab is the base package and isaaclab_physx is a separate optional extension, any deployment installing isaaclab without isaaclab_physx will now get an ImportError when initializing a Camera, whereas previously the code path worked fine (the internal has_kit() guard made it a graceful no-op).

This is a breaking change for installations without isaaclab_physx. Consider either:

1. Guarding the import with try/except ImportError
2. Keeping the RTX-settings logic inlined (behind the existing has_kit() guard) so the base package has no new cross-package dependency

[greptile-apps]

Unconditional GUI override setting even when the fast-path flag was never written

When needs_color_render is True (rgb/rgba requested), the first branch is skipped, so /rtx/sdg/force/disableColorRender is never set to True. However, the GUI override at lines 108–109 runs unconditionally and calls settings.set_bool("/rtx/sdg/force/disableColorRender", False) anyway. While harmless (setting a flag to False that was never set True), this is an unnecessary settings mutation that could surprise future maintainers.

The GUI override only needs to apply when the fast-path was attempted, so it should be nested:

if not needs_color_render:
    settings.set_bool("/rtx/sdg/force/disableColorRender", True)
    if settings.get("/isaaclab/has_gui"):
        settings.set_bool("/rtx/sdg/force/disableColorRender", False)

The existing test test_gui_overrides_fast_path already captures the original intent and can be updated accordingly.

[huidongc]

or even better, figure out the boolean value first and then make the set_bool() call just once.

[huidongc]

Why don't you bump the version?

[huidongc]

Same. Why not bump the version?

[huidongc]

If you like you can merge my wip branch - https://github.com/huidongc/IsaacLab/tree/dev/rendering-correctness-validation and double confirm no regression due to the refactoring:

./isaaclab.sh -p -m pytest source/isaaclab_tasks/test/test_rendering_correctness.py -v