Skip to content

Commit b51594f

Browse files
author
Horde
committed
refactor: move Fabric/USD dispatch from FabricFrameView to FrameView factory
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.
1 parent 6571f0f commit b51594f

2 files changed

Lines changed: 60 additions & 53 deletions

File tree

source/isaaclab/isaaclab/sim/views/frame_view.py

Lines changed: 48 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,24 @@
66
"""Backend-dispatching FrameView.
77
88
``FrameView(path, device=...)`` automatically selects the right backend:
9-
- PhysX: :class:`~isaaclab_physx.sim.views.FabricFrameView`
9+
- PhysX + Fabric enabled + supported device: :class:`~isaaclab_physx.sim.views.FabricFrameView`
10+
- PhysX without Fabric (or unsupported device): :class:`~isaaclab.sim.views.UsdFrameView`
1011
- Newton: :class:`~isaaclab_newton.sim.views.NewtonSiteFrameView`
1112
"""
1213

1314
from __future__ import annotations
1415

16+
import logging
17+
1518
from isaaclab.utils.backend_utils import FactoryBase
1619

1720
from .base_frame_view import BaseFrameView
21+
from .usd_frame_view import UsdFrameView
22+
23+
logger = logging.getLogger(__name__)
24+
25+
# Devices supported by Fabric GPU acceleration (USDRT SelectPrims + Warp).
26+
_FABRIC_SUPPORTED_DEVICES = ("cpu", "cuda", "cuda:0")
1827

1928

2029
class FrameView(FactoryBase, BaseFrameView):
@@ -23,26 +32,59 @@ class FrameView(FactoryBase, BaseFrameView):
2332
Callers use ``FrameView(prim_path, device=device)`` and get the
2433
correct implementation automatically:
2534
26-
- **PhysX / no backend**: :class:`~isaaclab_physx.sim.views.FabricFrameView`
27-
(Fabric GPU acceleration with USD fallback).
35+
- **PhysX + Fabric**: :class:`~isaaclab_physx.sim.views.FabricFrameView`
36+
(GPU-accelerated transforms via Warp + USDRT).
37+
- **PhysX without Fabric**: :class:`~isaaclab.sim.views.UsdFrameView`
38+
(standard USD operations).
2839
- **Newton**: :class:`~isaaclab_newton.sim.views.NewtonSiteFrameView`
2940
(GPU-resident site-based transforms).
3041
"""
3142

32-
_backend_class_names = {"physx": "FabricFrameView", "newton": "NewtonSiteFrameView"}
43+
_backend_class_names = {
44+
"physx": "FabricFrameView",
45+
"newton": "NewtonSiteFrameView",
46+
# "usd" is registered eagerly below — no dynamic import needed.
47+
}
3348

3449
@classmethod
3550
def _get_backend(cls, *args, **kwargs) -> str:
51+
from isaaclab.app.settings_manager import SettingsManager # noqa: PLC0415
3652
from isaaclab.sim.simulation_context import SimulationContext # noqa: PLC0415
3753

3854
ctx = SimulationContext.instance()
3955
if ctx is None:
40-
return "physx"
56+
return "usd"
57+
4158
manager_name = ctx.physics_manager.__name__.lower()
4259
if "newton" in manager_name:
4360
return "newton"
44-
return "physx"
61+
62+
# PhysX path — check if Fabric is enabled and the device is supported.
63+
settings = SettingsManager.instance()
64+
fabric_enabled = bool(settings.get("/physics/fabricEnabled", False))
65+
66+
device = kwargs.get("device", "cpu")
67+
if len(args) >= 2:
68+
device = args[1]
69+
70+
if fabric_enabled and device in _FABRIC_SUPPORTED_DEVICES:
71+
return "physx"
72+
73+
if fabric_enabled and device not in _FABRIC_SUPPORTED_DEVICES:
74+
logger.warning(
75+
f"Fabric mode is not supported on device '{device}'. "
76+
"USDRT SelectPrims and Warp fabric arrays are currently "
77+
f"only supported on {', '.join(_FABRIC_SUPPORTED_DEVICES)}. "
78+
"Falling back to UsdFrameView."
79+
)
80+
81+
return "usd"
4582

4683
def __new__(cls, *args, **kwargs) -> BaseFrameView:
4784
"""Create a new FrameView for the active physics backend."""
4885
return super().__new__(cls, *args, **kwargs)
86+
87+
88+
# Eagerly register UsdFrameView — it lives in isaaclab, not a backend package,
89+
# so FactoryBase's dynamic import (isaaclab_{backend}.sim.views) can't find it.
90+
FrameView.register("usd", UsdFrameView)

source/isaaclab_physx/isaaclab_physx/sim/views/fabric_frame_view.py

Lines changed: 12 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -15,20 +15,13 @@
1515
from pxr import Usd
1616

1717
import isaaclab.sim as sim_utils
18-
from isaaclab.app.settings_manager import SettingsManager
1918
from isaaclab.sim.views.base_frame_view import BaseFrameView
2019
from isaaclab.sim.views.usd_frame_view import UsdFrameView
2120
from isaaclab.utils.warp import ProxyArray
2221
from isaaclab.utils.warp import fabric as fabric_utils
2322

2423
logger = logging.getLogger(__name__)
2524

26-
# TODO: extend this to ``cuda:N`` once we wire up multi-GPU support for the view.
27-
# Recent Kit / USDRT releases do support multi-GPU ``SelectPrims``, but the
28-
# rest of the FabricFrameView wiring (selections, indexed arrays, etc.) still
29-
# assumes a single device — to be tackled in a follow-up.
30-
_fabric_supported_devices = ("cpu", "cuda", "cuda:0")
31-
3225

3326
def _to_float32_2d(a: wp.array | torch.Tensor) -> wp.array | torch.Tensor:
3427
"""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:
4942
class FabricFrameView(BaseFrameView):
5043
"""FrameView with Fabric GPU acceleration for the PhysX backend.
5144
52-
Uses composition: holds a :class:`UsdFrameView` internally for USD
53-
fallback and non-accelerated operations (local poses, visibility, scales
54-
when Fabric is disabled).
45+
This class is only instantiated when Fabric is enabled and the device is
46+
supported. The :class:`~isaaclab.sim.views.FrameView` factory dispatches
47+
to :class:`~isaaclab.sim.views.UsdFrameView` otherwise.
5548
56-
When Fabric is enabled, world-pose and scale operations use Warp kernels
57-
operating on ``omni:fabric:worldMatrix``. All other operations delegate
58-
to the internal USD view.
49+
Uses composition: holds a :class:`UsdFrameView` internally for operations
50+
that don't have a Fabric-accelerated path (local poses, visibility).
5951
60-
After every Fabric write (``set_world_poses``, ``set_scales``),
61-
:meth:`PrepareForReuse` is called on the ``PrimSelection`` to notify
62-
the FSD renderer that Fabric data has changed and to detect topology
63-
changes that require rebuilding internal mappings. Read operations
64-
do not call PrepareForReuse to avoid unnecessary renderer invalidation.
52+
World-pose and scale operations use Warp kernels operating on
53+
``omni:fabric:worldMatrix``. After every Fabric write
54+
(``set_world_poses``, ``set_scales``), :meth:`PrepareForReuse` is called
55+
on the ``PrimSelection`` to notify the FSD renderer that Fabric data has
56+
changed and to detect topology changes that require rebuilding internal
57+
mappings. Read operations do not call PrepareForReuse to avoid
58+
unnecessary renderer invalidation.
6559
6660
Pose getters return :class:`~isaaclab.utils.warp.ProxyArray`. Setters accept ``wp.array``.
6761
"""
@@ -89,18 +83,6 @@ def __init__(
8983
self._usd_view = UsdFrameView(prim_path, device=device, validate_xform_ops=validate_xform_ops, stage=stage)
9084
self._device = device
9185

92-
settings = SettingsManager.instance()
93-
self._use_fabric = bool(settings.get("/physics/fabricEnabled", False))
94-
95-
if self._use_fabric and self._device not in _fabric_supported_devices:
96-
logger.warning(
97-
f"Fabric mode is not supported on device '{self._device}'. "
98-
"USDRT SelectPrims and Warp fabric arrays are currently "
99-
f"only supported on {', '.join(_fabric_supported_devices)}. "
100-
"Falling back to standard USD operations. This may impact performance."
101-
)
102-
self._use_fabric = False
103-
10486
self._fabric_initialized = False
10587
self._fabric_usd_sync_done = False
10688
self._fabric_selection = None
@@ -146,10 +128,6 @@ def set_visibility(self, visibility, indices=None):
146128
# ------------------------------------------------------------------
147129

148130
def set_world_poses(self, positions=None, orientations=None, indices=None):
149-
if not self._use_fabric:
150-
self._usd_view.set_world_poses(positions, orientations, indices)
151-
return
152-
153131
if not self._fabric_initialized:
154132
self._initialize_fabric()
155133

@@ -188,9 +166,6 @@ def set_world_poses(self, positions=None, orientations=None, indices=None):
188166
self._fabric_usd_sync_done = True
189167

190168
def get_world_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray, ProxyArray]:
191-
if not self._use_fabric:
192-
return self._usd_view.get_world_poses(indices)
193-
194169
if not self._fabric_initialized:
195170
self._initialize_fabric()
196171
if not self._fabric_usd_sync_done:
@@ -241,10 +216,6 @@ def get_local_poses(self, indices: wp.array | None = None) -> tuple[ProxyArray,
241216
# ------------------------------------------------------------------
242217

243218
def set_scales(self, scales, indices=None):
244-
if not self._use_fabric:
245-
self._usd_view.set_scales(scales, indices)
246-
return
247-
248219
if not self._fabric_initialized:
249220
self._initialize_fabric()
250221

@@ -279,9 +250,6 @@ def set_scales(self, scales, indices=None):
279250
self._fabric_usd_sync_done = True
280251

281252
def get_scales(self, indices=None):
282-
if not self._use_fabric:
283-
return self._usd_view.get_scales(indices)
284-
285253
if not self._fabric_initialized:
286254
self._initialize_fabric()
287255
if not self._fabric_usd_sync_done:
@@ -404,9 +372,6 @@ def _initialize_fabric(self) -> None:
404372
)
405373
wp.synchronize()
406374

407-
# The constructor should have taken care of this, but double check here to avoid regressions
408-
assert self._device in _fabric_supported_devices
409-
410375
self._fabric_selection = fabric_stage.SelectPrims(
411376
require_attrs=[
412377
(usdrt.Sdf.ValueTypeNames.UInt, self._view_index_attr, usdrt.Usd.Access.Read),

0 commit comments

Comments
 (0)