Skip to content

Commit ce9ab26

Browse files
committed
Restore three-selection RO/RW design, drop eager world<->local flushes
Restores PR isaac-sim#5728's three-selection layout (`_trans_sel_ro`, `_world_sel_rw`, `_local_sel_rw`) with asymmetric Fabric access flags on `worldMatrix` and `localMatrix`. Those flags are what protect the user's write from being clobbered by Kit's per-tick `IFabricHierarchy.update_world_xforms`: * On `set_world_poses` (via `_world_sel_rw`, `localMatrix=RO`), Fabric does not recompute world from local -- the user's worldMatrix write survives until the renderer reads it. * On `set_local_poses` (via `_local_sel_rw`, `worldMatrix=RO`), Fabric recomputes world from the new local on the next tick -- the renderer reads the correct world. A single combined `worldMatrix=RW, localMatrix=RW` selection (the recent design on this branch) removed that protection. Fabric saw both attributes as user-authored and fell back to the hierarchy's canonical direction (local -> world), recomputing world from a stale local and silently overwriting the user's world write. That was the failure mode behind the `test_output_equal_to_usdcamera` regression and any other Camera + RTX path that drives world poses through Fabric. With the RO/RW protection back in place, the eager world<->local flushes introduced by commits "fix: flush Fabric world matrices after local writes" and the follow-up "set_world_poses eager local sync" are no longer needed and are removed. The `change_block` context manager and its companion helpers existed only to batch those eager flushes; with the flushes gone, the API has nothing to defer and is removed from both `BaseFrameView` and `FabricFrameView`. Class docstring now spells out the load-bearing role of the RO/RW layout so a future refactor doesn't reintroduce the single-selection shape. Tests: * Removed `test_set_local_*_updates_renderer_facing_fabric_world_matrix` (asserted an eager-update contract that the lazy design deliberately does not hold; correctness across the next render tick is provided by the RO/RW protection, not by an extra Warp kernel). * Removed the four `test_change_block_*` tests; the API is gone. * Inverted `test_interleaved_set_emits_no_warning` back to `test_interleaved_set_emits_warning`, restored `_dirty == LOCAL` assertions in `test_world_scales_roundtrip` and the symmetric `WORLD` assertion in `test_local_scales_roundtrip`, and updated `test_multi_view_per_view_dirty_isolation` to expect the lazy cross-view behavior. * Adapted `test_prepare_for_reuse_detects_topology_change` and `test_fabric_rebuild_after_topology_change` to poll/rebuild all three selections. Verified: * `pytest test_ray_caster_camera.py::test_output_equal_to_usdcamera` passes. * `pytest test_ray_caster_camera.py::test_output_equal_to_usd_camera_when_intrinsics_set` 4/4 pass. * `pytest test_views_xform_prim_fabric.py` 71 passed, 3 skipped (cuda:1). * `./isaaclab.sh -f` clean. Net diff -198 lines.
1 parent 2966561 commit ce9ab26

3 files changed

Lines changed: 232 additions & 430 deletions

File tree

source/isaaclab/isaaclab/sim/views/base_frame_view.py

Lines changed: 0 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from __future__ import annotations
99

1010
import abc
11-
import contextlib
1211
import warnings
1312

1413
import warp as wp
@@ -39,37 +38,6 @@ def device(self) -> str:
3938
"""Device where arrays are allocated (``"cpu"`` or ``"cuda:0"``)."""
4039
...
4140

42-
@contextlib.contextmanager
43-
def change_block(self, update_world_matrices: bool = True, update_local_matrices: bool = False):
44-
"""Batch multiple transform writes into one logical change.
45-
46-
Backends may use this context to defer expensive derived-state updates
47-
until the outermost block exits. The default implementation is a no-op
48-
so callers can use it with every FrameView backend.
49-
50-
Args:
51-
update_world_matrices: Whether derived world matrices should be
52-
updated before the outermost block exits. Backends may ignore
53-
this option when they do not maintain separate cached world
54-
matrices.
55-
update_local_matrices: Whether derived local matrices should be
56-
updated before the outermost block exits. Backends may ignore
57-
this option when they do not maintain separate cached local
58-
matrices.
59-
"""
60-
yield self
61-
62-
def changeBlock(
63-
self,
64-
updateWorldMatrices: bool = True,
65-
updateLocalMatrices: bool = False,
66-
):
67-
"""CamelCase alias for :meth:`change_block`."""
68-
return self.change_block(
69-
update_world_matrices=updateWorldMatrices,
70-
update_local_matrices=updateLocalMatrices,
71-
)
72-
7341
@abc.abstractmethod
7442
def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]:
7543
"""Get world-space positions and orientations for prims in the view.

0 commit comments

Comments
 (0)