diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 82c5eb49ba92..a13693c64171 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -144,6 +144,7 @@ Guidelines for modifications: * Patrick Yin * Paul Reeves * Peter Du +* Peter Verswyvelen * Philipp Reist * Piotr Barejko * Pulkit Goyal diff --git a/source/isaaclab/changelog.d/fix-fabric-prepare-for-reuse.rst b/source/isaaclab/changelog.d/fix-fabric-prepare-for-reuse.rst new file mode 100644 index 000000000000..20a6d385c094 --- /dev/null +++ b/source/isaaclab/changelog.d/fix-fabric-prepare-for-reuse.rst @@ -0,0 +1,8 @@ +Changed +^^^^^^^ + +* Updated :class:`~isaaclab.sensors.camera.Camera` to construct its internal + :class:`~isaaclab.sim.views.FrameView` without the now-removed + ``sync_usd_on_fabric_write`` kwarg. USD attributes on camera prims are + no longer kept in sync with Fabric writes; read poses through the view's + getters instead. diff --git a/source/isaaclab/isaaclab/sensors/camera/camera.py b/source/isaaclab/isaaclab/sensors/camera/camera.py index be52668dbd6c..675ee4a7bb7c 100644 --- a/source/isaaclab/isaaclab/sensors/camera/camera.py +++ b/source/isaaclab/isaaclab/sensors/camera/camera.py @@ -18,8 +18,7 @@ import isaaclab.sim as sim_utils import isaaclab.utils.sensors as sensor_utils from isaaclab.app.settings_manager import get_settings_manager -from isaaclab.renderers import BaseRenderer -from isaaclab.renderers.camera_render_spec import CameraRenderSpec +from isaaclab.renderers import BaseRenderer, CameraRenderSpec from isaaclab.sim.views import FrameView from isaaclab.utils import to_camel_case from isaaclab.utils.math import ( @@ -380,9 +379,7 @@ def _initialize_impl(self): # references to prims located in the stage. sim_ctx.render_context.ensure_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 Fabric sync bug is fixed. - self._view = FrameView(self.cfg.prim_path, device=self._device, stage=self.stage, sync_usd_on_fabric_write=True) + self._view = FrameView(self.cfg.prim_path, device=self._device, stage=self.stage) # Check that sizes are correct if self._view.count != self._num_envs: raise RuntimeError( diff --git a/source/isaaclab/isaaclab/sim/views/usd_frame_view.py b/source/isaaclab/isaaclab/sim/views/usd_frame_view.py index 7730c3dd735d..88392d54b2a0 100644 --- a/source/isaaclab/isaaclab/sim/views/usd_frame_view.py +++ b/source/isaaclab/isaaclab/sim/views/usd_frame_view.py @@ -72,8 +72,7 @@ def __init__( stage: USD stage to search for prims. Defaults to None, in which case the current active stage from the simulation context is used. **kwargs: Additional keyword arguments (ignored). Allows forward-compatible - construction when callers pass backend-specific options like - ``sync_usd_on_fabric_write``. + construction when callers pass backend-specific options. Raises: ValueError: If any matched prim is not Xformable or doesn't have standardized diff --git a/source/isaaclab/test/sensors/test_camera.py b/source/isaaclab/test/sensors/test_camera.py index daed8e95773d..99c93fec7323 100644 --- a/source/isaaclab/test/sensors/test_camera.py +++ b/source/isaaclab/test/sensors/test_camera.py @@ -1161,6 +1161,72 @@ def cleanup(self, render_data): Renderer._registry.pop(backend, None) +@pytest.mark.parametrize("device", ["cuda:0", "cpu"]) +@pytest.mark.isaacsim_ci +def test_camera_pose_update_reflected_in_render(setup_camera_device, device): + """Camera pose changes via FrameView should be visible in rendered depth. + + Moves the camera close then far, renders depth, and verifies that the mean + valid depth from the far position is significantly larger (>1.5×) than the + close position. This validates that Fabric-side pose writes (via + PrepareForReuse) and USD writes are correctly propagated to the RTX + renderer. + """ + sim, _unused_cam_cfg, dt = setup_camera_device + + cam_cfg = CameraCfg( + prim_path="/World/PoseTestCam", + height=128, + width=256, + update_period=0, + update_latest_camera_pose=True, + data_types=["distance_to_camera"], + spawn=sim_utils.PinholeCameraCfg( + focal_length=24.0, + focus_distance=400.0, + horizontal_aperture=20.955, + clipping_range=(0.1, 1.0e5), + ), + ) + camera = Camera(cam_cfg) + try: + sim.reset() + + target = torch.tensor([[0.0, 0.0, 0.0]], dtype=torch.float32, device=camera.device) + max_range = cam_cfg.spawn.clipping_range[1] + + # -- close position -- + eyes_close = torch.tensor([[2.0, 2.0, 2.0]], dtype=torch.float32, device=camera.device) + camera.set_world_poses_from_view(eyes_close, target) + sim.step() + camera.update(dt) + depth_close = camera.data.output["distance_to_camera"].clone() + + # -- far position -- + eyes_far = torch.tensor([[8.0, 8.0, 8.0]], dtype=torch.float32, device=camera.device) + camera.set_world_poses_from_view(eyes_far, target) + sim.step() + camera.update(dt) + depth_far = camera.data.output["distance_to_camera"].clone() + + # -- validate -- + valid_close = depth_close[depth_close < max_range] + valid_far = depth_far[depth_far < max_range] + + assert valid_close.numel() > 0, "No valid close-range depth pixels" + assert valid_far.numel() > 0, "No valid far-range depth pixels" + + mean_close = valid_close.mean().item() + mean_far = valid_far.mean().item() + + assert mean_far > mean_close * 1.5, ( + f"Far depth ({mean_far:.2f}) should be > 1.5× close depth ({mean_close:.2f}). " + "Camera pose change may not be reaching the renderer." + ) + finally: + del camera + + def _populate_scene(): """Add prims to the scene.""" # Ground-plane diff --git a/source/isaaclab/test/sensors/test_multi_mesh_ray_caster_camera.py b/source/isaaclab/test/sensors/test_multi_mesh_ray_caster_camera.py index 8657c938c691..7e7efe16d091 100644 --- a/source/isaaclab/test/sensors/test_multi_mesh_ray_caster_camera.py +++ b/source/isaaclab/test/sensors/test_multi_mesh_ray_caster_camera.py @@ -752,11 +752,11 @@ def test_output_equal_to_usd_camera_when_intrinsics_set(setup_simulation): # set camera position camera_warp.set_world_poses_from_view( - eyes=torch.tensor([[0.0, 0.0, 5.0]], device=camera_warp.device), + eyes=torch.tensor([[0.1, 0.0, 5.0]], device=camera_warp.device), targets=torch.tensor([[0.0, 0.0, 0.0]], device=camera_warp.device), ) camera_usd.set_world_poses_from_view( - eyes=torch.tensor([[0.0, 0.0, 5.0]], device=camera_usd.device), + eyes=torch.tensor([[0.1, 0.0, 5.0]], device=camera_usd.device), targets=torch.tensor([[0.0, 0.0, 0.0]], device=camera_usd.device), ) diff --git a/source/isaaclab/test/sensors/test_ray_caster_camera.py b/source/isaaclab/test/sensors/test_ray_caster_camera.py index a913d38dd833..752734936934 100644 --- a/source/isaaclab/test/sensors/test_ray_caster_camera.py +++ b/source/isaaclab/test/sensors/test_ray_caster_camera.py @@ -898,11 +898,11 @@ def test_output_equal_to_usd_camera_when_intrinsics_set(setup_sim, focal_length_ # set camera position camera_warp.set_world_poses_from_view( - eyes=torch.tensor([[0.001, 0.0, 5.0]], device=camera_warp.device), + eyes=torch.tensor([[0.1, 0.0, 5.0]], device=camera_warp.device), targets=torch.tensor([[0.0, 0.0, 0.0]], device=camera_warp.device), ) camera_usd.set_world_poses_from_view( - eyes=torch.tensor([[0.001, 0.0, 5.0]], device=camera_usd.device), + eyes=torch.tensor([[0.1, 0.0, 5.0]], device=camera_usd.device), targets=torch.tensor([[0.0, 0.0, 0.0]], device=camera_usd.device), ) diff --git a/source/isaaclab_physx/changelog.d/fix-fabric-prepare-for-reuse.rst b/source/isaaclab_physx/changelog.d/fix-fabric-prepare-for-reuse.rst new file mode 100644 index 000000000000..e7d842da72bd --- /dev/null +++ b/source/isaaclab_physx/changelog.d/fix-fabric-prepare-for-reuse.rst @@ -0,0 +1,12 @@ +Changed +^^^^^^^ + +* **Breaking:** Removed the ``sync_usd_on_fabric_write`` keyword argument from + :class:`~isaaclab_physx.sim.views.FabricFrameView`. Fabric writes + (``set_world_poses``, ``set_scales``) now notify the renderer via + ``PrepareForReuse()`` on the underlying ``PrimSelection`` instead of writing + back to USD, which is ~200x faster and avoids the stale USD shadow state the + old path produced. Callers passing ``sync_usd_on_fabric_write=True`` should + remove the argument; if they relied on USD reflecting Fabric writes, they + should now read Fabric poses directly via the view's getters or refresh USD + explicitly. diff --git a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py index de65e8501793..1bcff86d57ac 100644 --- a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py +++ b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py @@ -23,6 +23,12 @@ logger = logging.getLogger(__name__) +# TODO: extend this to ``cuda:N`` once we wire up multi-GPU support for the view. +# Recent Kit / USDRT releases do support multi-GPU ``SelectPrims``, but the +# rest of the FabricFrameView wiring (selections, indexed arrays, etc.) still +# assumes a single device — to be tackled in a follow-up. +_fabric_supported_devices = ("cpu", "cuda", "cuda:0") + def _to_float32_2d(a: wp.array | torch.Tensor) -> wp.array | torch.Tensor: """Ensure array is compatible with Fabric kernels (2-D float32). @@ -47,9 +53,15 @@ class FabricFrameView(BaseFrameView): fallback and non-accelerated operations (local poses, visibility, scales when Fabric is disabled). - When Fabric is enabled, world-pose and scale operations use GPU-accelerated - Warp kernels operating on ``omni:fabric:worldMatrix``. All other operations - delegate to the internal USD view. + When Fabric is enabled, world-pose and scale operations use Warp kernels + operating on ``omni:fabric:worldMatrix``. All other operations delegate + to the internal USD view. + + After every Fabric write (``set_world_poses``, ``set_scales``), + :meth:`PrepareForReuse` is called on the ``PrimSelection`` to notify + the FSD renderer that Fabric data has changed and to detect topology + changes that require rebuilding internal mappings. Read operations + do not call PrepareForReuse to avoid unnecessary renderer invalidation. Pose getters return :class:`~isaaclab.utils.warp.ProxyArray`. Setters accept ``wp.array``. """ @@ -59,27 +71,32 @@ def __init__( prim_path: str, device: str = "cpu", validate_xform_ops: bool = True, - sync_usd_on_fabric_write: bool = False, stage: Usd.Stage | None = None, + **kwargs, ): + """Initialize the view. + + Args: + prim_path: USD prim-path pattern to match. + device: Device for Warp arrays (``"cpu"`` or ``"cuda:0"``). + validate_xform_ops: Whether to validate prim xform-ops. + stage: USD stage; defaults to the current sim context's stage. + **kwargs: Additional keyword arguments (ignored). Matches the signature of + :class:`~isaaclab.sim.views.UsdFrameView` so that the top-level + :class:`~isaaclab.sim.views.FrameView` factory can forward backend-agnostic + kwargs without each backend having to know about every option. + """ self._usd_view = UsdFrameView(prim_path, device=device, validate_xform_ops=validate_xform_ops, stage=stage) self._device = device - self._sync_usd_on_fabric_write = sync_usd_on_fabric_write settings = SettingsManager.instance() self._use_fabric = bool(settings.get("/physics/fabricEnabled", False)) - if self._use_fabric and self._device == "cpu": - logger.warning( - "Fabric mode with Warp fabric-array operations is not supported on CPU devices. " - "Falling back to standard USD operations on the CPU. This may impact performance." - ) - self._use_fabric = False - - if self._use_fabric and self._device not in ("cuda", "cuda:0"): + if self._use_fabric and self._device not in _fabric_supported_devices: logger.warning( f"Fabric mode is not supported on device '{self._device}'. " - "USDRT SelectPrims and Warp fabric arrays only support cuda:0. " + "USDRT SelectPrims and Warp fabric arrays are currently " + f"only supported on {', '.join(_fabric_supported_devices)}. " "Falling back to standard USD operations. This may impact performance." ) self._use_fabric = False @@ -136,6 +153,8 @@ def set_world_poses(self, positions=None, orientations=None, indices=None): if not self._fabric_initialized: self._initialize_fabric() + self._prepare_for_reuse() + indices_wp = self._resolve_indices_wp(indices) count = indices_wp.shape[0] @@ -167,8 +186,6 @@ def set_world_poses(self, positions=None, orientations=None, indices=None): self._fabric_hierarchy.update_world_xforms() self._fabric_usd_sync_done = True - if self._sync_usd_on_fabric_write: - self._usd_view.set_world_poses(positions, orientations, indices) def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]: if not self._use_fabric: @@ -231,6 +248,8 @@ def set_scales(self, scales, indices=None): if not self._fabric_initialized: self._initialize_fabric() + self._prepare_for_reuse() + indices_wp = self._resolve_indices_wp(indices) count = indices_wp.shape[0] @@ -258,8 +277,6 @@ def set_scales(self, scales, indices=None): self._fabric_hierarchy.update_world_xforms() self._fabric_usd_sync_done = True - if self._sync_usd_on_fabric_write: - self._usd_view.set_scales(scales, indices) def get_scales(self, indices=None): if not self._use_fabric: @@ -297,6 +314,56 @@ def get_scales(self, indices=None): wp.synchronize() return scales_wp + # ------------------------------------------------------------------ + # Internal — PrepareForReuse (renderer notification + topology tracking) + # ------------------------------------------------------------------ + + def _prepare_for_reuse(self) -> None: + """Call PrepareForReuse on the PrimSelection to notify the renderer. + + PrepareForReuse serves two purposes: + + 1. **Renderer notification**: Tells FSD/Storm that Fabric data has + been (or will be) modified, so the next rendered frame reflects + the updated transforms. + 2. **Topology change detection**: Returns True when Fabric's + internal memory layout changed (e.g., prims added/removed). + In that case, view-to-fabric index mappings and fabricarrays + must be rebuilt. + """ + if self._fabric_selection is None: + return + + topology_changed = self._fabric_selection.PrepareForReuse() + if topology_changed: + logger.info("Fabric topology changed — rebuilding view-to-fabric index mapping.") + self._rebuild_fabric_arrays() + + def _rebuild_fabric_arrays(self) -> None: + """Rebuild fabricarray and view↔fabric mappings after a topology change. + + Note: Only index mappings and fabricarrays are rebuilt. Position/orientation/scale + buffers are *not* resized because ``self.count`` is derived from the USD prim-path + pattern (via ``_usd_view.count``) and does not change when Fabric rearranges its + internal memory layout. The assertion below guards this invariant. + """ + assert self.count == self._default_view_indices.shape[0], ( + f"Prim count changed ({self.count} vs {self._default_view_indices.shape[0]}). " + "Fabric topology change added/removed tracked prims — full re-initialization required." + ) + self._view_to_fabric = wp.zeros((self.count,), dtype=wp.uint32, device=self._fabric_device) + self._fabric_to_view = wp.fabricarray(self._fabric_selection, self._view_index_attr) + + wp.launch( + kernel=fabric_utils.set_view_to_fabric_array, + dim=self._fabric_to_view.shape[0], + inputs=[self._fabric_to_view, self._view_to_fabric], + device=self._fabric_device, + ) + wp.synchronize() + + self._fabric_world_matrices = wp.fabricarray(self._fabric_selection, "omni:fabric:worldMatrix") + # ------------------------------------------------------------------ # Internal — Fabric initialization # ------------------------------------------------------------------ @@ -337,34 +404,25 @@ def _initialize_fabric(self) -> None: ) wp.synchronize() - fabric_device = self._device - if self._device == "cuda": - logger.warning("Fabric device is not specified, defaulting to 'cuda:0'.") - fabric_device = "cuda:0" - elif self._device.startswith("cuda:"): - if self._device != "cuda:0": - logger.debug( - f"SelectPrims only supports cuda:0. Using cuda:0 for SelectPrims " - f"even though simulation device is {self._device}." - ) - fabric_device = "cuda:0" + # The constructor should have taken care of this, but double check here to avoid regressions + assert self._device in _fabric_supported_devices self._fabric_selection = fabric_stage.SelectPrims( require_attrs=[ (usdrt.Sdf.ValueTypeNames.UInt, self._view_index_attr, usdrt.Usd.Access.Read), (usdrt.Sdf.ValueTypeNames.Matrix4d, "omni:fabric:worldMatrix", usdrt.Usd.Access.ReadWrite), ], - device=fabric_device, + device=self._device, ) - self._view_to_fabric = wp.zeros((self.count,), dtype=wp.uint32, device=fabric_device) + self._view_to_fabric = wp.zeros((self.count,), dtype=wp.uint32, device=self._device) self._fabric_to_view = wp.fabricarray(self._fabric_selection, self._view_index_attr) wp.launch( kernel=fabric_utils.set_view_to_fabric_array, dim=self._fabric_to_view.shape[0], inputs=[self._fabric_to_view, self._view_to_fabric], - device=fabric_device, + device=self._device, ) wp.synchronize() @@ -376,13 +434,17 @@ def _initialize_fabric(self) -> None: self._fabric_dummy_buffer = wp.zeros((0, 3), dtype=wp.float32, device=self._device) self._fabric_world_matrices = wp.fabricarray(self._fabric_selection, "omni:fabric:worldMatrix") self._fabric_stage = fabric_stage - self._fabric_device = fabric_device + self._fabric_device = self._device self._fabric_initialized = True self._fabric_usd_sync_done = False def _sync_fabric_from_usd_once(self) -> None: - """Sync Fabric world matrices from USD once, on the first read.""" + """Sync Fabric world matrices from USD once, on the first read. + + ``set_world_poses`` and ``set_scales`` each set ``_fabric_usd_sync_done`` + themselves, so no explicit flag assignment is needed here. + """ if not self._fabric_initialized: self._initialize_fabric() @@ -391,13 +453,8 @@ def _sync_fabric_from_usd_once(self) -> None: orientations_usd = orientations_usd_ta.warp scales_usd = self._usd_view.get_scales() - prev_sync = self._sync_usd_on_fabric_write - self._sync_usd_on_fabric_write = False self.set_world_poses(positions_usd, orientations_usd) self.set_scales(scales_usd) - self._sync_usd_on_fabric_write = prev_sync - - self._fabric_usd_sync_done = True def _resolve_indices_wp(self, indices: wp.array | None) -> wp.array: """Resolve view indices as a Warp uint32 array.""" diff --git a/source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py b/source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py index 4376c0e0b8ea..f0c18ccb98c7 100644 --- a/source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py +++ b/source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py @@ -21,8 +21,9 @@ import pytest # noqa: E402 import torch # noqa: E402 +import warp as wp # noqa: E402 from frame_view_contract_utils import * # noqa: F401, F403, E402 -from frame_view_contract_utils import CHILD_OFFSET, ViewBundle # noqa: E402 +from frame_view_contract_utils import CHILD_OFFSET, ViewBundle, test_set_world_updates_local # noqa: E402 from isaaclab_physx.sim.views import FabricFrameView as FrameView # noqa: E402 from pxr import Gf, UsdGeom # noqa: E402 @@ -45,8 +46,6 @@ def test_setup_teardown(): def _skip_if_unavailable(device: str): if device.startswith("cuda") and not torch.cuda.is_available(): pytest.skip("CUDA not available") - if device == "cpu": - pytest.skip("Warp fabricarray operations on CPU have known issues") # ------------------------------------------------------------------ @@ -95,7 +94,7 @@ def factory(num_envs: int, device: str) -> ViewBundle: sim_utils.create_prim(f"/World/Parent_{i}/Child", "Camera", translation=CHILD_OFFSET, stage=stage) sim_utils.SimulationContext(sim_utils.SimulationCfg(dt=0.01, device=device, use_fabric=True)) - view = FrameView("/World/Parent_.*/Child", device=device, sync_usd_on_fabric_write=True) + view = FrameView("/World/Parent_.*/Child", device=device) return ViewBundle( view=view, get_parent_pos=_get_parent_positions, @@ -104,3 +103,133 @@ def factory(num_envs: int, device: str) -> ViewBundle: ) return factory + + +# ------------------------------------------------------------------ +# Override shared contract test with expected failure for Fabric. +# FabricFrameView.set_world_poses writes to Fabric worldMatrix only; the local +# pose (read via USD) does not reflect the change because there is no +# Fabric → USD writeback for local poses. This is tracked as Issue #5 +# (localMatrix: set_local_poses falls back to USD). +# ------------------------------------------------------------------ + + +@pytest.mark.parametrize("device", ["cpu", "cuda:0"]) +@pytest.mark.xfail( + reason=( + "Issue #5: FabricFrameView.set_world_poses writes to Fabric worldMatrix only. " + "get_local_poses reads from stale USD because there is no Fabric→USD " + "writeback for local poses." + ), + strict=True, +) +def test_set_world_updates_local(device, view_factory): # noqa: F811 + """Override the shared test to mark it as expected failure.""" + from frame_view_contract_utils import test_set_world_updates_local as _impl # noqa: PLC0415 + + _impl(device, view_factory) + + +# ------------------------------------------------------------------ +# Fabric-specific tests (not in shared contract) +# ------------------------------------------------------------------ + + +@wp.kernel +def _fill_position(out: wp.array(dtype=wp.float32, ndim=2), x: float, y: float, z: float): + i = wp.tid() + out[i, 0] = wp.float32(x) + out[i, 1] = wp.float32(y) + out[i, 2] = wp.float32(z) + + +@pytest.mark.parametrize("device", ["cpu", "cuda:0"]) +def test_fabric_set_world_does_not_write_back_to_usd(device, view_factory): + """Verify that set_world_poses in Fabric mode does NOT sync back to USD. + + This confirms the removal of sync_usd_on_fabric_write. After calling + set_world_poses, the USD prim's xformOps should still contain the + original (stale) values. + """ + bundle = view_factory(1, device) + view = bundle.view + + # Capture the original USD world position BEFORE any Fabric write + stage = sim_utils.get_current_stage() + prim = stage.GetPrimAtPath(view.prim_paths[0]) + xform_cache = UsdGeom.XformCache() + usd_tf_before = xform_cache.GetLocalToWorldTransform(prim) + usd_t_before = usd_tf_before.ExtractTranslation() + orig_usd_pos = torch.tensor([float(usd_t_before[0]), float(usd_t_before[1]), float(usd_t_before[2])]) + + # Write to Fabric — move to (99, 99, 99) + new_pos = wp.zeros((1, 3), dtype=wp.float32, device=device) + wp.launch(kernel=_fill_position, dim=1, inputs=[new_pos, 99.0, 99.0, 99.0], device=device) + view.set_world_poses(positions=new_pos) + + # Verify Fabric has the new position + fab_pos, _ = view.get_world_poses() + pos_torch = wp.to_torch(fab_pos) + assert torch.allclose(pos_torch, torch.tensor([[99.0, 99.0, 99.0]], device=device), atol=0.1), ( + f"Fabric should have new position, got {pos_torch}" + ) + + # Verify USD still has the ORIGINAL position (no writeback). Equality, not + # approximate — USD should literally not have moved, so any drift would + # indicate a residual writeback path. + xform_cache_after = UsdGeom.XformCache() + usd_tf_after = xform_cache_after.GetLocalToWorldTransform(prim) + usd_t_after = usd_tf_after.ExtractTranslation() + usd_pos_after = torch.tensor([float(usd_t_after[0]), float(usd_t_after[1]), float(usd_t_after[2])]) + assert torch.allclose(usd_pos_after, orig_usd_pos, atol=0.0), ( + f"USD should still have original position {orig_usd_pos}, but got {usd_pos_after}. " + f"sync_usd_on_fabric_write may not have been fully removed." + ) + + +@pytest.mark.parametrize("device", ["cpu", "cuda:0"]) +def test_fabric_rebuild_after_topology_change(device, view_factory, monkeypatch): + """Forcing the topology-changed branch on a write triggers + :meth:`_rebuild_fabric_arrays` and leaves the view in a state where + subsequent writes/reads still produce correct data. + + Real ``PrimSelection.PrepareForReuse`` reports topology change only when + Fabric reallocates internally, which is hard to provoke from a unit test. + Instead we monkeypatch ``_prepare_for_reuse`` on the instance to always + take the rebuild branch and verify the view remains usable. + """ + bundle = view_factory(2, device) + view = bundle.view + + # First write — initializes Fabric and binds _fabric_selection. + initial = wp.zeros((2, 3), dtype=wp.float32, device=device) + wp.launch(kernel=_fill_position, dim=2, inputs=[initial, 1.0, 2.0, 3.0], device=device) + view.set_world_poses(positions=initial) + + rebuild_calls = [] + real_rebuild = view._rebuild_fabric_arrays + + def spy_rebuild(): + rebuild_calls.append(True) + real_rebuild() + + def force_topology_changed(): + if view._fabric_selection is not None: + view._fabric_selection.PrepareForReuse() + spy_rebuild() + + monkeypatch.setattr(view, "_prepare_for_reuse", force_topology_changed) + + # Trigger another write — goes through the forced topology-change branch. + new = wp.zeros((2, 3), dtype=wp.float32, device=device) + wp.launch(kernel=_fill_position, dim=2, inputs=[new, 4.0, 5.0, 6.0], device=device) + view.set_world_poses(positions=new) + + assert rebuild_calls, "Forced topology-change branch did not invoke _rebuild_fabric_arrays" + + # Read back — proves the rebuilt _view_to_fabric and _fabric_world_matrices + # are still consistent. + ret_pos, _ = view.get_world_poses() + pos_torch = wp.to_torch(ret_pos) + expected = torch.tensor([[4.0, 5.0, 6.0], [4.0, 5.0, 6.0]], device=device) + assert torch.allclose(pos_torch, expected, atol=1e-7), f"Read after rebuild failed on {device}: {pos_torch}"