[Backport release/3.0.0-beta2] Fabric pose writer context - #7318
[Backport release/3.0.0-beta2] Fabric pose writer context#7318alexmillane wants to merge 2 commits into
Conversation
…r API (isaac-sim#5677) Two things: 1. **Local-space pose and scale ops are now GPU-fast on the Fabric backend.** They go through the same Warp-kernel path that world-space ops already used. The old USD fallback for local-space is ~100× slower (see the table below). 2. **All transform writes go through a small context manager.** You open a scope, do your writes inside it, and the scope handles the cleanup: ```python with view.xform_world_space_writer() as writer: writer.set_poses(positions=p, orientations=o) writer.set_scales(scales=s) ``` Use `view.xform_local_space_writer()` for local-space writes. Builds on Piotr's prototype at [`bareya/pbarejko/camera-update`](https://github.com/bareya/IsaacLab/tree/pbarejko/camera-update). A FrameView keeps two copies of each prim's transform in Fabric storage: the `omni:fabric:worldMatrix` and `omni:fabric:localMatrix` attributes. When you write one, the other has to be recomputed so they stay consistent. Without a scope, that recompute has to run after every single write. With a scope, it runs **once**, when the scope closes. You can call `set_poses` and `set_scales` as many times as you want inside the scope; on exit, one Warp kernel derives the other space and one `wp.synchronize()` runs. Empty scopes cost nothing. The scope also pauses **Fabric Hierarchy**'s transform-change tracking while you're writing, then restores it on exit. (Fabric itself is just the data store; Fabric Hierarchy, exposed through USDRT's `IFabricHierarchy`, is the plugin that watches writes to `omni:fabric:localMatrix` / `omni:fabric:worldMatrix` and keeps them mutually consistent across the hierarchy.) Its tracking is pull-based: a per-attribute listener records writes into a changelog, and the plugin drains and processes that changelog on the next call to `IFabricHierarchy::update_world_xforms()`. Pausing the listener does not block the writes themselves — they still land in Fabric storage — it just keeps them out of the changelog. With the changelog empty, the next tick has nothing to "catch up on" for these prims, and can't decide one of the two spaces is canonical and derive the other from it. See the next section for why that matters. Rules: - Only one writer can be open on a view at a time. Opening a second one raises `RuntimeError`. - While a writer is open, the view's own getters (`get_world_poses`, etc.) raise. Read through the writer (`writer.get_poses()`) inside the scope, or close the scope first. - **Do not advance the simulation or render from inside the scope.** No `sim.step()`, no `world.render()`, no `SimulationApp.update()`. The matrices are mid-write until exit, and rendering against that state would read torn data. Keep scopes short and close them before stepping. - If something raises inside the scope (a user-code bug, a notebook cell interrupt, etc.), the scope still runs the opposite-space recompute on exit so the world and local matrices stay consistent prim-by-prim. The partial write itself is not rolled back — if you need all-or-nothing, snapshot the matrices yourself before entering. correctness) A writer scope is synchronous Python code, so the renderer can't see anything mid-scope — no tick runs until we exit. The risk is on the **next** render/sim tick after the scope closes. On that next tick, Fabric Hierarchy runs its `IFabricHierarchy::update_world_xforms()` step, which can recompute `omni:fabric:worldMatrix` from `omni:fabric:localMatrix` (or vice versa). If it thinks one of the two was the user's most recent edit, it derives the other from it; the derived value — not what we wrote — is then what the **Fabric Scene Delegate (FSD)** hands to RTX on the render path. Three things in this PR keep that next-tick recompute from clobbering our work: 1. **Opposite-space derive at scope exit.** By the time the scope closes, `omni:fabric:worldMatrix` and `omni:fabric:localMatrix` are mutually consistent prim-by-prim, so any recompute Fabric Hierarchy does is a no-op. 2. **Fabric-Hierarchy change tracking is paused for the whole scope.** The writer calls `track_local_xform_changes(False)` and `track_world_xform_changes(False)` on enter (saving the prior state) and restores them on exit. While tracking is paused, the listeners don't record our writes in their changelog, so when the next tick runs there's nothing for `update_world_xforms()` to "catch up on". 3. **Two persistent selections with explicit read-only / read-write flags.** Outside the scope, the view's selection is fully read-only — both `omni:fabric:worldMatrix` and `omni:fabric:localMatrix` are flagged RO. Inside the scope, the writer flips to a fully read-write selection. ``` _sel_ro : worldMatrix=RO, localMatrix=RO (steady state, between writes) _sel_rw : worldMatrix=RW, localMatrix=RW (active during a writer scope) ``` Both selections are built once when the view is initialized and kept alive for its lifetime. The writer scope flips a single flag (`_is_rw`) on enter/exit; nothing is rebuilt. The RO steady state tells Fabric Hierarchy's next-tick `update_world_xforms()` that no attribute is user-authored, so it leaves both alone. - **Fabric (PhysX):** the fast GPU path described above. - **USD, Newton, OVPhysX:** the writer is a thin wrapper. Writes go straight to the backend's existing storage. No batching, no extra recompute (there is no second matrix to keep in sync). Same API across all four backends. Newton has two different "scale" ideas, and the API keeps them apart: - `NewtonSiteFrameView.set_scales(...)` (deprecated) writes the **Newton collision-shape geometry** size. - The writer's `set_scales(...)` writes per-site **transform scale**, matching what USD FrameView does. They operate on different state and are not merged. **New (recommended):** - `view.xform_world_space_writer()` / `view.xform_local_space_writer()` — a context manager with `set_poses`, `set_scales`, `get_poses`, `get_scales`. **Deprecated (still works, warns once per class on first use):** - `set_world_poses` / `set_local_poses` — use the writer scope. - `get_scales` / `set_scales` — use `get_local_scales` / `get_world_scales`, or the writer's `set_scales`. Per-iteration timings (lower is better): | Operation | USD (ms) | Fabric (ms) | Newton Site (ms) | |----------------------------|---------:|------------:|-----------------:| | Get World Poses | 8.6148 | 0.0375 | 0.0307 | | Set World Poses | 19.1357 | 0.1090 | 0.0368 | | Get Local Poses | 5.9556 | 0.0374 | 0.0497 | | Set Local Poses | 7.9794 | 0.0979 | 0.0790 | | Get World Scales | 13.1193 | 0.0371 | 0.0015 | | Set World Scales | 21.4571 | 0.1014 | 0.0612 | | Get Local Scales | 3.2544 | 0.0375 | 0.0022 | | Set Local Scales | 3.8350 | 0.0937 | 0.0587 | | Get Both (World+Local) | 14.7506 | 0.0738 | 0.1302 | | Interleaved World Set→Get | 28.1046 | 0.1432 | 0.1588 | | **Per-iter total** | **126.2** | **0.77** | **0.61** | Speedup vs USD (per-iter ops; one-time view construction excluded): | Operation | Fabric × | Newton Site × | |----------------------------|---------:|--------------:| | Get World Poses | 229.6× | 280.2× | | Set World Poses | 175.6× | 519.8× | | Get Local Poses | 159.0× | 119.9× | | Set Local Poses | 81.5× | 101.0× | | Get World Scales | 353.9× | 8911.5× | | Set World Scales | 211.6× | 350.4× | | Get Local Scales | 86.8× | 1489.9× | | Set Local Scales | 40.9× | 65.4× | | Get Both (World+Local) | 200.0× | 113.3× | | Interleaved World Set→Get | 196.2× | 177.0× | | **Overall (per-iter)** | **164×** | **207×** | One-time view construction (reported separately, not part of the per-iter total): USD 4.6 ms, Fabric 4.4 ms, Newton Site 1013 ms — the Newton number is dominated by stage population on the first call. Steady-state per-iteration cost is what the speedup row reflects. - Getters call `wp.synchronize()` before returning, so the returned `ProxyArray` is always immediately readable from both GPU and host code — no caller-side sync needed. (Both the cached and the per-indices paths sync; this used to be asymmetric.) --------- Co-authored-by: pv-nvidia <peter.verswyvelen@gmail.com> Co-authored-by: pv-nvidia <{"message":"Not Found","documentation_url":"https://docs.github.com/rest/users/emails#list-email-addresses-for-the-authenticated-user","status":"404"}>
github-actions
Bot
added
documentation
alexmillane
changed the title
alexmillane
changed the title
Greptile SummaryThe PR introduces context-managed world/local transform writers and extends scale and Fabric matrix handling across USD, Fabric, Newton, and OvPhysX backends to keep renderer-visible transforms synchronized after rebuilds.
Confidence Score: 3/5The PR is not safe to merge until failed writer entry can recover cleanly and world-scale writes correctly handle rotated, non-uniformly scaled parents. Backend initialization exceptions can permanently lock a FrameView, while the new USD world-scale conversion can author a transform that does not compose to the requested scale. Files Needing Attention: source/isaaclab/isaaclab/sim/views/xform_space_writer.py; source/isaaclab/isaaclab/sim/views/usd_frame_view.py Important Files Changed
Sequence DiagramsequenceDiagram
participant C as Caller
participant W as Space writer
participant B as FrameView backend
participant F as Fabric/USD state
participant R as Renderer
C->>W: enter world/local writer
W->>B: claim active writer
C->>W: set poses/scales
W->>F: write primary-space transform
C->>W: exit writer
W->>B: derive opposite-space transform
B->>F: synchronize matrices
F-->>R: renderer reads current world transform
Reviews (1): Last reviewed commit: "Shim needed for Arena compatibility." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| self._view._active_writer = self | ||
| self._enter_impl() |
There was a problem hiding this comment.
When Fabric initialization raises during __enter__, _active_writer has already been assigned and __exit__ is never invoked, causing every subsequent getter and writer scope on the view to fail as though the original writer were still active.
Knowledge Base Used: PhysX backend
| local_scale = Gf.Vec3d( | ||
| float(scales_np[idx][0] / parent_scale[0]), | ||
| float(scales_np[idx][1] / parent_scale[1]), | ||
| float(scales_np[idx][2] / parent_scale[2]), | ||
| ) |
There was a problem hiding this comment.
When a parent combines non-identity rotation with non-uniform scale, component-wise division by its world-matrix row lengths does not invert the composed transform, causing set_scales to author a local scale whose world-space readback and rendered scaling differ from the requested value.
Knowledge Base Used: Physics backend extensions
![isaaclab-review-bot[bot]](https://avatars.githubusercontent.com/u/157846462?s=60&v=4){kind=small}
There was a problem hiding this comment.
Isaac Lab Review Bot
The writer-scope backport introduces concrete API, lifecycle, caching, and documentation issues that should be corrected before merge. In particular, get_scales() changes its public return type, failed writer initialization can permanently lock a view, world/local Fabric scale results alias one cache, and the documentation and changelog do not match the implemented contract.
- Design and architecture: The context-managed writer design is coherent, but
FrameViewSpaceWriterBase.__enter__claims the per-view lock before invoking the fallible backend initialization hook. An initialization failure therefore leaves the view permanently marked as having an active writer. The lock must be released when_enter_impl()raises. - API:
get_scales()changes fromwp.arraytoProxyArray, which breaks callers that pass the result directly to Warp APIs; compatibility must be preserved or the breaking change handled according to the repository policy. The local-writer example incorrectly documentstranslations=although the public writer method acceptspositions=. Several changelog fragments also promiseDeprecationWarningbehavior that the implementation does not provide. - Implementation: The Fabric world- and local-scale getters reuse the same cached buffer and
ProxyArray, so a later getter overwrites an earlier result handle. Separate caches are needed. The two core changelog fragments also use an unsupportedNotescategory and must be reorganized into the permitted changelog sections.
Significant concerns. Posted 6 actionable findings inline.
Automated review; human maintainers own approval decisions.
| # Scale getter/setter convenience helpers. | ||
| # ------------------------------------------------------------------ | ||
|
|
||
| def get_scales(self, indices: wp.array | None = None) -> ProxyArray: |
There was a problem hiding this comment.
🟡 Warning · Api — get_scales runtime return type changed
get_scales previously declared and returned wp.array (UsdFrameView built one directly, Newton returned a raw gather buffer); it now returns ProxyArray, which is not a wp.array and cannot be passed to wp.launch/wp.clone without .warp. The changelog fragment states this helper "remains supported ... (not deprecated)" and never records the change. Record it under Changed with **Breaking:** and migration guidance, or keep returning the underlying array.
| f"({type(self._view._active_writer).__name__}). Exit the existing scope before " | ||
| "opening a new one." | ||
| ) | ||
| self._view._active_writer = self |
There was a problem hiding this comment.
🟡 Warning · Implementation — Failed enter leaves view permanently locked
__enter__ assigns _view._active_writer = self before calling _enter_impl(). If the backend hook raises (e.g. FabricFrameView._initialize_fabric failing to resolve a path in the Fabric selection), __exit__ never runs, so the flag stays set and every later getter and writer raises "active writer scope". Clear _active_writer if _enter_impl() fails.
| if use_cached: | ||
| wp.synchronize() | ||
| return scales_wp | ||
| return self._fabric_scales_ta |
There was a problem hiding this comment.
🟡 Warning · Implementation — World and local scale getters share a buffer
_get_world_scales_impl and _get_local_scales_impl both call _decompose_scales, which for indices=None writes into _fabric_scales_buf and returns the same _fabric_scales_ta. Calling get_world_scales() then get_local_scales() overwrites the first result in place, so both handles report local scales. Pose getters correctly keep separate world/local buffers; add a second scale buffer and ProxyArray selected per space.
| writing Fabric ``Matrix4d`` attributes (``omni:fabric:worldMatrix`` / | ||
| ``omni:fabric:localMatrix``). | ||
|
|
||
| Notes |
There was a problem hiding this comment.
🔵 Suggestion · Implementation — Changelog fragment uses invalid Notes category
Both new isaaclab fragments add a Notes heading (also xform-space-writer.rst). Repository changelog rules restrict fragment sections to Added, Changed, Deprecated, Removed, and Fixed, so the compiler has no bucket for these entries. Move the guidance under Changed or fold it into the corresponding Added bullets.
| .. code-block:: python | ||
|
|
||
| with view.xform_local_space_writer() as w: | ||
| w.set_poses(translations=t, orientations=o) |
There was a problem hiding this comment.
🔵 Suggestion · Api — Local writer example uses wrong keyword
The xform_local_space_writer docstring example calls w.set_poses(translations=t, orientations=o), but FrameViewSpaceWriterBase.set_poses and every concrete writer name the first parameter positions (the migrated contract test also switched to positions=). Copying the documented example raises TypeError. Update the example to positions=.
| Deprecated | ||
| ^^^^^^^^^^ | ||
|
|
||
| * Deprecated :meth:`~isaaclab_newton.sim.views.NewtonSiteFrameView.get_scales` |
There was a problem hiding this comment.
🔵 Suggestion · Api — Fragments promise unemitted DeprecationWarnings
This fragment and the newton/ovphysx/physx xform-space-writer fragments state that get_scales/set_scales and the set_world_poses/set_local_poses shims emit a DeprecationWarning, but no backend in this diff calls warnings.warn, and the isaaclab fragment simultaneously documents the same helpers as explicitly not deprecated. Align the fragments with the implemented behavior.
2 participants
Description
Backport #5677
Fixes
This solves an issue where on a stage rebuild, with GPU physics + fabrics enabled, some poses are incorrectly read in the renderer, leading to rendering artifacts. See attached video
This was solved upstream. Backporting this fix to Beta 2.
Type of change
Screenshots
disappearing_gripper.mp4
Checklist
pre-commitchecks with./isaaclab.sh --formatsource/<pkg>/changelog.d/for every touched package (do not editCHANGELOG.rstor bumpextension.toml— CI handles that)CONTRIBUTORS.mdor my name already exists there