Migrate OVPhysX FrameView + SceneDataBackend pose bindings onto OvPhysxView (view series)

[AntoineRichard]

🔗 Stacked on top of #6224 (Part 1 — OvPhysxView)

Branched off #6224 as a sibling of the asset/sensor migrations. Targets develop; until #6224 merges the diff includes the Part 1 OvPhysxView commits. Please review #6224 first.

Description

Two more RIGID_BODY_POSE binding consumers the asset (#6225-6227) and sensor (#6228-6232) passes didn't cover, migrated together in one PR:

• OvPhysxFrameView (sim/views/ovphysx_frame_view.py) — the frame view that cameras / XformPrim use. Builds one OvPhysxView + try_binding_for(RIGID_BODY_POSE) (preserving its explicit "matched zero bodies" error, since the view rejects a 0-count binding), and reads via read_into.
• OvPhysxSceneDataBackend (physics/ovphysx_manager.py) — the scene-data provider. Builds one OvPhysxView per distinct rigid-body pattern, stores it on the per-pattern entry, and reads via read_into; the per-entry merge into the transform buffer is unchanged.

Both are internal refactors — no public API change (neither exposes a root_view). The ContactBinding is unrelated (separate wheel API; see #6232). The scene-data backend's bypass-init unit test hand-seeds the internal entry dict, so it now seeds a matching view stub with read_into.

Type of change

• Internal refactor (non-breaking)

Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• Changelog fragment (.skip — internal refactor, no bump)

Testing

• test_views_xform_prim_ovphysx.py — cpu: 18 passed, cuda: 18 passed.
• test_ovphysx_scene_data_backend.py — 11 passed.

[greptile-apps]

Greptile Summary

This PR introduces OvPhysxView, a string-keyed binding manager over OVPhysX TensorBinding handles, and migrates OvPhysxFrameView and OvPhysxSceneDataBackend to use it in place of direct physx.create_tensor_binding() calls. The migration is a clean internal refactor with no public API changes.

• OvPhysxView (ovphysx_view.py): New 700-line class providing lazy binding creation, structured-dtype reinterpretation, device/dtype guards, a _read_views cache for object-identity read-cache warmth, and a try_binding_for / binding_for escape hatch for asset-internal use. A companion 587-line test suite exercises all paths against a fake PhysX.
• OvPhysxFrameView: Replaces the direct create_tensor_binding + shape[0]==0 check with OvPhysxView + try_binding_for, and binding.read() with root_view.read_into(); the world-only (_root_view=None) path is preserved correctly.
• OvPhysxSceneDataBackend: Replaces direct create_tensor_binding with OvPhysxView + binding_for, but the previously reachable row_count == 0 guard becomes dead code because binding_for already raises for zero-count bindings — routing the formerly-silent debug case into the except handler as a warning.

Confidence Score: 3/5

Safe to merge for OvPhysxFrameView and the new OvPhysxView itself; the scene-data backend needs one fix before merging.

The OvPhysxFrameView migration and the new OvPhysxView are correct and well-tested. The OvPhysxSceneDataBackend migration introduced a behavioural regression: binding_for raises for zero-count bindings, so the row_count == 0 guard at line 129 is unreachable and the logger.debug at line 130 is never emitted. Any USD prim that carries RigidBodyAPI but produces zero tensor-binding matches now silently triggers the except path and emits a WARNING instead of the intentional DEBUG; in scenes with partially-physics prims this would flood logs with misleading failure messages on every backend construction.

source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py — the zero-count guard is dead code and the associated debug log is unreachable.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py	New OvPhysxView binding manager; well-structured with strong dtype/device guards, though _read_views cache keys only on id(dst) without the binding, allowing a stale shape reinterpret if the same buffer is reused across bindings with different shapes.
source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py	Migrates to OvPhysxView+binding_for; the row_count == 0 guard (lines 129-131) is now unreachable dead code because binding_for already raises for zero-count bindings, turning the former silent DEBUG path for empty patterns into a spurious WARNING.
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_frame_view.py	Correctly migrates to OvPhysxView+try_binding_for; the None check replaces the old shape[0]==0 check idiomatically and the _root_view=None path for world-only mode is preserved.
source/isaaclab_ovphysx/test/physics/test_ovphysx_scene_data_backend.py	Adds a minimal view stub with read_into to each hand-seeded entry dict; matches the new production code path correctly.
source/isaaclab_ovphysx/test/sim/test_ovphysx_view.py	Comprehensive unit tests for OvPhysxView covering construction, read/write dispatch, dtype/device guards, cache behaviour, key aliases, and adversarial inputs; all against a fake PhysX backend.
source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/init.pyi	Adds OvPhysxView to all and the stub import; consistent with the new module.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant FV as OvPhysxFrameView
    participant MG as OvPhysxSceneDataBackend
    participant OV as OvPhysxView
    participant PX as PhysX (wheel)
    participant BD as TensorBinding

    Note over FV,MG: __init__ / initialize_impl (once at physics ready)

    FV->>OV: OvPhysxView(physx, pattern, device)
    FV->>OV: try_binding_for(RIGID_BODY_POSE)
    OV->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>OV: "TensorBinding (count>0) or raises"
    OV-->>FV: binding or None
    Note over FV: None → raise RuntimeError (0 bodies)

    MG->>OV: OvPhysxView(physx, pattern, device)
    MG->>OV: binding_for(RIGID_BODY_POSE)
    OV->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>OV: TensorBinding
    OV-->>MG: binding (stored in entry dict)

    Note over FV,MG: Per-step reads

    FV->>OV: read_into(rigid_body_pose, pose_buf)
    OV->>OV: _read_view(dst, binding) → cached float32 reinterpret
    OV->>BD: binding.read(float32_view)
    BD-->>OV: fills pose_buf in-place

    MG->>OV: read_into(rigid_body_pose, entry.pose_buf)
    OV->>OV: _read_view(dst, binding) → cached float32 reinterpret
    OV->>BD: binding.read(float32_view)
    BD-->>OV: fills pose_buf in-place
    MG->>MG: wp.copy into merged_transforms buffer

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant FV as OvPhysxFrameView
    participant MG as OvPhysxSceneDataBackend
    participant OV as OvPhysxView
    participant PX as PhysX (wheel)
    participant BD as TensorBinding

    Note over FV,MG: __init__ / initialize_impl (once at physics ready)

    FV->>OV: OvPhysxView(physx, pattern, device)
    FV->>OV: try_binding_for(RIGID_BODY_POSE)
    OV->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>OV: "TensorBinding (count>0) or raises"
    OV-->>FV: binding or None
    Note over FV: None → raise RuntimeError (0 bodies)

    MG->>OV: OvPhysxView(physx, pattern, device)
    MG->>OV: binding_for(RIGID_BODY_POSE)
    OV->>PX: create_tensor_binding(tensor_type, pattern)
    PX-->>OV: TensorBinding
    OV-->>MG: binding (stored in entry dict)

    Note over FV,MG: Per-step reads

    FV->>OV: read_into(rigid_body_pose, pose_buf)
    OV->>OV: _read_view(dst, binding) → cached float32 reinterpret
    OV->>BD: binding.read(float32_view)
    BD-->>OV: fills pose_buf in-place

    MG->>OV: read_into(rigid_body_pose, entry.pose_buf)
    OV->>OV: _read_view(dst, binding) → cached float32 reinterpret
    OV->>BD: binding.read(float32_view)
    BD-->>OV: fills pose_buf in-place
    MG->>MG: wp.copy into merged_transforms buffer

Loading

Comments Outside Diff (2)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/physics/ovphysx_manager.py, line 128-131 (link)

   Dead code + log-level regression for zero-match patterns

   view.binding_for(TT.RIGID_BODY_POSE) internally calls _binding(), which raises OvPhysxView.AttributeUnavailable whenever binding.count == 0 — so control never reaches line 128. The row_count == 0 guard is dead code and the logger.debug on line 130 is never emitted.

   The practical regression is that a pattern which discovers a prim via USD traversal but produces zero tensor-binding matches (a legitimate scenario the original author anticipated, given the explicit debug log) now falls into the except Exception handler at line 125 and emits a WARNING ("Failed to create RIGID_BODY_POSE binding …") instead. In scenes with many partially-physics prims this generates spurious warnings that would previously have been silent at the debug level.

2. source/isaaclab_ovphysx/isaaclab_ovphysx/sim/views/ovphysx_view.py, line 805-811 (link)

   _read_views cache does not key on binding — stale shape with a reused buffer

   The cache key is id(dst) alone, not (id(dst), binding). If a caller passes the same dst buffer to read_into for two different bindings with different flat shapes (e.g. rigid_body_pose followed by rigid_body_velocity on the same (N, 7) float32 buffer), the second call hits the cache, skips _as_binding_view, and hands the wheel a reinterpret view sized for the first binding's shape. The velocity binding (shape (N, 6)) would then write 6 floats per row into a 7-element view, silently corrupting the trailing element and leaving the last column stale.

   This cannot occur in the two call-sites introduced by this PR (each pose_buf is always paired with exactly one binding), but a future caller that reuses a buffer across attribute reads on the same view would get no validation error and silently wrong data.

_{Reviews (1): Last reviewed commit: "Merge branch 'develop' into antoiner/fea..." | Re-trigger Greptile}