Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions source/isaaclab/isaaclab/sensors/camera/camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import isaaclab.utils.sensors as sensor_utils
from isaaclab.app.settings_manager import get_settings_manager
from isaaclab.renderers import BaseRenderer, Renderer
from isaaclab.sim.views import FrameView
from isaaclab.sim.views import UsdFrameView
from isaaclab.utils import has_kit, to_camel_case
from isaaclab.utils.math import (
convert_camera_frame_orientation_convention,
Expand Down Expand Up @@ -405,9 +405,11 @@ def _initialize_impl(self):
# references to prims located in the stage.
self._renderer.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)
# Camera uses UsdFrameView directly (not FrameView/FabricFrameView) because
# the RTX renderer / Replicator reads camera poses from USD prim paths, not
# from Fabric. Writing to Fabric + sync_usd_on_fabric_write was wasteful —
# this bypasses Fabric entirely for camera transforms.
self._view = UsdFrameView(self.cfg.prim_path, device=self._device, stage=self.stage)
# Check that sizes are correct
if self._view.count != self._num_envs:
raise RuntimeError(
Expand Down
3 changes: 1 addition & 2 deletions source/isaaclab/isaaclab/sim/views/usd_frame_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,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
Expand Down
76 changes: 76 additions & 0 deletions source/isaaclab/test/sensors/test_tiled_camera.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,3 +195,79 @@ def _populate_scene():
sim_utils.define_rigid_body_properties(prim_path, sim_utils.RigidBodyPropertiesCfg())
sim_utils.define_mass_properties(prim_path, sim_utils.MassPropertiesCfg(mass=5.0))
sim_utils.define_collision_properties(prim_path, sim_utils.CollisionPropertiesCfg())


# ------------------------------------------------------------------
# Camera pose → render validation (PrepareForReuse / Fabric path)
# ------------------------------------------------------------------


@pytest.mark.parametrize(
"device, camera_cls",
[
pytest.param("cpu", TiledCamera, id="cpu-tiled"),
pytest.param("cpu", Camera, id="cpu-non_tiled"),
pytest.param("cuda:0", TiledCamera, id="cuda:0-tiled"),
pytest.param("cuda:0", Camera, id="cuda:0-non_tiled"),
],
)
Comment on lines +205 to +213
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing @pytest.mark.isaacsim_ci marker

Every other test in this file carries @pytest.mark.isaacsim_ci, which gates execution in the CI pipeline. The new test_camera_pose_update_reflected_in_render is missing it, so it will be silently skipped by CI runs that filter on that marker — defeating the intent of validating the Fabric/PrepareForReuse pose propagation fix.

def test_camera_pose_update_reflected_in_render(setup_camera, device, camera_cls):
"""Camera pose changes via FrameView should be visible in rendered depth.

Moves 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) or USD writes are correctly propagated to the
RTX renderer.
"""
sim, _unused_cam_cfg, dt = setup_camera

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_cls(cam_cfg)
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."
)
del camera
196 changes: 187 additions & 9 deletions source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ class FabricFrameView(BaseFrameView):
Warp kernels operating on ``omni:fabric:worldMatrix``. All other operations
delegate to the internal USD view.

After every Fabric write, :meth:`PrepareForReuse` is called on the
``PrimSelection`` to notify the renderer (FSD/Storm) that Fabric data
has changed.

All getters return ``wp.array``. Setters accept ``wp.array``.
"""

Expand All @@ -58,12 +62,11 @@ 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,
):
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))
Expand Down Expand Up @@ -134,6 +137,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]

Expand Down Expand Up @@ -165,8 +170,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=None):
if not self._use_fabric:
Expand All @@ -177,6 +180,8 @@ def get_world_poses(self, indices=None):
if not self._fabric_usd_sync_done:
self._sync_fabric_from_usd_once()

self._prepare_for_reuse()

indices_wp = self._resolve_indices_wp(indices)
count = indices_wp.shape[0]

Expand Down Expand Up @@ -228,6 +233,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]

Expand Down Expand Up @@ -255,8 +262,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:
Expand All @@ -267,6 +272,8 @@ def get_scales(self, indices=None):
if not self._fabric_usd_sync_done:
self._sync_fabric_from_usd_once()

self._prepare_for_reuse()

indices_wp = self._resolve_indices_wp(indices)
count = indices_wp.shape[0]

Expand Down Expand Up @@ -294,6 +301,93 @@ 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."""
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,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pv-nvidia , will this sync and kernel go away if we switch to indexedfabricarray? Is this a temporary solution?

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")
Comment on lines +329 to +342
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Incomplete rebuild after topology change may leave stale/wrong-sized buffers

_rebuild_fabric_arrays re-creates the fabricarrays and the _view_to_fabric / _fabric_to_view mappings, but the docstring explicitly documents this as the handler for "prims added/removed" — a scenario that changes self.count. The following count-dependent buffers are not rebuilt:

  • _default_view_indices (shape=(self.count,))
  • _fabric_positions_buf (shape=(self.count, 3))
  • _fabric_orientations_buf (shape=(self.count, 4))
  • _fabric_scales_buf (shape=(self.count, 3))

If self.count changes after a topology event, subsequent get_world_poses() / get_scales() calls that use these cached buffers will either produce wrong results or crash with a shape mismatch. _initialize_fabric creates all of them; _rebuild_fabric_arrays should mirror that.


# ------------------------------------------------------------------
# Context managers for raw Fabric access
# ------------------------------------------------------------------

def fabric_write(self):
"""Context manager for raw Fabric write operations.

Calls ``PrepareForReuse()`` on entry (notifying the renderer that
data is about to change) and ``update_world_xforms()`` +
``PrepareForReuse()`` on exit (propagating changes through the
hierarchy).

Example::

with view.fabric_write() as fab:
# fab.world_matrices is the fabricarray
wp.launch(my_kernel, dim=N, inputs=[fab.world_matrices, ...])
"""
return _FabricWriteContext(self)
Comment on lines +348 to +362
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Docstring incorrectly promises a second PrepareForReuse() call on exit

The docstring says: "and update_world_xforms() + PrepareForReuse() on exit", but _FabricWriteContext.__exit__ only calls wp.synchronize() and update_world_xforms() — there is no second PrepareForReuse() call. Either the exit should call self._view._prepare_for_reuse() after update_world_xforms(), or the docstring should be corrected to match the implementation.


def fabric_read(self):
"""Context manager for raw Fabric read operations.

Calls ``PrepareForReuse()`` on entry to ensure the view’s
fabricarray pointers are still valid after potential topology
changes.

Example::

with view.fabric_read() as fab:
wp.launch(my_read_kernel, dim=N, inputs=[fab.world_matrices, ...])
"""
return _FabricReadContext(self)

@property
def world_matrices(self) -> wp.fabricarray | None:
"""The raw Fabric world-matrix array (read-only property).

Returns None if Fabric is not initialized.
"""
return getattr(self, "_fabric_world_matrices", None)

@property
def view_to_fabric_mapping(self) -> wp.array | None:
"""View-index → Fabric-index mapping array."""
return getattr(self, "_view_to_fabric", None)

# ------------------------------------------------------------------
# Internal — Fabric initialization
# ------------------------------------------------------------------
Expand Down Expand Up @@ -384,11 +478,8 @@ def _sync_fabric_from_usd_once(self) -> None:
positions_usd, orientations_usd = self._usd_view.get_world_poses()
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

Expand All @@ -401,3 +492,90 @@ def _resolve_indices_wp(self, indices: wp.array | None) -> wp.array:
if indices.dtype != wp.uint32:
return wp.array(indices.numpy().astype("uint32"), dtype=wp.uint32, device=self._device)
return indices


# ======================================================================
# Context manager helpers (module-level, not inside FabricFrameView)
# ======================================================================


class _FabricWriteContext:
"""RAII context manager for Fabric write operations.

On entry: ensures Fabric is initialized, calls PrepareForReuse.
On exit (no exception): synchronizes, propagates hierarchy, marks sync done.
"""

__slots__ = ("_view",)

def __init__(self, view: FabricFrameView):
self._view = view

def __enter__(self):
if not self._view._fabric_initialized:
self._view._initialize_fabric()
if not self._view._fabric_usd_sync_done:
self._view._sync_fabric_from_usd_once()
self._view._prepare_for_reuse()
return self

def __exit__(self, exc_type, exc_val, exc_tb):
if exc_type is None:
wp.synchronize()
self._view._fabric_hierarchy.update_world_xforms()
self._view._fabric_usd_sync_done = True
return False

@property
def world_matrices(self) -> wp.fabricarray:
"""The fabricarray of omni:fabric:worldMatrix."""
return self._view._fabric_world_matrices

@property
def view_to_fabric(self) -> wp.array:
"""View-index to Fabric-index mapping."""
return self._view._view_to_fabric

@property
def count(self) -> int:
"""Number of prims in the view."""
return self._view.count


class _FabricReadContext:
"""RAII context manager for Fabric read operations.

On entry: ensures Fabric is initialized, calls PrepareForReuse.
On exit: no-op.
"""

__slots__ = ("_view",)

def __init__(self, view: FabricFrameView):
self._view = view

def __enter__(self):
if not self._view._fabric_initialized:
self._view._initialize_fabric()
if not self._view._fabric_usd_sync_done:
self._view._sync_fabric_from_usd_once()
self._view._prepare_for_reuse()
return self

def __exit__(self, exc_type, exc_val, exc_tb):
return False

@property
def world_matrices(self) -> wp.fabricarray:
"""The fabricarray of omni:fabric:worldMatrix."""
return self._view._fabric_world_matrices

@property
def view_to_fabric(self) -> wp.array:
"""View-index to Fabric-index mapping."""
return self._view._view_to_fabric

@property
def count(self) -> int:
"""Number of prims in the view."""
return self._view.count
Loading