[hougantc-nvda]

Description

Review stack · 4/5: #8787 → #8788 → #8789 → #8790 → #8791

Review only this PR’s changes. The PRs target main; the comparison isolates this layer from its predecessors.

Add optional robot-camera panels (PiP) and a desktop mirror to OVXR teleoperation. The live and MCAP replay scripts enable the mirror by default. Add --headless to disable the local window; headset streaming and selected headset PiP remain available. The generic OVXR adapter retains its opt-in mirror default. Both views consume the session's completed images.

• isaaclab_teleop: selects camera feeds and supplies configured CameraPanelConsumer and OwnedDesktopMirror instances through OvxrSessionBinding.frame_consumers.
• isaaclab_ov: owns camera allocation, scene publication and native resource lifetime through the existing render lease.
• Isaac Capture: owns panel presentation and the mirror (#1295, #1296).

GR1T2 and G1 locomanipulation expose a robot-view camera; fixed-base G1 adds two wrist feeds. Its default policy observation space includes these image keys and requires camera rendering, including non-XR use; the documented camera-disable option remains available. Kit Scene UI retains its feed settings. SO101 has no declared PiP camera.

The adapter uses a tuple of auxiliary render-product paths for both one and multiple cameras. The mirror uses OVUI's default Vulkan backend. Neither Lab nor Isaac Capture changes OMNIUI_BACKEND; conflicting initialized backends are rejected. Disabled consumers are not created. --headless takes precedence over the explicit --ovxr_desktop_mirror flag.

Panels use the existing renderer and a head-locked layout. The mirror shows completed eye or camera images; eye images exclude the separate PiP layers. Closing the mirror requests orderly application shutdown.

PiP/mirror require the matching CUDA 13 package selection. The mirror also requires Python 3.12+, Capture's desktop-mirror extra and a usable GPU-backed Vulkan display. See camera panels and desktop mirror for selection and display constraints.

Dependencies: public isaaccapture==1.7.93a1 from NVIDIA’s Python index, using the optional XR selection documented in #8788. The stack includes upstream's merged provider upgrade #8858; the installation recipe reuses its OVRTX, OVStage, OvPhysX and schema declarations. Project-wide Kit/provider defaults are unchanged.

The head-locked CLI override preserves the task's automatic layout and spacing in both live teleoperation and replay. The fixed-base G1 guide describes its existing authored steering-wheel collision meshes accurately.

Visual materials register before deferred camera-scene population and bind their addresses before the first scene publication.

Type of change

• New feature
• Bug fix
• Documentation update

Unsupported panel placements are rejected at the OVXR resolver boundary; the separate Kit Scene UI placements remain supported.

Testing

At this PR boundary (d6527672d1e1): 284 passed, 3 skipped, 10 deselected, 36 warnings in the selected renderer, adapter, live/replay and task-owner suites applicable to this layer. These boundary runs overlap; their counts are not additive. Repository formatting/lint and the Python 3.12 changelog check pass. The reused public-package environment passes uv pip check for all 268 installed packages.

The task regressions exercise real registered constructors, public config serialization/restoration, and the actual Asset copy/spawn boundary. The GR1T2 action consumes the spawned asset’s conditioned hand metadata without custom copy semantics. G1 bounds/validation tests cover multiple environments and non-identity joint ordering.

Cumulative runtime integration at 4e608f200e21: public Capture 1.7.93a1 and the published CLI passed IWER scene/control checks for GR1T2 CPU OvPhysX, GR1T2 split-GPU Newton, fixed-base G1 split-GPU Newton, G1 locomanipulation CPU OvPhysX with --headless, and SO101 CPU OvPhysX. Both reconstructed eyes, meaningful motion and Reset were inspected. Requested PiP feeds and the default Vulkan mirror worked; closing the mirror shut down each non-headless instance normally. Headless G1 created no mirror even with the explicit mirror flag, retained PiP/streaming and exited normally after a deliberate owner interrupt. GR1T2 Newton and headless G1 also passed disconnect/reconnect and resumed motion. SO101 passed clutch/gripper checks; cube retention is not qualified.

With the same packages, hardware, input, seed and warm caches, GR1T2 CPU delivered 49.2 median decoded video frames/sec versus baseline runs of 49.0 and 50.4; fixed-base G1 Newton delivered 32.8 versus 33.0. Each median uses three 10-second steady windows. These bounded comparisons showed no material streaming slowdown; they do not measure headset FPS or establish a full physics-performance guarantee.

Fresh physical-headset, full native Kit/Isaac Sim, and native MCAP-file replay qualification were not performed in this follow-up. The historical cold-attach and intermittent GR1T2 first-Play reports remain open; successful warm runs do not establish a fix. Existing native USD/asset and split-GPU camera-device warnings remain visible.

Checklist

• I have read and understood the contribution guidelines
• I have run the repository formatting checks and the separate Python 3.12 changelog check
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have validated the changed behavior with focused coverage and applied the test-audit criteria to any test changes
• I have added a changelog fragment under source/<pkg>/changelog.d/ for every touched package (do not edit CHANGELOG.rst or bump package versions — CI handles that)
• I have added my name to the CONTRIBUTORS.md or my name already exists there

The warnings checkbox remains open: native-runtime warnings and the separate Kit dependency selection are not fully qualified by this run.

[hougantc-nvda]

@greptile Please review the latest changes in this PR.

[greptile-apps]

Confidence Score: 4/5

[High impact] The PR adds a new XR rendering path that shares the renderer across teleoperation and simulation workflows.

Fix the publication test fixture before merging.

Findings

1.  Publication tests fail during setup ▶

Summary

This PR adds optional OVXR camera panels and a desktop mirror. The latest changes publish visual materials through the render lease and prepare registered humanoid tasks before scene construction.

• Live teleoperation and replay can use OVXR, with optional camera panels and a desktop mirror.
• OVRTX and OVStage can be borrowed safely by an external renderer.
• Robot cameras now supply view feeds for contributed humanoid teleoperation tasks.
• Contributed robot tasks can run with kitless physics backends and task-specific teleoperation helpers.
• Pink hand joints keep their expected order, and Warp proxy arrays expose their updated operations.
• Headless XR keeps updating its scene, and applications can control simulation-context cleanup.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Task[Registered task] --> Prepare[Prepare selected physics]
    Prepare --> Env[Construct environment]
    Env --> Materials[Register visual materials]
    Materials --> Publish[Publish scene through render lease]
    Publish --> Capture[Capture renders completed images]
    Capture --> Panels[Optional camera panels]
    Capture --> Mirror[Optional desktop mirror]

Loading

_{Reviews (4) · Last reviewed commit: "Bind material addresses after deferred c..." · Reviewed by Greptile}

[hougantc-nvda]

@greptile Please review the current head (5421767) again. Recheck the fixes and remaining issues against the published code, including the actual interfaces and callers at this PR boundary.

[hougantc-nvda]

@greptile please review this PR again at the latest head (4d4579389784e7d22681b2d32cf6a97275ab489d), including the changes since your previous review.

[isaaclab-review-bot]

Isaac Lab Review Bot

Needs attention: Design and architecture (2), Scope and over-engineering (2), API (1), Compatibility and deprecation (7), Implementation (1), Style consistency (5), Test quality (5).

• PR description: The title understates renderer leasing, task backend preparation, and lifecycle changes; replace stale comparison and validation revisions with head-specific results, plus reproducible baseline/candidate XR frame-rate, step-rate, and peak-memory measurements.
• Design and architecture: Backend preparation has duplicate owners in HumanoidTeleopEnv and action_policy, while hand-presence policy bypasses IsaacTeleopDevice through private lifecycle state; centralize dispatch and expose a device property.
• Scope and over-engineering: Minimal implementation: extend OVRTXRenderer, task configs, and teleop entrypoints; keep render_lease, g1_kitless_newton, packing_table, gr1t2_kitless_action/_cfg, steering_wheel, teleop_physics_cfg, action_policy, ov_libraries_xr, ovxr_session, and xr_scene_partition; merge publication_buffers into OVRTXRenderer, delete teleop_env, and remove packing-table forwarding wrappers. Remaining flow is task selection → leased renderer → Capture consumers. Suggested PR breakdown: core fixes; task preparation; renderer lease; OVXR lifecycle; panels and mirror.
• API: OvLibrariesXrConfig exposes OvLibrariesXrCameraConfig in its public contract, but the lazy package export omits that required construction type; add it to all and the stub import.
• Compatibility and deprecation: Breaking changes: existing G1/GR1T2 registry classes, fixed-base G1 observations, SO101 reset defaults, optional-backend importability, and TeleopSessionLifecycle.stop failure behavior changed without deprecation bridges.
• Implementation: G1NewtonBoundedPinkInverseKinematicsAction allocates and stacks per environment on every IK step; cache stacked Pink limits and apply intersections and clamps as batched tensor operations.
• Style consistency: Cross-module helpers are exposed through underscore-private names and inconsistent aliases, obscuring their supported ownership; give shared helpers public names and use consistent relative imports within packages.
• Test quality: Added tests grep or AST-recompile production source, assert private module locations, and include assertion-free or unrelated negative controls; replace them with executed public-boundary behavior tests or delete them.

Significant concerns. Posted 23 inline findings.

The full PR diff was reviewed; some supplemental surrounding file context was omitted.

Automated review; human maintainers own approval decisions.

[hougantc-nvda]

@isaaclab-review-bot review