Skip to content

Commit afa5b85

Browse files
committed
fix: add PrepareForReuse to FabricFrameView, remove sync_usd_on_fabric_write
- FabricFrameView calls PrepareForReuse after warp writes for renderer sync - Remove sync_usd_on_fabric_write workaround - Camera uses FrameView directly (PrepareForReuse handles renderer sync) - Remove incorrect CPU/device fallback warnings (Warp handles CPU Fabric fine)
1 parent 4085387 commit afa5b85

5 files changed

Lines changed: 219 additions & 33 deletions

File tree

source/isaaclab/isaaclab/sensors/camera/camera.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,8 +406,7 @@ def _initialize_impl(self):
406406
self._renderer.prepare_stage(self.stage, self._num_envs)
407407

408408
# Create a view for the sensor with Fabric enabled for fast pose queries.
409-
# TODO: remove sync_usd_on_fabric_write=True once the GPU Fabric sync bug is fixed.
410-
self._view = FrameView(self.cfg.prim_path, device=self._device, stage=self.stage, sync_usd_on_fabric_write=True)
409+
self._view = FrameView(self.cfg.prim_path, device=self._device, stage=self.stage)
411410
# Check that sizes are correct
412411
if self._view.count != self._num_envs:
413412
raise RuntimeError(

source/isaaclab/isaaclab/sim/views/usd_frame_view.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,7 @@ def __init__(
7272
stage: USD stage to search for prims. Defaults to None, in which case the current
7373
active stage from the simulation context is used.
7474
**kwargs: Additional keyword arguments (ignored). Allows forward-compatible
75-
construction when callers pass backend-specific options like
76-
``sync_usd_on_fabric_write``.
75+
construction when callers pass backend-specific options.
7776
7877
Raises:
7978
ValueError: If any matched prim is not Xformable or doesn't have standardized

source/isaaclab/test/sensors/test_tiled_camera.py

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,3 +195,79 @@ def _populate_scene():
195195
sim_utils.define_rigid_body_properties(prim_path, sim_utils.RigidBodyPropertiesCfg())
196196
sim_utils.define_mass_properties(prim_path, sim_utils.MassPropertiesCfg(mass=5.0))
197197
sim_utils.define_collision_properties(prim_path, sim_utils.CollisionPropertiesCfg())
198+
199+
200+
# ------------------------------------------------------------------
201+
# Camera pose → render validation (PrepareForReuse / Fabric path)
202+
# ------------------------------------------------------------------
203+
204+
205+
@pytest.mark.parametrize(
206+
"device, camera_cls",
207+
[
208+
pytest.param("cpu", TiledCamera, id="cpu-tiled"),
209+
pytest.param("cpu", Camera, id="cpu-non_tiled"),
210+
pytest.param("cuda:0", TiledCamera, id="cuda:0-tiled"),
211+
pytest.param("cuda:0", Camera, id="cuda:0-non_tiled"),
212+
],
213+
)
214+
def test_camera_pose_update_reflected_in_render(setup_camera, device, camera_cls):
215+
"""Camera pose changes via FrameView should be visible in rendered depth.
216+
217+
Moves camera close then far, renders depth, and verifies that the mean
218+
valid depth from the far position is significantly larger (>1.5×) than
219+
the close position. This validates that Fabric-side pose writes
220+
(via PrepareForReuse) or USD writes are correctly propagated to the
221+
RTX renderer.
222+
"""
223+
sim, _unused_cam_cfg, dt = setup_camera
224+
225+
cam_cfg = CameraCfg(
226+
prim_path="/World/PoseTestCam",
227+
height=128,
228+
width=256,
229+
update_period=0,
230+
update_latest_camera_pose=True,
231+
data_types=["distance_to_camera"],
232+
spawn=sim_utils.PinholeCameraCfg(
233+
focal_length=24.0,
234+
focus_distance=400.0,
235+
horizontal_aperture=20.955,
236+
clipping_range=(0.1, 1.0e5),
237+
),
238+
)
239+
camera = camera_cls(cam_cfg)
240+
sim.reset()
241+
242+
target = torch.tensor([[0.0, 0.0, 0.0]], dtype=torch.float32, device=camera.device)
243+
max_range = cam_cfg.spawn.clipping_range[1]
244+
245+
# -- close position --
246+
eyes_close = torch.tensor([[2.0, 2.0, 2.0]], dtype=torch.float32, device=camera.device)
247+
camera.set_world_poses_from_view(eyes_close, target)
248+
sim.step()
249+
camera.update(dt)
250+
depth_close = camera.data.output["distance_to_camera"].clone()
251+
252+
# -- far position --
253+
eyes_far = torch.tensor([[8.0, 8.0, 8.0]], dtype=torch.float32, device=camera.device)
254+
camera.set_world_poses_from_view(eyes_far, target)
255+
sim.step()
256+
camera.update(dt)
257+
depth_far = camera.data.output["distance_to_camera"].clone()
258+
259+
# -- validate --
260+
valid_close = depth_close[depth_close < max_range]
261+
valid_far = depth_far[depth_far < max_range]
262+
263+
assert valid_close.numel() > 0, "No valid close-range depth pixels"
264+
assert valid_far.numel() > 0, "No valid far-range depth pixels"
265+
266+
mean_close = valid_close.mean().item()
267+
mean_far = valid_far.mean().item()
268+
269+
assert mean_far > mean_close * 1.5, (
270+
f"Far depth ({mean_far:.2f}) should be > 1.5× close depth ({mean_close:.2f}). "
271+
"Camera pose change may not be reaching the renderer."
272+
)
273+
del camera

source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -47,9 +47,13 @@ class FabricFrameView(BaseFrameView):
4747
fallback and non-accelerated operations (local poses, visibility, scales
4848
when Fabric is disabled).
4949
50-
When Fabric is enabled, world-pose and scale operations use GPU-accelerated
51-
Warp kernels operating on ``omni:fabric:worldMatrix``. All other operations
52-
delegate to the internal USD view.
50+
When Fabric is enabled, world-pose and scale operations use Warp kernels
51+
operating on ``omni:fabric:worldMatrix``. All other operations delegate
52+
to the internal USD view.
53+
54+
After every Fabric write, :meth:`PrepareForReuse` is called on the
55+
``PrimSelection`` to notify the renderer (FSD/Storm) that Fabric data
56+
has changed.
5357
5458
Pose getters return :class:`~isaaclab.utils.warp.ProxyArray`. Setters accept ``wp.array``.
5559
"""
@@ -59,31 +63,15 @@ def __init__(
5963
prim_path: str,
6064
device: str = "cpu",
6165
validate_xform_ops: bool = True,
62-
sync_usd_on_fabric_write: bool = False,
6366
stage: Usd.Stage | None = None,
67+
**kwargs,
6468
):
6569
self._usd_view = UsdFrameView(prim_path, device=device, validate_xform_ops=validate_xform_ops, stage=stage)
6670
self._device = device
67-
self._sync_usd_on_fabric_write = sync_usd_on_fabric_write
6871

6972
settings = SettingsManager.instance()
7073
self._use_fabric = bool(settings.get("/physics/fabricEnabled", False))
7174

72-
if self._use_fabric and self._device == "cpu":
73-
logger.warning(
74-
"Fabric mode with Warp fabric-array operations is not supported on CPU devices. "
75-
"Falling back to standard USD operations on the CPU. This may impact performance."
76-
)
77-
self._use_fabric = False
78-
79-
if self._use_fabric and self._device not in ("cuda", "cuda:0"):
80-
logger.warning(
81-
f"Fabric mode is not supported on device '{self._device}'. "
82-
"USDRT SelectPrims and Warp fabric arrays only support cuda:0. "
83-
"Falling back to standard USD operations. This may impact performance."
84-
)
85-
self._use_fabric = False
86-
8775
self._fabric_initialized = False
8876
self._fabric_usd_sync_done = False
8977
self._fabric_selection = None
@@ -136,6 +124,8 @@ def set_world_poses(self, positions=None, orientations=None, indices=None):
136124
if not self._fabric_initialized:
137125
self._initialize_fabric()
138126

127+
self._prepare_for_reuse()
128+
139129
indices_wp = self._resolve_indices_wp(indices)
140130
count = indices_wp.shape[0]
141131

@@ -167,8 +157,6 @@ def set_world_poses(self, positions=None, orientations=None, indices=None):
167157

168158
self._fabric_hierarchy.update_world_xforms()
169159
self._fabric_usd_sync_done = True
170-
if self._sync_usd_on_fabric_write:
171-
self._usd_view.set_world_poses(positions, orientations, indices)
172160

173161
def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]:
174162
if not self._use_fabric:
@@ -179,6 +167,8 @@ def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray,
179167
if not self._fabric_usd_sync_done:
180168
self._sync_fabric_from_usd_once()
181169

170+
self._prepare_for_reuse()
171+
182172
indices_wp = self._resolve_indices_wp(indices)
183173
count = indices_wp.shape[0]
184174

@@ -231,6 +221,8 @@ def set_scales(self, scales, indices=None):
231221
if not self._fabric_initialized:
232222
self._initialize_fabric()
233223

224+
self._prepare_for_reuse()
225+
234226
indices_wp = self._resolve_indices_wp(indices)
235227
count = indices_wp.shape[0]
236228

@@ -258,8 +250,6 @@ def set_scales(self, scales, indices=None):
258250

259251
self._fabric_hierarchy.update_world_xforms()
260252
self._fabric_usd_sync_done = True
261-
if self._sync_usd_on_fabric_write:
262-
self._usd_view.set_scales(scales, indices)
263253

264254
def get_scales(self, indices=None):
265255
if not self._use_fabric:
@@ -270,6 +260,8 @@ def get_scales(self, indices=None):
270260
if not self._fabric_usd_sync_done:
271261
self._sync_fabric_from_usd_once()
272262

263+
self._prepare_for_reuse()
264+
273265
indices_wp = self._resolve_indices_wp(indices)
274266
count = indices_wp.shape[0]
275267

@@ -297,6 +289,46 @@ def get_scales(self, indices=None):
297289
wp.synchronize()
298290
return scales_wp
299291

292+
# ------------------------------------------------------------------
293+
# Internal — PrepareForReuse (renderer notification + topology tracking)
294+
# ------------------------------------------------------------------
295+
296+
def _prepare_for_reuse(self) -> None:
297+
"""Call PrepareForReuse on the PrimSelection to notify the renderer.
298+
299+
PrepareForReuse serves two purposes:
300+
301+
1. **Renderer notification**: Tells FSD/Storm that Fabric data has
302+
been (or will be) modified, so the next rendered frame reflects
303+
the updated transforms.
304+
2. **Topology change detection**: Returns True when Fabric's
305+
internal memory layout changed (e.g., prims added/removed).
306+
In that case, view-to-fabric index mappings and fabricarrays
307+
must be rebuilt.
308+
"""
309+
if self._fabric_selection is None:
310+
return
311+
312+
topology_changed = self._fabric_selection.PrepareForReuse()
313+
if topology_changed:
314+
logger.info("Fabric topology changed — rebuilding view-to-fabric index mapping.")
315+
self._rebuild_fabric_arrays()
316+
317+
def _rebuild_fabric_arrays(self) -> None:
318+
"""Rebuild fabricarray and view↔fabric mappings after a topology change."""
319+
self._view_to_fabric = wp.zeros((self.count,), dtype=wp.uint32, device=self._fabric_device)
320+
self._fabric_to_view = wp.fabricarray(self._fabric_selection, self._view_index_attr)
321+
322+
wp.launch(
323+
kernel=fabric_utils.set_view_to_fabric_array,
324+
dim=self._fabric_to_view.shape[0],
325+
inputs=[self._fabric_to_view, self._view_to_fabric],
326+
device=self._fabric_device,
327+
)
328+
wp.synchronize()
329+
330+
self._fabric_world_matrices = wp.fabricarray(self._fabric_selection, "omni:fabric:worldMatrix")
331+
300332
# ------------------------------------------------------------------
301333
# Internal — Fabric initialization
302334
# ------------------------------------------------------------------
@@ -391,11 +423,8 @@ def _sync_fabric_from_usd_once(self) -> None:
391423
orientations_usd = orientations_usd_ta.warp
392424
scales_usd = self._usd_view.get_scales()
393425

394-
prev_sync = self._sync_usd_on_fabric_write
395-
self._sync_usd_on_fabric_write = False
396426
self.set_world_poses(positions_usd, orientations_usd)
397427
self.set_scales(scales_usd)
398-
self._sync_usd_on_fabric_write = prev_sync
399428

400429
self._fabric_usd_sync_done = True
401430

source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py

Lines changed: 85 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,9 @@
2121

2222
import pytest # noqa: E402
2323
import torch # noqa: E402
24+
import warp as wp # noqa: E402
2425
from frame_view_contract_utils import * # noqa: F401, F403, E402
25-
from frame_view_contract_utils import CHILD_OFFSET, ViewBundle # noqa: E402
26+
from frame_view_contract_utils import CHILD_OFFSET, ViewBundle, test_set_world_updates_local # noqa: E402
2627
from isaaclab_physx.sim.views import FabricFrameView as FrameView # noqa: E402
2728

2829
from pxr import Gf, UsdGeom # noqa: E402
@@ -95,7 +96,7 @@ def factory(num_envs: int, device: str) -> ViewBundle:
9596
sim_utils.create_prim(f"/World/Parent_{i}/Child", "Camera", translation=CHILD_OFFSET, stage=stage)
9697

9798
sim_utils.SimulationContext(sim_utils.SimulationCfg(dt=0.01, device=device, use_fabric=True))
98-
view = FrameView("/World/Parent_.*/Child", device=device, sync_usd_on_fabric_write=True)
99+
view = FrameView("/World/Parent_.*/Child", device=device)
99100
return ViewBundle(
100101
view=view,
101102
get_parent_pos=_get_parent_positions,
@@ -104,3 +105,85 @@ def factory(num_envs: int, device: str) -> ViewBundle:
104105
)
105106

106107
return factory
108+
109+
110+
# ------------------------------------------------------------------
111+
# Override shared contract test with expected failure for Fabric.
112+
# FabricFrameView.set_world_poses writes to Fabric worldMatrix only; the local
113+
# pose (read via USD) does not reflect the change because there is no
114+
# Fabric → USD writeback for local poses. This is tracked as Issue #5
115+
# (localMatrix: set_local_poses falls back to USD).
116+
# ------------------------------------------------------------------
117+
118+
119+
@pytest.mark.xfail(
120+
reason=(
121+
"Issue #5: FabricFrameView.set_world_poses writes to Fabric worldMatrix only. "
122+
"get_local_poses reads from stale USD because there is no Fabric→USD "
123+
"writeback for local poses."
124+
),
125+
strict=True,
126+
)
127+
def test_set_world_updates_local(device, view_factory): # noqa: F811
128+
"""Override the shared test to mark it as expected failure."""
129+
from frame_view_contract_utils import test_set_world_updates_local as _impl # noqa: PLC0415
130+
131+
_impl(device, view_factory)
132+
133+
134+
# ------------------------------------------------------------------
135+
# Fabric-specific tests (not in shared contract)
136+
# ------------------------------------------------------------------
137+
138+
139+
@wp.kernel
140+
def _fill_position(out: wp.array(dtype=wp.float32, ndim=2), x: float, y: float, z: float):
141+
i = wp.tid()
142+
out[i, 0] = wp.float32(x)
143+
out[i, 1] = wp.float32(y)
144+
out[i, 2] = wp.float32(z)
145+
146+
147+
@pytest.mark.parametrize("device", ["cuda:0"])
148+
def test_fabric_set_world_does_not_write_back_to_usd(device, view_factory):
149+
"""Verify that set_world_poses in Fabric mode does NOT sync back to USD.
150+
151+
This confirms the removal of sync_usd_on_fabric_write. After calling
152+
set_world_poses, the USD prim's xformOps should still contain the
153+
original (stale) values.
154+
"""
155+
bundle = view_factory(1, device)
156+
view = bundle.view
157+
158+
# Capture the original USD world position BEFORE any Fabric write
159+
stage = sim_utils.get_current_stage()
160+
prim = stage.GetPrimAtPath(view.prim_paths[0])
161+
xform_cache = UsdGeom.XformCache()
162+
usd_tf_before = xform_cache.GetLocalToWorldTransform(prim)
163+
usd_t_before = usd_tf_before.ExtractTranslation()
164+
orig_usd_pos = torch.tensor([float(usd_t_before[0]), float(usd_t_before[1]), float(usd_t_before[2])])
165+
166+
# Write to Fabric — move to (99, 99, 99)
167+
new_pos = wp.zeros((1, 3), dtype=wp.float32, device=device)
168+
wp.launch(kernel=_fill_position, dim=1, inputs=[new_pos, 99.0, 99.0, 99.0], device=device)
169+
view.set_world_poses(positions=new_pos)
170+
171+
# Verify Fabric has the new position
172+
fab_pos, _ = view.get_world_poses()
173+
pos_torch = wp.to_torch(fab_pos)
174+
assert torch.allclose(pos_torch, torch.tensor([[99.0, 99.0, 99.0]], device=device), atol=0.1), (
175+
f"Fabric should have new position, got {pos_torch}"
176+
)
177+
178+
# Verify USD still has the ORIGINAL position (no writeback)
179+
xform_cache_after = UsdGeom.XformCache()
180+
usd_tf_after = xform_cache_after.GetLocalToWorldTransform(prim)
181+
usd_t_after = usd_tf_after.ExtractTranslation()
182+
usd_pos_after = torch.tensor([float(usd_t_after[0]), float(usd_t_after[1]), float(usd_t_after[2])])
183+
assert torch.allclose(usd_pos_after, orig_usd_pos, atol=0.1), (
184+
f"USD should still have original position {orig_usd_pos}, but got {usd_pos_after}. "
185+
f"sync_usd_on_fabric_write may not have been fully removed."
186+
)
187+
188+
189+

0 commit comments

Comments
 (0)