feat: Fabric-accelerated get/set_local_poses via indexedfabricarray

[pv-nvidia]

feat: Fabric-accelerated get/set_local_poses via indexedfabricarray

Adds GPU-accelerated get_local_poses / set_local_poses (and the explicit
get/set_local_scales, get/set_world_scales) to FabricFrameView, using
wp.indexedfabricarray over Fabric's omni:fabric:worldMatrix and
omni:fabric:localMatrix. Both directions stay in sync via lazy Warp-kernel
recomputes (no round-trip through USD).

Builds on Piotr's prototype at
bareya/pbarejko/camera-update.

Benchmark (1024 prims, 50 iterations, A6000)

Operation	USD (ms)	Fabric (ms)	Speedup
Get World Poses	13.56	0.067	201×
Set World Poses	29.97	0.123	244×
Interleaved Set→Get	43.78	0.159	276×
Get Local Poses	6.15	0.064	96×
Set Local Poses	8.61	0.053	163×

Why two layers of indirection: wp.indexedfabricarray

Naive wp.fabricarray(selection, attr) already wraps a Fabric attribute as a
Warp array, but it indexes by selection order, which is opaque to user
code and unstable across Fabric reallocation. wp.indexedfabricarray(fa, indices) overlays a view-side index array on top, so kernels can write
fabric_matrices[view_index] and have it land at the right Fabric slot
regardless of internal layout. Topology changes are detected per-selection
via PrepareForReuse(); the indices and indexed arrays are rebuilt lazily
on the next access.

Why RO/RW PrimSelections are required

The separate RO/RW PrimSelections are not an implementation detail; they are the synchronization contract with Fabric's hierarchy updater. Fabric/Kit runs IFabricHierarchy.update_world_xforms() on the render path, after Isaac Lab has written matrices but before Hydra/RTX consumes omni:fabric:worldMatrix. That updater decides which direction is authoritative from the access flags on the most recent selection:

• worldMatrix=RW, localMatrix=RO means the user authored world transforms; Fabric must not rebuild world from stale local data.
• worldMatrix=RO, localMatrix=RW means the user authored local transforms; Fabric may rebuild world from the new local data.
• worldMatrix=RW, localMatrix=RW is ambiguous and unsafe for this API: Fabric can treat local as canonical and overwrite a just-authored world matrix before the renderer reads it.

This is why the branch uses three persistent selections instead of one combined read/write selection. Dirty tracking keeps Isaac Lab's local/world getters consistent on demand; the asymmetric RO/RW selections keep the renderer-facing Fabric world matrices correct across the render tick.

The design that matters most: three selections with asymmetric RO/RW access

This is the load-bearing correctness choice and the reason the
implementation kept iterating. Three persistent PrimSelection handles
cover the same prims with different per-attribute access flags:

_trans_sel_ro  :  worldMatrix=RO, localMatrix=RO   reads + sync helpers
_world_sel_rw  :  worldMatrix=RW, localMatrix=RO   set_world_poses / _scales
_local_sel_rw  :  worldMatrix=RO, localMatrix=RW   set_local_poses / _scales

Kit runs IFabricHierarchy.update_world_xforms() as part of every render
tick, and that update respects the access flags: if localMatrix is
marked RO on the most recent write, Fabric leaves the user's worldMatrix
write alone; if localMatrix is marked RW, Fabric treats it as the
canonical source and recomputes world from local.

A single combined worldMatrix=RW, localMatrix=RW selection (one earlier
revision of this branch) removes that protection. Fabric sees both
attributes as user-authored, falls back to the hierarchy's canonical
direction (local → world), and recomputes worldMatrix from the still-stale
localMatrix. The user's worldMatrix write is silently overwritten before
the renderer reads it. That was the failure mode behind the
test_output_equal_to_usdcamera regression and any other Camera + RTX path
that drives world poses through Fabric (the RTX delegate consumes
omni:fabric:worldMatrix directly via Hydra's fabric transform reader).

The asymmetric RO/RW layout is the right primitive for that protection,
and it works with the lazy consistency model below rather than fighting it.

Lazy world ↔ local consistency

Setters mark the opposite direction stale on a small enum
(_DirtyFlag.{NONE,WORLD,LOCAL}); the recompute happens only when the
opposite getter is called. No eager Warp kernels run on the hot setter
path. Concretely:

• set_world_poses / set_world_scales write worldMatrix and set
  _dirty = LOCAL. The next get_local_* recomputes localMatrix = inv(parent_world) * world via a Warp kernel.
• set_local_poses / set_local_scales write localMatrix and set
  _dirty = WORLD. The next get_world_* recomputes `world = parent_world
  • local`.

If the user interleaves the two setters on the same view in a single
frame, the second one flushes the stale direction before writing (correct
but pays an extra kernel launch). A one-time warning per view instance
flags this so users notice the perf hazard. Sticking to one setter
exclusively keeps the path O(1) kernel launches.

Renderer correctness across the lazy gap is provided by the RO/RW
protection above, not by Isaac Lab doing extra work: between the user's
write and the renderer's read, the only thing that runs is Kit's
update_world_xforms tick, and that tick is well-behaved because the
access flags tell it what to leave alone.

Getter note: indexed Fabric getters intentionally return freshly allocated
ProxyArray buffers without an unconditional wp.synchronize(). Consumers
that need immediate host-visible data should synchronize or copy explicitly;
GPU consumers keep normal Warp stream ordering.

Earlier iterations on this PR (resolved)

For reviewers who saw the older shape of this branch:

• An interim revision collapsed the three selections into one combined
  ReadWrite(world+local) selection. That broke the renderer-clobber
  protection described above; the latest commit restores the three-selection
  shape.
• The same interim revision added an eager _flush_deferred_local_writes()
  to set_local_poses and a symmetric eager _sync_local_from_world_if_dirty()
  to set_world_poses as workarounds for the lost protection. These are
  removed in the latest commit -- they're unnecessary once the RO/RW layout
  is back, and they made every setter pay an extra kernel.
• A change_block / changeBlock context manager was added on
  BaseFrameView and FabricFrameView to batch the now-removed eager
  flushes. With nothing to defer, the API has been removed too.

Net change vs. the previous push on this branch: -198 lines. Net change
vs. develop is dominated by the new set/get_local_* API and the new
indexed Warp kernels in isaaclab.utils.warp.fabric.

Tests

New / kept:

• test_set_local_poses_roundtrip, test_set_local_then_get_world_with_rotated_parent,
  test_set_world_then_get_local_with_rotated_parent — local⇄world
  correctness, including a 90° Z-rotated parent that catches matrix-storage
  convention bugs.
• test_initial_seed_with_scaled_parent — the initial USD-seed path
  composes scales correctly across non-unit-scale parents.
• test_multi_view_per_view_dirty_isolation — two views on the same stage
  don't clear each other's dirty flag (regression for an earlier
  class-level-flag bug).
• test_prepare_for_reuse_detects_topology_change — polls all three
  selections.
• test_fabric_rebuild_after_topology_change — rebuilds each selection's
  indexed array after a simulated topology change.
• test_interleaved_set_emits_warning — the one-time interleave warning.

Removed in the latest commit (obsolete contracts):

• test_set_local_*_updates_renderer_facing_fabric_world_matrix — asserted
  the eager-update contract that the lazy design deliberately does not
  hold; renderer correctness is now provided by the RO/RW protection.
• The four test_change_block_* tests — the API is gone.

Verified:

• pytest test_ray_caster_camera.py::test_output_equal_to_usdcamera — passes
  (was the original failure that motivated the redesign).
• pytest test_ray_caster_camera.py::test_output_equal_to_usd_camera_when_intrinsics_set —
  4/4 pass.
• pytest test_views_xform_prim_fabric.py — 71 passed, 3 skipped (cuda:1).
• ./isaaclab.sh -f — clean.

Files changed

• source/isaaclab/isaaclab/sim/views/base_frame_view.py
• source/isaaclab/isaaclab/utils/warp/fabric.py (new indexed kernels)
• source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py
• source/isaaclab_physx/test/sim/test_views_xform_prim_fabric.py
• source/isaaclab/test/sim/frame_view_contract_utils.py (extended contracts)
• source/isaaclab/changelog.d/fabric-local-poses.rst
• source/isaaclab_physx/changelog.d/fabric-local-poses.rst

[isaaclab-review-bot]

🔍 Code Review Update

Review of new commits: PR was rebased and expanded since last review.
Previously reviewed: b15d6235 | Now reviewing: 80838c00

Summary

The PR has been expanded from 5 to 6 commits, adding Fabric-accelerated get/set_local_poses:

1. Service locator infrastructure on SimulationContext
2. Service locator tests + changelog
3. Indexed Fabric transform kernels in isaaclab.utils.warp.fabric
4. FabricStageCache as a shared hierarchy handle
5. (NEW) Merge commit to consolidate branches
6. (NEW) FabricFrameView rewrite with get/set_local_poses + dirty tracking

✅ New Additions Since Last Review

• Fabric-accelerated local poses: set_local_poses / get_local_poses now use wp.indexedfabricarray to read/write omni:fabric:localMatrix directly on the GPU
• Bidirectional world↔local sync:
  • set_world_poses → recomputes localMatrix via _sync_local_from_world()
  • set_local_poses → marks _world_dirty, world recomputed on next get_world_poses
• Per-view dirty tracking: _world_dirty flag is instance-scoped, so concurrent views on the same stage don't clear each other's flag
• Parent matrix handling: _build_parent_indexed_array() + _compute_parent_fabric_indices() for parent world matrix lookups
• Topology-adaptive: PrepareForReuse() calls + _rebuild_trans_ro_arrays() for automatic recovery
• Comprehensive tests: 13 new integration tests covering local/world consistency, rotated/scaled parents, multi-view isolation

🔧 Remaining Observations

[Minor] Index array dtype mismatch still present

The kernels declare indices: ArrayUInt32, but _compute_fabric_indices() returns dtype=wp.int32:

# fabric_frame_view.py
return wp.array(indices, dtype=wp.int32, device=self._device)

Warp will silently cast, so this works in practice. Suggestion: switch to dtype=wp.uint32 for consistency with the kernel signatures. Not blocking.

[Minor] Undefined buffer references in get_local_poses

get_local_poses references self._fabric_local_translations_buf and self._fabric_local_orientations_buf:

if use_cached:
    translations_wp = self._fabric_local_translations_buf
    orientations_wp = self._fabric_local_orientations_buf

These don't appear to be initialized in _initialize_fabric(). Verify these buffers are created alongside the existing world-pose buffers.

📋 Architecture Notes

The world↔local propagation design is clean:

• Write world → update local: _sync_local_from_world() runs update_indexed_local_matrix_from_world kernel immediately after world writes
• Write local → lazy world update: _world_dirty flag defers update_indexed_world_matrix_from_local until the next world read

This asymmetry makes sense: world writes are typically followed by physics steps (which don't need locals), while local writes are often followed by world reads for rendering.

📋 Verdict

LGTM — the new local-pose acceleration is a significant feature addition. The bidirectional sync logic is well-designed, and the test coverage is comprehensive (33 test functions across all new modules). The minor dtype observation from the previous review remains, plus one potential undefined buffer issue to double-check.

---

Automated review by isaaclab-review-bot • Reviewed at 80838c00

[isaaclab-review-bot]

🔍 Code Review Update

Review of new commits: PR was rebased and expanded since last review.
Previously reviewed: eb5582ec | Now reviewing: 9ff3155

Summary

This PR delivers a well-architected feature: GPU-accelerated local-pose operations for FabricFrameView. The implementation is comprehensive and addresses the core limitation where local poses previously fell back to USD round-trips.

✅ Strengths

1. Clean Architecture

• FabricStageCache provides shared hierarchy handles via the service locator pattern, avoiding per-view duplication
• Three persistent selections (trans_sel_ro, world_sel_rw, local_sel_rw) cleanly separate read vs. write access patterns
• Factory dispatch in FrameView._get_backend() correctly routes to UsdFrameView when Fabric is unavailable

2. Robust World↔Local Consistency

• Bidirectional dirty tracking: set_local_poses marks _world_dirty, deferred until next world read
• _sync_local_from_world() / _sync_world_from_local_if_dirty() keep matrices consistent
• Per-view dirty flags prevent concurrent views from clearing each other's pending syncs

3. Topology-Adaptive Design

• PrepareForReuse() + lazy array rebuild in _get_*_array() handles Fabric memory layout changes
• _rebuild_trans_ro_arrays() consolidates index and indexed-array refresh

4. Excellent Test Coverage

• 13+ new integration tests covering rotated parents, scaled parents, multi-view isolation
• test_set_local_then_get_world_with_rotated_parent validates transpose-convention correctness
• test_multi_view_per_view_dirty_isolation catches per-stage vs. per-view flag bugs

🔧 Minor Observations

[Minor] Index array dtype mismatch

_compute_fabric_indices() returns dtype=wp.int32, but kernels declare indices: ArrayUInt32:

return wp.array(indices, dtype=wp.int32, device=self._device)

Warp silently casts, so this works. Suggestion: use dtype=wp.uint32 for consistency with kernel signatures. Not blocking.

[Nit] Docstring transpose-convention note

The docstrings for update_indexed_local_matrix_from_world and update_indexed_world_matrix_from_local explain the transpose identity well. Consider adding a brief note that this relies on Fabric's row-major storage convention for future maintainers.

[Style] Empty sentinel shape

_fabric_empty_2d_array_sentinel uses shape (0, 0):

self._fabric_empty_2d_array_sentinel = wp.zeros((0, 0), dtype=wp.float32, device=self._device)

This is fine since the kernels gate on shape[0] > 0, but (0, 3) or (0, 4) might be slightly more self-documenting for unused position/quaternion slots.

📋 Architecture Notes

The asymmetric sync strategy is well-reasoned:

• Write world → sync local immediately: _sync_local_from_world() runs right after world writes because downstream code (e.g., rendering) typically reads locals soon after
• Write local → lazy world sync: _world_dirty flag defers the world = parent * local kernel until the next world read, avoiding unnecessary computation when multiple local writes occur before a world read

📋 CI Status

CI checks are currently pending. The pre-commit and changelog checks have passed.

📋 Verdict

LGTM — This is a significant feature addition that completes the Fabric acceleration story for FabricFrameView. The bidirectional sync logic is sound, test coverage is thorough, and the codebase is well-documented. The minor dtype observation is non-blocking.

---

Automated review by isaaclab-review-bot • Reviewed at 9ff31550

---

Update (d13ed99→7f1a012c): Reviewed large incremental batch (50+ files changed). This batch contains no changes to the core FabricFrameView implementation — the Fabric local-poses feature code is identical to the previously reviewed state.

Key changes in this batch (all outside the PR's core feature scope):

1. isaaclab_ppisp made fully optional: All packages (isaaclab_physx, isaaclab_newton, isaaclab_ov, isaaclab_ovphysx) removed hard isaaclab_ppisp from dependencies. Lazy try/except ModuleNotFoundError with _raise_missing_ppisp_error() helper provides actionable install hints only when isp_cfg is actually set. Clean pattern, consistently applied.

2. Cloner refactoring to ReplicateContext classes: physx_replicate, newton_physics_replicate, and ovphysx_replicate legacy functions now delegate to new PhysxReplicateContext, NewtonReplicateContext, and OvPhysxReplicateContext classes. Asset constructors call queue_*_replication(cfg) to register on REPLICATION_QUEUE. Well-structured — separates queueing from execution, enables batched replication.

3. MJWarp ls_parallel deprecated: Config field emits DeprecationWarning via __post_init__(), _build_solver() ignores it. Removed from all test configurations.

4. Newton manager lifecycle fix: close() now calls super().close() before cls.clear(), ensuring sensor invalidation callbacks fire while Newton state is still alive, preventing stale registrations leaking.

5. OvPhysX/OVRTX runtime guards: import_ovphysx() helper with actionable install messages, ovphysx==0.4.13 pin.

6. PhysxManager rigid body view: Now detects name collisions between rigid and non-rigid prims (e.g., JointWrenchSensor frame prim vs. rigid link with same name) and uses exact paths for ambiguous names.

7. Version bumps and changelog consolidation: All changelog fragment files moved into proper versioned CHANGELOG.rst entries.

Previous inline comments status:

• ⏳ P1 (Missing wp.synchronize() in indexed getter paths): Still present — no changes to fabric_frame_view.py in this batch. Remains non-blocking per previous assessment.
• ✅ P2 (Redundant kernel launch): Fixed in earlier commits (confirmed).

No new issues found in the incremental changes. All additions are well-implemented and follow established patterns. LGTM.

---

Automated incremental review by isaaclab-review-bot • Reviewed at 7f1a012c

---

Update (7f1a012→999cf3c6): Major architectural refactor reviewed.

Changes in this batch

The lazy dirty-flag mechanism (_DirtyFlag enum, _dirty field, _sync_*_if_dirty helpers, interleaved-write warning) has been removed entirely and replaced with a context-managed writer scope API (FrameViewSpaceWriterBase). This is a substantial and well-executed improvement:

1. New xform_space_writer() context manager: Opens a scoped write session ("world" or "local"). On __exit__, derives the opposite-space matrices once via a single Warp kernel and calls wp.synchronize() once. Clean, predictable, no more subtle interleaving bugs.

2. Single-active-writer invariant: Per-view lock prevents concurrent scopes — RuntimeError on double-entry. Well-tested.

3. Hierarchy listener pause/restore: _FabricWriterMixin._enter_impl() pauses track_local_xform_changes / track_world_xform_changes during writes, restoring prior state on exit. Correctly handles pre-paused listeners (no accidental re-enable).

4. Deprecated shims preserved: set_world_poses() / set_local_poses() remain as thin wrappers that open one-shot writer scopes internally + emit DeprecationWarning. Clean migration path.

5. set_world_scales / set_local_scales removed (breaking): Since these were introduced in this release cycle without stable downstream users, removal without deprecation is appropriate. Changelog documents it.

6. Test coverage: Comprehensive new tests for the writer contract — single-active invariant, derive-on-exit counting, empty-scope no-op, getter-raises-inside-scope, hierarchy tracking restore.

Previous comments status

• ✅ P2 (Redundant kernel launch): Fixed — the _sync_fabric_from_usd_initial() path now only composes localMatrix then calls _recompute_world_from_local_all(). The redundant world compose is gone.
• ⏳ P1 (Missing wp.synchronize() in indexed getter paths): Still present in the non-cached decompose paths. Remains non-blocking per previous assessment (callers on the same CUDA stream see correct ordering; only cross-stream or immediate .numpy() usage is affected).
• ✅ Minor observations (dtype, docstring, sentinel shape): These were non-blocking nits and remain unchanged. The sentinel shape (0, 0) is now less relevant since it is only used internally by the writer compose kernel.

New observations

No new issues found. The architectural shift from lazy dirty-flag tracking to eager derive-on-scope-exit is a clear improvement in correctness and readability. The _FabricWriterMixin pattern cleanly separates Fabric lifecycle management from the write logic. LGTM.

---

Automated incremental review by isaaclab-review-bot • Reviewed at 999cf3c6

[pv-nvidia]

Superseded by the consolidated PR #5728 (pv/fabric-full-stack).