Address Greptile feedback on Fabric write path

- Replace assert in _rebuild_fabric_arrays with RuntimeError so the
  prim-count invariant survives python -O.
- Factor a single _compose_fabric_transform helper used by
  set_world_poses, set_scales, and the initial USD->Fabric sync. The
  sync now does one combined kernel launch with one PrepareForReuse,
  removing the prior double-call concern.
- Refresh the changelog fragments accordingly.

Author: pv-nvidia

source/isaaclab/changelog.d/fix-fabric-prepare-for-reuse.rst

@@ -3,6 +3,9 @@ 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.
+  ``sync_usd_on_fabric_write`` keyword (see the corresponding
+  ``isaaclab_physx`` entry).  No user-facing migration is required —
+  callers reading camera poses through the sensor's data buffers are
+  unaffected.  Direct readers of the camera prim's USD attributes during a
+  simulation step should switch to the sensor getters, since USD attributes
+  are no longer updated in lock-step with Fabric writes.

source/isaaclab_physx/changelog.d/feat-frame-view-enable-mgpu.rst

@@ -1,9 +1,9 @@
-Changed
-^^^^^^^
+Fixed
+^^^^^
 
-* Removed the ``cuda:0``-only restriction in
-  :class:`~isaaclab_physx.sim.views.FabricFrameView`.  USDRT ``SelectPrims``
-  now accepts any CUDA device index, so Fabric acceleration runs on the
-  simulation device (e.g., ``cuda:1``) instead of silently falling back to
-  the slower USD path.  This unblocks distributed training where each
-  process is pinned to a specific GPU.
+* Fixed :class:`~isaaclab_physx.sim.views.FabricFrameView` falling back to
+  the slow USD path on every CUDA device other than ``cuda:0``.  USDRT
+  ``SelectPrims`` now accepts any CUDA device index, so Fabric acceleration
+  runs on the simulation device the view was constructed with (e.g.
+  ``cuda:1``).  This unblocks distributed training where each rank is
+  pinned to a non-primary GPU.

source/isaaclab_physx/changelog.d/fix-fabric-prepare-for-reuse.rst

@@ -3,10 +3,27 @@ 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.
+  (:meth:`~isaaclab_physx.sim.views.FabricFrameView.set_world_poses`,
+  :meth:`~isaaclab_physx.sim.views.FabricFrameView.set_scales`) now notify the
+  renderer via ``PrepareForReuse`` on the underlying ``PrimSelection`` and
+  detect Fabric topology changes, instead of writing back to USD.  This is
+  ~200x faster and removes the stale USD shadow state the old path produced.
+  Migration: drop the ``sync_usd_on_fabric_write=True`` argument; if you
+  previously relied on USD reflecting Fabric writes, read poses through the
+  view's getters or refresh USD explicitly at your sync point.
+
+* Combined the initial USD→Fabric sync into a single Fabric write so
+  ``PrepareForReuse`` is invoked exactly once per logical update (positions,
+  orientations, and scales are composed in one kernel launch).  This avoids
+  the possibility of a second non-idempotent ``PrepareForReuse`` call masking
+  a topology-change signal that should have triggered a fabricarray rebuild.
+
+Fixed
+^^^^^
+
+* Fixed the topology-change invariant guard in
+  :class:`~isaaclab_physx.sim.views.FabricFrameView` not surviving
+  ``python -O``.  The check now raises :class:`RuntimeError` instead of using
+  ``assert`` so the prim-count mismatch between view and Fabric is reported
+  at every optimisation level rather than silently producing wrong poses or
+  out-of-bounds kernel indices.

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

@@ -134,43 +134,7 @@ def set_world_poses(self, positions=None, orientations=None, indices=None):
         if not self._use_fabric:
             self._usd_view.set_world_poses(positions, orientations, indices)
             return
-
-        if not self._fabric_initialized:
-            self._initialize_fabric()
-
-        self._prepare_for_reuse()
-
-        indices_wp = self._resolve_indices_wp(indices)
-        count = indices_wp.shape[0]
-
-        dummy = wp.zeros((0, 3), dtype=wp.float32, device=self._device)
-        positions_wp = _to_float32_2d(positions) if positions is not None else dummy
-        orientations_wp = (
-            _to_float32_2d(orientations)
-            if orientations is not None
-            else wp.zeros((0, 4), dtype=wp.float32, device=self._device)
-        )
-
-        wp.launch(
-            kernel=fabric_utils.compose_fabric_transformation_matrix_from_warp_arrays,
-            dim=count,
-            inputs=[
-                self._fabric_world_matrices,
-                positions_wp,
-                orientations_wp,
-                dummy,
-                False,
-                False,
-                False,
-                indices_wp,
-                self._view_to_fabric,
-            ],
-            device=self._fabric_device,
-        )
-        wp.synchronize()
-
-        self._fabric_hierarchy.update_world_xforms()
-        self._fabric_usd_sync_done = True
+        self._compose_fabric_transform(positions=positions, orientations=orientations, indices=indices)
 
     def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]:
         if not self._use_fabric:
@@ -229,7 +193,15 @@ def set_scales(self, scales, indices=None):
         if not self._use_fabric:
             self._usd_view.set_scales(scales, indices)
             return
+        self._compose_fabric_transform(scales=scales, indices=indices)
+
+    def _compose_fabric_transform(self, positions=None, orientations=None, scales=None, indices=None):
+        """Write the given subset of (position, orientation, scale) into Fabric in one kernel launch.
 
+        Components left as ``None`` are skipped via empty input arrays — the kernel reads them
+        from the existing Fabric matrix. Always invokes :meth:`_prepare_for_reuse` exactly once
+        per write, even when multiple components are updated together.
+        """
         if not self._fabric_initialized:
             self._initialize_fabric()
 
@@ -238,17 +210,19 @@ def set_scales(self, scales, indices=None):
         indices_wp = self._resolve_indices_wp(indices)
         count = indices_wp.shape[0]
 
-        dummy3 = wp.zeros((0, 3), dtype=wp.float32, device=self._device)
-        dummy4 = wp.zeros((0, 4), dtype=wp.float32, device=self._device)
-        scales_wp = _to_float32_2d(scales)
+        empty3 = wp.zeros((0, 3), dtype=wp.float32, device=self._device)
+        empty4 = wp.zeros((0, 4), dtype=wp.float32, device=self._device)
+        positions_wp = _to_float32_2d(positions) if positions is not None else empty3
+        orientations_wp = _to_float32_2d(orientations) if orientations is not None else empty4
+        scales_wp = _to_float32_2d(scales) if scales is not None else empty3
 
         wp.launch(
             kernel=fabric_utils.compose_fabric_transformation_matrix_from_warp_arrays,
             dim=count,
             inputs=[
                 self._fabric_world_matrices,
-                dummy3,
-                dummy4,
+                positions_wp,
+                orientations_wp,
                 scales_wp,
                 False,
                 False,
@@ -332,10 +306,11 @@ def _rebuild_fabric_arrays(self) -> None:
         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."
-        )
+        if self.count != self._default_view_indices.shape[0]:
+            raise RuntimeError(
+                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)
 
@@ -424,19 +399,20 @@ def _initialize_fabric(self) -> None:
     def _sync_fabric_from_usd_once(self) -> None:
         """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.
+        Combines position/orientation/scale into a single Fabric write so
+        :meth:`_prepare_for_reuse` (and its underlying ``PrepareForReuse``) is invoked
+        exactly once across the full sync.
         """
         if not self._fabric_initialized:
             self._initialize_fabric()
 
         positions_usd_ta, orientations_usd_ta = self._usd_view.get_world_poses()
-        positions_usd = positions_usd_ta.warp
-        orientations_usd = orientations_usd_ta.warp
         scales_usd = self._usd_view.get_scales()
-
-        self.set_world_poses(positions_usd, orientations_usd)
-        self.set_scales(scales_usd)
+        self._compose_fabric_transform(
+            positions=positions_usd_ta.warp,
+            orientations=orientations_usd_ta.warp,
+            scales=scales_usd,
+        )
 
     def _resolve_indices_wp(self, indices: wp.array | None) -> wp.array:
         """Resolve view indices as a Warp uint32 array."""