diff --git a/source/isaaclab/changelog.d/fix-camera-data-physics-dependency.changed.rst b/source/isaaclab/changelog.d/fix-camera-data-physics-dependency.changed.rst new file mode 100644 index 000000000000..466413df0e20 --- /dev/null +++ b/source/isaaclab/changelog.d/fix-camera-data-physics-dependency.changed.rst @@ -0,0 +1 @@ +* Changed RTX cameras to preserve authored exposure when only ``CameraCfg.isp_cfg`` is set. diff --git a/source/isaaclab/changelog.d/fix-camera-data-physics-dependency.skip b/source/isaaclab/changelog.d/fix-camera-data-physics-dependency.skip new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab/isaaclab/sensors/camera/camera.py b/source/isaaclab/isaaclab/sensors/camera/camera.py index 1861aa096152..afce70a1409b 100644 --- a/source/isaaclab/isaaclab/sensors/camera/camera.py +++ b/source/isaaclab/isaaclab/sensors/camera/camera.py @@ -7,7 +7,6 @@ import logging import sys -import weakref from collections.abc import Sequence from typing import TYPE_CHECKING, Any, Literal, cast @@ -19,7 +18,6 @@ from ... import sim as sim_utils from ...app.logging_utils import force_log_level -from ...physics import PhysicsEvent, PhysicsManager from ...renderers import BaseRenderer, CameraRenderSpec from ...sim.views import FrameView from ...utils.math import ( @@ -208,7 +206,6 @@ def __init__(self, cfg: CameraCfg): RuntimeError: If no camera prim is found at the given path. ValueError: If the provided data types are not supported by the camera or active renderer. """ - self._prepare_renderer_handle = None # perform check on supported data types self._check_supported_data_types(cfg) # initialize base class @@ -641,23 +638,12 @@ def _initialize_impl(self): sim_ctx = sim_utils.SimulationContext.instance() if sim_ctx is None: raise RuntimeError("SimulationContext is not initialized.") - # Normally created in ``__init__``; only missing when the camera was built without a simulation. - if self._renderer is None: - self._renderer = sim_ctx.get_or_create_backend(self.cfg.renderer_cfg) - # Per-camera USD setup must run **before** ``ensure_prepare_stage`` so renderers that snapshot # the stage (ovrtx's ``stage.Export``) capture the overrides in their exported USD. Every camera - # already did this in :meth:`_prepare_renderer_callback`; repeating it is harmless and covers + # already did this in :meth:`_prepare_initialize_callback`; repeating it is harmless and covers # cameras initialized without that callback. - cam_paths = tuple(str(p.GetPath()) for p in sim_utils.find_matching_prims(self.cfg.prim_path, self.stage)) - render_spec = CameraRenderSpec( - cfg=self.cfg, - device=str(sim_ctx.device), - num_instances=self._num_envs, - camera_prim_paths=cam_paths, - view_count=self._num_envs, - ) - self._renderer.prepare_cameras(self.stage, render_spec) + render_spec = self._prepare_renderer(self._num_envs) + cam_paths = render_spec.camera_prim_paths # Stage preprocessing must happen before creating the view because the view keeps # references to prims located in the stage. @@ -1027,31 +1013,7 @@ def _env_mask_has_any(self, env_mask: wp.array) -> bool: Internal simulation callbacks. """ - def _register_callbacks(self): - """Register camera stage preparation before the ordinary sensor initialization callbacks.""" - super()._register_callbacks() - physics_mgr_cls = sim_utils.SimulationContext.instance().physics_manager - obj_ref = weakref.proxy(self) - - def _prepare(event): - obj_ref._prepare_renderer_callback(event) - - # All cameras must author their USD overrides before the first camera exports the shared stage. - # Sensor initialization runs at order 10. Preserve weak ownership and backend exception forwarding. - self._prepare_renderer_handle = physics_mgr_cls.register_callback( - lambda payload: PhysicsManager.safe_callback_invoke(_prepare, payload, physics_manager=physics_mgr_cls), - PhysicsEvent.PHYSICS_READY, - order=9, - ) - - def _clear_callbacks(self) -> None: - """Release the camera preparation subscription along with the ordinary sensor subscriptions.""" - if self._prepare_renderer_handle is not None: - self._prepare_renderer_handle.deregister() - self._prepare_renderer_handle = None - super()._clear_callbacks() - - def _prepare_renderer_callback(self, event): + def _prepare_initialize_callback(self, event): """Apply this camera's renderer USD overrides before any camera initializes. A renderer that exports the stage when the first camera initializes, such as OVRTX, then sees @@ -1059,16 +1021,17 @@ def _prepare_renderer_callback(self, event): """ if self._is_initialized: return + self._prepare_renderer() + + def _prepare_renderer(self, num_views: int | None = None) -> CameraRenderSpec: + """Resolve camera prims and apply renderer overrides before shared-stage export.""" sim_ctx = sim_utils.SimulationContext.instance() if self._renderer is None: self._renderer = sim_ctx.get_or_create_backend(self.cfg.renderer_cfg) - # Matches the environment count that sensor initialization derives next. - clone_plan = sim_ctx.get_clone_plan() - if clone_plan is not None: - num_views = len(clone_plan.topology.world_prototype_layout) - else: - num_views = len(sim_utils.find_matching_prims(self.cfg.prim_path, self.stage)) cam_paths = tuple(str(p.GetPath()) for p in sim_utils.find_matching_prims(self.cfg.prim_path, self.stage)) + if num_views is None: + clone_plan = sim_ctx.get_clone_plan() + num_views = len(clone_plan.topology.world_prototype_layout) if clone_plan is not None else len(cam_paths) render_spec = CameraRenderSpec( cfg=self.cfg, device=str(sim_ctx.device), @@ -1077,6 +1040,7 @@ def _prepare_renderer_callback(self, event): view_count=num_views, ) self._renderer.prepare_cameras(self.stage, render_spec) + return render_spec def _invalidate_initialize_callback(self, event): """Invalidates the scene elements.""" diff --git a/source/isaaclab/isaaclab/sensors/sensor_base.py b/source/isaaclab/isaaclab/sensors/sensor_base.py index b23b1fc43036..5998df0ef062 100644 --- a/source/isaaclab/isaaclab/sensors/sensor_base.py +++ b/source/isaaclab/isaaclab/sensors/sensor_base.py @@ -17,6 +17,7 @@ import weakref from abc import ABC, abstractmethod from collections.abc import Callable, Sequence +from contextlib import ExitStack from typing import TYPE_CHECKING, Any import warp as wp @@ -344,13 +345,23 @@ def _invoke(callback_name, event): getattr(obj_ref, callback_name)(event) # Backend-agnostic: PHYSICS_READY initializes sensors; STOP invalidates them. - self._initialize_handle = physics_mgr_cls.register_callback( + self._initialize_handle = ExitStack() + prepare_handle = physics_mgr_cls.register_callback( + lambda payload: PhysicsManager.safe_callback_invoke( + _invoke, "_prepare_initialize_callback", payload, physics_manager=physics_mgr_cls + ), + PhysicsEvent.PHYSICS_READY, + order=9, + ) + self._initialize_handle.callback(prepare_handle.deregister) + initialize_handle = physics_mgr_cls.register_callback( lambda payload: PhysicsManager.safe_callback_invoke( _invoke, "_initialize_callback", payload, physics_manager=physics_mgr_cls ), PhysicsEvent.PHYSICS_READY, order=10, ) + self._initialize_handle.callback(initialize_handle.deregister) self._invalidate_initialize_handle = physics_mgr_cls.register_callback( lambda payload: PhysicsManager.safe_callback_invoke( _invoke, "_invalidate_initialize_callback", payload, physics_manager=physics_mgr_cls @@ -372,6 +383,10 @@ def _invoke(callback_name, event): IsaacEvents.PRIM_DELETION, ) + def _prepare_initialize_callback(self, event): + """Prepare shared stage state before any sensor's initialization callback runs.""" + pass + def _initialize_callback(self, event): """Initializes the scene elements. @@ -404,7 +419,7 @@ def _on_prim_deletion(self, event) -> None: def _clear_callbacks(self) -> None: """Clears the callbacks.""" if self._initialize_handle is not None: - self._initialize_handle.deregister() + self._initialize_handle.close() self._initialize_handle = None if self._invalidate_initialize_handle is not None: self._invalidate_initialize_handle.deregister() diff --git a/source/isaaclab/test/renderers/test_camera_output_contract.py b/source/isaaclab/test/renderers/test_camera_output_contract.py index abe6c61b699c..fc7f5efb04ba 100644 --- a/source/isaaclab/test/renderers/test_camera_output_contract.py +++ b/source/isaaclab/test/renderers/test_camera_output_contract.py @@ -395,7 +395,9 @@ def export_stage(stage, num_envs): cameras = [] for prim, data_types in zip(prims, (["rgb"], ["rgb_radiance"])): camera = Camera.__new__(Camera) - camera.cfg = SimpleNamespace(prim_path=str(prim.GetPath()), data_types=data_types, height=2, width=3) + camera.cfg = SimpleNamespace( + prim_path=str(prim.GetPath()), data_types=data_types, height=2, width=3, isp_cfg=None + ) camera.stage = stage camera._device = "cpu" camera._num_envs = 2 diff --git a/source/isaaclab/test/sensors/test_opencv_distortion.py b/source/isaaclab/test/sensors/test_opencv_distortion.py index 6eb473d085f6..6d276bc0f451 100644 --- a/source/isaaclab/test/sensors/test_opencv_distortion.py +++ b/source/isaaclab/test/sensors/test_opencv_distortion.py @@ -192,7 +192,6 @@ def _camera_for_prims(prims, width=640, height=480, device="cpu"): fake._ALL_INDICES = wp.array(np.arange(len(prims)), dtype=wp.int32, device=device) fake._ALL_ENV_MASK = wp.ones(len(prims), dtype=wp.bool, device=device) # attributes touched by ``__del__``/``_clear_callbacks`` when the fake object is garbage collected - fake._prepare_renderer_handle = None fake._initialize_handle = None fake._invalidate_initialize_handle = None fake._prim_deletion_handle = None diff --git a/source/isaaclab/test/sensors/test_sensor_destructors.py b/source/isaaclab/test/sensors/test_sensor_destructors.py index 99f910f60927..a5d057d9b8f3 100644 --- a/source/isaaclab/test/sensors/test_sensor_destructors.py +++ b/source/isaaclab/test/sensors/test_sensor_destructors.py @@ -77,7 +77,6 @@ def cleanup(self, _render_data): view = _View() renderer = _Renderer() camera = object.__new__(Camera) - camera._prepare_renderer_handle = None camera._view = view camera._renderer = renderer camera._render_data = None diff --git a/source/isaaclab_ov/changelog.d/fix-camera-data-physics-dependency.skip b/source/isaaclab_ov/changelog.d/fix-camera-data-physics-dependency.skip new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py b/source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py index 355932a343d0..03e604152bf6 100644 --- a/source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py +++ b/source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py @@ -488,11 +488,9 @@ def prepare_cameras(self, stage: Any, spec: CameraRenderSpec) -> None: When ``spec.cfg.isp_cfg`` is set, resolves it (sentinel discovery + normalization) via :func:`isaaclab_ppisp.resolve_and_normalize` so - :mod:`isaaclab` does not need to know about PPISP. Then pins - ``exposure:*`` to neutral and applies ``OmniRtxCameraExposureAPI_1`` so - the RTX exposure model OVRTX embeds does not compound on top of the - ISP. Exposure is also neutralized when ``rgb_radiance`` is requested. Otherwise, the - camera prim's authored exposure is left alone. + :mod:`isaaclab` does not need to know about PPISP. Camera exposure is neutralized + only when ``rgb_radiance`` is requested; otherwise authored exposure is preserved, + including when an ISP is configured. """ if spec.cfg.isp_cfg is not None: try: @@ -502,8 +500,7 @@ def prepare_cameras(self, stage: Any, spec: CameraRenderSpec) -> None: camera_prim_path = spec.camera_prim_paths[0] if spec.camera_prim_paths else None spec.cfg.isp_cfg = resolve_and_normalize(spec.cfg.isp_cfg, stage, camera_prim_path) - needs_neutral_exposure = spec.cfg.isp_cfg is not None or "rgb_radiance" in spec.cfg.data_types - if needs_neutral_exposure and spec.camera_prim_paths: + if "rgb_radiance" in spec.cfg.data_types and spec.camera_prim_paths: _apply_rtx_exposure_overrides(stage, list(spec.camera_prim_paths)) def prepare_stage(self, stage: Any, num_envs: int) -> None: diff --git a/source/isaaclab_ov/test/test_ovrtx_renderer_contract.py b/source/isaaclab_ov/test/test_ovrtx_renderer_contract.py index cf8ecfc4e025..61085e954bde 100644 --- a/source/isaaclab_ov/test/test_ovrtx_renderer_contract.py +++ b/source/isaaclab_ov/test/test_ovrtx_renderer_contract.py @@ -570,26 +570,41 @@ def test_ovrtx_hdr_color_fills_requested_hdr_outputs(monkeypatch, data_types): np.testing.assert_array_equal(data.output[name].warp.numpy(), 2.0) -@pytest.mark.parametrize("data_types", [["rgb_hdr"], ["rgb_radiance"]]) -def test_ovrtx_prepare_cameras_neutralizes_exposure_for_radiance(data_types): +@pytest.mark.parametrize( + ("data_types", "isp_cfg", "radiance"), + [(["rgb_hdr"], None, False), (["rgb_radiance"], None, True), (["rgb"], object(), False)], +) +def test_ovrtx_prepare_cameras_neutralizes_exposure_for_radiance(monkeypatch, data_types, isp_cfg, radiance): """Radiance neutralizes camera exposure; standalone HDR keeps the authored camera settings.""" from pxr import Sdf, Usd, UsdGeom - authored = {"exposure": 2.0, "exposure:fStop": 2.8, "exposure:iso": 800.0, "exposure:responsivity": 0.5} - neutral = {"exposure": 0.0, "exposure:fStop": 1.0, "exposure:iso": 0.0, "exposure:responsivity": 1.0} + monkeypatch.setattr("isaaclab_ppisp.resolve_and_normalize", lambda cfg, *args: cfg) + authored = { + "exposure": 2.0, + "exposure:fStop": 2.8, + "exposure:iso": 800.0, + "exposure:responsivity": 0.5, + "exposure:time": 0.125, + } + neutral = { + "exposure": 0.0, + "exposure:fStop": 1.0, + "exposure:iso": 0.0, + "exposure:responsivity": 1.0, + "exposure:time": 1.0, + } stage = Usd.Stage.CreateInMemory() camera = UsdGeom.Camera.Define(stage, "/World/Camera").GetPrim() for name, value in authored.items(): camera.CreateAttribute(name, Sdf.ValueTypeNames.Float).Set(value) camera.CreateAttribute("omni:rtx:autoExposure:enabled", Sdf.ValueTypeNames.Bool).Set(True) spec = types.SimpleNamespace( - cfg=types.SimpleNamespace(data_types=data_types, isp_cfg=None), camera_prim_paths=("/World/Camera",) + cfg=types.SimpleNamespace(data_types=data_types, isp_cfg=isp_cfg), camera_prim_paths=("/World/Camera",) ) _make_ovrtx_renderer_without_backend().prepare_cameras(stage, spec) - radiance = "rgb_radiance" in data_types for name, value in (neutral if radiance else authored).items(): - assert camera.GetAttribute(name).Get() == value + assert camera.GetAttribute(name).Get() == pytest.approx(value) assert camera.GetAttribute("omni:rtx:autoExposure:enabled").Get() is not radiance schemas = camera.GetMetadata("apiSchemas") applied = [] if schemas is None else [*schemas.prependedItems, *schemas.explicitItems, *schemas.appendedItems] diff --git a/source/isaaclab_physx/changelog.d/fix-camera-data-physics-dependency.skip b/source/isaaclab_physx/changelog.d/fix-camera-data-physics-dependency.skip new file mode 100644 index 000000000000..e69de29bb2d1 diff --git a/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py b/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py index 083fd76ba769..35931df7205c 100644 --- a/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py +++ b/source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py @@ -261,11 +261,9 @@ def prepare_cameras(self, stage: Any, spec: CameraRenderSpec) -> None: When ``spec.cfg.isp_cfg`` is set, resolves it (sentinel discovery + normalization) via :func:`isaaclab_ppisp.resolve_and_normalize` so - :mod:`isaaclab` does not need to know about PPISP. Then pins - ``exposure:*`` to neutral and applies ``OmniRtxCameraExposureAPI_1`` so - RTX's physical-camera exposure model does not compound on top of the - ISP. Exposure is also neutralized when ``rgb_radiance`` is requested. Otherwise, the - camera prim's authored exposure is left alone. + :mod:`isaaclab` does not need to know about PPISP. Camera exposure is neutralized + only when ``rgb_radiance`` is requested; otherwise authored exposure is preserved, + including when an ISP is configured. :attr:`~isaaclab.sensors.camera.CameraCfg.background_color` is applied per-render-product in :meth:`create_render_data` via USD attributes. @@ -281,8 +279,7 @@ def prepare_cameras(self, stage: Any, spec: CameraRenderSpec) -> None: if spec.cfg.isp_cfg is not None or not _HDR_DATA_TYPES.isdisjoint(spec.cfg.data_types): # Gaussian splats otherwise skip tonemapping and never reach the HdrColor AOV. get_settings_manager().set("/rtx/rtpt/gaussian/skipTonemapping/enabled", False) - needs_neutral_exposure = spec.cfg.isp_cfg is not None or "rgb_radiance" in spec.cfg.data_types - if needs_neutral_exposure and spec.camera_prim_paths: + if "rgb_radiance" in spec.cfg.data_types and spec.camera_prim_paths: _apply_rtx_exposure_overrides(stage, list(spec.camera_prim_paths)) def supported_output_types(self) -> dict[RenderBufferKind, RenderBufferSpec]: diff --git a/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py b/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py index 88330b030651..dde51e826d90 100644 --- a/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py +++ b/source/isaaclab_physx/test/renderers/test_isaac_rtx_renderer_contract.py @@ -87,26 +87,6 @@ def test_native_fabric_geometry_needs_no_separate_publication(monkeypatch): provider.get_geometry_points.assert_not_called() -def test_renderer_marks_rtx_sensor_rendering_when_created(monkeypatch): - """Creating the renderer, which cameras do during scene construction, marks RTX sensor rendering.""" - _install_omni_stubs(monkeypatch) - import isaaclab_physx.renderers.isaac_rtx_renderer as rtx_renderer - from isaaclab_physx.renderers.isaac_rtx_renderer_cfg import IsaacRtxRendererCfg - - settings = MagicMock() - settings.get.return_value = False - with ( - patch.object(rtx_renderer, "get_settings_manager", return_value=settings), - patch.object(rtx_renderer, "enable_extension"), - patch.object(rtx_renderer, "apply_isaac_rtx_global_settings"), - patch.object(rtx_renderer, "ensure_rtx_hydra_engine_attached"), - ): - rtx_renderer.IsaacRtxRenderer(IsaacRtxRendererCfg()) - - settings.set.assert_any_call("/isaaclab/render/rtx_sensors", True) - settings.set.assert_any_call("/physics/fabricUpdateTransformations", True) - - @pytest.mark.parametrize( ("data_types", "isp_cfg", "routes_hdr"), [(["rgb"], None, False), (["rgb_hdr"], None, True), (["rgb_radiance"], None, True), (["rgb"], object(), True)], @@ -128,29 +108,44 @@ def test_prepare_cameras_routes_gaussian_hdr_for_hdr_cameras(monkeypatch, data_t assert (hdr_call in settings.set.call_args_list) is routes_hdr -@pytest.mark.parametrize("data_types", [["rgb_hdr"], ["rgb_radiance"]]) -def test_prepare_cameras_neutralizes_exposure_for_radiance(monkeypatch, data_types): +@pytest.mark.parametrize( + ("data_types", "isp_cfg", "radiance"), + [(["rgb_hdr"], None, False), (["rgb_radiance"], None, True), (["rgb"], object(), False)], +) +def test_prepare_cameras_neutralizes_exposure_for_radiance(monkeypatch, data_types, isp_cfg, radiance): """Radiance neutralizes camera exposure; standalone HDR keeps the authored camera settings.""" _install_omni_stubs(monkeypatch) from isaaclab_physx.renderers.isaac_rtx_renderer import IsaacRtxRenderer from pxr import Sdf, Usd, UsdGeom - authored = {"exposure": 2.0, "exposure:fStop": 2.8, "exposure:iso": 800.0, "exposure:responsivity": 0.5} - neutral = {"exposure": 0.0, "exposure:fStop": 1.0, "exposure:iso": 0.0, "exposure:responsivity": 1.0} + monkeypatch.setattr("isaaclab_ppisp.resolve_and_normalize", lambda cfg, *args: cfg) + authored = { + "exposure": 2.0, + "exposure:fStop": 2.8, + "exposure:iso": 800.0, + "exposure:responsivity": 0.5, + "exposure:time": 0.125, + } + neutral = { + "exposure": 0.0, + "exposure:fStop": 1.0, + "exposure:iso": 0.0, + "exposure:responsivity": 1.0, + "exposure:time": 1.0, + } stage = Usd.Stage.CreateInMemory() camera = UsdGeom.Camera.Define(stage, "/World/Camera").GetPrim() for name, value in authored.items(): camera.CreateAttribute(name, Sdf.ValueTypeNames.Float).Set(value) camera.CreateAttribute("omni:rtx:autoExposure:enabled", Sdf.ValueTypeNames.Bool).Set(True) spec = SimpleNamespace( - cfg=SimpleNamespace(data_types=data_types, isp_cfg=None), camera_prim_paths=("/World/Camera",) + cfg=SimpleNamespace(data_types=data_types, isp_cfg=isp_cfg), camera_prim_paths=("/World/Camera",) ) IsaacRtxRenderer.__new__(IsaacRtxRenderer).prepare_cameras(stage, spec) - radiance = "rgb_radiance" in data_types for name, value in (neutral if radiance else authored).items(): - assert camera.GetAttribute(name).Get() == value + assert camera.GetAttribute(name).Get() == pytest.approx(value) assert camera.GetAttribute("omni:rtx:autoExposure:enabled").Get() is not radiance schemas = camera.GetMetadata("apiSchemas") applied = [] if schemas is None else [*schemas.prependedItems, *schemas.explicitItems, *schemas.appendedItems] @@ -493,6 +488,8 @@ def _record_global_settings(*_args): rtx_renderer.IsaacRtxRenderer(IsaacRtxRendererCfg()) assert call_order == ["enable", "global_settings"] + settings.set.assert_any_call("/isaaclab/render/rtx_sensors", True) + settings.set.assert_any_call("/physics/fabricUpdateTransformations", True) @pytest.mark.parametrize("configured_value", [None, False, True])