From 5e6be23389b399fc63e223a494d0a1913955cec5 Mon Sep 17 00:00:00 2001 From: Peter Verswyvelen Date: Sun, 17 May 2026 19:55:30 +0000 Subject: [PATCH] refactor: move Fabric/USD dispatch from FabricFrameView to FrameView factory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FabricFrameView had an internal _use_fabric flag that fell back to UsdFrameView when Fabric was disabled or the device was unsupported. This violated single-responsibility: FabricFrameView pretended to be one class but sometimes behaved as another. Now the FrameView factory handles all dispatch: - PhysX + Fabric enabled + supported device → FabricFrameView - PhysX without Fabric (or unsupported device) → UsdFrameView - Newton → NewtonSiteFrameView FabricFrameView no longer checks _use_fabric or _fabric_supported_devices. It assumes Fabric is available (the factory guarantees this). UsdFrameView is eagerly registered on the factory since it lives in isaaclab (not a backend package), so FactoryBase's dynamic import (isaaclab_{backend}.sim.views) can't discover it. --- .../isaaclab/isaaclab/sim/views/frame_view.py | 50 ++++++++++++++-- .../sim/views/fabric_frame_view.py | 59 ++++--------------- 2 files changed, 57 insertions(+), 52 deletions(-) diff --git a/source/isaaclab/isaaclab/sim/views/frame_view.py b/source/isaaclab/isaaclab/sim/views/frame_view.py index 95c5f0bb6850..eaa172529d52 100644 --- a/source/isaaclab/isaaclab/sim/views/frame_view.py +++ b/source/isaaclab/isaaclab/sim/views/frame_view.py @@ -6,15 +6,25 @@ """Backend-dispatching FrameView. ``FrameView(path, device=...)`` automatically selects the right backend: -- PhysX: :class:`~isaaclab_physx.sim.views.FabricFrameView` +- PhysX + Fabric enabled + supported device: :class:`~isaaclab_physx.sim.views.FabricFrameView` +- PhysX without Fabric (or unsupported device): :class:`~isaaclab.sim.views.UsdFrameView` +- OVPhysX: :class:`~isaaclab_ovphysx.sim.views.OvPhysxFrameView` - Newton: :class:`~isaaclab_newton.sim.views.NewtonSiteFrameView` """ from __future__ import annotations +import logging + from isaaclab.utils.backend_utils import FactoryBase from .base_frame_view import BaseFrameView +from .usd_frame_view import UsdFrameView + +logger = logging.getLogger(__name__) + +# Devices supported by Fabric GPU acceleration (USDRT SelectPrims + Warp). +_FABRIC_SUPPORTED_DEVICES = ("cpu", "cuda", "cuda:0") class FrameView(FactoryBase, BaseFrameView): @@ -23,8 +33,10 @@ class FrameView(FactoryBase, BaseFrameView): Callers use ``FrameView(prim_path, device=device)`` and get the correct implementation automatically: - - **PhysX / no backend**: :class:`~isaaclab_physx.sim.views.FabricFrameView` - (Fabric GPU acceleration with USD fallback). + - **PhysX + Fabric**: :class:`~isaaclab_physx.sim.views.FabricFrameView` + (GPU-accelerated transforms via Warp + USDRT). + - **PhysX without Fabric**: :class:`~isaaclab.sim.views.UsdFrameView` + (standard USD operations). - **OVPhysX**: :class:`~isaaclab_ovphysx.sim.views.OvPhysxFrameView` (Warp-native, reads body poses via an OVPhysX ``RIGID_BODY_POSE`` tensor binding). @@ -36,22 +48,50 @@ class FrameView(FactoryBase, BaseFrameView): "physx": "FabricFrameView", "ovphysx": "OvPhysxFrameView", "newton": "NewtonSiteFrameView", + # "usd" is registered eagerly below — no dynamic import needed. } @classmethod def _get_backend(cls, *args, **kwargs) -> str: + from isaaclab.app.settings_manager import SettingsManager # noqa: PLC0415 from isaaclab.sim.simulation_context import SimulationContext # noqa: PLC0415 ctx = SimulationContext.instance() if ctx is None: - return "physx" + return "usd" + manager_name = ctx.physics_manager.__name__.lower() if "newton" in manager_name: return "newton" if "ovphysx" in manager_name: return "ovphysx" - return "physx" + + # PhysX path — check if Fabric is enabled and the device is supported. + settings = SettingsManager.instance() + fabric_enabled = bool(settings.get("/physics/fabricEnabled", False)) + + device = kwargs.get("device", "cpu") + if len(args) >= 2: + device = args[1] + + if fabric_enabled and device in _FABRIC_SUPPORTED_DEVICES: + return "physx" + + if fabric_enabled and device not in _FABRIC_SUPPORTED_DEVICES: + logger.warning( + f"Fabric mode is not supported on device '{device}'. " + "USDRT SelectPrims and Warp fabric arrays are currently " + f"only supported on {', '.join(_FABRIC_SUPPORTED_DEVICES)}. " + "Falling back to UsdFrameView." + ) + + return "usd" def __new__(cls, *args, **kwargs) -> BaseFrameView: """Create a new FrameView for the active physics backend.""" return super().__new__(cls, *args, **kwargs) + + +# Eagerly register UsdFrameView — it lives in isaaclab, not a backend package, +# so FactoryBase's dynamic import (isaaclab_{backend}.sim.views) can't find it. +FrameView.register("usd", UsdFrameView) diff --git a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py index 1bcff86d57ac..f61259779957 100644 --- a/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py +++ b/source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py @@ -15,7 +15,6 @@ from pxr import Usd import isaaclab.sim as sim_utils -from isaaclab.app.settings_manager import SettingsManager from isaaclab.sim.views.base_frame_view import BaseFrameView from isaaclab.sim.views.usd_frame_view import UsdFrameView from isaaclab.utils.warp import ProxyArray @@ -23,12 +22,6 @@ logger = logging.getLogger(__name__) -# TODO: extend this to ``cuda:N`` once we wire up multi-GPU support for the view. -# Recent Kit / USDRT releases do support multi-GPU ``SelectPrims``, but the -# rest of the FabricFrameView wiring (selections, indexed arrays, etc.) still -# assumes a single device — to be tackled in a follow-up. -_fabric_supported_devices = ("cpu", "cuda", "cuda:0") - def _to_float32_2d(a: wp.array | torch.Tensor) -> wp.array | torch.Tensor: """Ensure array is compatible with Fabric kernels (2-D float32). @@ -49,19 +42,20 @@ def _to_float32_2d(a: wp.array | torch.Tensor) -> wp.array | torch.Tensor: class FabricFrameView(BaseFrameView): """FrameView with Fabric GPU acceleration for the PhysX backend. - Uses composition: holds a :class:`UsdFrameView` internally for USD - fallback and non-accelerated operations (local poses, visibility, scales - when Fabric is disabled). + This class is only instantiated when Fabric is enabled and the device is + supported. The :class:`~isaaclab.sim.views.FrameView` factory dispatches + to :class:`~isaaclab.sim.views.UsdFrameView` otherwise. - When Fabric is enabled, world-pose and scale operations use Warp kernels - operating on ``omni:fabric:worldMatrix``. All other operations delegate - to the internal USD view. + Uses composition: holds a :class:`UsdFrameView` internally for operations + that don't have a Fabric-accelerated path (local poses, visibility). - After every Fabric write (``set_world_poses``, ``set_scales``), - :meth:`PrepareForReuse` is called on the ``PrimSelection`` to notify - the FSD renderer that Fabric data has changed and to detect topology - changes that require rebuilding internal mappings. Read operations - do not call PrepareForReuse to avoid unnecessary renderer invalidation. + World-pose and scale operations use Warp kernels operating on + ``omni:fabric:worldMatrix``. After every Fabric write + (``set_world_poses``, ``set_scales``), :meth:`PrepareForReuse` is called + on the ``PrimSelection`` to notify the FSD renderer that Fabric data has + changed and to detect topology changes that require rebuilding internal + mappings. Read operations do not call PrepareForReuse to avoid + unnecessary renderer invalidation. Pose getters return :class:`~isaaclab.utils.warp.ProxyArray`. Setters accept ``wp.array``. """ @@ -89,18 +83,6 @@ def __init__( self._usd_view = UsdFrameView(prim_path, device=device, validate_xform_ops=validate_xform_ops, stage=stage) self._device = device - settings = SettingsManager.instance() - self._use_fabric = bool(settings.get("/physics/fabricEnabled", False)) - - if self._use_fabric and self._device not in _fabric_supported_devices: - logger.warning( - f"Fabric mode is not supported on device '{self._device}'. " - "USDRT SelectPrims and Warp fabric arrays are currently " - f"only supported on {', '.join(_fabric_supported_devices)}. " - "Falling back to standard USD operations. This may impact performance." - ) - self._use_fabric = False - self._fabric_initialized = False self._fabric_usd_sync_done = False self._fabric_selection = None @@ -146,10 +128,6 @@ def set_visibility(self, visibility, indices=None): # ------------------------------------------------------------------ def set_world_poses(self, positions=None, orientations=None, indices=None): - if not self._use_fabric: - self._usd_view.set_world_poses(positions, orientations, indices) - return - if not self._fabric_initialized: self._initialize_fabric() @@ -188,9 +166,6 @@ def set_world_poses(self, positions=None, orientations=None, indices=None): self._fabric_usd_sync_done = True def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]: - if not self._use_fabric: - return self._usd_view.get_world_poses(indices) - if not self._fabric_initialized: self._initialize_fabric() if not self._fabric_usd_sync_done: @@ -241,10 +216,6 @@ def get_local_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, # ------------------------------------------------------------------ def set_scales(self, scales, indices=None): - if not self._use_fabric: - self._usd_view.set_scales(scales, indices) - return - if not self._fabric_initialized: self._initialize_fabric() @@ -279,9 +250,6 @@ def set_scales(self, scales, indices=None): self._fabric_usd_sync_done = True def get_scales(self, indices=None): - if not self._use_fabric: - return self._usd_view.get_scales(indices) - if not self._fabric_initialized: self._initialize_fabric() if not self._fabric_usd_sync_done: @@ -404,9 +372,6 @@ def _initialize_fabric(self) -> None: ) wp.synchronize() - # The constructor should have taken care of this, but double check here to avoid regressions - assert self._device in _fabric_supported_devices - self._fabric_selection = fabric_stage.SelectPrims( require_attrs=[ (usdrt.Sdf.ValueTypeNames.UInt, self._view_index_attr, usdrt.Usd.Access.Read),