Add perspective video recording via Newton GL viewer for Kitless backends; Fix for Kitfull backends camera position

[bdilinila]

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 introduces a pluggable VideoRecorder abstraction for perspective video recording that works across both Kitless (Newton GL viewer) and Kitfull (Isaac Sim / omni.replicator) backends, and fixes the camera position bug in Kitfull backends by explicitly calling set_camera_view before creating the render product. The old inline omni.replicator logic in three env classes (DirectRLEnv, DirectMARLEnv, ManagerBasedRLEnv) is replaced by delegating to the new VideoRecorder.

Key concerns:

• Critical (manager_based_rl_env.py): self.video_recorder is referenced in ManagerBasedRLEnv.render() but is never assigned. Only cfg.video_recorder.render_mode is set before super().__init__() — neither ManagerBasedRLEnv.__init__() nor the unmodified ManagerBasedEnv.__init__() creates the actual VideoRecorder instance. This will raise AttributeError at runtime for any manager-based environment using --video.
• Bug (newton_gl_perspective_video.py, line 632–633): No guard against camera_eye == camera_lookat, which causes a ZeroDivisionError when normalizing the look direction.
• Concern (newton_gl_perspective_video.py, render_rgb_array): initialize_scene_data_provider() is called on every frame inside render_rgb_array(), which appears to be an initialization method rather than an accessor; this should likely be called once and cached.
• Style (test_video_recorder.py): The test file lives inside the production source package rather than a dedicated tests/ directory, and its fake config includes fields (video_mode, fallback_camera_cfg, video_num_tiles) not present on the real VideoRecorderCfg, reducing the tests' fidelity.

Confidence Score: 1/5

• Not safe to merge — manager-based environments will throw AttributeError when --video is used, and the Newton GL backend has an unguarded division-by-zero.
• A critical runtime AttributeError in ManagerBasedRLEnv (self.video_recorder never assigned), plus a ZeroDivisionError in the Newton GL camera setup, and a per-frame re-initialization concern block merging until fixed.
• source/isaaclab/isaaclab/envs/manager_based_rl_env.py (missing self.video_recorder assignment) and source/isaaclab_newton/isaaclab_newton/video_recording/newton_gl_perspective_video.py (division by zero + initialize_scene_data_provider called every frame).

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/envs/manager_based_rl_env.py	Forwards render_mode to VideoRecorderCfg before super().init(), but never assigns self.video_recorder — causing an AttributeError at runtime when --video is used with manager-based environments.
source/isaaclab_newton/isaaclab_newton/video_recording/newton_gl_perspective_video.py	Implements headless Newton GL perspective capture; has a ZeroDivisionError when camera_eye equals camera_lookat, and calls initialize_scene_data_provider() on every rendered frame rather than caching the provider.
source/isaaclab/isaaclab/envs/utils/video_recorder.py	New VideoRecorder class that dispatches to kit or newton_gl backend based on physics/renderer detection; logic is clean, though _resolve_video_backend relies on physics_manager being a class type (which it is per simulation_context.py).
source/isaaclab/isaaclab/envs/utils/video_recorder_cfg.py	Defines VideoRecorderCfg with camera position, resolution, and render_mode fields; missing fields (video_mode, fallback_camera_cfg, video_num_tiles) referenced in tests create a mismatch with the test suite.
source/isaaclab_physx/isaaclab_physx/video_recording/isaacsim_kit_perspective_video.py	Implements Kit perspective capture via omni.replicator; sets camera view lazily on first render call and handles warmup with a blank frame; no cleanup/close method for the render product.
source/isaaclab/isaaclab/envs/direct_rl_env.py	Correctly instantiates self.video_recorder before sim.reset(), delegates render() to the recorder, and removes the old omni.replicator inline logic.
source/isaaclab/isaaclab/envs/utils/test_video_recorder.py	Unit tests bypass init via object.new and SimpleNamespace, which avoids testing actual construction; test config includes fields not present in VideoRecorderCfg; file is placed inside the production package rather than a test directory.

Sequence Diagram

sequenceDiagram
    participant Env as Environment (__init__)
    participant VRCfg as VideoRecorderCfg
    participant VR as VideoRecorder
    participant Backend as Kit / Newton GL Backend

    Env->>VRCfg: cfg.video_recorder.render_mode = render_mode
    Env->>VR: VideoRecorder(cfg, scene)
    VR->>VR: _resolve_video_backend(scene)
    alt Kit backend (PhysX / Isaac RTX)
        VR->>Backend: IsaacsimKitPerspectiveVideo(cfg)
    else Newton GL backend
        VR->>Backend: NewtonGlPerspectiveVideo(cfg)
    end

    loop Each render() call
        Env->>VR: render_rgb_array()
        VR->>Backend: render_rgb_array()
        alt Kit
            Backend->>Backend: set_camera_view (lazy, first call only)
            Backend->>Backend: annotator.get_data()
        else Newton GL
            Backend->>Backend: _ensure_viewer() (lazy)
            Backend->>Backend: viewer.begin_frame / log_state / end_frame
            Backend->>Backend: viewer.get_frame().numpy()
        end
        Backend-->>VR: np.ndarray (H×W×3)
        VR-->>Env: np.ndarray (H×W×3)
    end

Loading

Comments Outside Diff (3)

1. source/isaaclab_newton/isaaclab_newton/video_recording/newton_gl_perspective_video.py, line 631-633 (link)

   Division by zero when camera_eye == camera_lookat

   If camera_eye and camera_lookat are equal (or very close), length will be zero (or near-zero), causing a ZeroDivisionError on the normalization lines. There is no guard against this case.

   dx, dy, dz = lx - ex, ly - ey, lz - ez
   length = math.sqrt(dx**2 + dy**2 + dz**2)
   dx, dy, dz = dx / length, dy / length, dz / length  # ZeroDivisionError if length == 0

   A minimal guard should be added:

   if length < 1e-6:
       raise ValueError(
           "camera_eye and camera_lookat are identical (or too close); "
           "cannot determine camera direction."
       )

2. source/isaaclab_newton/isaaclab_newton/video_recording/newton_gl_perspective_video.py, line 648-653 (link)

   initialize_scene_data_provider() called on every frame

   render_rgb_array() calls sim.initialize_scene_data_provider() on every invocation:

   sdp = sim.initialize_scene_data_provider()
   state = sdp.get_newton_state()

   Calling an initialize method every frame is likely incorrect and potentially expensive or side-effect-laden. Looking at _ensure_viewer(), it already calls initialize_scene_data_provider() once during setup to get the model. For subsequent per-frame calls, the intent is presumably to retrieve the already-initialized provider (e.g., via a get_scene_data_provider() accessor), not to re-initialize it. If the simulation context doesn't have a separate getter, initialize_scene_data_provider() may be idempotent — but that should be verified and documented rather than relied upon silently.

3. source/isaaclab/isaaclab/envs/utils/test_video_recorder.py, line 286-295 (link)

   Test config contains fields not present in VideoRecorderCfg

   The default test config references three fields — video_mode, fallback_camera_cfg, and video_num_tiles — that do not exist on the actual VideoRecorderCfg class:

   _DEFAULT_CFG = dict(
       render_mode="rgb_array",
       video_mode="perspective",        # not in VideoRecorderCfg
       fallback_camera_cfg=None,        # not in VideoRecorderCfg
       video_num_tiles=-1,              # not in VideoRecorderCfg
       ...
   )

   Because the tests create recorders via SimpleNamespace and bypass __init__, these extra fields never raise an error, but they create false confidence that the tested API matches the real one. Consider removing spurious fields or explaining why they are needed.

_{Last reviewed commit: "Merge branch 'develo..."}