Move TiledCamera implementation into Camera and deprecate TiledCamera. #5162
Move TiledCamera implementation into Camera and deprecate TiledCamera. #5162nvsekkin wants to merge 4 commits intoisaac-sim:developfrom
Conversation
nvsekkin
changed the title
nvsekkin
changed the title
|
|
||
| # Note: Semantic Versioning is used: https://semver.org/ | ||
| version = "4.5.25" | ||
| version = "4.5.26" |
There was a problem hiding this comment.
maybe bump minor at least, since there is a breaking change and deprecation warning.
There was a problem hiding this comment.
Good point — you're right. This PR removes render_product_paths, changes the info format from dict to list[dict], and deprecates TiledCamera/TiledCameraCfg. Per semver, that warrants at least a minor bump.
4.6.0 would be more appropriate than 4.5.26 here. 👍
| into a single implementation. :class:`Camera` now delegates all rendering to the | ||
| :class:`~isaaclab.renderers.Renderer` abstraction (same approach :class:`TiledCamera` used). | ||
| The public API is unchanged for :class:`Camera` users. | ||
| * **Breaking (TiledCamera users):** :attr:`~isaaclab.sensors.camera.CameraData.info` now correctly |
There was a problem hiding this comment.
this was a bug from before, fixed while moving TiledCamera impl into Camera
There was a problem hiding this comment.
Makes sense — the old TiledCamera returning a flat dict for info violated the CameraData contract (which documents list[dict] keyed per-camera). Fixing it during the unification is the right call. The changelog entry reads clearly: it calls out the migration path (info[data_type] → info[cam_idx][data_type]) which is what downstream users need. 👍
|
|
||
| # Deprecation warnings from TiledCamera/TiledCameraCfg are expected in this file; | ||
| # the deprecation mechanism itself is validated in test_tiled_camera.py. | ||
| pytestmark = pytest.mark.filterwarnings("ignore::DeprecationWarning") |
There was a problem hiding this comment.
this is needed otherwise, the TiledCamera gets noisy in tests
There was a problem hiding this comment.
Makes sense — since this test exercises TiledCamera behavior (not the deprecation path itself), suppressing the warnings here keeps the output clean. And the inline comment + the note that test_tiled_camera.py covers the deprecation mechanism makes the intent clear. 👍
| settings = get_settings_manager() | ||
| isaac_sim_version = get_isaac_sim_version() | ||
|
|
||
| if isaac_sim_version.major >= 6: |
There was a problem hiding this comment.
This was inside Camera impl, but should be in the renderer
There was a problem hiding this comment.
Makes sense — the disableColorRender settings toggle and the Isaac Sim version gating are renderer-specific concerns, so create_render_data() in the renderer is the right place for them. 👍
Greptile SummaryThis PR unifies
Confidence Score: 3/5Not safe to merge as-is: the semantic_filter regression silently breaks existing Camera users, and render_product_paths is removed without a prior deprecation cycle per project policy. Two P1 findings block merge: (1) semantic_filter is silently dropped — any Camera user with a non-default filter gets unfiltered segmentation with no warning or error; (2) render_product_paths is removed in the same release it would have been deprecated, violating AGENTS.md policy. The rest of the change is well-structured and the deprecation wiring for TiledCamera/TiledCameraCfg is correct. source/isaaclab/isaaclab/sensors/camera/camera.py (semantic_filter removal, render_product_paths removal) and source/isaaclab_physx/isaaclab_physx/renderers/isaac_rtx_renderer.py (semantic_filter not applied in create_render_data). Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Camera
participant Renderer as IsaacRtxRenderer
participant RepAPI as omni.replicator.core
User->>Camera: Camera(cfg)
Camera->>Camera: __init__() — spawn prim, set RTX flag
User->>Camera: sim.reset() → _initialize_impl()
Camera->>Renderer: Renderer(cfg.renderer_cfg)
Camera->>Renderer: prepare_stage(stage, num_envs) [no-op for RTX]
Camera->>Renderer: create_render_data(sensor)
Renderer->>RepAPI: rep.create.render_product_tiled(cameras, tile_res)
Renderer->>RepAPI: get_annotator(type) × N [per data_type]
Renderer-->>Camera: IsaacRtxRenderData
Camera->>Camera: _create_buffers()
Camera->>Camera: _update_intrinsic_matrices()
Camera->>Camera: _update_poses() → renderer.update_camera() [no-op]
Camera->>Renderer: set_outputs(render_data, data_dict)
User->>Camera: camera.update(dt) → _update_buffers_impl()
Camera->>Renderer: update_transforms() [no-op]
Camera->>Renderer: render(render_data)
Renderer->>RepAPI: ensure_isaac_rtx_render_update()
Renderer->>RepAPI: annotator.get_data() × N
Renderer->>Renderer: reshape_tiled_image kernel × N
Renderer->>Renderer: apply depth clipping
Renderer-->>Camera: output tensors written in-place
User->>Camera: camera.data.output["rgb"]
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -443,12 +406,19 @@ def _initialize_impl(self): | |||
| " rendering." | |||
| ) | |||
|
|
|||
| import omni.replicator.core as rep | |||
| from omni.syntheticdata.scripts.SyntheticData import SyntheticData | |||
|
|
|||
| # Initialize parent class | |||
| super()._initialize_impl() | |||
| # Create a view for the sensor with Fabric enabled for fast pose queries, otherwise position will be stale. | |||
|
|
|||
| self.renderer = Renderer(self.cfg.renderer_cfg) | |||
| logger.info("Using renderer: %s", type(self.renderer).__name__) | |||
|
|
|||
| # Stage preprocessing must happen before creating the view because the view keeps | |||
| # references to prims located in the stage. | |||
| self.renderer.prepare_stage(self.stage, self._num_envs) | |||
|
|
|||
| # Create a view for the sensor with Fabric enabled for fast pose queries. | |||
| # TODO: remove sync_usd_on_fabric_write=True once the GPU (cuda:0) Fabric sync bug in | |||
| # renderer.update_transforms() is fixed. Without it, poses are stale on the GPU path. | |||
| self._view = XformPrimView( | |||
| self.cfg.prim_path, device=self._device, stage=self.stage, sync_usd_on_fabric_write=True | |||
| ) | |||
| @@ -464,10 +434,6 @@ def _initialize_impl(self): | |||
| # Create frame count buffer | |||
| self._frame = torch.zeros(self._view.count, device=self._device, dtype=torch.long) | |||
|
|
|||
| # Attach the sensor data types to render node | |||
| self._render_product_paths: list[str] = list() | |||
| self._rep_registry: dict[str, list[rep.annotators.Annotator]] = {name: list() for name in self.cfg.data_types} | |||
|
|
|||
| # Convert all encapsulated prims to Camera | |||
| for cam_prim in self._view.prims: | |||
| # Obtain the prim path | |||
| @@ -476,135 +442,38 @@ def _initialize_impl(self): | |||
| if not cam_prim.IsA(UsdGeom.Camera): | |||
| raise RuntimeError(f"Prim at path '{cam_prim_path}' is not a Camera.") | |||
| # Add to list | |||
| sensor_prim = UsdGeom.Camera(cam_prim) | |||
| self._sensor_prims.append(sensor_prim) | |||
|
|
|||
| # Get render product | |||
| # From Isaac Sim 2023.1 onwards, render product is a HydraTexture so we need to extract the path | |||
| render_prod_path = rep.create.render_product(cam_prim_path, resolution=(self.cfg.width, self.cfg.height)) | |||
| if not isinstance(render_prod_path, str): | |||
| render_prod_path = render_prod_path.path | |||
| self._render_product_paths.append(render_prod_path) | |||
|
|
|||
| # Check if semantic types or semantic filter predicate is provided | |||
| if isinstance(self.cfg.semantic_filter, list): | |||
| semantic_filter_predicate = ":*; ".join(self.cfg.semantic_filter) + ":*" | |||
| elif isinstance(self.cfg.semantic_filter, str): | |||
| semantic_filter_predicate = self.cfg.semantic_filter | |||
| else: | |||
| raise ValueError(f"Semantic types must be a list or a string. Received: {self.cfg.semantic_filter}.") | |||
| # set the semantic filter predicate | |||
| # copied from rep.scripts.writes_default.basic_writer.py | |||
| SyntheticData.Get().set_instance_mapping_semantic_filter(semantic_filter_predicate) | |||
|
|
|||
| # Iterate over each data type and create annotator | |||
| # TODO: This will move out of the loop once Replicator supports multiple render products within a single | |||
| # annotator, i.e.: rep_annotator.attach(self._render_product_paths) | |||
| for name in self.cfg.data_types: | |||
| # note: we are verbose here to make it easier to understand the code. | |||
| # if colorize is true, the data is mapped to colors and a uint8 4 channel image is returned. | |||
| # if colorize is false, the data is returned as a uint32 image with ids as values. | |||
| if name == "semantic_segmentation": | |||
| init_params = { | |||
| "colorize": self.cfg.colorize_semantic_segmentation, | |||
| "mapping": json.dumps(self.cfg.semantic_segmentation_mapping), | |||
| } | |||
| elif name == "instance_segmentation_fast": | |||
| init_params = {"colorize": self.cfg.colorize_instance_segmentation} | |||
| elif name == "instance_id_segmentation_fast": | |||
| init_params = {"colorize": self.cfg.colorize_instance_id_segmentation} | |||
| else: | |||
| init_params = None | |||
|
|
|||
| # Resolve device name | |||
| if "cuda" in self._device: | |||
| device_name = self._device.split(":")[0] | |||
| else: | |||
| device_name = "cpu" | |||
|
|
|||
| # TODO: this is a temporary solution because replicator has not exposed the annotator yet | |||
| # once it's exposed, we can remove this | |||
| if name == "albedo": | |||
| rep.AnnotatorRegistry.register_annotator_from_aov( | |||
| aov="DiffuseAlbedoSD", output_data_type=np.uint8, output_channels=4 | |||
| ) | |||
| if name in self.SIMPLE_SHADING_MODES: | |||
| rep.AnnotatorRegistry.register_annotator_from_aov( | |||
| aov=self.SIMPLE_SHADING_AOV, output_data_type=np.uint8, output_channels=4 | |||
| ) | |||
|
|
|||
| # Map special cases to their corresponding annotator names | |||
| simple_shading_cases = {key: self.SIMPLE_SHADING_AOV for key in self.SIMPLE_SHADING_MODES} | |||
| special_cases = { | |||
| "rgba": "rgb", | |||
| "depth": "distance_to_image_plane", | |||
| "albedo": "DiffuseAlbedoSD", | |||
| **simple_shading_cases, | |||
| } | |||
| # Get the annotator name, falling back to the original name if not a special case | |||
| annotator_name = special_cases.get(name, name) | |||
| # Create the annotator node | |||
| rep_annotator = rep.AnnotatorRegistry.get_annotator(annotator_name, init_params, device=device_name) | |||
|
|
|||
| # attach annotator to render product | |||
| rep_annotator.attach(render_prod_path) | |||
| # add to registry | |||
| self._rep_registry[name].append(rep_annotator) | |||
|
|
|||
| # Create internal buffers | |||
| self._sensor_prims.append(UsdGeom.Camera(cam_prim)) | |||
|
|
|||
| # View needs to exist before creating render data | |||
| self.render_data = self.renderer.create_render_data(self) | |||
|
|
|||
| # Create internal buffers (includes intrinsic matrix and pose init) | |||
| self._create_buffers() | |||
There was a problem hiding this comment.
semantic_filter silently dropped
The old Camera._initialize_impl applied the per-sensor semantic filter before attaching annotators:
SyntheticData.Get().set_instance_mapping_semantic_filter(semantic_filter_predicate)That call has been removed and is not present anywhere in the new IsaacRtxRenderer.create_render_data. Any camera configured with a non-default semantic_filter (e.g. ["class"]) will now receive unfiltered segmentation output with no warning. CameraCfg.semantic_filter defaults to "*:*" so the regression is silent for the most common case but breaks any user who relied on filtered semantics.
| def _create_buffers(self): | ||
| """Create buffers for storing data.""" | ||
| # create the data object | ||
| # -- intrinsic matrix | ||
| self._data.intrinsic_matrices = torch.zeros((self._view.count, 3, 3), device=self._device) | ||
| self._update_intrinsic_matrices(self._ALL_INDICES) | ||
| # -- pose of the cameras | ||
| self._data.pos_w = torch.zeros((self._view.count, 3), device=self._device) | ||
| self._data.quat_w_world = torch.zeros((self._view.count, 4), device=self._device) | ||
| # -- intrinsic matrix | ||
| self._data.intrinsic_matrices = torch.zeros((self._view.count, 3, 3), device=self._device) | ||
| self._update_poses(self._ALL_INDICES) | ||
| self._data.image_shape = self.image_shape | ||
| # -- output data | ||
| # lazy allocation of data dictionary | ||
| # since the size of the output data is not known in advance, we leave it as None | ||
| # the memory will be allocated when the buffer() function is called for the first time. | ||
| self._data.output = {} | ||
| # -- output data (eagerly pre-allocated so renderer.set_outputs() can hold tensor references) | ||
| data_dict = dict() | ||
| if "rgba" in self.cfg.data_types or "rgb" in self.cfg.data_types: | ||
| data_dict["rgba"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 4), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| if "rgb" in self.cfg.data_types: | ||
| data_dict["rgb"] = data_dict["rgba"][..., :3] | ||
| if "albedo" in self.cfg.data_types: | ||
| data_dict["albedo"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 4), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| for data_type in self.SIMPLE_SHADING_TYPES: | ||
| if data_type in self.cfg.data_types: | ||
| data_dict[data_type] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 3), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| if "distance_to_image_plane" in self.cfg.data_types: | ||
| data_dict["distance_to_image_plane"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.float32 | ||
| ).contiguous() | ||
| if "depth" in self.cfg.data_types: | ||
| data_dict["depth"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.float32 | ||
| ).contiguous() | ||
| if "distance_to_camera" in self.cfg.data_types: | ||
| data_dict["distance_to_camera"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.float32 | ||
| ).contiguous() | ||
| if "normals" in self.cfg.data_types: | ||
| data_dict["normals"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 3), device=self.device, dtype=torch.float32 | ||
| ).contiguous() | ||
| if "motion_vectors" in self.cfg.data_types: | ||
| data_dict["motion_vectors"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 2), device=self.device, dtype=torch.float32 | ||
| ).contiguous() | ||
| if "semantic_segmentation" in self.cfg.data_types: | ||
| if self.cfg.colorize_semantic_segmentation: | ||
| data_dict["semantic_segmentation"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 4), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| else: | ||
| data_dict["semantic_segmentation"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.int32 | ||
| ).contiguous() | ||
| if "instance_segmentation_fast" in self.cfg.data_types: | ||
| if self.cfg.colorize_instance_segmentation: | ||
| data_dict["instance_segmentation_fast"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 4), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| else: | ||
| data_dict["instance_segmentation_fast"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.int32 | ||
| ).contiguous() | ||
| if "instance_id_segmentation_fast" in self.cfg.data_types: | ||
| if self.cfg.colorize_instance_id_segmentation: | ||
| data_dict["instance_id_segmentation_fast"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 4), device=self.device, dtype=torch.uint8 | ||
| ).contiguous() | ||
| else: | ||
| data_dict["instance_id_segmentation_fast"] = torch.zeros( | ||
| (self._view.count, self.cfg.height, self.cfg.width, 1), device=self.device, dtype=torch.int32 | ||
| ).contiguous() | ||
|
|
||
| self._data.output = data_dict | ||
| self._data.info = [{name: None for name in self.cfg.data_types} for _ in range(self._view.count)] | ||
| self.renderer.set_outputs(self.render_data, self._data.output) | ||
|
|
||
| def _update_intrinsic_matrices(self, env_ids: Sequence[int]): | ||
| """Compute camera's matrix of intrinsic parameters. |
There was a problem hiding this comment.
cfg.data_types order
_create_buffers constructs data_dict in a fixed, hardcoded order regardless of the order in cfg.data_types. The old replicator-based Camera iterated cfg.data_types directly, preserving user-specified order. For example, data_types=["distance_to_camera", "depth"] now yields output.keys() == ["depth", "distance_to_camera"]. Code that iterates or zips the output dict while assuming it matches data_types order will silently process data out of sequence. The test test_camera_data_types_ordering hard-codes this new order rather than asserting insertion-order parity, which may mask the regression.
| # Broadcast renderer info (e.g. segmentation label mappings) to all per-camera entries | ||
| renderer_info = getattr(self.render_data, "renderer_info", None) | ||
| if renderer_info: | ||
| for data_type, metadata in renderer_info.items(): | ||
| for cam_idx in range(self._view.count): | ||
| self._data.info[cam_idx][data_type] = metadata |
There was a problem hiding this comment.
renderer_info broadcast ignores env_ids mask
The renderer_info loop broadcasts updated segmentation metadata to every camera index (range(self._view.count)) on every update call, even when only a subset of environments (env_ids) triggered the update. For a tiled renderer that always renders all cameras this is consistent, but it also means a partial-reset cycle unconditionally overwrites info for cameras that were not in env_ids. This is a minor consistency concern but could cause stale-or-fresh confusion in selective-reset workflows.
| class_type: type["TiledCamera"] | str = "{DIR}.tiled_camera:TiledCamera" | ||
|
|
||
| def __post_init__(self): | ||
| warnings.warn( | ||
| "TiledCameraCfg is deprecated. Use CameraCfg directly.", | ||
| DeprecationWarning, | ||
| stacklevel=2, | ||
| ) |
There was a problem hiding this comment.
__post_init__ does not chain to parent
TiledCameraCfg.__post_init__ emits the deprecation warning but never calls super().__post_init__(). @configclass wraps a user-defined __post_init__ with its own _custom_post_init for the current class only — the parent CameraCfg's combined __post_init__ is not automatically invoked. If CameraCfg or SensorBaseCfg ever gains a __post_init__ with validation logic, TiledCameraCfg instantiation will silently skip it.
| class_type: type["TiledCamera"] | str = "{DIR}.tiled_camera:TiledCamera" | |
| def __post_init__(self): | |
| warnings.warn( | |
| "TiledCameraCfg is deprecated. Use CameraCfg directly.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| def __post_init__(self): | |
| warnings.warn( | |
| "TiledCameraCfg is deprecated. Use CameraCfg directly.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| ) | |
| super().__post_init__() |
| """ | ||
|
|
||
| cfg: TiledCameraCfg | ||
| """The configuration parameters.""" | ||
|
|
||
| def __init__(self, cfg: TiledCameraCfg): | ||
| """Initializes the tiled camera sensor. | ||
|
|
||
| Args: | ||
| cfg: The configuration parameters. | ||
|
|
||
| Raises: | ||
| RuntimeError: If no camera prim is found at the given path. | ||
| ValueError: If the provided data types are not supported by the camera. | ||
| """ | ||
| self.renderer: Renderer | None = None | ||
| self.render_data = None | ||
| super().__init__(cfg) | ||
|
|
||
| def __del__(self): | ||
| """Unsubscribes from callbacks and detach from the replicator registry.""" | ||
| # unsubscribe from callbacks | ||
| SensorBase.__del__(self) | ||
| # cleanup render resources (renderer may be None if never initialized) | ||
| if hasattr(self, "renderer") and self.renderer is not None: | ||
| self.renderer.cleanup(getattr(self, "render_data", None)) | ||
|
|
||
| def __str__(self) -> str: | ||
| """Returns: A string containing information about the instance.""" | ||
| # message for class | ||
| return ( | ||
| f"Tiled Camera @ '{self.cfg.prim_path}': \n" | ||
| f"\tdata types : {list(self.data.output.keys())} \n" | ||
| f"\tsemantic filter : {self.cfg.semantic_filter}\n" | ||
| f"\tcolorize semantic segm. : {self.cfg.colorize_semantic_segmentation}\n" | ||
| f"\tcolorize instance segm. : {self.cfg.colorize_instance_segmentation}\n" | ||
| f"\tcolorize instance id segm.: {self.cfg.colorize_instance_id_segmentation}\n" | ||
| f"\tupdate period (s): {self.cfg.update_period}\n" | ||
| f"\tshape : {self.image_shape}\n" | ||
| f"\tnumber of sensors : {self._view.count}" | ||
| def __init__(self, cfg): | ||
| warnings.warn( | ||
| "TiledCamera is deprecated and will be removed in a future release. " | ||
| "Use Camera directly — it now uses the same Renderer abstraction.", | ||
| DeprecationWarning, | ||
| stacklevel=2, |
There was a problem hiding this comment.
TiledCamera.__init__ type annotation omitted
The deprecated __init__ has no type annotation for cfg. Adding it preserves IDE/type-checker support for the deprecated alias and avoids untyped-argument warnings.
| """ | |
| cfg: TiledCameraCfg | |
| """The configuration parameters.""" | |
| def __init__(self, cfg: TiledCameraCfg): | |
| """Initializes the tiled camera sensor. | |
| Args: | |
| cfg: The configuration parameters. | |
| Raises: | |
| RuntimeError: If no camera prim is found at the given path. | |
| ValueError: If the provided data types are not supported by the camera. | |
| """ | |
| self.renderer: Renderer | None = None | |
| self.render_data = None | |
| super().__init__(cfg) | |
| def __del__(self): | |
| """Unsubscribes from callbacks and detach from the replicator registry.""" | |
| # unsubscribe from callbacks | |
| SensorBase.__del__(self) | |
| # cleanup render resources (renderer may be None if never initialized) | |
| if hasattr(self, "renderer") and self.renderer is not None: | |
| self.renderer.cleanup(getattr(self, "render_data", None)) | |
| def __str__(self) -> str: | |
| """Returns: A string containing information about the instance.""" | |
| # message for class | |
| return ( | |
| f"Tiled Camera @ '{self.cfg.prim_path}': \n" | |
| f"\tdata types : {list(self.data.output.keys())} \n" | |
| f"\tsemantic filter : {self.cfg.semantic_filter}\n" | |
| f"\tcolorize semantic segm. : {self.cfg.colorize_semantic_segmentation}\n" | |
| f"\tcolorize instance segm. : {self.cfg.colorize_instance_segmentation}\n" | |
| f"\tcolorize instance id segm.: {self.cfg.colorize_instance_id_segmentation}\n" | |
| f"\tupdate period (s): {self.cfg.update_period}\n" | |
| f"\tshape : {self.image_shape}\n" | |
| f"\tnumber of sensors : {self._view.count}" | |
| def __init__(self, cfg): | |
| warnings.warn( | |
| "TiledCamera is deprecated and will be removed in a future release. " | |
| "Use Camera directly — it now uses the same Renderer abstraction.", | |
| DeprecationWarning, | |
| stacklevel=2, | |
| def __init__(self, cfg: "TiledCameraCfg"): |
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR unifies Camera and TiledCamera by moving TiledCamera's renderer-based implementation into Camera and deprecating TiledCamera as a thin subclass alias. The new Camera class delegates rendering to the Renderer abstraction (via IsaacRtxRenderer), removing the old per-camera replicator annotator approach. The PR also fixes a silent API contract violation where TiledCamera.data.info returned a flat dict instead of the documented list[dict].
Design Assessment
This is a well-designed refactoring that improves the framework's architecture.
Problem being solved: Redundant implementations — Camera and TiledCamera maintained separate codepaths for essentially the same functionality, with TiledCamera using the cleaner Renderer abstraction while Camera used legacy per-camera replicator annotators.
Alternative approaches considered:
- Keep both implementations, share code via inheritance → Rejected correctly because it would leave confusing API surface with unclear when to use which
- Extract common logic to a base class → Would add complexity without clear benefit; full consolidation is cleaner
- Current approach: Unify into
Camera, deprecateTiledCamera→ Best choice for consistency and maintainability
Framework layering: ✅ Correct. The PR keeps rendering logic in the Renderer abstraction layer where it belongs, and the sensor layer (Camera) just orchestrates initialization and buffer management. No boundary violations.
Will this survive refactors? Yes — the Renderer abstraction is cleanly separated, so Camera doesn't depend on RTX internals.
Design is sound.
Architecture Impact
Breaking changes:
render_product_pathsproperty removed — PR author confirmed zero external usages (only internal references incamera.py,isaac_rtx_renderer.py,ovrtx_renderer.py).TiledCamera.data.infoformat change from flatdicttolist[dict]— This fixes a documented API contract violation. Users who accessedinfo["semantic_segmentation"]must now useinfo[0]["semantic_segmentation"].
Downstream consumers that use TiledCamera:
VisuoTactileSensor(isaaclab_contrib) — Will emit deprecation warning but continue to workobservations.py— Type hints updated, behavior unchanged- Various task envs — All use
TiledCameraCfg, will work through the deprecation shim
No silent breakage — the deprecation warnings provide clear migration guidance.
Implementation Verdict
Ship it — Clean implementation with comprehensive test coverage. One minor note below.
Test Coverage
Excellent. The PR:
- Ports 6 unique tests from
test_tiled_camera.py→test_camera.py(multi-regex init, all annotators, segmentation non-colorize, normals unit length, data types ordering, frame offset) - Slims
test_tiled_camera.pyto 4 focused deprecation/compatibility tests - Fixes
infoassertions intest_tiled_camera.pyandtest_multi_tiled_camera.pyto use the correctedlist[dict]format - Tests cover both
cuda:0andcpudevices
The tests properly validate the unified behavior and the deprecation mechanism.
CI Status
Only labeler check has run so far (passed). Full test suite hasn't executed yet — recommend waiting for CI to complete before merge.
Findings
🔵 Improvement: tiled_camera_cfg.py:28 — Missing super().__post_init__() call
While CameraCfg and SensorBaseCfg don't currently have __post_init__ methods, calling super().__post_init__() is a defensive practice that prevents silent bugs if a parent class adds __post_init__ in the future. Consider:
def __post_init__(self):
warnings.warn(
"TiledCameraCfg is deprecated. Use CameraCfg directly.",
DeprecationWarning,
stacklevel=2,
)
super().__post_init__() # Forward-compatibleThis is a minor improvement, not blocking.
Overall: This is a clean, well-tested refactoring that improves framework consistency and fixes a documented API violation. The deprecation path is properly implemented with clear warnings. Ready to merge once CI completes.
1 participant
Description
Unify
CameraandTiledCameraby moving TiledCamera's renderer-based implementation intoCameraand makingTiledCameraa deprecated subclass alias.What changed
camera.py:_initialize_implnow usesRenderer(cfg.renderer_cfg)+renderer.prepare_stage()+renderer.create_render_data()instead of replicator imports and per-camera render product loops_create_buffersuses eager pre-allocation +renderer.set_outputs()instead of lazy allocation_rep_registry,_render_product_paths,_create_annotator_data(),_process_annotator_output(),render_product_pathsproperty,omni.replicator.coreimport,SyntheticDataimporttiled_camera.py:class TiledCamera(Camera)subclass that emits aDeprecationWarningon instantiationtiled_camera_cfg.py:DeprecationWarningin__post_init__Bug fix —
infoformat:CameraDatacontract by using a flatdictforinfoinstead of the documentedlist[dict[str, Any]]. The unified Camera maintains the correctlist[dict]format.Test suite:
test_tiled_camera.py→test_camera.py(multi-regex init, all annotators, segmentation non-colorize, normals unit length, data types ordering, frame offset)test_tiled_camera.pyto 4 focused deprecation/compatibility testsinfoassertions intest_tiled_camera.pyandtest_multi_tiled_camera.pyto useinfo[0]["key"]instead ofinfo["key"]Breaking changes
render_product_pathsproperty removed (confirmed zero external usage across entire repo — only referenced internally incamera.py,isaac_rtx_renderer.py, andovrtx_renderer.py)Type of change
Screenshots
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there