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 20 commits into
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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
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.
bareya
changed the title
bareya
force-pushed
the
pbarejko/camera-update
branch
from
7e72a1b to
421c53f
Compare
![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 fixes a bug where camera pose updates via set_world_poses_from_view were not reflected in rendered output when using Fabric. The fix replaces a cached fabricarray with a fresh SelectPrims + fabricarray on every access, ensuring Fabric's journaling system marks the worldMatrix attribute as dirty so downstream renderers observe the update. A regression test is included that verifies both Camera and TiledCamera produce different depth images when repositioned.
Architecture Impact
XformPrimView — The core change: _fabric_world_matrices cache is replaced by _get_world_matrices_as_fabricarray() which recreates the SelectPrims + fabricarray on every call (both reads and writes). This affects all Fabric-backed pose get/set operations. Callers of XformPrimView.set_world_poses() / get_world_poses() will now pay the SelectPrims overhead per call.
Camera — sync_usd_on_fabric_write=True is removed. The old Camera relied on USD mirroring for the renderer to see updates; the new approach relies on Fabric dirty-marking instead. This is correct since the RTX renderer reads from Fabric when it's enabled.
IsaacRtxRenderer — Unused import added, docstring removed from update_camera. The unused import will cause the pre-commit check to fail (and already has).
Implementation Verdict
Minor fixes needed — Correct approach to the Fabric journaling bug, but 3 issues to address: unused import causing CI failure, debug artifacts in the test, and unnecessary overhead on read paths.
Test Coverage
The test is well-designed — it exercises the exact bug path (set_world_poses_from_view → render → verify depth changes) for both Camera and TiledCamera on both CPU and GPU. The 1.5x depth ratio assertion is a reasonable heuristic. However, the test contains debug image-saving code with a PIL dependency that should be removed before merge.
CI Status
❌ pre-commit failed — ruff formatting issues in camera.py and test_tiled_camera.py, plus the unused convert_camera_frame_orientation_convention import in isaac_rtx_renderer.py.
Branch Status
develop. The PR is mergeable (no conflicts) but consider rebasing to pick up recent changes.
Findings
🟡 Warning: isaac_rtx_renderer.py — Unused import causes CI failure. The added from isaaclab.utils.math import convert_camera_frame_orientation_convention is never used in this file. ruff correctly flags this. Remove it.
🟡 Warning: xform_prim_view.py — _get_world_matrices_as_fabricarray() called on read paths adds unnecessary overhead. The SelectPrims + fabricarray recreation is needed for writes (to mark dirty for journaling), but read paths (_get_world_poses_fabric, _get_scales_fabric) don't need dirty-marking. The old code's comment noted this cached approach "eliminates the 0.06-0.30ms variability from creating fabricarray each call." Consider splitting into a cached path for reads and a fresh path for writes.
🔵 Improvement: test_tiled_camera.py — Debug image-saving code should be removed. The _save_depth_image helper with its PIL dependency, print statements, and /tmp file writes is debug scaffolding. Tests should not leave artifacts on disk or depend on optional libraries for non-assertion logic.
🔵 Improvement: isaac_rtx_renderer.py — Docstring removed from update_camera. The base class has a docstring, but this override silently drops it. Either keep the docstring explaining this is a no-op, or at minimum add a one-line comment.
|
|
||
| from isaaclab.app.settings_manager import get_settings_manager | ||
| from isaaclab.renderers import BaseRenderer | ||
| from isaaclab.utils.math import convert_camera_frame_orientation_convention |
There was a problem hiding this comment.
Unused import — convert_camera_frame_orientation_convention is not referenced anywhere in this file. This is already causing the pre-commit CI check to fail.
| from isaaclab.utils.math import convert_camera_frame_orientation_convention |
| # Use cached fabricarray for world matrices | ||
| # This eliminates the 0.06-0.30ms variability from creating fabricarray each call | ||
| world_matrices = self._fabric_world_matrices | ||
| world_matrices = self._get_world_matrices_as_fabricarray() |
There was a problem hiding this comment.
The _get_world_matrices_as_fabricarray() method recreates SelectPrims + fabricarray on every call, which is necessary for writes to trigger Fabric's journaling dirty-marking. However, read paths (_get_world_poses_fabric, _get_scales_fabric) don't need to mark the attribute as dirty — they're only reading. The old code noted this cached approach eliminated 0.06–0.30ms variability per call.
Consider keeping a cached fabricarray for reads and only recreating on writes. For example, store the result of _get_world_matrices_as_fabricarray() after each write and reuse it for subsequent reads until the next write invalidates it.
There was a problem hiding this comment.
👍 Totally fair — no rush on a draft. Just flagging it so it doesn't get lost when you move toward finalizing. The caching suggestion is there whenever you're ready to revisit the read path performance.
| img = depth[0].squeeze().cpu().float() | ||
| valid = img[img < max_range] | ||
| if valid.numel() > 0: | ||
| lo, hi = valid.min(), valid.max() |
There was a problem hiding this comment.
The _save_depth_image function with its PIL import, print statements, and /tmp file writes is debug scaffolding that shouldn't ship in a production test. It introduces a dependency on PIL (not used elsewhere in test code), writes files to /tmp on every test run, and adds print noise to test output.
Remove the entire _save_depth_image function and the lines that call it (lines 1700–1702). The test's assertions are sufficient to verify correctness.
| Both camera types must produce different depth images when the | ||
| camera is repositioned from close to far. | ||
| """ | ||
| sim, __unused_camera_cfg, dt = setup_camera |
There was a problem hiding this comment.
Inconsistent unused-variable naming. The rest of this file uses _ for unused values (e.g., sim, _, dt = setup_camera at line 1725). Use the same convention here.
| sim, __unused_camera_cfg, dt = setup_camera | |
| sim, _, dt = setup_camera |
| ): | ||
| """No-op for Replicator - uses USD camera prims directly. | ||
| See :meth:`~isaaclab.renderers.base_renderer.BaseRenderer.update_camera`.""" | ||
| pass |
There was a problem hiding this comment.
The docstring explaining that update_camera is a no-op for RTX (which reads camera prims directly from USD/Fabric) was removed. The other no-op method update_transforms (line 171) keeps its docstring. Either restore the docstring or add a brief comment explaining why this is a no-op — it helps the next person understand the design.
| pass | |
| ): | |
| """No-op for Isaac RTX — camera transforms are read directly from the scene graph. | |
| See :meth:`~isaaclab.renderers.base_renderer.BaseRenderer.update_camera`.""" | |
| pass |
| """ | ||
| import usdrt | ||
|
|
||
| sel = self._fabric_stage.SelectPrims( |
There was a problem hiding this comment.
Because we re-create SelectPrims, the test succeeds. We should be caching SelectPrims and use SelectPrims::PrepareForReuse.
There was a problem hiding this comment.
Agreed — recreating SelectPrims on every call is wasteful and only done here as a workaround to trigger Fabric journaling dirty-marking.
Caching the PrimSelection in _initialize_fabric() (where we already create one for the index mapping) and calling PrepareForReuse before each access would be the right fix. Something like:
def _get_world_matrices_as_fabricarray(self) -> wp.fabricarray:
self._fabric_selection.PrepareForReuse()
return wp.fabricarray(self._fabric_selection, "omni:fabric:worldMatrix")That should give us dirty-marking without the SelectPrims allocation overhead each frame. Would you like to roll this into this PR, or track it as a follow-up?
| """ | ||
| import usdrt | ||
|
|
||
| if True: |
There was a problem hiding this comment.
if True: dead code — PrepareForReuse() else-branch is unreachable
Both _get_world_matrices_as_fabricarray() (here) and _get_local_matrices_as_fabricarray() (line 489) have if True: guards with dead else: self._*_selection.PrepareForReuse() branches. This looks like WIP scaffolding for the caching optimization we discussed earlier.
Either wire up a real condition (e.g. a _cache_selections flag) so the PrepareForReuse path can actually be exercised, or remove the dead branches and leave a # TODO noting the planned caching.
There was a problem hiding this comment.
This is intentional, we are waiting for a fix in PrepareForReuse
There was a problem hiding this comment.
Got it — makes sense to keep the scaffolding while waiting for the fix. A # TODO: switch to PrepareForReuse once fixed comment above the if True: would help the next reader.
| translations_wp = wp.zeros((0, 3), dtype=wp.float32).to(self._device) | ||
|
|
||
| if orientations is not None: | ||
| orientations_wp = wp.from_torch(orientations) |
There was a problem hiding this comment.
Docstring inaccuracy in set_local_poses
The docstring says "re-registers it through :pyobj:IFabricHierarchy" but the code only adds to _dirty_stages. The actual update_world_xforms() call happens lazily on the next get_world_poses or set_world_poses. Consider updating the docstring to accurately describe the deferred propagation:
| orientations_wp = wp.from_torch(orientations) | |
| """Write local poses to Fabric ``omni:fabric:localMatrix`` via a Warp kernel. | |
| After composing the local matrix the method marks world transforms as | |
| dirty so that a subsequent world-space read or write will call | |
| :meth:`update_world_xforms` to propagate the change through the | |
| hierarchy. | |
| """ |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot — Follow-up
New commits (4c2ac4a→c9b68de): Refactored Fabric backend to use cached PrimSelection + PrepareForReuse() pattern (addresses earlier caching discussion). One bug in the new code.
| self._fabric_indices = self._compute_fabric_indices(self._local_sel_rw) | ||
| self._world_ifa_ro = self._build_array(self._trans_sel_ro, self._WORLD_MATRIX_ATTR) | ||
| self._local_ifa_ro = self._build_array(self._trans_sel_ro, self._LOCAL_MATRIX_ATTR) | ||
| return self._local_ifa_rw |
There was a problem hiding this comment.
🔴 Bug: _get_local_ro_array returns the wrong array and mixes selections
Two issues here:
-
Returns
self._local_ifa_rwinstead ofself._local_ifa_ro— this method is the read-only accessor used byget_local_poses(), but it returns the read-write indexed fabric array. -
Cross-selection index mismatch on topology change —
_fabric_indicesis recomputed fromself._local_sel_rw, but then_build_arrayfor_world_ifa_roand_local_ifa_rousesself._trans_sel_ro. Different selections can have different path orderings, so the indices may not align.
Compare with _get_world_ro_array which correctly uses _trans_sel_ro throughout.
| return self._local_ifa_rw | |
| def _get_local_ro_array(self) -> wp.indexedfabricarray: | |
| # import usdrt | |
| # return self._select_indexed(self._LOCAL_MATRIX_ATTR, usdrt.Usd.Access.Read) | |
| if self._trans_sel_ro.PrepareForReuse(): | |
| self._fabric_indices = self._compute_fabric_indices(self._trans_sel_ro) | |
| self._world_ifa_ro = self._build_array(self._trans_sel_ro, self._WORLD_MATRIX_ATTR) | |
| self._local_ifa_ro = self._build_array(self._trans_sel_ro, self._LOCAL_MATRIX_ATTR) | |
| return self._local_ifa_ro |
…and shared contract tests (#5179) # Description ### Summary Rewrites the Newton `XformPrimView` from scratch with correct local-pose semantics, a clean site-based architecture, and a shared test contract that enforces the same invariants across all backends (USD, Fabric, Newton). **Key changes:** - **Fix local poses**: `get_local_poses` / `set_local_poses` now correctly compute parent-relative transforms on GPU (`inv(parent_world) * prim_world`) instead of incorrectly returning world poses - **Fix set_world_poses**: Updates `site_local` offset instead of writing `body_q` directly (which would move the parent body) - **Guard against misuse**: Raises `ValueError` if prim path resolves to a physics body or collision shape — XformPrimView is for non-physics child prims only (cameras, sensors, markers) - **Warp-native API**: All inputs/outputs are `wp.array` — no torch/list conversion overhead - **Factory dispatch**: `from isaaclab.sim.views import XformPrimView` now auto-selects the correct backend (USD, Fabric, Newton) via `XformPrimViewFactory` - **Composition over inheritance**: PhysX `FabricXformPrimView` uses composition (`self._usd_view`) instead of inheriting from `UsdXformPrimView` - **Explicit class names**: `UsdXformPrimView`, `FabricXformPrimView`, `NewtonSiteXformPrimView` — no more ambiguous `XformPrimView` in every package - **Shared contract tests**: 16 test functions in `xform_contract_tests.py` that any backend imports and runs by providing a `view_factory` fixture - **Benchmark updates**: Both benchmark scripts support Newton, use warp-native arrays, and include per-backend round-trip verification ### Type of change - [x] Bug fix (Newton local poses were fundamentally broken — `local == world`) - [x] New feature (shared contract test infrastructure, factory dispatch) - [x] Breaking change (Newton `XformPrimView` renamed to `NewtonSiteXformPrimView`, PhysX to `FabricXformPrimView`; indices parameter changed from `Sequence[int]` to `wp.array`) - [x] Documentation update ### Expected failures - `test_set_world_updates_local[cuda:0]` in Fabric — pre-existing limitation: `set_world_poses` writes to `omni:fabric:worldMatrix` but `get_local_poses` reads from USD, so local poses are stale after a Fabric world write. This will be fixed by the Fabric backend PR (#4923) which adds `omni:fabric:localMatrix` support. ### Test results | Backend | Passed | Failed | Skipped | |---|---|---|---| | Newton | 40 | 0 | 0 | | USD | 45 | 0 | 0 | | Fabric | 15 | 1 (xfail) | 16 (CPU) | | Camera | 20 | 0 | 0 | | TiledCamera | 61 | 0 | 0 | | RayCaster | 5 | 0 | 0 | ### Benchmark (1024 prims, 50 iterations, RTX 5090) ``` ======================================================================================================================== BENCHMARK RESULTS: 1024 prims, 50 iterations ======================================================================================================================== Operation Isaaclab Usd (ms) Isaaclab Fabric (ms) Isaaclab Newton Site (ms) ------------------------------------------------------------------------------------------------------------------------ Initialization 3.7168 3.6596 39.0608 Get World Poses 6.6730 0.0296 0.0180 Set World Poses 15.5574 0.0640 0.0186 Get Local Poses 4.6086 4.5637 0.0216 Set Local Poses 6.4680 6.6221 0.0218 Get Both (World+Local) 12.1240 4.7361 0.0374 Interleaved World Set->Get 23.4141 0.1050 0.0344 ======================================================================================================================== Total 72.5619 19.7800 39.2126 ======================================================================================================================== ``` ### Checklist - [x] I have read and understood the contribution guidelines - [ ] I have run the pre-commit checks with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Antoine Richard <antoiner@nvidia.com> Co-authored-by: Kelly Guo <kellyg@nvidia.com>
…and shared contract tests (isaac-sim#5179) # Description ### Summary Rewrites the Newton `XformPrimView` from scratch with correct local-pose semantics, a clean site-based architecture, and a shared test contract that enforces the same invariants across all backends (USD, Fabric, Newton). **Key changes:** - **Fix local poses**: `get_local_poses` / `set_local_poses` now correctly compute parent-relative transforms on GPU (`inv(parent_world) * prim_world`) instead of incorrectly returning world poses - **Fix set_world_poses**: Updates `site_local` offset instead of writing `body_q` directly (which would move the parent body) - **Guard against misuse**: Raises `ValueError` if prim path resolves to a physics body or collision shape — XformPrimView is for non-physics child prims only (cameras, sensors, markers) - **Warp-native API**: All inputs/outputs are `wp.array` — no torch/list conversion overhead - **Factory dispatch**: `from isaaclab.sim.views import XformPrimView` now auto-selects the correct backend (USD, Fabric, Newton) via `XformPrimViewFactory` - **Composition over inheritance**: PhysX `FabricXformPrimView` uses composition (`self._usd_view`) instead of inheriting from `UsdXformPrimView` - **Explicit class names**: `UsdXformPrimView`, `FabricXformPrimView`, `NewtonSiteXformPrimView` — no more ambiguous `XformPrimView` in every package - **Shared contract tests**: 16 test functions in `xform_contract_tests.py` that any backend imports and runs by providing a `view_factory` fixture - **Benchmark updates**: Both benchmark scripts support Newton, use warp-native arrays, and include per-backend round-trip verification ### Type of change - [x] Bug fix (Newton local poses were fundamentally broken — `local == world`) - [x] New feature (shared contract test infrastructure, factory dispatch) - [x] Breaking change (Newton `XformPrimView` renamed to `NewtonSiteXformPrimView`, PhysX to `FabricXformPrimView`; indices parameter changed from `Sequence[int]` to `wp.array`) - [x] Documentation update ### Expected failures - `test_set_world_updates_local[cuda:0]` in Fabric — pre-existing limitation: `set_world_poses` writes to `omni:fabric:worldMatrix` but `get_local_poses` reads from USD, so local poses are stale after a Fabric world write. This will be fixed by the Fabric backend PR (isaac-sim#4923) which adds `omni:fabric:localMatrix` support. ### Test results | Backend | Passed | Failed | Skipped | |---|---|---|---| | Newton | 40 | 0 | 0 | | USD | 45 | 0 | 0 | | Fabric | 15 | 1 (xfail) | 16 (CPU) | | Camera | 20 | 0 | 0 | | TiledCamera | 61 | 0 | 0 | | RayCaster | 5 | 0 | 0 | ### Benchmark (1024 prims, 50 iterations, RTX 5090) ``` ======================================================================================================================== BENCHMARK RESULTS: 1024 prims, 50 iterations ======================================================================================================================== Operation Isaaclab Usd (ms) Isaaclab Fabric (ms) Isaaclab Newton Site (ms) ------------------------------------------------------------------------------------------------------------------------ Initialization 3.7168 3.6596 39.0608 Get World Poses 6.6730 0.0296 0.0180 Set World Poses 15.5574 0.0640 0.0186 Get Local Poses 4.6086 4.5637 0.0216 Set Local Poses 6.4680 6.6221 0.0218 Get Both (World+Local) 12.1240 4.7361 0.0374 Interleaved World Set->Get 23.4141 0.1050 0.0344 ======================================================================================================================== Total 72.5619 19.7800 39.2126 ======================================================================================================================== ``` ### Checklist - [x] I have read and understood the contribution guidelines - [ ] I have run the pre-commit checks with `./isaaclab.sh --format` - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there --------- Signed-off-by: Kelly Guo <kellyg@nvidia.com> Co-authored-by: Antoine Richard <antoiner@nvidia.com> Co-authored-by: Kelly Guo <kellyg@nvidia.com>
…ic_write (#5380) ## Summary Replace the `sync_usd_on_fabric_write` workaround in `FabricFrameView` with proper `PrepareForReuse()` calls on the Fabric `PrimSelection`. This tells the renderer (FSD/Storm) that Fabric data has changed, so the next rendered frame reflects updated transforms — eliminating the need to copy Fabric writes back to USD. ## Motivation The existing `sync_usd_on_fabric_write` flag worked by mirroring every Fabric write back to USD, which defeated the performance benefits of Fabric. With `PrepareForReuse()`, the rendering pipeline is properly notified of Fabric data changes without any USD writeback. Additionally, the old code incorrectly fell back to USD for CPU devices — Warp handles CPU Fabric buffers correctly, so the fallback was unnecessary. This addresses two of the issues raised in @pbarejko Piotr's review of PR #4923: - **Issue #1** (USD write-back): Fabric writes no longer sync back to USD - **Issue #4** (PrepareForReuse): Renderer notification via `PrepareForReuse()` instead of USD writeback ## Changes ### Core (FabricFrameView) - Call `_prepare_for_reuse()` in write paths (`set_world_poses`, `set_scales`) to notify the renderer - Remove `sync_usd_on_fabric_write` parameter (accepted via `**kwargs` for backward compat) - Remove incorrect CPU/device fallback warnings — Warp handles CPU Fabric buffers correctly - Add `_rebuild_fabric_arrays()` for topology change recovery when `PrepareForReuse()` returns True, with assertion guarding the prim-count invariant ### Camera - Remove `sync_usd_on_fabric_write=True` from FrameView construction in `camera.py` ## Benchmark Results 1024 prims, 50 iterations, NVIDIA L40 GPU: | Operation | USD (ms) | Fabric (ms) | Speedup | |---|---|---|---| | Get World Poses | 14.71 | 0.07 | **203x** | | Set World Poses | 40.75 | 0.16 | **259x** | | Interleaved Set→Get | 55.90 | 0.24 | **232x** | | Get Local Poses | 11.08 | 11.12 | 1.0x | | Set Local Poses | 16.14 | 16.28 | 1.0x | Local poses fall back to USD (expected — Fabric only accelerates world poses via `omni:fabric:worldMatrix`). ## Tests Added | Test | What it validates | |------|------------------| | `test_camera_pose_update_reflected_in_render` | Camera pose changes propagate to rendered depth (close vs far) for CPU/GPU, tiled/non-tiled | | `test_fabric_set_world_does_not_write_back_to_usd` | Fabric writes stay in Fabric, USD prim unchanged | | `test_set_world_updates_local` (xfail) | Documents Issue #5: `set_world_poses` doesn't update local pose in Fabric mode | ## Test Results | Test Suite | Passed | Skipped | Xfailed | Total | |---|---|---|---|---| | Fabric contract tests (`test_views_xform_prim_fabric.py`) | 17 | 16 | 1 | 34 | | USD contract tests (`test_views_xform_prim.py`) | 45 | 0 | 0 | 45 | | Camera render test (`test_tiled_camera.py`) | 8 | 0 | 0 | 8 | ## Type of change - Performance improvement (removes redundant USD writeback on Fabric operations) ## Checklist - [x] I have read and understood the [contribution guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html) - [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with `./isaaclab.sh --format` - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there *No doc changes needed (parameter wasn't referenced in any docs)*
3 participants
This MR isn't ready to be merged
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