Introduce OrientedBoundingBox - #961
Conversation
Signed-off-by: zhx06 <zihaox@nvidia.com>
Greptile SummaryIntroduces rotation-aware oriented bounding boxes throughout placement and collision handling.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code defect was identified. The reviewed OBB, solver, validator, mesh-collision, ObjectReference, and cuRobo paths consistently use xyzw quaternions, compose local-to-world rotations in the same order, preserve pose-relative box centers, and keep matching relation losses and validators geometrically aligned. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
USD[USD geometry] --> OBB[Local OrientedBoundingBox]
OBB --> ENV[Per-environment boxes]
ENV --> STATE[Relation solver state]
STATE --> LOSS[Relation and overlap losses]
STATE --> POSE[Full quaternion placement poses]
POSE --> VALIDATE[Placement validators]
POSE --> MESH[Warp mesh collision transforms]
POSE --> CUROBO[cuRobo oriented cuboids]
VALIDATE --> RESULT[Placement result]
MESH --> RESULT
CUROBO --> RESULT
Reviews (1): Last reviewed commit: "introduce oriented bbox" | Re-trigger Greptile |
| return manager.get_query_spheres(box_mesh).to(device) | ||
|
|
||
|
|
||
| def _bbox_proxy_mesh(bbox: OrientedBoundingBox) -> trimesh.Trimesh | None: |
There was a problem hiding this comment.
🔵 Duplicated OBB→box-mesh conversion
This builds a trimesh box from an OBB (extents, quaternion_matrix(np.roll(...)), translate) — the same steps NoOverlapValidator._collision_mesh_or_bbox_proxy runs in placement_validators.py. Could the conversion live once, e.g. as a small OrientedBoundingBox.to_box_mesh() (or a shared helper both call), so the two copies can't drift apart?
| from typing import TYPE_CHECKING, ClassVar, cast | ||
|
|
||
| from isaaclab_arena.relations.collision_mode import CollisionMode, get_object_collision_mode, object_uses_mesh_collision | ||
| from isaaclab_arena.relations.no_overlap_mesh import transform_points_between_frames |
There was a problem hiding this comment.
🔵 Generic geometry helper imported from the collision module
transform_points_between_frames is a plain quaternion frame transform, but it lives in no_overlap_mesh.py and is pulled in here. Would a shared home (e.g. utils/pose or a small transforms util) fit better, so the validator doesn't reach into the collision-loss module for basic geometry?
| radius = torch.bmm(rotation.abs(), self.half_extents.unsqueeze(-1)).squeeze(-1) | ||
| return self.center - radius, self.center + radius | ||
|
|
||
| def is_axis_aligned(self, atol: float = 1e-5) -> torch.Tensor: |
There was a problem hiding this comment.
🔵 is_axis_aligned looks unused
The only caller I can find is its own unit test — no production use. Is it needed now, or can it be dropped until something actually uses it?
🤖 Isaac Lab-Arena Review BotSummaryThis PR replaces the axis-aligned bounding box with a batched Findings🔵 Improvement — Notes (non-blocking)The no-overlap collision loss changes from an overlap-volume product ( Test CoverageGood. The changed suites cover the new OBB math, rotated reference objects, mesh/OBB collision parity, and reachability-cuboid orientation, including negative/identity-fallback cases. These are pure-Python (non-sim) tests, so the inner/outer VerdictShip it (minor cleanups optional). |
Summary
Add oriented bounding box support
##Detailed description