Skip to content

Removed OVRTX 0.2.x compatibility code paths#5778

Merged
huidongc merged 4 commits into
isaac-sim:developfrom
huidongc:remove-ovrtx0.2-code-path
May 26, 2026
Merged

Removed OVRTX 0.2.x compatibility code paths#5778
huidongc merged 4 commits into
isaac-sim:developfrom
huidongc:remove-ovrtx0.2-code-path

Conversation

@huidongc
Copy link
Copy Markdown
Collaborator

Description

Removed OVRTX 0.2.x compatibility code paths from :class:~isaaclab_ov.renderers.OVRTXRenderer.

Type of change

  • Code cleanup

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

@huidongc huidongc requested a review from pbarejko May 26, 2026 09:53
@github-actions github-actions Bot added the isaac-lab Related to Isaac Lab team label May 26, 2026
@huidongc huidongc changed the title Removed OVRTX 0.2.x compatibility code paths from :class:~isaaclab_ov.renderers.OVRTXRenderer Removed OVRTX 0.2.x compatibility code paths May 26, 2026
Copy link
Copy Markdown

@isaaclab-review-bot isaaclab-review-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good cleanup removing OVRTX 0.2.x compatibility code. The simplification of the codebase by removing version checks, fallback paths, and the _usd_handles tracking mechanism is welcome.

I noticed one issue to address:


Update (ea9f0d8):
✅ Both issues from my previous review have been addressed:

  1. Legacy kernels removed from ovrtx_renderer_kernels.py
  2. Exception handling added around reset_stage()

LGTM


Update (26b6953):
Reviewed additional test cleanup: removed legacy kernel test classes (TestExtractAllDepthTilesKernelLegacy, TestRandomColorsFromIdsKernelLegacy) and associated imports/references. Clean test maintenance aligning with kernel removals.

✅ No issues found in the incremental changes.


Update (cc7b3d1):
Minor changelog wording refinement: "compatibility code paths" → "legacy OVRTX 0.2.x code paths". No functional changes.

✅ No issues.

Comment thread source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 26, 2026

Greptile Summary

This PR removes all OVRTX 0.2.x compatibility code paths from OVRTXRenderer, assuming OVRTX ≥ 0.3.0 as the minimum supported version. The version-gated branching (_IS_OVRTX_0_3_0_OR_NEWER) is eliminated throughout the renderer and USD helper.

  • ovrtx_renderer.py: Drops _OVRTX_VERSION/_IS_OVRTX_0_3_0_OR_NEWER constants, removes legacy kernel imports (extract_all_depth_tiles_kernel_legacy, generate_random_colors_from_ids_kernel_legacy), unconditionally sets read_gpu_transforms=True, and replaces the per-handle remove_usd cleanup loop with a single reset_stage() call.
  • ovrtx_usd.py: Removes the enable_scene_partition_workaround parameter from create_scene_partition_attributes; the function now exclusively writes omni:scenePartition on Camera prims via a continue-based guard, dropping the primvars:omni:scenePartition fallback that was needed for OVRTX < 0.3.0's missing primvar inheritance.

Confidence Score: 4/5

Safe to merge; the cleanup is mechanical and the logic is straightforward, with one minor gap in error handling during teardown.

The reset_stage() call in cleanup() is now unguarded: if it throws, self._renderer is never nulled out, leaving the renderer in a partially torn-down state on any subsequent cleanup attempt. The previous remove_usd loop wrapped each call in a try/except so teardown always completed. All other removals are clean and correct.

The cleanup method in ovrtx_renderer.py deserves a second look for the missing exception guard around reset_stage().

Important Files Changed

Filename Overview
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py Removes all OVRTX 0.2.x version-gated branches: drops _IS_OVRTX_0_3_0_OR_NEWER flag, switches USD loading unconditionally to open_usd_from_string, replaces per-handle remove_usd cleanup with reset_stage() (now without exception handling), and eliminates legacy kernel variants.
source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_usd.py Removes enable_scene_partition_workaround parameter and the associated primvars:omni:scenePartition loop over non-camera prims; logic is simplified to a continue-based guard that only writes omni:scenePartition on Camera prims.
source/isaaclab_ov/changelog.d/remove-legacy-ovrtx-code-path.rst New changelog fragment documenting the removal of OVRTX 0.2.x compatibility code paths.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OVRTXRenderer.__init__] --> B[create_render_data]
    B --> C{_initialized_scene?}
    C -- No --> D[_initialize_from_spec]
    D --> E[build_render_product_as_string]
    E --> F[Combine exported USD + render product string]
    F --> G{temp_usd_dir set?}
    G -- Yes --> H[Write combined USD to file for debugging]
    H --> I[open_usd_from_string]
    G -- No --> I
    I --> J{use_ovrtx_cloning?}
    J -- Yes --> K[_clone_environments_in_ovrtx + _update_scene_partitions_after_clone]
    K --> L[bind_attribute cameras + objects]
    J -- No --> L
    L --> M[render loop: update_camera to step to _process_render_frame]
    M --> N[cleanup: unbind to reset_stage]
Loading

Reviews (1): Last reviewed commit: "Removed OVRTX 0.2.x compatibility code p..." | Re-trigger Greptile

Comment thread source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py
@huidongc huidongc merged commit fa9ce1a into isaac-sim:develop May 26, 2026
38 of 40 checks passed
@huidongc huidongc deleted the remove-ovrtx0.2-code-path branch May 26, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

isaac-lab Related to Isaac Lab team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants