Skip to content

Commit 4e6af98

Browse files
[Backport release/3.0.0] Replace map transforms with pre-owned and reused buffer (#7233) (#7566)
# Description Backport of #7233 (*Replace map transforms with pre-owned and reused buffer*) to `release/3.0.0`. This supersedes #7300, which was the original cherry-pick by @pbarejko. That PR was approved on 2026-08-24 but has been unmergeable (`mergeable=false`, `dirty`) ever since, so the fix never reached the release branch. Authorship of the commit is preserved as @pbarejko. ## Why it conflicted `ovrtx_renderer.py` diverged structurally between `develop` and `release/3.0.0`. The backport of #7347 (landed as #7353) was not a line-for-line cherry-pick — on `develop` #7347 changed 11 lines of that file, while the release-side version changed 1348 (240+/1108−) and restructured code that now only exists in that shape on the release branch: - `_close_legacy` was rewritten from explicit per-binding unbinds into a `for attr, name in (...)` loop - `except Exception as e` was renamed to `as exc` - the close dispatch was restructured The three-week-old cherry-pick was written against the pre-#7353 shape, hence the conflicts. ## Conflict resolution - The four renderer conflicts covering the buffer-write change itself take the #7233 side. - `_close_legacy` keeps **the release branch's** loop form and adds only #7233's semantic contribution (`self._object_transform_buffer = None`). Taking `develop`'s shape here would have silently reverted #7353. - The test-file conflict was an add/add at the same line; all three tests are kept. - The follow-up `Reformat` commit was a single blank line already covered by the resolution, so it drops out as a no-op. ## One deliberate change from the original PR #7233 predates #7169, so it wrote `cuda_stream=wp.get_stream(self._device).cuda_stream`. `release/3.0.0` has since taken #7169 and standardized on `self._warp_device.stream.cuda_stream` — that caching *is* #7169's fix. Applying the original patch verbatim would have reintroduced a per-call device lookup into the per-frame transform path, partially undoing a fix already on the branch. Both new call sites and the two new tests are aligned to the release idiom instead. This is the one substantive delta from what was approved on #7300 and is the part most worth a reviewer's attention. ## Type of change - Bug fix (non-breaking change which fixes an issue) ## Release backport This PR targets `release/3.0.0` directly; the change is already on `develop` via #7233. ## Testing - `source/isaaclab_ov/test/test_ovrtx_deformable_bindings.py` — 19 passed - `source/isaaclab_ov/test/test_ovrtx_renderer_contract.py` — 37 passed - Both new tests (`test_update_transforms_writes_caller_owned_buffer`, `test_update_camera_writes_without_mapping`) were confirmed to **fail** against the unpatched release renderer and pass with the change. ## 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 added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package - [x] I have added my name to the `CONTRIBUTORS.md` or my name already exists there Co-authored-by: Piotr Barejko <pbarejko@nvidia.com>
1 parent 91b540d commit 4e6af98

4 files changed

Lines changed: 95 additions & 15 deletions

File tree

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fixed
2+
^^^^^
3+
4+
* Fixed OVRTX object and camera transform updates to write a caller-owned GPU buffer instead of mapping and unmapping OVRTX memory every frame.

source/isaaclab_ov/isaaclab_ov/renderers/ovrtx_renderer.py

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@
8585
decode_stable_id_map,
8686
decode_stable_id_semantic_id_map,
8787
)
88-
from .ovrtx_mapping import map_attribute_for_warp_writes
8988
from .ovrtx_renderer_cfg import OVRTXRendererCfg
9089
from .ovrtx_renderer_kernels import (
9190
compute_cable_points_world_kernel,
@@ -422,8 +421,16 @@ def _create_object_scale_array(self, object_paths: list[str]) -> wp.array:
422421
return wp.array(scales, dtype=wp.vec3f, device=self._device)
423422

424423
def _init_fields_legacy(self) -> None:
424+
"""Initialize the legacy-path instance fields.
425+
426+
Counterpart to :meth:`_init_fields_ovstage`. Only fields the ovstage path never touches live
427+
here: the ``bind_attribute``/``bind_array_attribute`` handles and the caller-owned object
428+
transform buffer. State shared by both paths (``_object_newton_indices``, the particle
429+
offset/count lists) stays in :meth:`__init__`.
430+
"""
425431
self._camera_xform_binding = None
426432
self._object_xform_binding = None
433+
self._object_transform_buffer: wp.array | None = None
427434
self._deformable_points_binding = None
428435
self._particle_points_binding = None
429436
self._particle_workaround_applied = False
@@ -563,6 +570,7 @@ def _setup_xform_bindings_legacy(self):
563570
raise RuntimeError("Failed to create OVRTX object bindings")
564571
self._object_newton_indices = wp.array(newton_indices, dtype=wp.int32, device=self._device)
565572
self._object_scales = self._create_object_scale_array(object_paths)
573+
self._object_transform_buffer = wp.zeros(len(newton_indices), dtype=wp.mat44d, device=self._device)
566574

567575
def _setup_deformable_bindings_legacy(self, num_envs: int):
568576
try:
@@ -719,7 +727,13 @@ def set_outputs(self, render_data: OVRTXRenderData, output_data: dict[str, Proxy
719727
)
720728

721729
def _update_transforms_legacy(self) -> None:
722-
if self._object_xform_binding is None or self._object_newton_indices is None or self._object_scales is None:
730+
"""Sync transforms to OVRTX."""
731+
if (
732+
self._object_xform_binding is None
733+
or self._object_newton_indices is None
734+
or self._object_scales is None
735+
or self._object_transform_buffer is None
736+
):
723737
return
724738
from isaaclab_newton.physics import NewtonManager
725739

@@ -729,15 +743,21 @@ def _update_transforms_legacy(self) -> None:
729743
body_q = getattr(newton_state, "body_q", None)
730744
if body_q is None:
731745
return
732-
with map_attribute_for_warp_writes(
733-
self._object_xform_binding, self._warp_device, wp.mat44d
734-
) as ovrtx_transforms:
735-
wp.launch(
736-
kernel=sync_newton_transforms_kernel,
737-
dim=len(self._object_newton_indices),
738-
inputs=[ovrtx_transforms, self._object_newton_indices, body_q, self._object_scales],
739-
device=self._device,
740-
)
746+
747+
wp.launch(
748+
kernel=sync_newton_transforms_kernel,
749+
dim=len(self._object_newton_indices),
750+
inputs=[self._object_transform_buffer, self._object_newton_indices, body_q, self._object_scales],
751+
device=self._device,
752+
)
753+
# Blocking ``write()`` so the buffer stays valid until OVRTX finishes reading it.
754+
# ``DataAccess.ASYNC`` + the Warp CUDA stream let OVRTX read in place and wait
755+
# on-GPU for the kernel; ``SYNC`` is rejected for GPU buffers.
756+
self._object_xform_binding.write(
757+
self._object_transform_buffer,
758+
data_access=DataAccess.ASYNC,
759+
cuda_stream=self._warp_device.stream.cuda_stream,
760+
)
741761

742762
def _update_geometries_legacy(self) -> None:
743763
if self._deformable_points_binding is not None:
@@ -801,10 +821,11 @@ def _update_camera_legacy(
801821
device=self._device,
802822
)
803823
if self._camera_xform_binding is not None:
804-
with map_attribute_for_warp_writes(
805-
self._camera_xform_binding, self._warp_device, wp.mat44d
806-
) as transforms_view:
807-
wp.copy(transforms_view, camera_transforms)
824+
self._camera_xform_binding.write(
825+
camera_transforms,
826+
data_access=DataAccess.ASYNC,
827+
cuda_stream=self._warp_device.stream.cuda_stream,
828+
)
808829

809830
def read_output(self, render_data: OVRTXRenderData, camera_data: CameraData) -> None:
810831
assert camera_data.info is not None, "CameraData.info should be created in CameraData.allocate"
@@ -1063,6 +1084,7 @@ def _safe_unbind(binding, name: str) -> None:
10631084
):
10641085
_safe_unbind(getattr(self, attr), name)
10651086
setattr(self, attr, None)
1087+
self._object_transform_buffer = None
10661088
self._deformable_particle_offsets = []
10671089
self._deformable_particle_counts = []
10681090
self._particle_visual_offsets = []

source/isaaclab_ov/test/test_ovrtx_deformable_bindings.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,3 +590,56 @@ def _write(query, attribute, **kwargs):
590590
assert tensors[0].data == particle_q[1:4].ptr
591591
assert tensors[0].shape_tuple == (3,)
592592
assert tensors[0].dtype.lanes == 3
593+
594+
595+
def test_update_transforms_writes_caller_owned_buffer(monkeypatch: pytest.MonkeyPatch):
596+
"""Object xforms fill a persistent GPU buffer and blocking ASYNC write, not map/unmap."""
597+
renderer, _ = _make_renderer_without_backend()
598+
buffer = object()
599+
renderer._object_xform_binding = _FakePointsBinding("omni:xform")
600+
renderer._object_newton_indices = [0, 1]
601+
renderer._object_scales = object()
602+
renderer._object_transform_buffer = buffer
603+
604+
monkeypatch.setattr(NewtonManager, "get_state", classmethod(lambda cls: SimpleNamespace(body_q=object())))
605+
launch_kwargs: dict = {}
606+
607+
def _capture_launch(*args, **kwargs):
608+
launch_kwargs.update(kwargs)
609+
610+
monkeypatch.setattr(ovrtx_renderer_module.wp, "launch", _capture_launch)
611+
renderer._warp_device = SimpleNamespace(stream=SimpleNamespace(cuda_stream=99))
612+
613+
renderer.update_transforms()
614+
615+
assert launch_kwargs["inputs"][0] is buffer
616+
assert launch_kwargs["dim"] == 2
617+
assert renderer._object_xform_binding.written is buffer
618+
assert renderer._object_xform_binding.write_kwargs["data_access"] is DataAccess.ASYNC
619+
assert renderer._object_xform_binding.write_kwargs["cuda_stream"] == 99
620+
621+
622+
def test_update_camera_writes_without_mapping(monkeypatch: pytest.MonkeyPatch):
623+
"""Camera xforms are handed to ``write()`` instead of copied into a mapped OVRTX buffer."""
624+
renderer, _ = _make_renderer_without_backend()
625+
renderer._camera_xform_binding = _FakePointsBinding("omni:xform")
626+
camera_transforms = []
627+
628+
monkeypatch.setattr(ovrtx_renderer_module, "convert_camera_frame_orientation_convention_wp", lambda **kwargs: None)
629+
monkeypatch.setattr(ovrtx_renderer_module.wp, "empty", lambda *args, **kwargs: object())
630+
631+
def _fake_zeros(*args, **kwargs):
632+
arr = object()
633+
camera_transforms.append(arr)
634+
return arr
635+
636+
monkeypatch.setattr(ovrtx_renderer_module.wp, "zeros", _fake_zeros)
637+
monkeypatch.setattr(ovrtx_renderer_module.wp, "launch", lambda *args, **kwargs: None)
638+
renderer._warp_device = SimpleNamespace(stream=SimpleNamespace(cuda_stream=7))
639+
640+
positions = SimpleNamespace(shape=(2,), warp=object())
641+
renderer.update_camera(object(), positions, SimpleNamespace(warp=object()), object())
642+
643+
assert renderer._camera_xform_binding.written is camera_transforms[0]
644+
assert renderer._camera_xform_binding.write_kwargs["data_access"] is DataAccess.ASYNC
645+
assert renderer._camera_xform_binding.write_kwargs["cuda_stream"] == 7

source/isaaclab_ov/test/test_ovrtx_renderer_contract.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,7 @@ def test_ovrtx_close_releases_legacy_renderer_state():
682682
]
683683
assert renderer._camera_xform_binding is None
684684
assert renderer._object_xform_binding is None
685+
assert renderer._object_transform_buffer is None
685686
assert renderer._deformable_points_binding is None
686687
assert renderer._particle_points_binding is None
687688
assert renderer._cable_points_binding is None

0 commit comments

Comments
 (0)