Skip to content

Add Collision Check for Background Objects - #835

Merged
zhx06 merged 21 commits into
mainfrom
zxiao/feature/background_collision_check
Jul 14, 2026
Merged

Add Collision Check for Background Objects#835
zhx06 merged 21 commits into
mainfrom
zxiao/feature/background_collision_check

Conversation

@zhx06

@zhx06 zhx06 commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Add background collision checking to relation-based object placement

Detailed description

  • Background objects without relations were invisible to the placement solver, so placed objects could overlap furniture.
  • Fixed, relation-free scene geometry is now treated as passive obstacles during placement; in MESH mode fixtures merge into one aggregate collision mesh, with AABB fallback for meshless objects.
  • Placed objects now clear nearby fixtures (RoboCasa kitchen example below).
kitchen_place_and_view

@greptile-apps

greptile-apps Bot commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR introduces passive background-object collision checking into the placement solver pipeline. Previously, relation-free fixed geometry (e.g. kitchen cabinets, walls) was invisible to the solver and placed objects could overlap it; now such objects are treated as immovable obstacles whose bounding boxes the solver must avoid.

  • New background_colliders.py module provides build_placement_region (computes the XY/Z envelope above anchor surfaces) and find_background_colliders (spatially culls background USD prims to only those intersecting that region), keeping the number of new collision objects small.
  • RelationSolverState now caches world bboxes for all fixed obstacles (anchors + background collision objects) once at construction, replacing the per-gradient-step get_world_bounding_box() calls flagged in a prior review comment.
  • Scene.get_collision_objects() auto-discovers relation-free, USD-backed assets with a fixed pose and feeds them as obstacles to solve_and_apply_relation_placement, wiring the feature into ArenaEnvBuilder with no additional user code required for the common case.

Confidence Score: 5/5

Safe to merge; the change is purely additive and defaults to no-op behaviour when no collision objects are discovered.

The bounding-box math through both the culling path (ComputeWorldBound → scale → rotate → translate) and the ObjectReference path (local bbox → scale → rotate → translate by world position) is algebraically equivalent and confirmed by seven dedicated unit tests. The prior performance concern about per-step world-bbox recomputation is addressed by the new _fixed_obstacle_world_bboxes cache in RelationSolverState. When collision_objects is empty or omitted the code falls back to the existing behaviour exactly, so there is no regression risk for existing scenes.

No files require special attention.

Important Files Changed

Filename Overview
isaaclab_arena/relations/background_colliders.py New module: build_placement_region computes the AABB envelope above anchor surfaces; find_background_colliders walks the background USD, spatially culls prims to the region, and drops enclosing shells. The bounding-box pipeline (ComputeWorldBound → scale → rotate → translate) mirrors what ObjectReference.get_world_bounding_box() produces, keeping culling consistent with the solver view.
isaaclab_arena/relations/relation_solver_state.py Adds collision_objects storage and pre-computes world bboxes for all fixed obstacles (anchors + background) once in init, resolving the per-step get_world_bounding_box() performance concern from a previous review. The disjoint-set assertion correctly guards against an object appearing both as an optimizable and a fixed obstacle.
isaaclab_arena/relations/object_placer.py collision_objects is threaded through place(), get_ranked_placement_candidates_per_env(), _solve_and_rank_placement_candidates(), and _validate_no_overlap(). The new background-overlap check in _validate_no_overlap correctly skips anchors and uses the same clearance margin as the pairwise check.
isaaclab_arena/scene/scene.py get_collision_objects() applies six distinct filters (type, Background subclass, has-relations, usd_path, fixed Pose) to auto-discover passive collision obstacles; each filter is now covered by _test_scene_get_collision_objects_filters, addressing the prior review comment requesting this test.
isaaclab_arena/assets/object_reference.py get_world_bounding_box() is refactored to rotate by the parent's orientation rather than the prim's composed world orientation; mathematically equivalent for the supported axis-aligned use case and verified by two new unit tests. isaaclab_prim_path_to_original_prim_path is promoted to a @staticmethod (no behavior change).
isaaclab_arena/relations/relation_solver.py _compute_no_overlap_loss now iterates fixed_obstacles = anchors + collision_objects, using the cached state.get_fixed_obstacle_world_bbox() for both. The on_pairs skip set correctly omits collision objects (they can never be On-relation parents), so placed objects are always checked against background obstacles.
isaaclab_arena/relations/pooled_object_placer.py Stores collision_objects in init and forwards them to get_ranked_placement_candidates_per_env on each pool fill; multi-env correctness is verified by the new test_pooled_object_placer_multi_env_avoids_obstacle test.
isaaclab_arena/tests/test_background_colliders.py Seven tests covering build_placement_region padding, ObjectReference world bbox (no parent pose, 90° yaw), and find_background_colliders (in-region, out-of-region, enclosing shell, anchor exclusion, group recursion, explicit prim paths, nested paths).
isaaclab_arena/tests/test_relation_solver_background_collision.py Nine tests exercising the solver (loss accumulation, full solve push-off, no-op without obstacles), state validation (disjoint assertion), ObjectPlacer validation, and PooledObjectPlacer (single-env and multi-env).
isaaclab_arena/environments/arena_env_builder.py Minimal change: calls get_collision_objects() and forwards the result to solve_and_apply_relation_placement. No existing logic is altered.
isaaclab_arena_examples/relations/isaac_sim_kitchen_background_collision_notebook.py New end-to-end example demonstrating the feature on the RoboCasa kitchen counter; includes a smoke test that asserts at least one fixture (the sink) is discovered.

Reviews (7): Last reviewed commit: "change filtering mechanism" | Re-trigger Greptile

Comment thread isaaclab_arena/scene/scene.py Outdated
Comment thread isaaclab_arena/relations/relation_solver.py Outdated
Comment thread isaaclab_arena/scene/scene.py Outdated
Comment thread isaaclab_arena/tests/test_relation_solver_background_collision.py Outdated
@zhx06
zhx06 force-pushed the zxiao/feature/background_collision_check branch from 5ae33dc to 9deb1a8 Compare July 1, 2026 18:08

@arena-review-bot arena-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Isaac Lab-Arena Review Bot

Summary

This PR teaches the placement solver to treat relation-free background geometry as fixed collision obstacles, so placed objects no longer overlap furniture/fixtures. It threads a collision_objects list through the interface → placer → pooled placer → solver → state, caches the fixed obstacles' world bounding boxes once per solve, and adds Scene.get_collision_objects() plus a USD-fixture discovery helper. The solver-side logic (fixed-obstacle no-overlap loss, validation, disjointness assertion) is clean and well tested. My main note is a design question about two parallel discovery mechanisms, plus a couple of small style points.

Design, Boundaries & Scope

The PR ships two ways to obtain collision objects that never meet:

  • the automatic builder path (_solve_relationsScene.get_collision_objects()), which passes every relation-free scene asset to the solver as an obstacle with no spatial culling; and
  • the region-culling helpers in background_colliders.py (build_placement_region + find_background_colliders), which are invoked only from the example notebook.

background_colliders.py's own module docstring motivates itself as keeping the solver cheap by not "feeding every prim in the scene into the all-pairs no-overlap term" — yet the production path does exactly the un-culled thing (over whole assets). Is the intent for the builder to eventually use the discovery/culling helpers, or is coarse whole-asset collision the intended automatic default with fine-grained discovery reserved for manual use? Worth a short comment either way so the two paths don't silently drift. Not a blocker — both paths are individually correct.

Findings

See inline comments.

Test Coverage

Strong. Solver loss/solve, placer + pooled-placer forwarding, validation reject/accept, Scene.get_collision_objects() filtering, and both discovery helpers are covered. Pure-solver tests correctly run without the sim-app harness (the solver is sim-agnostic), and the USD/Background tests correctly use the run_simulation_app_function inner/outer pattern and land in Phase 1 (headless, no cameras/subprocess). No coverage gaps worth blocking on.

Verdict

Minor fixes needed

Comment thread isaaclab_arena/relations/background_colliders.py Outdated
Comment thread isaaclab_arena/environments/relation_solver_interface.py Outdated
Comment thread isaaclab_arena/environments/arena_env_builder.py Outdated

@arena-review-bot arena-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Isaac Lab-Arena Review Bot

Summary

This teaches the placement solver to avoid static scene geometry that carries no relations: Scene.get_collision_objects() collects relation-free, fixed-pose assets and threads them through the placer/solver/validation as passive fixed obstacles, plus an opt-in background_colliders module for region-culled per-fixture discovery. The implementation is clean, the bbox-caching is a nice touch, and the tests cover the solver loss, the full solve, validation, and the scene filter well. My main question is about blast radius — the automatic path changes the default placement behavior for every build (details below); the code itself I couldn't fault.

Design, Boundaries & Scope

The automatic path changes default placement behavior for everyone. _solve_relations now calls get_collision_objects() on every build, so any relation-free, fixed-pose Object/ObjectReference becomes a hard no-overlap constraint by default. Two things make me want to confirm this is intended:

  • Coarse whole-asset AABBs can over-reject. A large or concave relation-free background prop contributes its full axis-aligned box, so placements in empty space within that box are now rejected even though nothing physically overlaps.
  • The failure mode is quiet. When the added constraints can't be satisfied, solve_and_apply_relation_placement falls back to best-loss layouts that failed strict validation and only prints a warning (had_fallbacks). The solver's overlap-volume loss is also flat when an object starts fully enclosed by an obstacle (the PR's own test notes this), so an object spawned inside a coarse background box may not get pushed out — it just lands in the fallback path. Net effect: scenes that placed cleanly before could now silently degrade to fallback layouts.

Given the PR's goal is to change this default, this may well be acceptable — but could it be worth gating (e.g. a builder/CLI flag defaulting the automatic path off, or on but opt-out) so existing scenes aren't surprised? At minimum, is the increased fallback frequency something you've checked against the existing environment suite?

Separately, relations/background_colliders.py (the fine-grained region-culling path) is currently wired up only by the example notebook and its own tests — nothing in core uses it. Is it intended as public API for task authors now, or could it live alongside the example until a real task consumes it? (Just questioning whether the extra core surface earns its place yet.)

Findings

See the inline comment on arena_env_builder.py.

Test Coverage

Good. Pure-solver/validation tests (test_relation_solver_background_collision.py) correctly avoid the sim harness; the ones needing USD/Scene (_test_scene_get_collision_objects_filters, test_background_colliders.py) use the inner/outer run_simulation_app_function pattern, matching the existing no-marker Phase-1 convention for in-process sim tests. Coverage spans the loss term, the full solve, ObjectPlacer/PooledObjectPlacer forwarding, and the scene filter, including negative cases. No gaps I'd block on.

Verdict

Minor fixes needed — mainly confirming the default-behavior change is intended (and ideally checked against the existing scene suite).

Comment thread isaaclab_arena/environments/arena_env_builder.py Outdated

@arena-review-bot arena-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Isaac Lab-Arena Review Bot

Summary

This PR makes the placement solver avoid static background geometry, not just anchors: a new collision_objects channel threads fixed obstacles through RelationSolverStateRelationSolverObjectPlacer/PooledObjectPlacer, Scene.get_collision_objects() auto-discovers relation-free fixed assets, and relations/background_colliders.py offers an opt-in fine-grained culling path for large scenes. The solver/state changes are clean (constant-bbox caching, disjoint-set assert, gradient only to placed objects) and well tested across the pure-Python and sim paths. My main concern is a correctness gap in the opt-in region culling, plus a question on where background_colliders belongs.

Design, Boundaries & Scope

relations/background_colliders.py is the only module in relations/ that opens USD stages and walks prims (open_stage, UsdGeom.BBoxCache) — the solver files there are pure torch. Its actual job is reading background USD geometry and emitting ObjectReferences, which reads more like scene/asset work sitting next to Scene.get_collision_objects(), and it's currently consumed only by the example notebook. Would scene/ (or an assets util) be a more natural home, keeping relations/ as pure placement math? Not a blocker — a question worth a sentence in the PR.

One heads-up (not a finding): _solve_relations now calls get_collision_objects() unconditionally, so every relation-solving scene with a relation-free, fixed-pose Object/ObjectReference gains new obstacles by default. That's the PR's intent and had_fallbacks still surfaces failures, so it's not silent — just worth confirming no existing scene relies on the old overlap-with-furniture behavior.

Findings

🟡 Warning: isaaclab_arena/relations/background_colliders.py:51build_placement_region pads the region only in Z; an object placed On an anchor can straddle its edge (more so under random_yaw_init), so a wall/backsplash just outside the anchor footprint is culled yet still collidable — contradicting the module's own "a nearby wall or backsplash is a real constraint". Suggest padding XY by the largest object's half-extent + clearance. (inline)

🔵 Improvement: isaaclab_arena/tests/test_background_colliders.py:195_test_build_placement_region is pure (MagicMock + torch), so it doesn't need a persistent SimApp; its siblings in test_relation_solver_background_collision.py run as plain functions. (inline)

Test Coverage

Strong. Pure solver/state/placer behavior is covered without a SimApp (loss present/absent, full solve pushes off obstacle, disjoint-object assert, validation accept/reject, pooled multi-env), and the USD-discovery + get_world_bounding_box paths run under the persistent-SimApp inner/outer pattern (Phase 1 — correct, since run_simulation_app_function runs in-process, not via with_subprocess). New-file copyright years (2026) are correct. The kitchen example has a real-asset smoke test asserting the sink is discovered.

Verdict

Minor fixes needed

Comment thread isaaclab_arena/relations/background_colliders.py Outdated
Comment thread isaaclab_arena/tests/test_background_colliders.py Outdated

@arena-review-bot arena-review-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Isaac Lab-Arena Review Bot

Summary

This PR makes the placement solver aware of fixed background geometry so placed objects stop overlapping furniture, with a coarse whole-asset path enabled automatically inside solve_relations and an opt-in per-fixture culling module (background_colliders) for large scenes. The implementation is clean, the get_world_bounding_box fix is correct (the local bbox is already axis-aligned in the parent's frame, so applying only the parent rotation is right), and test coverage is thorough. One design question below about the default-behavior change.

Design, Boundaries & Scope

The new obstacle-avoidance is wired unconditionally into _solve_relations (arena_env_builder.py:88): whenever solve_relations is enabled, every relation-free fixed Object/ObjectReference now becomes a collision obstacle. That's a good correctness improvement, but it silently changes the solved layout for any existing scene that already had such objects — placed positions (and thus the layouts a given placement_seed reproduces) will shift, and placements that previously succeeded may now fall back if the fixed object blocks the region. For an eval library that's a reproducibility consideration worth a deliberate call. Is the always-on coarse path the intended default, or should it be gated behind a flag (default off) so existing benchmarks don't move without an explicit opt-in? Flagging as a question, not a blocker — inline at the call site.

Findings

No correctness issues found. See the design question above.

Test Coverage

Strong. test_background_colliders.py covers region construction, spatial culling (near kept, far/behind culled, enclosing shell dropped, anchor excluded), explicit and nested prim paths, and the two get_world_bounding_box frames — all via the inner/outer run_simulation_app_function pattern, matching the sibling example tests. test_relation_solver_background_collision.py covers the pure-torch solver/validation paths (loss presence, full-solve escape, the disjoint-object assertion, and the pooled multi-env production path) with substantive assertions against actual overlap. The kitchen smoke test asserts a real fixture (sink_main_group) is discovered. Nothing missing.

Verdict

Minor fixes needed — confirm the default-behavior intent; otherwise ship it.

Comment thread isaaclab_arena/environments/arena_env_builder.py Outdated
@zhx06
zhx06 force-pushed the zxiao/feature/background_collision_check branch 3 times, most recently from 5b87fba to 5c04c72 Compare July 7, 2026 23:44

@qianl-nv qianl-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Zihao, change is well structured and thorough.

A few nits, feel free to merge after fixing.

One question: is the goal for v0.3 to switch to mesh mode by default? Are we still keeping AABB mode then?
This MR currently don't provide a user interface through CLI/config to switch to the mesh mode with policy_runner/eval_runner scripts. What's your plan for it?

Comment thread isaaclab_arena/scene/scene.py Outdated
Comment thread isaaclab_arena/assets/object_base.py Outdated
Comment thread isaaclab_arena/relations/placement_events.py Outdated
Comment thread isaaclab_arena/relations/background_collision_object.py Outdated
Comment thread isaaclab_arena/relations/background_collision_object.py Outdated
Comment thread isaaclab_arena/scene/scene.py Outdated

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review #1.

The results look great!

I have a few comments. I'll have another review after you take a look.

Comment thread isaaclab_arena/assets/dummy_object.py Outdated
Comment thread isaaclab_arena/assets/object_reference.py Outdated
Comment thread isaaclab_arena/assets/object_reference.py Outdated
Comment thread isaaclab_arena/assets/object_reference.py
Comment thread isaaclab_arena/scene/scene.py Outdated
Comment thread isaaclab_arena/relations/background_collision_object.py Outdated
Comment thread isaaclab_arena/relations/background_collision_object.py Outdated
Comment thread isaaclab_arena/relations/placement_events.py Outdated
Comment thread isaaclab_arena/relations/placement_events.py Outdated
Comment thread isaaclab_arena/relations/relation_solver.py Outdated
@zhx06
zhx06 force-pushed the zxiao/feature/background_collision_check branch 2 times, most recently from f0e916a to 5d62ac6 Compare July 9, 2026 19:30

@alexmillane alexmillane left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few more small comments. Feel free to merge once addressed.

Comment thread isaaclab_arena/assets/object_reference.py
Comment thread isaaclab_arena/environments/arena_env_builder.py Outdated
Comment thread isaaclab_arena/environments/arena_env_builder.py Outdated
Comment thread isaaclab_arena/relations/background_collision_object.py Outdated
@zhx06
zhx06 force-pushed the zxiao/feature/background_collision_check branch 2 times, most recently from 3a37c8b to 03cf56e Compare July 14, 2026 00:33
zhx06 added 7 commits July 13, 2026 19:21
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
zhx06 added 14 commits July 13, 2026 19:21
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
Signed-off-by: zhx06 <zihaox@nvidia.com>
@zhx06
zhx06 force-pushed the zxiao/feature/background_collision_check branch from 03cf56e to 67e9c0c Compare July 14, 2026 02:22
@zhx06
zhx06 merged commit 1afe366 into main Jul 14, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants