Repository navigation
Consolidate camera preparation and RTX exposure policy - #5
Open
mataylor-nvidia wants to merge 7 commits into
Open
mataylor-nvidia wants to merge 7 commits into
mataylor-nvidia wants to merge 7 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to isaac-sim#8352, targeting its source branch. Camera preparation and shared RTX exposure policy now have one owner each, with duplicate setup and coverage removed. RTX exposure is now neutralized only when
rgb_radianceis requested;isp_cfgalone preserves authored exposure. No public API changes.Move physics subscription management out of
CameraintoSensorBase. Its preparation callback runs at order 9 before sensor initialization at order 10, preserving weak ownership, exception forwarding, cleanup, and preparation of every camera before shared-stage export. Other sensors use a no-op hook: one extra lifecycle callback, no per-step work. Both initialization subscriptions are owned by one_initialize_handleExitStack and released with one close call; no tuple or_prepare_initialize_handlefield remains. Cleanup is registered immediately after each subscription so partial construction remains safe, and manually constructed sensor fixtures retain_initialize_handle = None.Consolidate renderer acquisition, prim lookup, and render-spec construction in
Camera. Resolve paths once per preparation call and retain the initialization fallback without cached state.Centralize the radiance exposure decision in the existing internal RTX helper. Both RTX backends pass their camera spec directly; PPISP normalization and backend-specific Gaussian routing remain with their existing owners.
Replace duplicate backend exposure tests with shared HDR/radiance/ISP cases using explicit expected values. Merge the RTX constructor settings assertions into the existing initialization-order test. Retain backend output extraction, Gaussian routing, buffer aliasing, and multi-camera preparation-order coverage.
The exposure-policy cleanup changes production code by +13/-12 lines and tests by +25/-64 lines (38 fewer lines overall). A core changelog entry documents the exposure behavior change; existing skip fragments cover the internal refactors.
Validation
On the final cleanup:
Camera output contract: 18 passed.
Shared RTX exposure policy: 3 passed.
Isaac RTX renderer contract: 35 passed.
OVRTX renderer contract, excluding integration: 55 passed, 5 deselected, using a fresh temporary directory.
Mutation check: bypassing the exposure-policy guard makes the standalone HDR case fail with the expected ISO mismatch; restoring it returns all 3 cases to passing.
Full
uv run isaaclab --format, changelog gate against the PR base, andgit diff --check: passed.On the final handle-consolidation commit: camera output contract (including preparation order and callback cleanup) 18 passed, sensor destructors 4 passed, and OpenCV distortion 46 passed. Full formatting/lint, changelog, and diff checks passed. Renderer-only results above are reused from the preceding exposure-policy commit.
Full Isaac Sim/RTX/OVRTX integration and PhysX runtime fixture tests remain unverified. The full OVRTX attempt stopped producing output and was terminated; the non-integration rerun passed. Initial checks encountered pytest cache/temp-directory issues; final focused runs disabled the cache and OVRTX used a fresh temporary directory. Existing PyTorch deprecation warnings were emitted.
Latest exposure-policy change
Removed the
isp_cfgcheck from the shared exposure helper. Onlyrgb_radiancetriggers neutral exposure; ISP normalization, pipeline setup, and Gaussian HDR routing retain their existing behavior. Updated both renderer docstrings and the core changelog.Validation on this commit: shared exposure tests 3 passed, Isaac RTX contracts 35 passed, OVRTX non-integration contracts 55 passed (5 integration cases deselected). Restoring the previous condition makes the ISP-only regression case fail; restoring the fix passes. Full formatting/lint, changelog, and diff checks passed. Earlier lifecycle test results above remain applicable.
The clean documentation build was attempted but blocked when an Omniverse Kit import requested EULA acceptance. The EULA was not accepted on the user's behalf.