diff --git a/source/isaaclab_newton/changelog.d/task-cleanup-dex-part01.rst b/source/isaaclab_newton/changelog.d/task-cleanup-dex-part01.rst new file mode 100644 index 000000000000..af5a8a106c46 --- /dev/null +++ b/source/isaaclab_newton/changelog.d/task-cleanup-dex-part01.rst @@ -0,0 +1,5 @@ +Fixed +^^^^^ + +* Fixed detached articulation links with Newton and Isaac RTX by falling back + from unvalidated cubric adapter versions. diff --git a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py index e009580a699a..3bc889f36e49 100644 --- a/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py +++ b/source/isaaclab_newton/isaaclab_newton/physics/_cubric.py @@ -250,10 +250,9 @@ def initialize(self) -> bool: def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: """Verify the acquired IAdapter is compatible with this shim's vtable offsets. - Major mismatches and older minors return False (CPU fallback). Higher - minors are accepted under the semver compatibility contract but emit a - loud warning, so any silent ABI break — the failure mode that motivated - this verification — gets flagged early rather than miscalled. + Only the exact version whose vtable and transform behavior have been + validated is accepted. Any mismatch returns False so the caller uses + the safe CPU hierarchy fallback. """ get_desc_addr = _read_u64(fw_ptr + _FW_OFF_GET_INTERFACE_PLUGIN_DESC) if get_desc_addr == 0: @@ -288,7 +287,7 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: continue major = ctypes.c_uint32.from_address(entry_addr + 8).value minor = ctypes.c_uint32.from_address(entry_addr + 12).value - if major != _IA_EXPECTED_MAJOR or minor < _IA_EXPECTED_MINOR: + if not (major == _IA_EXPECTED_MAJOR and minor == _IA_EXPECTED_MINOR): logger.warning( "cubric IAdapter version incompatible with this shim: plugin " "reports v%d.%d, shim is pinned to v%d.%d. Falling back to " @@ -299,17 +298,6 @@ def _verify_iadapter_version(fw_ptr: int, ia_ptr: int) -> bool: _IA_EXPECTED_MINOR, ) return False - if minor > _IA_EXPECTED_MINOR: - logger.warning( - "cubric IAdapter minor version newer than this shim was validated " - "against: plugin reports v%d.%d, shim is pinned to v%d.%d. Proceeding " - "under semver minor-compatibility — if transforms misbehave, verify " - "the vtable layout against omni/cubric/IAdapter.h.", - major, - minor, - _IA_EXPECTED_MAJOR, - _IA_EXPECTED_MINOR, - ) return True logger.warning( diff --git a/source/isaaclab_newton/test/physics/test_cubric.py b/source/isaaclab_newton/test/physics/test_cubric.py new file mode 100644 index 000000000000..9dc5adb985ec --- /dev/null +++ b/source/isaaclab_newton/test/physics/test_cubric.py @@ -0,0 +1,34 @@ +# Copyright (c) 2022-2026, The Isaac Lab Project Developers (https://github.com/isaac-sim/IsaacLab/blob/main/CONTRIBUTORS.md). +# All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause + +import ctypes + +from isaaclab_newton.physics._cubric import CubricBindings + + +def _verify_version(major: int, minor: int) -> bool: + """Run the cubric ABI verifier against an in-memory plugin descriptor.""" + interface_name = ctypes.create_string_buffer(b"omni::cubric::IAdapter\0") + interface = (ctypes.c_uint64 * 2)() + interface[0] = ctypes.addressof(interface_name) + ctypes.c_uint32.from_address(ctypes.addressof(interface) + 8).value = major + ctypes.c_uint32.from_address(ctypes.addressof(interface) + 12).value = minor + + plugin_descriptor = (ctypes.c_ubyte * 56)() + ctypes.c_uint64.from_address(ctypes.addressof(plugin_descriptor) + 40).value = ctypes.addressof(interface) + ctypes.c_uint64.from_address(ctypes.addressof(plugin_descriptor) + 48).value = 1 + + get_descriptor_type = ctypes.CFUNCTYPE(ctypes.c_void_p, ctypes.c_void_p) + get_descriptor = get_descriptor_type(lambda _: ctypes.addressof(plugin_descriptor)) + framework = (ctypes.c_uint64 * 13)() + framework[12] = ctypes.cast(get_descriptor, ctypes.c_void_p).value + + return CubricBindings._verify_iadapter_version(ctypes.addressof(framework), 1) + + +def test_cubric_adapter_rejects_unvalidated_minor_version(): + """A newer minor ABI must use the safe CPU transform-hierarchy fallback.""" + assert _verify_version(0, 1) + assert not _verify_version(0, 2) diff --git a/source/isaaclab_visualizers/changelog.d/task-cleanup-dex-part01.rst b/source/isaaclab_visualizers/changelog.d/task-cleanup-dex-part01.rst new file mode 100644 index 000000000000..9cc2d06cddda --- /dev/null +++ b/source/isaaclab_visualizers/changelog.d/task-cleanup-dex-part01.rst @@ -0,0 +1,4 @@ +Fixed +^^^^^ + +* Fixed Newton visualization marker cleanup during interpreter shutdown. diff --git a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py index 5e2c39c93659..80829e3b44d4 100644 --- a/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py +++ b/source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py @@ -80,14 +80,16 @@ def __init__(self, cfg: VisualizationMarkersCfg, visible: bool = True): } sim = sim_utils.SimulationContext.instance() - if sim is not None: - sim.vis_marker_registry.set_group(self.group_id, self) + self._registry = sim.vis_marker_registry if sim is not None else None + if self._registry is not None: + self._registry.set_group(self.group_id, self) def close(self) -> None: """Remove marker backend from the simulation marker registry.""" - sim = sim_utils.SimulationContext.instance() - if sim is not None: - sim.vis_marker_registry.remove_group(self.group_id) + registry = getattr(self, "_registry", None) + self._registry = None + if registry is not None: + registry.remove_group(self.group_id) def infer_device(self) -> torch.device: """Infer the device from current marker state.""" diff --git a/source/isaaclab_visualizers/test/test_newton_adapter.py b/source/isaaclab_visualizers/test/test_newton_adapter.py index 448a71834813..d006acc5f3be 100644 --- a/source/isaaclab_visualizers/test/test_newton_adapter.py +++ b/source/isaaclab_visualizers/test/test_newton_adapter.py @@ -14,6 +14,7 @@ import torch import warp as wp from isaaclab_visualizers.newton import NewtonVisualizer, NewtonVisualizerCfg +from isaaclab_visualizers.newton import newton_visualization_markers as newton_markers from isaaclab_visualizers.newton.newton_visualizer import NewtonViewerGL from isaaclab_visualizers.newton_adapter import ( VISUALIZER_INFINITE_PLANE_SIZE, @@ -116,6 +117,49 @@ def test_newton_visualizer_cfg_exposes_particle_options(): assert cfg.particle_color == (0.1, 0.2, 0.3) +def test_newton_marker_registry_lifecycle(monkeypatch: pytest.MonkeyPatch): + """Construction caches the registry; close survives context teardown and is idempotent.""" + + class _Registry: + def __init__(self) -> None: + self.groups: dict[str, object] = {} + + def set_group(self, group_id: str, marker) -> None: + self.groups[group_id] = marker + + def remove_group(self, group_id: str) -> None: + self.groups.pop(group_id) + + class _FakeContext: + def __init__(self, registry: _Registry) -> None: + self.vis_marker_registry = registry + + class _FakeSimulationContext: + current: object | None = None + + @classmethod + def instance(cls): + return cls.current + + registry = _Registry() + monkeypatch.setattr(newton_markers.sim_utils, "SimulationContext", _FakeSimulationContext) + _FakeSimulationContext.current = _FakeContext(registry) + + marker = newton_markers.NewtonVisualizationMarkers( + newton_markers.VisualizationMarkersCfg(prim_path="/Visuals/test", markers={}), visible=False + ) + assert registry.groups == {marker.group_id: marker} + + # the context is torn down before markers close during interpreter shutdown + _FakeSimulationContext.current = None + + marker.close() + marker.close() + + assert marker._registry is None + assert registry.groups == {} + + def test_newton_visualizer_cfg_exposes_world_spacing(): cfg = NewtonVisualizerCfg(world_spacing=(2.0, 2.0, 0.0))