feat: Fabric-accelerated get/set_local_poses via indexedfabricarray

[pv-nvidia]

Motivation

Resolves the core feature request: Fabric-accelerated local-pose operations for FabricFrameView. Previously, only world poses were GPU-accelerated via Fabric -- local poses fell back to USD round-trips, defeating the purpose of Fabric for parent-child hierarchies.

Also introduces an explicit local/world scales API across all BaseFrameView implementations, fixing the long-standing ambiguity where set_scales meant different things on USD (local) vs Fabric (world).

Changes

New operations on FabricFrameView:

Method	Description
get_local_poses()	Read local pos/quat from omni:fabric:localMatrix via indexed kernels
set_local_poses()	Write local pos/quat -> mark world matrix dirty
get_world_scales()	Read scale from Fabric world matrix (with lazy local->world sync)
set_world_scales()	Write scale to world matrix -> mark local matrix dirty
get_local_scales()	Read scale from Fabric local matrix (with lazy world->local sync)
set_local_scales()	Write scale to local matrix -> mark world matrix dirty

Scales API redesign (all backends):

Backend	get/set_local_scales	get/set_world_scales	get/set_scales (deprecated)
UsdFrameView	xformOp:scale	world-matrix decomposition	-> local
FabricFrameView	localMatrix decompose	worldMatrix decompose	-> world
OvPhysxFrameView	-> UsdFrameView	-> UsdFrameView	-> local
NewtonSiteFrameView	shape_scale	shape_scale	-> world

The deprecated get_scales()/set_scales() still work but emit DeprecationWarning and delegate to the backend-appropriate default (preserving prior behavior).

Lazy dirty propagation:

• set_local_poses() / set_local_scales() marks world matrix dirty -> next get_world_poses() / get_world_scales() recomputes child_world = parent_world * child_local
• set_world_poses() / set_world_scales() marks local matrix dirty -> next get_local_poses() / get_local_scales() recomputes child_local = inv(parent_world) * child_world
• No eager kernel launches on the setter path -- only a flag flip. Propagation fires lazily on the first stale read.
• Per-view tracking: multiple views on the same stage do not interfere with each other's dirty state

Interleave detection:

If a user interleaves set_world_poses and set_local_poses on the same view within a frame, the second setter flushes the first's stale direction before writing. This is correct but costs an extra kernel -- a one-time warning is logged.

Topology recovery:

Each PrimSelection is checked via PrepareForReuse() on every accessor call. If topology changed (Fabric reallocated), the per-selection fabric indices and indexed arrays are rebuilt automatically.

Infrastructure used:

• Indexed fabric kernels for GPU-side matrix operations
• wp.indexedfabricarray for zero-copy access to Fabric data
• SettingsManager _use_fabric check with full USD fallback when Fabric is disabled

Performance

bench_set_world_poses.py (1024 prims, 200 iters, 2x L40)

Operation	USD (ms/call)	Fabric (ms/call)	Speedup
set_world_poses	29.26	0.070	417x
get_world_poses	12.24	0.040	307x
interleaved set->get	41.96	0.114	369x

bench_scales.py (1024 prims, 200 iters, 2x L40)

Operation	USD (ms/call)	Fabric (ms/call)	Speedup
get_local_scales	4.80	0.040	121x
get_world_scales	19.34	0.040	483x
set_local_scales	5.64	0.053	106x
set_world_scales	29.12	0.052	565x
interleaved set_local->get_world	25.02	0.141	178x

benchmark_xform_prim_view.py (100 prims, 50 iters, 2x L40)

Operation	USD (ms)	Fabric (ms)	Speedup
get_world_poses	1.24	0.041	30x
set_world_poses	2.88	0.069	42x
get_local_poses	0.87	0.043	21x
set_local_poses	1.21	0.068	18x
get both (world+local)	2.13	0.081	26x
interleaved world set->get	4.19	0.111	38x

Tests

• Local-pose round-trips (set->get identity)
• World<->local propagation correctness with rotated parents
• Per-view dirty isolation (two views, same stage)
• Topology-change recovery (simulated rebuild)
• Scaled parent/child combinations
• Local/world scales round-trips
• Multi-GPU tests (cuda:1, gated by ISAACLAB_TEST_MULTI_GPU=1)
• Interleaved set_world/set_local partial-index correctness
• Interleave warning emission + one-time-only assertion
• Deprecated get_scales/set_scales still functional (emits warning)

[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

Reviewing new commits: PR rebased and updated 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 (0dbfcfc): Reviewed incremental changes. New commits add the isaaclab_ppisp package (PPISP pipeline + Warp kernels), HDR output (RGB_HDR) to both Isaac RTX and OVRTX renderers, PPISP integration in renderer backends, tiled camera views for Kit/Newton visualizers, test improvements (force_interval_events, reduced step counts), and continued FabricFrameView refinements (multi-GPU support, SimulationContext requirement). No new issues found — code is well-structured, tests are comprehensive, and the new package follows established project patterns. Previous minor observations (dtype, sentinel shape, docstring nit) remain non-blocking and unaddressed. LGTM.

---

Update (cff9c36): Significant refactor of FabricFrameView replacing the custom view_index_attr + fabricarray pattern with wp.indexedfabricarray — cleaner, more idiomatic use of the Warp Fabric API. Key changes: per-selection lazy rebuild via PrepareForReuse() in each _get_*_array() accessor, new _sync_fabric_from_usd_initial() that properly seeds both parent and child matrices (including scale), and removal of the multi-GPU device string mentions from docstrings. Tests updated to remove the xfail on test_set_world_updates_local (feature now works!) and added excellent new coverage for rotated parents, scaled parents, and multi-view dirty-flag isolation. No new issues found. Previous minor observations (int32 vs uint32 dtype, sentinel shape) still apply but remain non-blocking. LGTM.

---

Update (0b3280d): Commit history rebased/squashed into a single feature commit. Code is functionally identical to the previously-reviewed cff9c36f — no new issues introduced. Previous minor observations (int32 vs uint32 dtype, sentinel shape) still non-blocking. LGTM.

---

Update (c07b2b5): Incremental changes improve docs (multi-GPU device string clarification), add TODO comments for follow-up PRs (#5673, #5674), harden multi-GPU test skipping in _skip_if_unavailable(), and replace .torch property access with torch.as_tensor() in two tests. All quality improvements, no new issues. LGTM.

---

Update (179eead): Continuation of the previous commit's cleanup — torch.as_tensor() replacement is now applied comprehensively across all remaining test assertions (interleaved set/get tests, multi-view isolation, scaled-parent tests). Changelog expanded to better describe lazy dirty tracking, interleave detection with performance warning, and topology recovery. No new issues. LGTM.

---

Update (c40ca6b): Incremental diff from 179eead8 completes the same cleanup pass: adds a set_scales/get_scales TODO noting world-vs-local scale divergence from UsdFrameView, and finishes the torch.as_tensor() migration in all remaining interleaved-set and multi-view test assertions. No new issues. LGTM.

---

Update (3c2b9ff): Commit rebased/squashed again. The diff from c40ca6b0 shows the full feature state as a single commit — changelog file added (fabric-local-poses.rst), fabric_frame_view.py contains all previously-reviewed changes (indexedfabricarray, lazy dirty tracking, per-view flags, _sync_fabric_from_usd_initial with parent scale seeding), and tests include the full suite (rotated/scaled parents, multi-view isolation, interleave warning). Functionally identical to the previously reviewed state. No new issues. LGTM.

---

Update (f526b6c): Another rebase onto updated base branch (diverged: 1 ahead, 1 behind from 3c2b9ff9). Code is functionally identical — same single squashed feature commit, same changelog, same tests. No new issues. LGTM.

---

Update (12705d0): Another rebase onto updated base branch (diverged: 1 ahead, 1 behind from f526b6c7). Same single squashed feature commit — code is functionally identical to previous review. No new issues. LGTM.

---

Update (dc728d0): Rebase onto updated base. No functional changes. LGTM maintained.

---

Update (207608e): Minor cleanup — removed helper @wp.func wrappers (_local_from_world_transposed, _world_from_local_transposed) and inlined the matrix math directly in the kernels. Functionally identical, slightly cleaner code. No new issues. LGTM.

---

Update (945c2a1): ✅ Significant improvement — This commit addresses the world-vs-local scale divergence noted in earlier reviews (the set_scales/get_scales TODO). The new design:

• Deprecates get_scales()/set_scales() with DeprecationWarning across all BaseFrameView subclasses
• Introduces explicit get_local_scales()/set_local_scales() (operates on xformOp:scale/localMatrix) and get_world_scales()/set_world_scales() (composed scale)
• Preserves backwards compat via _get_scales_default()/_set_scales_default() that route to the appropriate method per backend (USD→local, Fabric→world)
• Full implementation across USD, Fabric, Newton, and OVPhysX backends
• New tests for local/world scale roundtrips on Fabric
• Changelog updated documenting the deprecation

Clean API design that resolves the previous ambiguity. No new issues. LGTM.

---

Update (d1dcadd): Bug fixes in UsdFrameView scale methods: (1) replaced GetRow()[:3] with explicit element access (world_mtx[i][j]) for correct matrix extraction, (2) added explicit float() casts for scale division results. Also updated _sync_fabric_from_usd_initial() in FabricFrameView to use get_local_scales() instead of deprecated get_scales() — aligns with the new API. Good follow-up fixes. No new issues. LGTM.

---

Update (68ff2d0): Completes the scale API deprecation cleanup: removed deprecated get_scales()/set_scales() methods from NewtonSiteFrameView and OvPhysXFrameView. The OvPhysXFrameView.get_scales() was renamed to get_local_scales() with updated docstrings, and set_local_scales() now has an explicit docstring noting the USD-only limitation. Clean follow-through on the deprecation introduced in 945c2a16. No new issues. LGTM.

---

Update (6370a2d): Large incremental diff reviewed (40+ files changed). Key additions:

1. Stale sensor data fix (#4970): Contact sensor, IMU, PVA, and joint wrench sensor kernels now gate on timestamp[env] == 0.0 to skip envs not stepped since their last reset. Comprehensive regression tests added across both PhysX and Newton backends. Well-implemented.

2. Asset initialization refactor: All asset classes (Articulation, RigidObject, RigidObjectCollection, DeformableObject, ContactSensor, FrameTransformer, SurfaceGripper, JointWrenchSensor, RayCaster, Imu, Pva) migrated to resolve_matching_prims_from_source() — cleaner, clone-plan-aware resolution replacing the old find_first_matching_prim() + manual suffix reconstruction.

3. Legacy OVRTX 0.2.x removal: Version checks, legacy kernels (extract_all_depth_tiles_kernel_legacy, generate_random_colors_from_ids_kernel_legacy), tempfile fallback, packaging.version import all removed. Clean.

4. pyproject.toml migration: All three packages (isaaclab_ov, isaaclab_ovphysx, isaaclab_physx) moved from setup.py + extension.toml to modern pyproject.toml with importlib.metadata versioning.

5. Newton test additions: New tests for per-env gravity tracking, collision decimation, stale data after reset, world-local site injection (tuple key now includes per_world bool), and frame view clone-plan resolution.

6. FabricFrameView refinements: _DirtyFlag enum for bidirectional world↔local lazy sync, interleave detection with one-time warning, _sync_fabric_from_usd_initial() now properly seeds parent world matrices with scale.

Previous minor observations (int32 vs uint32 dtype in fabric indices, sentinel shape (0,0) vs (0,3), docstring transpose-convention nit) remain unaddressed but were explicitly non-blocking. No new issues found. LGTM.

---

Update (d024384): Scale getters (get_local_scales()/get_world_scales()) now return ProxyArray across all backends (USD, Fabric, Newton, OvPhysX), completing the consistent return-type story started with pose getters. Fabric cached path returns the pre-built _fabric_scales_ta ProxyArray. _sync_fabric_from_usd_initial() simplified by removing duck-type fallback (now relies on guaranteed ProxyArray from get_local_scales()). Shear detection warning added during parent transform initialization — good defensive measure. Six new contract tests for scale API added. Tests updated to use .torch property. No new issues. LGTM.

---

Update (8b8a0e0): Docstring-only cleanup in base_frame_view.py: removed implementation-detail notes from scale method docstrings, shortened shear warning note in get_world_scales, and updated deprecation wording in get_scales()/set_scales() to reference _get_scales_default/_set_scales_default delegates. No functional changes. LGTM.

---

Update (5e62111): Reviewed incremental changes. This commit improves the deprecated get_scales()/set_scales() API:

1. Deprecation warning optimization: Added a class-level _scales_deprecation_warned flag to emit the deprecation warning only once per process, rather than on every call. This is a good UX improvement — repeated warnings clutter logs without adding value.

2. Method renaming for clarity: Renamed _get_scales_default → _get_scales_impl and _set_scales_default → _set_scales_impl across all backend implementations (USD, Fabric, Newton, OvPhysX). The new names better convey that these are backend-specific implementations, not "defaults."

3. Docstring consistency: Updated docstrings to match the renamed methods.

Changes are clean and applied consistently across all four backend implementations. No issues found. LGTM.

---

Update (3d4f504): Docstring/comment-only cleanup — all references to the deprecated set_scales/get_scales in comments, logger warnings, and class-level documentation are now updated to set_world_scales/get_world_scales, consistent with the API rename introduced in earlier commits. Also adds a missing RST heading underline and a ThreeDWorld reference link in ecosystem.rst (unrelated docs fix). No functional changes. LGTM.

---

Update (fbd4512): Reviewed incremental changes from 3d4f5047→fbd45128.

This commit is a substantial rewrite touching 9 files with significant architectural changes:

• Scales API fully materialized: get_local_scales()/set_local_scales()/get_world_scales()/set_world_scales() implemented across all backends (USD, Fabric, Newton, OvPhysX) with proper ProxyArray return types
• Deprecation layer: get_scales()/set_scales() deprecated with one-time DeprecationWarning, routing through _get_scales_impl()/_set_scales_impl() per backend
• _sync_fabric_from_usd_initial() rewrite: Now seeds only localMatrix + parent worldMatrix, then derives child world via _recompute_world_from_local() — eliminates the previously redundant child worldMatrix kernel launch
• Benchmark script fixes: wp.clone(positions.warp) and .torch property access for ProxyArray
• Test coverage: 6 new scale contract tests, interleave tests, multi-view isolation test, rotated/scaled parent tests

Previous inline comments (on review 4358209137):

• ✅ P2 (Redundant kernel launch): Fixed — _sync_fabric_from_usd_initial() no longer composes a child worldMatrix that is immediately overwritten by _recompute_world_from_local()
• ⏳ P1 (Missing wp.synchronize() in non-cached getter paths): Still present — non-cached returns in get_world_poses, get_local_poses, get_world_scales, and get_local_scales still lack synchronization before returning the ProxyArray

No new issues found. LGTM.

---

Update (d6a0db8): Rebase onto updated base branch (300 files changed in diff, but primarily due to base branch updates). Core feature files (fabric_frame_view.py, fabric.py, tests) have minor updates — no functional changes to the Fabric-accelerated local poses implementation.

Previous inline comments status:

• ✅ P2 (Redundant kernel launch): Fixed — confirmed _sync_fabric_from_usd_initial() only composes localMatrix + parent worldMatrix, then derives child world via _recompute_world_from_local()
• ⏳ P1 (Missing wp.synchronize() in non-cached getter paths): Still present — get_world_poses, get_local_poses, get_world_scales, and get_local_scales all lack wp.synchronize() before returning when indices is provided. The cached path (indices=None) correctly synchronizes.

Previous minor observations (int32 vs uint32 dtype in _compute_fabric_indices, sentinel shape (0,0)) remain non-blocking. Code is production-ready with the caveat that callers using indexed getters should be aware of async kernel execution. LGTM.

Update (d6a0db8): Large batch of changes: version bumps across all sub-packages (release changelogs consolidated from changelog.d/ fragments), isaaclab_tasks restructured to core/contrib layout (v2.0.0), import omni.physics.tensors.api as physx → import omni.physics.tensors as physx for wheel compatibility, and resolve_matching_prims_from_source/get_all_matching_child_prims call sites simplified across all backends (removing manual empty-check + expected-count validation now handled by the utility itself). FabricFrameView code is functionally identical to previously-reviewed c07b2b58/179eead8 — no new logic changes. Previous minor observations (int32 vs uint32 dtype, sentinel shape) still non-blocking. No new issues found. LGTM.

---

Update (48e9a08): Reviewed incremental changes from d6a0db8e→48e9a083. 4 files changed:

1. Changelog additions (fabric-local-poses.rst across isaaclab, isaaclab_newton, isaaclab_ovphysx): Properly documents the new explicit scale API (get_local_scales/set_local_scales/get_world_scales/set_world_scales) with deprecation notices for the old get_scales/set_scales methods. Each backend's changelog correctly describes its semantics (Newton: local==world since shape_scale is absolute; OvPhysX: delegates to UsdFrameView; USD: operates on xformOp:scale). Well-written.

2. New scale tests (test_views_xform_prim.py): Two new parametrized tests (test_set_local_scales_then_get_world_scales, test_set_world_scales_then_get_local_scales) exercise the USD-specific cross-space scale conversion under a non-uniform scaled parent (2.0, 1.0, 1.0). The helper _make_scaled_parent_child_view() is clean and reusable. Tests correctly validate the world = parent * local and local = world / parent relationships. Good coverage of an edge case the shared contract tests cannot cover (unit-scale parents only).

No new issues found. Previous minor observations (int32 vs uint32 dtype, sentinel shape) remain non-blocking. LGTM.

[pv-nvidia]

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