Migrate OVPhysX RigidObject onto OvPhysxView (view series, part 3)

[AntoineRichard]

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

This PR depends on #6224 and is branched off it (a sibling of the Articulation
migration #6225, not stacked on it). It targets develop, but because #6224 has not
merged yet, the diff below includes the Part 1 OvPhysxView commits as well as the
Part 3 migration. Please review #6224 first. Once #6224 merges, this branch will be
rebased onto develop and the diff will shrink to the rigid-object migration only.

Description

Part 3 of the OVPhysX view-migration series. It migrates the OVPhysX RigidObject
onto the OvPhysxView binding manager introduced in #6224 (Part 1), mirroring the
Articulation migration in #6225 (Part 2).

The series dogfoods OvPhysxView on the real asset classes to surface weaknesses and to
consolidate the scattered create_tensor_binding / binding.read / binding.write calls
behind one managed surface.

What changed

• RigidObject._initialize_impl builds a single OvPhysxView(physx, pattern, device) and
  eagerly creates the rigid-body bindings through it. RigidObject keeps its fail-loud
  contract (a missing core type raises a helpful RuntimeError) by creating each via
  binding_for rather than the best-effort try_binding_for the articulation uses.
• RigidObjectData is constructed from the view; counts come from the view's bindings,
  and both _get_binding helpers delegate to OvPhysxView.try_binding_for.
• Every write goes through root_view.set_attribute(...) — pose/velocity/wrench plus
  the CPU-only mass/COM/inertia properties (which the asset pre-stages to pinned CPU
  buffers, so the source is already on the binding's native device and the view's
  no-implicit-staging policy is satisfied). Writes now get the view's read-only/device/
  dtype/contiguity guards.
• Every read goes through root_view.read_into(...). RigidObjectData._read_binding_into
  routes through the view, which caches the float32 reinterpret per destination buffer
  and derives the structured layout from the binding shape — this path previously rebuilt
  the reinterpret on every read, so it is a strict read-cache improvement. CPU-only reads
  keep their pinned-host staging.
• RigidObject.root_view now returns the OvPhysxView instead of a raw
  dict[TensorType, TensorBinding] (breaking — see migration note).

Breaking change / migration

RigidObject.root_view returns an OvPhysxView, not a dict:

• root_view[tensor_type] → root_view.try_binding_for(tensor_type)
• full-tensor read → root_view.get_attribute(tensor_type)

Type of change

• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Bug fix (non-breaking change which fixes an issue)
• This change requires a documentation update

Checklist

• I have run the pre-commit checks with ./isaaclab.sh --format
• I have added tests that prove my fix is effective or that my feature works (rigid-object tests exercise the migrated read/write paths)
• I have updated the changelog (source/isaaclab_ovphysx/changelog.d/)
• I have updated the documentation accordingly
• I have added my changes to the right changelog.d fragment(s)

Testing

Run kitless; cpu and cuda must be separate invocations (process-global device lock):

42 passed, 10 xfailed              # test_rigid_object.py -k cpu
41 passed, 2 skipped, 8 xfailed    # test_rigid_object.py -k cuda
1 passed                           # test_rigid_object_helpers.py

[greptile-apps]

Greptile Summary

This PR migrates RigidObject and RigidObjectData from direct dict[TensorType, TensorBinding] management to the OvPhysxView binding manager introduced in Part 1 (#6224). All reads route through OvPhysxView.read_into (gaining a per-destination float32 reinterpret cache) and all writes route through OvPhysxView.set_attribute (gaining read-only, device, dtype, and contiguity guards). The public root_view property type changes from dict to OvPhysxView — a documented breaking change.

• RigidObject._initialize_impl now constructs an OvPhysxView eagerly and creates all six required bindings up front via binding_for, preserving the fail-loud contract with descriptive RuntimeError messages.
• RigidObjectData receives the OvPhysxView instead of a raw dict; _read_binding_into delegates reinterpret-view construction and caching to the view, removing the per-call wp.array(..., copy=False) rebuild.
• CPU-only property writes (mass, COM, inertia) continue to stage through pre-allocated pinned-host buffers; the staging path is unchanged in semantics but now validated by the view's device guard.

Confidence Score: 4/5

The migration is mechanically correct and the CPU-only staging semantics are preserved; the only annotation gap is a root_view return type that omits None for the pre-init window, which misleads static analysis but does not affect runtime behavior.

All six required bindings are eagerly validated at init with descriptive errors; read/write paths are guarded by the view's device, dtype, and shape checks. The annotation mismatch on root_view and the dead _get_binding shim are cosmetic. No data-path logic changes introduce correctness concerns.

rigid_object.py for the root_view return type and the now-unreachable _get_binding shim; rigid_object_data.py for the redundant binding_for call in the CPU-only staging branch of _read_binding_into.

Important Files Changed

Filename	Overview
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object/rigid_object.py	Migrates all binding reads/writes through OvPhysxView; root_view property return annotation misses
source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object/rigid_object_data.py	Replaces raw dict-based binding access with OvPhysxView; _read_binding_into makes a redundant binding_for call in the staging-buffer allocation path when check_shapes=False.
source/isaaclab_ovphysx/test/assets/test_rigid_object.py	Only updates the _MATERIAL_GAP_REASON string to reflect the new OvPhysxView terminology; no functional test changes.
source/isaaclab_ovphysx/changelog.d/antoiner-feat-ovphysx_rigidobject_view.major.rst	Changelog fragment correctly documents the breaking change: root_view now returns OvPhysxView instead of dict[TensorType, TensorBinding].

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Caller
    participant RigidObject
    participant OvPhysxView
    participant TensorBinding
    participant OvPhysX

    Note over RigidObject,OvPhysX: Initialization
    RigidObject->>OvPhysxView: OvPhysxView(physx, pattern, device)
    loop for each TensorType
        RigidObject->>OvPhysxView: binding_for(tt)
        OvPhysxView->>OvPhysX: create_tensor_binding(tensor_type, pattern)
        OvPhysX-->>OvPhysxView: TensorBinding
        OvPhysxView-->>RigidObject: TensorBinding cached
    end

    Note over Caller,TensorBinding: Write path
    Caller->>RigidObject: write_root_link_pose_to_sim_index
    RigidObject->>OvPhysxView: set_attribute(RIGID_BODY_POSE, data, indices)
    OvPhysxView->>OvPhysxView: device/dtype/shape guards
    OvPhysxView->>TensorBinding: write(float32_view, indices)

    Note over Caller,TensorBinding: Read path
    Caller->>RigidObject: data.root_link_pose_w
    RigidObject->>OvPhysxView: read_into(RIGID_BODY_POSE, dst)
    OvPhysxView->>OvPhysxView: _read_view reinterpret cache
    OvPhysxView->>TensorBinding: read(float32_reinterpret)

    Note over Caller,TensorBinding: CPU-only write
    Caller->>RigidObject: set_masses_index
    RigidObject->>RigidObject: wp.launch + wp.copy to pinned CPU
    RigidObject->>OvPhysxView: set_attribute(RIGID_BODY_MASS, cpu_buf, indices)
    OvPhysxView->>TensorBinding: write(cpu_float32_view, indices)

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 Caller
    participant RigidObject
    participant OvPhysxView
    participant TensorBinding
    participant OvPhysX

    Note over RigidObject,OvPhysX: Initialization
    RigidObject->>OvPhysxView: OvPhysxView(physx, pattern, device)
    loop for each TensorType
        RigidObject->>OvPhysxView: binding_for(tt)
        OvPhysxView->>OvPhysX: create_tensor_binding(tensor_type, pattern)
        OvPhysX-->>OvPhysxView: TensorBinding
        OvPhysxView-->>RigidObject: TensorBinding cached
    end

    Note over Caller,TensorBinding: Write path
    Caller->>RigidObject: write_root_link_pose_to_sim_index
    RigidObject->>OvPhysxView: set_attribute(RIGID_BODY_POSE, data, indices)
    OvPhysxView->>OvPhysxView: device/dtype/shape guards
    OvPhysxView->>TensorBinding: write(float32_view, indices)

    Note over Caller,TensorBinding: Read path
    Caller->>RigidObject: data.root_link_pose_w
    RigidObject->>OvPhysxView: read_into(RIGID_BODY_POSE, dst)
    OvPhysxView->>OvPhysxView: _read_view reinterpret cache
    OvPhysxView->>TensorBinding: read(float32_reinterpret)

    Note over Caller,TensorBinding: CPU-only write
    Caller->>RigidObject: set_masses_index
    RigidObject->>RigidObject: wp.launch + wp.copy to pinned CPU
    RigidObject->>OvPhysxView: set_attribute(RIGID_BODY_MASS, cpu_buf, indices)
    OvPhysxView->>TensorBinding: write(cpu_float32_view, indices)

Loading

Comments Outside Diff (1)

1. source/isaaclab_ovphysx/isaaclab_ovphysx/assets/rigid_object/rigid_object.py, line 1130-1146 (link)

   _get_binding is now dead code with a misleading docstring

   All internal callers were replaced with direct self._root_view.set_attribute(...), self._root_view.binding_for(...), or self._root_view.try_binding_for(...) calls. The method survives only as a shim, but its docstring still says "creating it on first access" which is now handled by the view. If this is kept for subclass/external compatibility, the docstring should be updated to reflect the delegation; if it is internal-only, it can be removed to avoid confusion.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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

[greptile-apps]

Redundant binding_for call in the staging-allocation branch when check_shapes=False

When check_shapes is False, the shape-check block is skipped and no local binding variable is set. The staging-buffer allocation (line 991) then calls self._view.binding_for(tensor_type) to obtain the shape. The old code fetched binding unconditionally at the top of the function and reused it in both branches. Consider hoisting the binding fetch before the check_shapes guard so both the shape check and the staging allocation share the same cached object.