Skip to content

Commit 91069ae

Browse files
committed
Make world-dirty tracking per-view in FabricFrameView
The previous _static_dirty_stages set was keyed by stage_id, but _sync_world_from_local_if_dirty only recomputes worldMatrix for the *calling view's* children before clearing the flag. With two views on the same stage, view B's world-read would clear the flag set by view A's set_local_poses, leaving A's worlds silently stale. Replace the class-level set with a per-instance bool. Each view now tracks its own dirty state, which matches the actual scope of the recompute kernel and removes a mutable ClassVar. Also: - Raise a clearer error when _compute_parent_fabric_indices is asked to look up the parent of a root-level prim (rsplit produces ""), instead of bubbling up the generic "not found in selection" message. - Document on the remaining _static_hierarchy_cache that it is not thread-safe by design (Isaac Lab's loop is single-threaded; adding a lock would negate the per-stage caching benefit). - Update the module docstring to reflect the per-view dirty model and drop the stale reference to IFabricHierarchy.update_world_xforms.
1 parent c958fdf commit 91069ae

1 file changed

Lines changed: 23 additions & 11 deletions

File tree

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

Lines changed: 23 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
* Read/write happens via ``wp.indexedfabricarray``, so the view-to-fabric mapping is
1818
baked into the array itself and the kernels just dereference ``ifa[view_index]``.
1919
* World ↔ local consistency is maintained:
20-
- After ``set_local_poses``: stage is marked dirty; the next world read calls
21-
``IFabricHierarchy.update_world_xforms()`` to propagate local → world.
20+
- After ``set_local_poses``: the view is marked dirty; the next world read
21+
fires a Warp kernel that recomputes ``child_world = parent_world *
22+
child_local`` for this view's children. Tracking is per-view so that
23+
multiple views on the same stage don't clear each other's dirty flag.
2224
- After ``set_world_poses``: a Warp kernel recomputes child localMatrix from
2325
parent worldMatrix on the fly using a parent indexed fabric array, so the
2426
next ``get_local_poses`` returns consistent values.
@@ -92,12 +94,12 @@ class FabricFrameView(BaseFrameView):
9294
_LOCAL_MATRIX_NAME = "omni:fabric:localMatrix"
9395

9496
# Stage-level shared state. Multiple FabricFrameView instances on the same stage
95-
# share one IFabricHierarchy handle; the dirty-stages set tracks which stages
96-
# need ``update_world_xforms()`` before a world read. ``ClassVar`` plus the
97-
# ``_static_`` prefix make it explicit (and enforceable by type checkers) that
98-
# these are class-level — never shadow them with ``self.<name> = ...``.
97+
# share one IFabricHierarchy handle. ``ClassVar`` plus the ``_static_`` prefix
98+
# make it explicit (and enforceable by type checkers) that this is class-level —
99+
# never shadow it with ``self.<name> = ...``. The cache is not protected by a
100+
# lock; Isaac Lab's simulation loop is single-threaded, and adding locking would
101+
# negate the per-stage hit-cache cost it exists to avoid.
99102
_static_hierarchy_cache: ClassVar[dict[int, object]] = {}
100-
_static_dirty_stages: ClassVar[set[int]] = set()
101103

102104
def __init__(
103105
self,
@@ -139,6 +141,10 @@ def __init__(
139141
self._stage_id: int | None = None
140142
self._stage = None
141143
self._fabric_hierarchy = None
144+
# Set by ``set_local_poses``; cleared by ``_sync_world_from_local_if_dirty``.
145+
# Per-view (not per-stage) so concurrent views on the same stage don't clear
146+
# each other's flag.
147+
self._world_dirty: bool = False
142148

143149
# Selections.
144150
self._trans_sel_ro = None
@@ -313,8 +319,8 @@ def set_local_poses(self, translations=None, orientations=None, indices=None):
313319
)
314320
wp.synchronize()
315321

316-
# Mark the stage dirty so the next world read calls update_world_xforms().
317-
FabricFrameView._static_dirty_stages.add(self._stage_id)
322+
# Mark this view's worlds stale so the next world read recomputes them.
323+
self._world_dirty = True
318324

319325
def get_local_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]:
320326
if not self._use_fabric:
@@ -443,7 +449,7 @@ def _sync_world_from_local_if_dirty(self) -> None:
443449
does ``child_world = parent_world * child_local`` per child, leaving the
444450
Fabric-side localMatrix untouched.
445451
"""
446-
if self._stage_id is None or self._stage_id not in FabricFrameView._static_dirty_stages:
452+
if not self._world_dirty:
447453
return
448454
# Make sure the parent indexed array is up-to-date with the current trans selection.
449455
if self._parent_world_ifa_ro is None:
@@ -460,7 +466,7 @@ def _sync_world_from_local_if_dirty(self) -> None:
460466
device=self._device,
461467
)
462468
wp.synchronize()
463-
FabricFrameView._static_dirty_stages.discard(self._stage_id)
469+
self._world_dirty = False
464470

465471
def _sync_local_from_world(self, indices_wp: wp.array) -> None:
466472
"""Recompute child ``localMatrix`` from (parent worldMatrix, child worldMatrix).
@@ -558,6 +564,12 @@ def _compute_parent_fabric_indices(self, selection) -> wp.array:
558564
indices: list[int] = []
559565
for prim_path in self.prim_paths:
560566
parent_path = prim_path.rsplit("/", 1)[0]
567+
if parent_path == "":
568+
raise RuntimeError(
569+
f"Child prim '{prim_path}' is at stage root and has no parent prim. "
570+
"FabricFrameView requires every prim to have a non-pseudoroot parent "
571+
"with Fabric world+local matrices."
572+
)
561573
fabric_idx = path_to_fabric_idx.get(parent_path)
562574
if fabric_idx is None:
563575
raise RuntimeError(

0 commit comments

Comments
 (0)