Keep USD cloning enabled for OVRTX scenes

[ooctipus]

Summary

• Keep USD cloning enabled at CloneCfg creation when the scene config requests an OVRTX renderer.
• Add the required isaaclab changelog fragment.

Testing

• pre-commit run ruff-format --files source/isaaclab/isaaclab/scene/interactive_scene.py source/isaaclab/changelog.d/zhengyuz-fix-ovrtx-usd-cloning.rst
• pre-commit run ruff --files source/isaaclab/isaaclab/scene/interactive_scene.py
• python3 -m py_compile source/isaaclab/isaaclab/scene/interactive_scene.py
• python3 tools/changelog/cli.py check release/3.0.0-beta2
• git diff --check release/3.0.0-beta2

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR fixes USD cloning behavior for OVRTX (ray-tracing) scenes running with Newton-replicated physics. Previously, when a Newton-replicated scene was used without Kit, USD cloning was disabled (clone_usd=False). This broke OVRTX camera sensors that depend on USD stage data being present per environment.

The fix:

1. Adds a _should_clone_usd() static method that returns True when either: the scene is NOT Newton-replicated, Kit is available, OR "ovrtx" is in the requested runtime types.
2. Adds a _cfg_renderer_types() method that scans scene config entries for renderer declarations before sensors are constructed, allowing early detection of OVRTX cameras.
3. Combines visualizer types with cfg renderer types into requested_runtime_types to inform the clone decision.

Findings

1. Logic is correct and well-structured. The _should_clone_usd method cleanly encapsulates the decision with a clear boolean expression. The early scan of cfg renderer types ensures OVRTX is detected even before sensor construction.

2. Test coverage is good. Parametrized tests cover all four key scenarios for _should_clone_usd, and _cfg_renderer_types is tested with a lightweight mock object.

3. Minor note: The _cfg_renderer_types method uses InteractiveSceneCfg.__dataclass_fields__ to filter out built-in fields. This works correctly for dataclass-based configs but assumes InteractiveSceneCfg remains a dataclass — a reasonable assumption given the codebase architecture.

4. Testing gap noted by author: The author mentions that the full pytest suite could not be run locally due to a missing lazy_loader dependency. The unit tests for the new logic are self-contained and should pass independently.

No issues found.

Verdict

No issues found — clean, focused bugfix with appropriate test coverage.

---

Update (61ab2f9): The implementation was significantly simplified. The _should_clone_usd() and _cfg_renderer_types() helper methods from the previous revision have been removed in favor of a single inline condition:

clone_usd=not is_newton_replicated_scene or has_kit() or "ovrtx" in requested_viz_types,

This is cleaner and uses the already-available requested_viz_types set (built earlier in __init__) rather than re-scanning config fields. A changelog entry was also added. No new issues — the simplification is an improvement.

---

Update (08ba7e0): The clone_usd logic has been moved from the initial ClonerCfg() constructor to a post-sensor-construction override:

sensor_renderer_types = self._sensor_renderer_types()
self.cloner_cfg.clone_usd = not is_newton_replicated_scene or has_kit() or "ovrtx" in sensor_renderer_types

Instead of relying on requested_viz_types (from resolve_visualizer_types()), it now uses _sensor_renderer_types() which inspects the actual constructed sensor objects' renderer_cfg.renderer_type. This is more precise — it checks what renderers are actually needed by sensors rather than what the visualizer system reports. The override correctly occurs after _add_entities_from_cfg() populates sensors but before clone_environments() executes. No new issues.

---

Update (b267b16): The approach has been revised to detect OVRTX renderers before sensors are constructed, moving the clone_usd decision back to the initial CloneCfg() call.

Key changes:

1. New _cfg_renderer_types() method — scans scene config entries (not constructed sensors) for renderer_cfg.renderer_type values. This enables early detection of OVRTX requirements before _add_entities_from_cfg() runs.
2. Updated clone_usd logic — now uses requested_renderer_types (from _cfg_renderer_types()) at construction time rather than waiting for sensors to be built.

This approach is preferable because:

• It determines clone_usd at the correct time — before cloning occurs
• It doesn't require overwriting cloner_cfg.clone_usd post-hoc
• The cfg-scanning approach correctly handles cases where renderer requirements are declared in config but sensors aren't yet instantiated

The _cfg_renderer_types() implementation properly handles both direct asset configs and RigidObjectCollectionCfg (iterating .rigid_objects.values()), with safe attribute access using getattr() defaults.

No new issues — this is a clean improvement over the previous post-sensor-construction override approach.

---

Update (e4dca43): New commit adds a _source_world_root() static method to NewtonSiteFrameView that resolves the cloned world root from a clone-plan source path and destination template. This fixes env-local camera transform resolution in heterogeneous clone plans — previously, _resolve_source_prim() used the full source_root path as the reference prim, which could be incorrect for child prims under the env root.

The implementation correctly handles edge cases (None inputs, missing {} placeholder, prefix/suffix mismatch, nested paths). A new parametrized test validates the behavior. Changelog updated to reflect the additional fix.

No new issues.

---

Update (79abd9b): Reviewed incremental changes. New logic in newton_site_frame_view.py correctly resolves ref_path to the env-level prim by trimming source_root using the destination_template pattern — this preserves env-local camera transforms in heterogeneous clone plans. Changelog updated to reflect the broader fix. No new issues found. Previous inline suggestions (test fidelity, one-level nesting scan) remain as non-blocking recommendations.

---

Update (7aefc99): The standalone _source_world_root() static method has been removed and its logic inlined directly into _resolve_source_prim(). The ref-path resolution now happens in-place: it splits destination_template on {}, checks if source_root starts with the destination prefix, and extracts the env-level prim path. Functionally equivalent to the previous revision — just cleaner since the logic is only used in one place. Changelog also updated. No new issues.

---

Update (28ca761): The _cfg_renderer_types() method has been removed entirely. Instead of scanning config entries pre-construction, the PR now uses a deferred override approach:

1. Initial clone_usd is set conservatively: not is_newton_replicated_scene or has_kit() (no renderer-type check at init).
2. After _aggregate_scene_data_requirements() completes (which happens after sensors are constructed via _add_entities_from_cfg()), it appends the OVRTX check: self.cloner_cfg.clone_usd = self.cloner_cfg.clone_usd or "ovrtx" in self._sensor_renderer_types().

This is a return to the post-construction override pattern (similar to the 08ba7e0 revision) but positioned at a later, more appropriate point — after scene data requirements are aggregated and all sensor renderer types are known. The _sensor_renderer_types() method (which inspects actual constructed sensor objects) is reused rather than maintaining a separate config-scanning method.

Trade-off: The clone_usd value in cloner_cfg is initially set without OVRTX awareness and then patched before clone_environments() runs. This works correctly since cloning hasn't occurred yet at that point, but it's slightly less self-documenting than having the full condition at construction time. However, it eliminates the need for the _cfg_renderer_types() helper and its assumptions about config structure.

No new issues — functionally correct and simpler than the previous revision.

---

Update (17304f5): Significant architectural improvement. The OVRTX detection and Kit-requirement logic has been lifted from InteractiveScene into the launcher layer, with results communicated via settings:

Key changes:

1. sim_launcher.py refactored — New _compute_runtime_intent() centralizes all backend/renderer/visualizer detection into a single pass. _collect_renderer_types() recursively finds RendererCfg nodes. _sync_runtime_intent_settings() persists results (needs_kit, renderer_types, etc.) to /isaaclab/runtime/ settings so downstream code can query them without re-scanning.

2. SimulationContext gains query methods — resolve_renderer_types(), has_requested_renderer_type(), and runtime_needs_kit() read launcher-computed intent from settings. The runtime_needs_kit() method gracefully falls back to has_kit() when settings are unavailable (e.g., non-launcher paths).

3. InteractiveScene simplified — No longer imports has_kit or implements _cfg_renderer_types(). The clone_usd decision now delegates to self.sim.runtime_needs_kit() and self.sim.has_requested_renderer_type("ovrtx"), making the scene code cleaner and decoupled from config-scanning logic.

4. _iter_config_children() helper — Properly handles Mapping, list/tuple, and dataclass-like objects, fixing the previous vars(node) approach that would fail on dict/list config nodes.

Assessment: This is a well-structured separation of concerns — the launcher computes intent once at startup and publishes it via settings, while scene/simulation code simply queries the pre-computed values. The clone_usd condition is now both early (set at CloneCfg construction) and correct (uses launcher-detected OVRTX presence). The fallback in runtime_needs_kit() ensures backward compatibility for non-launcher execution paths.

No new issues found.

---

Update (a90707b): Cleanup and simplification of the launcher/settings approach from the previous revision:

Changes in interactive_scene.py:

• Removed the from isaaclab.utils.version import has_kit import (no longer needed here — fully delegated to SimulationContext).
• Removed _cfg_renderer_types() private method entirely — OVRTX detection is now handled exclusively by the launcher via settings.
• Replaced inline requested_renderer_types set with a simple has_ovrtx_renderer = self.sim.has_requested_renderer_type("ovrtx") call.
• The clone_usd condition is now: not is_newton_replicated_scene or self.sim.runtime_needs_kit() or has_ovrtx_renderer — clean, readable, and fully delegated.

Changes in simulation_context.py:

• Added has_requested_renderer_type(renderer_type) — reads /isaaclab/runtime/has_{type}_renderer from settings.
• Added runtime_needs_kit() — reads /isaaclab/runtime/needs_kit from settings with fallback to has_kit() when unset (backward compatibility for non-launcher paths).

Changes in sim_launcher.py:

• Refactored the Kit-vs-OVRTX conflict check: has_ovrtx_renderer is now computed once via _scan_config() and reused for both the conflict check and settings publication.
• Added has_kit_visualizer computation.
• Settings are now published (needs_kit, has_kit_cameras, has_kit_visualizer, has_ovrtx_renderer) via get_settings_manager() after the Kit/Newton launch decision is made.
• The Newton-standalone visualizer path condition tightened from if visualizer_types or visualizer_explicit_none to if not needs_kit and (visualizer_types or visualizer_explicit_none) — correctly avoids overriding Kit's visualizer setup.

Assessment: This revision trims the previous approach nicely — removing the intermediate _compute_runtime_intent() / _collect_renderer_types() / _iter_config_children() helpers from the prior commit in favor of reusing the existing _scan_config() utility. The result is fewer new abstractions while maintaining the same clean separation of concerns (launcher detects → settings store → scene queries). The SimulationContext methods are minimal and well-named. The conditional guard on the Newton visualizer path (if not needs_kit) is a good correctness improvement.

No new issues found.

---

Update (88f219d): Final simplification — the scene now reads launcher-published settings directly via self.sim.get_setting() instead of going through SimulationContext helper methods.

Changes in interactive_scene.py:

• Removed the _cfg_renderer_types() method entirely (previously scanned config entries for renderer declarations).
• Removed the requested_renderer_types set that combined viz and cfg renderer types.
• Now reads /isaaclab/runtime/needs_kit and /isaaclab/runtime/has_ovrtx_renderer directly from settings manager, with appropriate fallbacks (has_kit() when setting is unset).
• The clone_usd condition becomes: not is_newton_replicated_scene or needs_kit or has_ovrtx_renderer — maximally simple.

Changes in sim_launcher.py:

• has_ovrtx_renderer is now computed once via _scan_config() with a new _is_ovrtx_renderer predicate and reused for both the Kit-conflict check and settings publication.
• New settings published after launch decision: needs_kit, has_kit_cameras, has_kit_visualizer, has_ovrtx_renderer — giving downstream code a clean query interface.
• The Kit-vs-OVRTX conflict check is refactored: has_ovrtx_renderer is computed unconditionally (not nested inside the "kit" in early_visualizer_types branch), making the flow flatter and the setting always available.
• Newton-standalone visualizer path guard tightened: if not needs_kit and (visualizer_types or visualizer_explicit_none) — correctly avoids overriding Kit's visualizer setup.
• Adds has_kit_visualizer computation combining visualizer_types and visualizer_intent.

Assessment: This commit removes the intermediate SimulationContext helper methods (has_requested_renderer_type(), runtime_needs_kit()) that were introduced in the previous revision, replacing them with direct settings reads in the scene. This is simpler (fewer abstractions, no new public API on SimulationContext) while maintaining the same architecture: launcher detects → settings store → scene queries. The fallback to has_kit() when the setting is None ensures backward compatibility for non-launcher paths.

No new issues found.

---

Update (29f5db7): Reviewed incremental changes (88f219d → 29f5db7). New commits make isaaclab_ppisp and ovphysx optional with actionable install-error messages, replace the Numba-based SoftDTW with a pure-Torch implementation, add multi-backend physics presets to stack/place tasks, remove fold_preset_tokens in favor of direct Hydra parsing, and split env config classes into separate *_cfg.py modules. All changes are well-structured with proper import guards and good test coverage. No new issues found. Previous inline comments on unmodified files still stand as optional improvements.

---

Update (25a81b1): Reviewed incremental changes (29f5db7 → 25a81b1). The _cfg_renderer_types() method has been fully removed. The scene now reads /isaaclab/runtime/needs_kit (with has_kit() fallback) and /isaaclab/runtime/has_ovrtx_renderer directly from settings — addressing my earlier concern about the one-level nesting limitation in config scanning. In sim_launcher.py, the has_ovrtx_renderer detection is lifted out of the Kit-visualizer branch and computed unconditionally, then published to settings alongside needs_kit, has_kit_cameras, and has_kit_visualizer. The Newton-standalone visualizer path is correctly guarded with if not needs_kit. No new issues found.

[isaaclab-review-bot]

PR Review: Keep USD cloning enabled for OVRTX scenes

Overall Assessment: This is a solid fix for preserving USD cloning when OVRTX renderers are configured.

What This PR Does

1. Adds early renderer detection via _cfg_renderer_types() - scans scene cfg entries for renderer configurations before sensors are constructed
2. Introduces _should_clone_usd() helper - cleanly encapsulates the decision logic for when to clone USD
3. Combines visualizer and cfg renderer types to ensure OVRTX cameras are detected early in the clone decision

Strengths

• Good separation of concerns - The static _should_clone_usd method makes the decision logic clear and testable
• Well-tested - Parametrized tests cover the key combinations (Newton replicated, Kit available, OVRTX requested)
• Early detection pattern - _cfg_renderer_types() scanning before sensor construction is the right approach to catch OVRTX cameras early

Suggestions & Questions

1. String matching on "ovrtx" - The check "ovrtx" in requested_runtime_types assumes the renderer type string contains "ovrtx". Should this be case-insensitive? Are there variants like "OVRTX" or "OVRtx" that could slip through?

   # Consider:
   any("ovrtx" in rt.lower() for rt in requested_runtime_types)

2. Default renderer type fallback - In _cfg_renderer_types(), you default to "default" if renderer_type is missing:

   renderer_types.append(getattr(rcfg, "renderer_type", "default"))

   Is "default" ever meaningful here? If renderer_type is missing, should we skip appending entirely since we only care about detecting OVRTX?

3. Type annotation consistency - _cfg_renderer_types() returns list[str] but requested_runtime_types is set[str]. The conversion happens implicitly via set(self._cfg_renderer_types()). Minor, but could make the method return a set directly for consistency.

4. Edge case: empty renderer_cfg - What happens if renderer_cfg exists but has no renderer_type attribute? The fallback to "default" handles this, but worth verifying that's the intended behavior.

5. Test for RigidObjectCollectionCfg - The test for _cfg_renderer_types only tests a simple SimpleNamespace with a direct renderer_cfg. Consider adding a test case that exercises the RigidObjectCollectionCfg branch with asset_cfg.rigid_objects.values().

Minor Nits

• The PR description notes tests weren't run locally due to missing lazy_loader - CI should validate, but worth confirming these pass in the CI environment.

Overall, this looks like a well-thought-out fix. The suggestions above are minor improvements that could make the code more robust.

[greptile-apps]

Greptile Summary

This PR ensures USD cloning is preserved for Newton-replicated scenes when an OVRTX renderer is requested — either via Kit availability or via OVRTX camera configs declared directly on the scene cfg. The clone_usd decision is extracted into a testable _should_clone_usd static method, and a new _cfg_renderer_types scan runs before sensors are constructed so that OVRTX cameras are detected at the right moment.

• Adds _should_clone_usd(is_newton_replicated_scene, kit_available, requested_runtime_types) which is a clean, pure-function extraction of the previous inline boolean; backward-compatible for all non-OVRTX scenes.
• Adds _cfg_renderer_types() which walks self.cfg.__dict__ (filtered against InteractiveSceneCfg.__dataclass_fields__) to collect renderer_type strings before _add_entities_from_cfg runs, so the clone decision sees the full type set.
• Covers both new helpers with focused unit tests; the test for _cfg_renderer_types uses a SimpleNamespace mock rather than a real @configclass subclass.

Confidence Score: 4/5

Safe to merge; the change is backward-compatible for all non-OVRTX Newton scenes and only widens the clone_usd=True envelope for the OVRTX camera case.

The core logic in _should_clone_usd is a clean boolean extraction with full parametrized coverage. The _cfg_renderer_types scan is structurally sound for the current configclass implementation, but the unit test exercises it through a SimpleNamespace rather than a real @configclass subclass, leaving a small gap between what the test validates and what production code sees. No functional regression risk for existing scenes.

The _cfg_renderer_types method in interactive_scene.py and its corresponding test deserve a second look — specifically, the one-level nesting assumption and the SimpleNamespace-based test mock.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/scene/interactive_scene.py	Adds _should_clone_usd static method and _cfg_renderer_types instance method; wires them together so USD cloning is preserved for Newton replicated scenes when an OVRTX renderer is detected in the scene cfg before sensors are built. Logic is backward-compatible and the factoring is clean.
source/isaaclab/test/scene/test_interactive_scene.py	Adds parametrized tests for _should_clone_usd covering 4 cases (correct) and a unit test for _cfg_renderer_types using a SimpleNamespace mock. The mock works structurally but does not exercise the actual @configclass dict path.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["InteractiveScene.__init__"] --> B["resolve_visualizer_types() -> requested_viz_types"]
    A --> C["_cfg_renderer_types() -> scan scene cfg fields for renderer_cfg"]
    B --> D["requested_runtime_types = viz_types | cfg_renderer_types"]
    C --> D
    A --> E["compute is_newton_replicated_scene"]
    A --> F["has_kit()"]
    D --> G["_should_clone_usd(is_newton_replicated_scene, kit_available, requested_runtime_types)"]
    E --> G
    F --> G
    G -->|"not newton_replicated OR kit OR ovrtx in types"| H["clone_usd = True"]
    G -->|"newton_replicated AND no kit AND no ovrtx"| I["clone_usd = False"]
    H --> J["CloneCfg(..., clone_usd=True)"]
    I --> J

Loading

_{Reviews (1): Last reviewed commit: "Keep USD cloning for OVRTX scenes" | Re-trigger Greptile}

[greptile-apps]

Test fidelity gap — SimpleNamespace vs real configclass

scene.cfg is a bare SimpleNamespace, so the assertion that num_envs and replicate_physics are silently filtered out relies on them happening to be in InteractiveSceneCfg.__dataclass_fields__. If the @configclass decorator ever stores instance data differently from a plain __dict__ (e.g., through __slots__ or a custom __getattr__), this test would still pass while the production code fails. Using an actual @configclass subclass of InteractiveSceneCfg (mirroring what __init__ receives) would make the contract explicit and catch any future configclass implementation drift.

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!

[greptile-apps]

_cfg_renderer_types only inspects one level of nesting

For a RigidObjectCollectionCfg the code correctly expands rigid_objects.values() before looking for renderer_cfg. But all other asset_cfg types are wrapped in a single-element list, so only the top-level object is checked. If an OVRTX camera is wrapped in a compound config type that is neither a RigidObjectCollectionCfg nor carries renderer_cfg directly on itself (e.g., a future multi-camera container), it would silently be missed and clone_usd would return the wrong value for that scene. A docstring caveat noting this one-level scan constraint would help future maintainers.

[isaaclab-review-bot]

🤖 Re-review Update (88f219d)

Reviewed incremental changes since a90707b.

Changes in interactive_scene.py:

• Direct settings access via self.sim.get_setting() instead of wrapper methods:

  needs_kit = self.sim.get_setting("/isaaclab/runtime/needs_kit")
  needs_kit = has_kit() if needs_kit is None else bool(needs_kit)
  has_ovrtx_renderer = bool(self.sim.get_setting("/isaaclab/runtime/has_ovrtx_renderer"))

• The _cfg_renderer_types() method has been removed entirely (handled by launcher).

Changes in sim_launcher.py:

• Moved has_ovrtx_renderer computation earlier, before the Kit compatibility check.
• Added has_kit_visualizer computation.
• Settings published via get_settings_manager() with set_bool() calls.
• Newton visualizer path condition guards against overriding Kit setup: if not needs_kit and (visualizer_types or visualizer_explicit_none).

Assessment

This revision removes the runtime_needs_kit() and has_requested_renderer_type() wrapper methods from SimulationContext. The direct get_setting() approach is simpler — it avoids adding new public API surface. The fallback logic (has_kit() if needs_kit is None) handles the non-launcher path correctly.

No new issues found. The implementation is clean and minimal.

[huidongc]

This fix is needed in develop too. Otherwise the OVRTX render backend will get incorrect positions/orientations of the camera.