feat(benchmark): add first-frame screenshot and scene metadata capture to benchmark_non_rl

[preeves-nv]

Summary

• Adds --preview_dir argument to benchmark_non_rl.py
• When set, captures a PNG screenshot of the first rendered frame immediately after env.reset() (before the measurement loop), plus a scene_meta.json with renderer name, shadow settings, and light count
• New capture_scene_preview() utility in utils.py, reusable by other benchmark scripts
• Both outputs are best-effort — failures log a warning and do not abort the benchmark

Motivation

When reviewing performance numbers on the dashboard it is hard to tell whether a regression is real or caused by a rendering issue (missing textures, wrong scene config, extra lights). A first-frame preview + scene descriptor gives reviewers that context without having to re-run or dig into job logs.

Test plan

• Run benchmark_non_rl.py with --preview_dir /tmp/bench_preview on a GTL runner and verify screenshot.png and scene_meta.json are written
• Run without --preview_dir and verify no change in benchmark behaviour or output
• Verify CI passes (no existing tests affected — new code is entirely behind the optional flag)

🤖 Generated with Claude Code

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR adds optional first-frame screenshot capture and scene metadata logging to the non-RL benchmark script. The implementation is well-isolated behind the --preview_dir flag, uses best-effort error handling, and correctly gates execution to rank 0 only in distributed scenarios. However, there is one clear bug and several minor issues.

Architecture Impact

Self-contained. The new capture_scene_preview() utility is only called from benchmark_non_rl.py when the optional flag is set. No existing code paths are affected when the flag is not used. The function is designed to be reusable by other benchmark scripts as noted in the PR description.

Implementation Verdict

Minor fixes needed — one bug must be fixed before merge.

Test Coverage

No tests are provided. This is acceptable for a best-effort diagnostic utility behind an optional flag in a benchmark script, but the manual test plan in the PR description should be executed before merge.

CI Status

No CI checks available yet — cannot assess.

Findings

🔴 Critical: benchmark_non_rl.py:145 — os.path.abs does not exist, should be os.path.abspath

log_root_path = os.path.abs(f"benchmark/{args_cli.task}")

This will raise AttributeError: module 'os.path' has no attribute 'abs' when --video is used. While this is in a pre-existing code path (not introduced by this PR), the diff shows the line was present before and the PR touches surrounding logic. This is a latent bug that should be fixed opportunistically — though strictly speaking it's not introduced by this PR.

🟡 Warning: utils.py:109 — numpy import is unused in the screenshot path

import numpy as np

The np import is not used — env.render() returns an array and Image.fromarray() accepts it directly. This is dead code that adds a small overhead. Remove the import.

🟡 Warning: utils.py:112-113 — Missing type validation for render output

rgb = env.render()
if rgb is not None and rgb.size > 0:

The check rgb.size > 0 assumes rgb has a .size attribute (numpy array). If env.render() returns something unexpected (e.g., a list or dict in edge cases with custom environments), this could raise AttributeError. Consider:

if rgb is not None and hasattr(rgb, 'size') and rgb.size > 0:

Or catch AttributeError in the existing exception handler.

🟡 Warning: utils.py:93-150 — Function docstring claims env should be "unwrapped" but env wrapper is passed

Args:
    env: The unwrapped IsaacLab gym environment, created with render_mode="rgb_array".

In benchmark_non_rl.py:158, the wrapped env is passed (potentially wrapped with RecordVideo). The function works correctly because env.render() propagates through wrappers, but the docstring is misleading. Update to: "The IsaacLab gym environment (may be wrapped)."

🔵 Improvement: utils.py:130-131 — Consider logging renderer setting path for debugging

When /rtx/shadows/enabled is None, it's unclear whether shadows are disabled or the setting doesn't exist. Consider:

shadows = settings.get("/rtx/shadows/enabled")
meta["shadows_enabled"] = shadows if shadows is not None else "unknown"

🔵 Improvement: benchmark_non_rl.py:27 — Argument help text should mention distributed behavior

parser.add_argument("--preview_dir", type=str, default=None, help="If set, capture a first-frame screenshot and scene metadata into this directory.")

The preview is only captured on rank 0. Consider updating help text: "...this directory (rank 0 only in distributed mode)."

🔵 Improvement: utils.py:137-138 — stage.Traverse() iteration could be expensive on large scenes

For complex USD stages, traversing all prims to count lights adds overhead. This is likely acceptable for a one-time diagnostic capture, but consider adding a warning in the docstring that this may be slow on large scenes, or adding an early-exit after finding N lights.

[preeves-nv]

Note on the failing docs check: The "Build & deploy docs / Build Docs" failure is not caused by this PR. The error is no module named isaaclab_tasks during Sphinx autosummary builder-inited — a docs CI environment issue unrelated to the scripts/benchmarks/ changes here. The docs.yaml workflow has no path filters so it fires on every PR. Will need an admin override to merge past this, or a fix to install isaaclab_tasks in the docs build environment.

[preeves-nv]

Update on the docs check: The previous comment was incorrect. The "Build & deploy docs / Build Docs" failure was caused by this PR targeting main instead of develop — PRs against main trigger a stricter docs build job that fails due to a missing isaaclab_tasks install. The PR has been retargeted to develop, which triggers "Build Latest Docs" instead — the correct job for contribution PRs. The failing check is gone and CI is now clean.