Update Visualizer Integration Tests

[matthewtrepte]

Description

Improve the visualizer integration test

• Unskip all tests
• Add retry loop for pause/replay step to reduce flakyness
• Fix camera pose of Kit Viz + Newton_MJWarp case
• Add debug image capture mode (disabled by default)
• Add simple tiled image check

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

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

[isaaclab-review-bot]

🔍 Isaac Lab Code Review

Thank you for this substantial PR improving the visualizer integration tests! I've conducted a thorough review focusing on test quality, Isaac Lab conventions, and potential issues.

✅ What's Working Well

1. Comprehensive Test Refactoring: The new test_visualizer_integration.py is significantly more robust than the previous implementation, with better structure and more thorough coverage of play/pause/resume cycles.

2. Multi-Backend Testing: The parameterized tests covering both PhysX and Newton MJWarp backends are well-designed and follow pytest best practices.

3. Debug Infrastructure: The debug image capture system (_WRITE_VIS_DEBUG_FRAMES, _save_visualizer_debug_image) is a valuable addition for diagnosing flaky tests.

4. Clean Config Reorganization (new in 6055e74): The VisualizerCfg fields are now logically grouped with section comments (Primary interactive camera, Tiled camera settings, Partial visualization, Markers, Live Plots, Internal). This improves readability significantly.

5. Improved UI Naming: The Newton visualizer's ImageLogger controls renamed to "Tiled Camera View" with a dedicated _draw_tiled_camera_view_controls method provides a clearer user experience.

6. Proper Test Separation (new in 6055e74): Splitting tiled camera tests into dedicated test_*_tiled_camera_motion functions is a much cleaner architecture than mixing assertions in the main motion tests.

🔎 Outstanding Items

Still Open from Initial Review:

1. _WRITE_VIS_DEBUG_FRAMES = True (P1) — Should be False for default CI runs to avoid writing debug PNGs to disk and PIL import dependency.

2. Missing pyglet headless configuration (P1) — Newton ViewerGL tests may fail on headless CI without explicit pyglet.options["headless"] = True before AppLauncher import.

3. Missing __import__("rerun") guard (P2) — Inconsistent with the viser handling; missing rerun package will raise ImportError instead of clean skip.

📋 Changes in This Update (ed71805 → 6055e74)

File	Change
visualizer_cfg.py	✅ Reorganized fields with section comments, improved docstrings
newton_visualizer.py	✅ New _draw_tiled_camera_view_controls() method, renamed section to "Tiled Camera View"
test_visualizer_integration.py	✅ Split tiled camera assertions into separate test_*_tiled_camera_motion test functions
changelog.d/*.rst	✅ Added proper changelog entry, removed .skip file

🏁 Summary

Excellent progress! This update addresses the test organization concerns by cleanly separating tiled camera tests into dedicated functions, and improves the config documentation significantly.

Remaining blockers before merge:

• Set _WRITE_VIS_DEBUG_FRAMES = False
• Add pyglet headless configuration
• Add rerun import guard

---

Automated review by Isaac Lab Review Bot | Updated for commit 6055e74

---

Update (dbcf34b): New commits add documentation reorganization (visualization.rst moved to overview/core-concepts), typed preset CLI integration for RL scripts, teleop replay stats enhancements, FabricFrameView multi-GPU fix, and various test/changelog updates. These changes do not touch the visualizer test files — the three items flagged above remain open.

---

Update (004a12c): Added changelog entry (changelog.d/mtrepte-update_viz_integration_test.rst) describing tiled camera default updates. No code changes in this commit — the three items flagged above remain open.

[greptile-apps]

Greptile Summary

This PR replaces the old test_visualizer_cartpole_integration.py with a new, more comprehensive test_visualizer_integration.py that unskips all visualizer tests, adds retry logic for flaky pause/replay steps, fixes the Kit Viz + Newton MJWarp camera pose, and introduces a debug image capture mode. It also adds new SceneDataBackend and SceneDataProvider abstractions in isaaclab.physics and isaaclab.scene, and adjusts KitVisualizer to allow camera sensor creation in headless mode while skipping only the UI image window.

• _WRITE_VIS_DEBUG_FRAMES = True is set in the new test file, contradicting the PR description of "disabled by default"; this causes PNG files to be written on every CI run and introduces a hard Pillow dependency.
• pyglet.options[\"headless\"] = True was present in the deleted file with an explicit comment explaining its necessity for Newton's ViewerGL but was not carried over, risking display-related failures on headless CI.
• SceneDataProvider._walk_camera_prims assigns world_id=0 to cameras outside /World/envs/ paths, which may silently conflate global cameras with env_0 slot data."

Confidence Score: 3/5

Two test-file issues need fixing before merging: the debug PNG write flag is enabled by default and the pyglet headless guard was dropped, both of which can cause unexpected CI failures.

The debug flag being left as True will write PNG files on every CI run and adds an implicit Pillow requirement; the missing pyglet headless setup is a regression from the deleted file that the original comment explicitly called out as required for headless Newton ViewerGL. Both issues are in the test path, not production code, but can cause CI runs to fail or behave unexpectedly.

source/isaaclab_visualizers/test/test_visualizer_integration.py needs the debug flag corrected and the pyglet headless setup restored; source/isaaclab/isaaclab/scene/scene_data_provider.py warrants a second look at how non-env-path cameras are handled in _walk_camera_prims.

Important Files Changed

Filename	Overview
source/isaaclab_visualizers/test/test_visualizer_integration.py	New consolidated integration test replacing the deleted file; _WRITE_VIS_DEBUG_FRAMES is True (should be False per PR description) and the pyglet headless setup required for Newton's ViewerGL on headless CI was dropped.
source/isaaclab_visualizers/isaaclab_visualizers/kit/kit_visualizer.py	Moves the headless-mode guard for camera setup to allow camera sensor creation in headless mode while skipping only the UI image window; splits _update_camera_image_panel guard so camera pose updates run even without an image provider.
source/isaaclab/isaaclab/scene/scene_data_provider.py	New SceneDataProvider with Warp conversion kernels and USD camera/env traversal. Non-env-path cameras silently get world_id=0 in _walk_camera_prims, which can mix global cameras into env_0 slots.
source/isaaclab/isaaclab/physics/scene_data_backend.py	New abstract SceneDataBackend interface with four Warp struct formats; clean separation from isaaclab.scene to avoid AppLauncher import ordering issues.
source/isaaclab_visualizers/test/test_visualizer_cartpole_integration.py	Deleted file intentionally replaced by test_visualizer_integration.py.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["KitVisualizer._setup_camera_sensor_view()"] --> B{uses_camera_sensor_view?}
    B -- No --> Z[return]
    B -- Yes --> C{cameras_enabled?}
    C -- No --> ERR[raise RuntimeError]
    C -- Yes --> D[Create / resolve camera sensor]
    D --> E{runtime_headless?}
    E -- No --> F[_setup_camera_image_window]
    E -- Yes --> G[Log: running without image panel]
    H["KitVisualizer._update_camera_image_panel()"] --> I{camera_sensor is None?}
    I -- Yes --> Z2[return]
    I -- No --> J[Update owned camera poses]
    J --> K[camera_sensor.update]
    K --> L{camera_image_provider is None?}
    L -- Yes --> Z3[return]
    L -- No --> M[camera_rgb_batch → upload to panel]

Loading

_{Reviews (1): Last reviewed commit: "sync" | Re-trigger Greptile}

[greptile-apps]

Debug flag enabled in test code — contradicts PR description

The PR description states that debug image capture is "disabled by default," but _WRITE_VIS_DEBUG_FRAMES = True means every CI run will write PNG files to logs/viz_integration_captures. This also introduces a hard runtime dependency on PIL (imported inside _save_visualizer_debug_image); if Pillow is not in the test dependencies, every test that reaches frame-capture paths will raise an ImportError.

Suggested change

	_WRITE_VIS_DEBUG_FRAMES = True
	_WRITE_VIS_DEBUG_FRAMES = False
	"""Whether to emit visualizer debug PNGs during integration tests."""

[greptile-apps]

Missing pyglet headless configuration required for Newton ViewerGL

The deleted test_visualizer_cartpole_integration.py contained an explicit comment explaining this requirement: "Pyglet must use HeadlessWindow (EGL) before pyglet.window is imported so Newton ViewerGL can construct without an X11 display." The new file omits import pyglet; pyglet.options["headless"] = True before AppLauncher. On headless CI machines without X11/Wayland, pyglet may fall back to a crashing backend at window-construction time, causing Newton visualizer tests to fail with a display error.

Suggested change

	from __future__ import annotations
	import warp as wp
	from isaaclab.app import AppLauncher
	# launch Kit app
	simulation_app = AppLauncher(headless=True, enable_cameras=True).app
	from __future__ import annotations
	import warp as wp
	# Pyglet must use HeadlessWindow (EGL) before ``pyglet.window`` is imported so Newton
	# ViewerGL can construct without an X11 display (matches ``headless=True`` on NewtonVisualizerCfg).
	import pyglet
	pyglet.options["headless"] = True
	from isaaclab.app import AppLauncher
	# launch Kit app
	simulation_app = AppLauncher(headless=True, enable_cameras=True).app

[greptile-apps]

Missing __import__("rerun") guard inconsistent with viser handling

For viser (line 255) the code guards with __import__("viser") before the lazy import, allowing a clean skip if the package is absent. The __import__("rerun") guard present in the deleted file was removed, so a missing rerun package now produces an ImportError from the from isaaclab_visualizers.rerun import ... line rather than a clean skip signal.

[greptile-apps]

Non-env-path cameras silently assigned world_id=0

In _walk_camera_prims, world_id is reset to 0 at the top of each iteration. Any camera prim whose path does not match _ENV_PATH_RE (e.g. /World/Camera) gets world_id=0, and the if world_id > num_envs: num_envs = world_id branch updates num_envs from -1 to 0. After num_envs += 1 the result is 1, so that global camera is placed into per_world_pos[0] — indistinguishable from an env_0 camera. Callers iterating the result by environment index will silently receive a global camera pose as if it belongs to env_0.