Add PPISP camera QA demos and discovery fallback

[moennen]

Description

This PR adds QA-oriented PPISP camera demo workflows for USD-authored Gaussian scenes and fixes a PPISP auto-discovery edge case.

Summary:

• Adds scripts/demos/sensors/ppisp_camera.py for Kit-based PPISP validation with newton and isaac_rtx renderers.
• Adds scripts/demos/sensors/ppisp_camera_ovrtx.py for kit-less OVRTX PPISP validation.
• Supports duplicated-env rendering from an input USD/USDZ scene so QA can exercise tiled camera output with num_envs > 1.
• Saves baseline, PPISP, and absolute-difference image grids for visual validation.
• Fixes auto_camera_ppisp_cfg() so a matching RenderProduct without a PPISP child does not prevent discovery of a later matching RenderProduct that does contain PPISP.
• Adds a unit test covering that PPISP discovery fallback.
• Adds missing isaaclab_ppisp docs/changelog scaffolding and a changelog fragment for the QA workflows.

Motivation/context:
Existing unit tests cover PPISP renderer integration, but QA needed a user-facing workflow that validates PPISP through CameraSensor-style rendering on a real USD-authored Gaussian scene. These
demos provide that workflow without requiring training jobs or task-specific environment changes.

Dependencies:

• A USD/USDZ scene containing PPISP-authored RenderProduct metadata.
• For OVRTX validation, an environment with the optional isaaclab_ov / ovrtx stack installed.
• OVRTX PPISP/HDR validation requires:
  OVRTX_rtx_rtpt_gaussian_skipTonemapping_enabled=0

Fixes #

Type of change

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

Screenshots

The demo scripts generate baseline / PPISP / diff image grids under the selected --output_dir. Attach representative generated images here if desired.

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]

---

Update (4f5b859): New commits address both previous inline comments:

• ✅ Unused rgb_hdr data type — Both demo scripts now use only data_types=["rgb"] in make_camera.
• ✅ Unreachable final raise — resolve_source_camera_binding is restructured in both scripts; the dead-code branch is removed.

No new issues found in the incremental changes.

---

Update (35794d8): Reviewed incremental diff (CI refactoring, auto_camera_ppisp_cfg bug fix, test docstring updates, rendering-correctness test reorganization). No new issues. Previous fixes remain intact. LGTM.

[greptile-apps]

Greptile Summary

This PR fixes a discovery bug in auto_camera_ppisp_cfg where an early return None would abort the search as soon as a camera-matching RenderProduct without a PPISP child was encountered, hiding any later RenderProduct that did carry PPISP. It also adds two QA demo scripts (ppisp_camera.py and ppisp_camera_ovrtx.py) for validating tiled PPISP output with duplicated-env scenes.

• cfg.py fix: removes the premature return None so the stage traversal continues past plain (non-PPISP) RenderProduct prims; a targeted unit test covers this fallback path.
• Demo scripts: both scripts load a USD scene, duplicate it across N envs, and periodically save baseline / PPISP / diff image grids; the OVRTX variant runs kit-less and adds make_matched_camera_prims_visible for render-product discovery.

Confidence Score: 4/5

The core bug fix is minimal and correct; the demo scripts are additive and carry only minor quality issues.

The one-line fix to cfg.py is correct and well-tested. The two new demo scripts work as intended but have a stale docstring on the fixed function, an unused rgb_hdr data type that requests an extra render pass with no consumer, and dead unreachable code in both scripts' resolve_source_camera_binding. None of these affect correctness of the primary fix or the existing library.

The docstring in cfg.py and the data_types list in ppisp_camera_ovrtx.py are the two spots worth a second look.

Important Files Changed

Filename	Overview
source/isaaclab_ppisp/isaaclab_ppisp/cfg.py	One-line bug fix removes the early return None that prevented the loop from continuing when a matching RenderProduct lacked a PPISP child; docstring is now stale and should be updated to reflect the new exhaustive-search behavior.
source/isaaclab_ppisp/test/test_ppisp.py	Adds a well-targeted regression test that exercises the fallback case: a plain RenderProduct (no PPISP child) followed by one that has PPISP; correctly asserts the cfg is found and its inputs match.
scripts/demos/sensors/ppisp_camera.py	New Kit-based PPISP QA demo; logic is sound, but resolve_source_camera_binding contains unreachable dead code (the final raise after an always-taken if-branch), and find_ppisp_camera_bindings is called twice in main() when the result could be reused.
scripts/demos/sensors/ppisp_camera_ovrtx.py	New kit-less OVRTX PPISP QA demo; same unreachable-code issue as ppisp_camera.py, plus the PPISP camera requests rgb_hdr data that is never consumed by run_simulator, wasting a render pass.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[auto_camera_ppisp_cfg\ncalled with camera_prim_path] --> B[Traverse stage prims]
    B --> C{prim type ==\nRenderProduct?}
    C -- No --> B
    C -- Yes --> D{camera rel\ntargets camera_prim_path?}
    D -- No --> B
    D -- Yes --> E{PPISP child\nshader exists?}
    E -- No --> B
    E -- Yes --> F[Parse & return PpispCfg]
    B -- exhausted --> G[return None]
    style E fill:#c8f7c5,stroke:#27ae60
    style G fill:#fadbd8,stroke:#e74c3c
    H[OLD behavior removed\nreturn None immediately\nif matching RP lacks PPISP] -.->|was here| E

Loading

Comments Outside Diff (1)

1. source/isaaclab_ppisp/isaaclab_ppisp/cfg.py, line 240-246 (link)

   The docstring still describes the old (pre-fix) behavior. It now incorrectly states the function returns None when a matching RenderProduct has no PPISP child — the whole point of this PR is that the loop now continues in that case. A future reader consulting this docstring will be misled about what the fallback behavior actually is.

_{Reviews (1): Last reviewed commit: "Add PPISP camera QA demos" | Re-trigger Greptile}

[greptile-apps]

The PPISP camera is configured with data_types=["rgb", "rgb_hdr"], but run_simulator only ever accesses data.output["rgb"]. The "rgb_hdr" channel is never read, so it requests an extra render pass (and any associated GPU memory / bandwidth) with no benefit.

Suggested change

	ppisp_camera = make_camera(
	camera_prim_path,
	ppisp_cfg=ppisp_cfg,
	width=width,
	height=height,
	data_types=["rgb", "rgb_hdr"],
	)
	ppisp_camera = make_camera(
	camera_prim_path,
	ppisp_cfg=ppisp_cfg,
	width=width,
	height=height,
	data_types=["rgb"],
	)

[greptile-apps]

Unreachable final raise in resolve_source_camera_binding

After the for-loop completes without returning, camera_prim_path is by definition absent from ppisp_camera_prim_paths (if it were present the loop would have matched and returned). Therefore the if camera_prim_path not in ppisp_camera_prim_paths: branch is always taken, and the final raise RuntimeError(f"Failed to resolve PPISP binding for camera: ...") on line 229 is dead code. The same pattern is duplicated in ppisp_camera_ovrtx.py.

[isaaclab-review-bot]

Re-review update (6b0b554): Reviewed incremental changes since previous review (63bbb5c).

✅ Good changes:

• The cfg.py fix for auto_camera_ppisp_cfg discovery (removing premature return None) correctly addresses a bug where generated RenderProduct prims without PPISP could mask later camera-bound PPISP render products. Test coverage added.
• Documentation updates in teleop_replay_agent.py appropriately generalize "Kit" references to "simulator".
• Changelog and docs additions are appropriate.

⚠️ Previous comments still open:
The two P2 inline comments from the earlier review remain unaddressed in the PPISP demo scripts:

1. Unused rgb_hdr data type in ppisp_camera_ovrtx.py
2. Unreachable final raise in resolve_source_camera_binding (both demo scripts)

[isaaclab-review-bot]

Review Update (13204ce)

✅ Both previous concerns have been addressed:

1. data_types fix: OVRTX demo now correctly uses only ["rgb"] - no unused rgb_hdr channel
2. Dead code fix: resolve_source_camera_binding refactored to eliminate unreachable final raise

Additional improvement in this commit:

• Fixed auto_camera_ppisp_cfg discovery so generated RenderProduct prims without PPISP do not mask later camera-bound PPISP render products (with corresponding test coverage)

LGTM 👍

[moennen]

@kellyguo11 : here is the QA PR (cannot add you as a reviewer)

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot - Follow-up Review

Commit: ac44ee0

Previous Issues Status

✅ Fixed: Unused "rgb_hdr" data type removed from make_camera - now only requests ["rgb"].

✅ Fixed: Unreachable code in resolve_source_camera_binding removed - function rewritten with cleaner binding lookup logic.

New Changes in This Commit

• Docstring improvements across PPISP test files (ISP → PPISP, clarified invariants)
• Bug fix for auto_camera_ppisp_cfg discovery (with new test case)
• Changelog and README additions for isaaclab_ppisp

Summary

All previous review comments have been addressed. No new issues identified.

LGTM ✅

[isaaclab-review-bot]

Re-review Update (6132e0e)

Both previous concerns have been addressed:

✅ P2 (data_types): ppisp_camera_ovrtx.py now uses data_types=["rgb"] instead of requesting unused rgb_hdr.

✅ P2 (unreachable code): The dead raise RuntimeError after the loop has been removed. The logic now cleanly returns the binding when found, or raises a clear error with available options if no match exists.

Additionally, this commit:

• Fixes auto_camera_ppisp_cfg discovery to continue scanning when a RenderProduct without PPISP is encountered (with test coverage)
• Improves docstring consistency (ISP → PPISP terminology)
• Adds changelog and README documentation

LGTM 👍

[isaaclab-review-bot]

🤖 Review Update (1f8ab7a)

✅ Both previous concerns addressed:

1. Unused rgb_hdr in ppisp_camera_ovrtx.py — Fixed. The make_camera function now uses data_types=["rgb"] only.

2. Unreachable final raise in resolve_source_camera_binding — Fixed in both ppisp_camera.py and ppisp_camera_ovrtx.py. The dead code has been replaced with a clear error message listing available PPISP-bound cameras.

LGTM — no new issues found in the incremental changes.