Commit efd9d1e
authored
perf: add PrepareForReuse to FabricFrameView, remove sync_usd_on_fabric_write (#5380)
## Summary
Replace the `sync_usd_on_fabric_write` workaround in `FabricFrameView`
with proper `PrepareForReuse()` calls on the Fabric `PrimSelection`.
This tells the renderer (FSD/Storm) that Fabric data has changed, so the
next rendered frame reflects updated transforms — eliminating the need
to copy Fabric writes back to USD.
## Motivation
The existing `sync_usd_on_fabric_write` flag worked by mirroring every
Fabric write back to USD, which defeated the performance benefits of
Fabric. With `PrepareForReuse()`, the rendering pipeline is properly
notified of Fabric data changes without any USD writeback.
Additionally, the old code incorrectly fell back to USD for CPU devices
— Warp handles CPU Fabric buffers correctly, so the fallback was
unnecessary.
This addresses two of the issues raised in @pbarejko Piotr's review of
PR #4923:
- **Issue #1** (USD write-back): Fabric writes no longer sync back to
USD
- **Issue #4** (PrepareForReuse): Renderer notification via
`PrepareForReuse()` instead of USD writeback
## Changes
### Core (FabricFrameView)
- Call `_prepare_for_reuse()` in write paths (`set_world_poses`,
`set_scales`) to notify the renderer
- Remove `sync_usd_on_fabric_write` parameter (accepted via `**kwargs`
for backward compat)
- Remove incorrect CPU/device fallback warnings — Warp handles CPU
Fabric buffers correctly
- Add `_rebuild_fabric_arrays()` for topology change recovery when
`PrepareForReuse()` returns True, with assertion guarding the prim-count
invariant
### Camera
- Remove `sync_usd_on_fabric_write=True` from FrameView construction in
`camera.py`
## Benchmark Results
1024 prims, 50 iterations, NVIDIA L40 GPU:
| Operation | USD (ms) | Fabric (ms) | Speedup |
|---|---|---|---|
| Get World Poses | 14.71 | 0.07 | **203x** |
| Set World Poses | 40.75 | 0.16 | **259x** |
| Interleaved Set→Get | 55.90 | 0.24 | **232x** |
| Get Local Poses | 11.08 | 11.12 | 1.0x |
| Set Local Poses | 16.14 | 16.28 | 1.0x |
Local poses fall back to USD (expected — Fabric only accelerates world
poses via `omni:fabric:worldMatrix`).
## Tests Added
| Test | What it validates |
|------|------------------|
| `test_camera_pose_update_reflected_in_render` | Camera pose changes
propagate to rendered depth (close vs far) for CPU/GPU, tiled/non-tiled
|
| `test_fabric_set_world_does_not_write_back_to_usd` | Fabric writes
stay in Fabric, USD prim unchanged |
| `test_set_world_updates_local` (xfail) | Documents Issue #5:
`set_world_poses` doesn't update local pose in Fabric mode |
## Test Results
| Test Suite | Passed | Skipped | Xfailed | Total |
|---|---|---|---|---|
| Fabric contract tests (`test_views_xform_prim_fabric.py`) | 17 | 16 |
1 | 34 |
| USD contract tests (`test_views_xform_prim.py`) | 45 | 0 | 0 | 45 |
| Camera render test (`test_tiled_camera.py`) | 8 | 0 | 0 | 8 |
## Type of change
- Performance improvement (removes redundant USD writeback on Fabric
operations)
## Checklist
- [x] I have read and understood the [contribution
guidelines](https://isaac-sim.github.io/IsaacLab/main/source/refs/contributing.html)
- [x] I have run the [`pre-commit` checks](https://pre-commit.com/) with
`./isaaclab.sh --format`
- [ ] I have made corresponding changes to the documentation
- [x] My changes generate no new warnings
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] I have updated the changelog and the corresponding version in the
extension's `config/extension.toml` file
- [x] I have added my name to the `CONTRIBUTORS.md` or my name already
exists there
*No doc changes needed (parameter wasn't referenced in any docs)*1 parent bf646bb commit efd9d1e
10 files changed
Lines changed: 323 additions & 54 deletions
File tree
- source
- isaaclab_physx
- changelog.d
- isaaclab_physx/sim/views
- test/sim
- isaaclab
- changelog.d
- isaaclab
- sensors/camera
- sim/views
- test/sensors
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
144 | 144 | | |
145 | 145 | | |
146 | 146 | | |
| 147 | + | |
147 | 148 | | |
148 | 149 | | |
149 | 150 | | |
| |||
Lines changed: 8 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
18 | 18 | | |
19 | 19 | | |
20 | 20 | | |
21 | | - | |
22 | | - | |
| 21 | + | |
23 | 22 | | |
24 | 23 | | |
25 | 24 | | |
| |||
380 | 379 | | |
381 | 380 | | |
382 | 381 | | |
383 | | - | |
384 | | - | |
385 | | - | |
| 382 | + | |
386 | 383 | | |
387 | 384 | | |
388 | 385 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
76 | | - | |
| 75 | + | |
77 | 76 | | |
78 | 77 | | |
79 | 78 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1161 | 1161 | | |
1162 | 1162 | | |
1163 | 1163 | | |
| 1164 | + | |
| 1165 | + | |
| 1166 | + | |
| 1167 | + | |
| 1168 | + | |
| 1169 | + | |
| 1170 | + | |
| 1171 | + | |
| 1172 | + | |
| 1173 | + | |
| 1174 | + | |
| 1175 | + | |
| 1176 | + | |
| 1177 | + | |
| 1178 | + | |
| 1179 | + | |
| 1180 | + | |
| 1181 | + | |
| 1182 | + | |
| 1183 | + | |
| 1184 | + | |
| 1185 | + | |
| 1186 | + | |
| 1187 | + | |
| 1188 | + | |
| 1189 | + | |
| 1190 | + | |
| 1191 | + | |
| 1192 | + | |
| 1193 | + | |
| 1194 | + | |
| 1195 | + | |
| 1196 | + | |
| 1197 | + | |
| 1198 | + | |
| 1199 | + | |
| 1200 | + | |
| 1201 | + | |
| 1202 | + | |
| 1203 | + | |
| 1204 | + | |
| 1205 | + | |
| 1206 | + | |
| 1207 | + | |
| 1208 | + | |
| 1209 | + | |
| 1210 | + | |
| 1211 | + | |
| 1212 | + | |
| 1213 | + | |
| 1214 | + | |
| 1215 | + | |
| 1216 | + | |
| 1217 | + | |
| 1218 | + | |
| 1219 | + | |
| 1220 | + | |
| 1221 | + | |
| 1222 | + | |
| 1223 | + | |
| 1224 | + | |
| 1225 | + | |
| 1226 | + | |
| 1227 | + | |
| 1228 | + | |
| 1229 | + | |
1164 | 1230 | | |
1165 | 1231 | | |
1166 | 1232 | | |
| |||
Lines changed: 2 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
752 | 752 | | |
753 | 753 | | |
754 | 754 | | |
755 | | - | |
| 755 | + | |
756 | 756 | | |
757 | 757 | | |
758 | 758 | | |
759 | | - | |
| 759 | + | |
760 | 760 | | |
761 | 761 | | |
762 | 762 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
898 | 898 | | |
899 | 899 | | |
900 | 900 | | |
901 | | - | |
| 901 | + | |
902 | 902 | | |
903 | 903 | | |
904 | 904 | | |
905 | | - | |
| 905 | + | |
906 | 906 | | |
907 | 907 | | |
908 | 908 | | |
| |||
Lines changed: 12 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
0 commit comments