Test for Camera and Tiled Camera transforms on CPU and GPU#4923
Test for Camera and Tiled Camera transforms on CPU and GPU#4923bareya wants to merge 5 commits intoisaac-sim:developfrom
Conversation
Greptile SummaryThis PR fixes a bug where repositioning a Key changes:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Camera as TiledCamera
participant View as XformPrimView
participant Fabric
participant USD
participant Renderer as IsaacRtxRenderer
participant Replicator
User->>Camera: set_world_poses_from_view(eyes, targets)
Camera->>View: set_world_poses(positions, orientations)
View->>Fabric: write pose (sync_usd_on_fabric_write=False)
Note over USD: USD pose is STALE here
User->>Camera: update(dt)
Camera->>Camera: _update_buffers_impl(env_mask)
Camera->>Camera: _update_poses(env_ids)
Camera->>View: get_world_poses(env_ids)
View->>Fabric: read pose
Fabric-->>View: fresh positions/orientations
View-->>Camera: pos_w, quat_w_world
Camera->>Renderer: update_camera(render_data, pos_w, quat_w_world, intrinsics)
Renderer->>Renderer: convert orientations world→opengl
Renderer->>View: _set_world_poses_usd(positions, orientations_opengl)
View->>USD: write ALL poses
Note over USD: USD now has latest poses
Camera->>Renderer: render(render_data)
Renderer->>Replicator: ensure_isaac_rtx_render_update()
Replicator->>USD: read camera transforms
USD-->>Replicator: fresh transforms
Replicator-->>Renderer: rendered frame
Renderer-->>Camera: output buffers filled
Last reviewed commit: 67bd3e4 |
|
repost: My worry is that for tiled camera we almost always directly read and write to fabric array |
|
This requires longer investigation. |
| # Create a view for the sensor with Fabric enabled for fast pose queries, otherwise position will be stale. | ||
| self._view = XformPrimView( | ||
| self.cfg.prim_path, | ||
| device=self._device, | ||
| stage=self.stage, | ||
| ) |
There was a problem hiding this comment.
Inaccurate comment copied from camera.py
The comment says "with Fabric enabled for fast pose queries, otherwise position will be stale" — but sync_usd_on_fabric_write=True is intentionally not passed here. In camera.py that flag is what makes the comment true; here it is absent by design (the docstring in update_camera even explains the trade-off: "TiledCamera disables sync_usd_on_fabric_write for performance").
The stale-pose problem in TiledCamera is solved by the new update_camera() path that writes directly to USD, not by Fabric–USD sync. Copying the camera.py comment verbatim will mislead future readers into thinking the XformPrimView is configured identically to the base class.
Consider updating it to accurately describe the approach used here:
| # Create a view for the sensor with Fabric enabled for fast pose queries, otherwise position will be stale. | |
| self._view = XformPrimView( | |
| self.cfg.prim_path, | |
| device=self._device, | |
| stage=self.stage, | |
| ) | |
| # Create a view for the sensor. NOTE: sync_usd_on_fabric_write is intentionally NOT set here | |
| # for performance. Instead, USD poses are written directly in update_camera() before each render | |
| # so that Replicator picks up the latest transforms. | |
| self._view = XformPrimView( | |
| self.cfg.prim_path, | |
| device=self._device, | |
| stage=self.stage, | |
| ) |
| orientations_opengl = convert_camera_frame_orientation_convention(orientations, origin="world", target="opengl") | ||
| sensor._view._set_world_poses_usd(positions, orientations_opengl) |
There was a problem hiding this comment.
All camera USD poses rewritten on every _update_poses call
update_camera receives the full self._data.pos_w / self._data.quat_w_world tensors (all view.count cameras) even when _update_poses was called for only a subset of env_ids. _set_world_poses_usd then iterates over every prim via Sdf.ChangeBlock, so cameras whose pose didn't change still incur USD-write and stage-notification overhead.
In large-scale training runs with many parallel environments this could become a non-trivial cost per camera.update() call.
A targeted write would limit the overhead to only the cameras that were actually moved:
orientations_opengl = convert_camera_frame_orientation_convention(
orientations, origin="world", target="opengl"
)
# Write only the subset passed to _update_poses (if indices are threaded through)
sensor._view._set_world_poses_usd(positions, orientations_opengl)Consider threading the env_ids / indices down from _update_poses into update_camera and forwarding them to _set_world_poses_usd(positions[env_ids], orientations_opengl[env_ids], indices=env_ids). This keeps the fix correct while avoiding redundant USD writes for unchanged cameras.
Fix camera update.
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
List any dependencies that are required for this change.
Fixes # (issue)
Type of change
Screenshots
Please attach before and after screenshots of the change if applicable.
Checklist
pre-commitchecks with./isaaclab.sh --formatconfig/extension.tomlfileCONTRIBUTORS.mdor my name already exists there