fix(urdf,physics): keep each link's collision decomposition

[ecto]

Problem

The URDF importer collapsed each link to a single geometry taken from its first <visual>, and PhysicsWorld derived the collider — and, absent an authored <inertial>, the mass properties — from that one mesh. URDF links routinely declare several <collision> elements, a convex decomposition of the link, and those were ignored entirely.

The pieces exist precisely because the single mesh is a bad collider. XLeRobot's base_link declares 3 visuals and 8 collisions; a cart body approximated by one mesh instead of its 8-piece decomposition gets the wrong contact surface.

Design

IR — PartDef::colliders: Option<Vec<NodeId>>, a list, not a union node. Unioning the pieces back together throws away the decomposition, and CsgOp::Union is a real boolean, so it would weld the meshes into one shape and run the boolean pipeline to do it. A list maps 1:1 onto phyz's Body::collisions: Vec<GeomInstance>, which already existed and which phyz-env's contact pass already iterates. Absent means "the collider is root" — what every non-URDF authoring path wants.

Reader. One DAG root per <collision>, sharing the primitive re-centering and <origin> placement with <visual> through a new geometry_subtree helper. Origins bake into the subtree, so no separate per-collider transform is needed in the IR. <visual> still drives the rendered root; collider roots are never scene entries. A link with no <visual> reuses its first collider root rather than duplicating the subtree.

Physics. from_document_with_colliders was the natural seam — eval_instance now returns a Vec<Geometry>, each piece transformed into the body frame by the same map the visual mesh takes. Body::geometry mirrors the first piece so phyz's single-shape consumers (and the GPU contact pass, which only understands Sphere/Box/Capsule/Cylinder) keep working. Ground contact gathers candidates across all of a body's shapes before capping the manifold at 4 points, matching phyz's own semantics — a body resting on two of eight pieces keeps support under both instead of spending the whole budget on whichever piece was listed first.

An unresolvable collider piece (a package:// mesh with no file behind it) is skipped rather than fatal; if none resolve, the part's own geometry stands in as before.

Mass properties deliberately unchanged — still the part's own geometry or the authored <inertial>. The decomposition describes the contact surface, not the material distribution, and its pieces routinely overlap, so summing their volumes would over-count. There's a test pinning this.

validate_document walks the new collider refs too: nothing else points at those nodes, so a dangling one would otherwise surface as a physics-eval failure far from its cause.

Tests

Against the vendored asset (vcad-kernel-urdf/tests/xlerobot_collision_decomposition.rs): base_link keeps all 8 <collision> pieces and Moving_Jaw all 3 — distinct, resolvable, none doubling as the render root — and each resolves to its own _part*.ply convex mesh rather than the .STL the link renders as.

Unit-level (vcad-kernel-urdf): per-piece <origin> xyz+rpy placement; the render root is the visual, not a collision piece; no collider root leaks into roots; no <collision> ⇒ no colliders; collision-only link reuses piece 0 and contributes exactly one scene entry.

Physics (vcad-kernel-physics/tests/urdf_collision_decomposition.rs): two 50 mm pads separated by a 250 mm gap stay two shapes and neither bridges the gap (any collapse — first-visual, union, or hull — would); collider z-extent proves <collision> won over <visual>; mass still 4 kg from the visual slab, not 0.25 kg from the pads.

Full workspace cargo test green (0 failures), cargo clippy clean on the touched crates, cargo fmt --check clean, npm run ir:check up to date, TS build + tests green.

Merge note

The XLeRobot work landed on main mid-flight and brought its own fix for the same quick-xml prerequisite — a link interleaving <visual>/<collision> (exactly what a decomposed link looks like) failed the whole parse with duplicate field "visual". Main's normalize_link_child_order + group_children_by_tag supersede this branch's reorder_children generalization; they're equivalent in effect and main's handles a self-closing <link .../> more precisely. See the merge commit and this comment.

Known gaps, left untouched

• Multi-visual links still render only <visual>[0]. XLeRobot's base_link has 3. Fixing it needs an IR grouping op — Union is a boolean and would weld the meshes.
• A link with no <visual>, several <collision> pieces, and no <inertial> takes its mass from piece 0 alone. Pre-existing; summing overlapping pieces isn't obviously better, so it's documented in the code rather than changed.
• examples/xlerobot.vcad was imported before this change and so carries no colliders. It still loads and simulates exactly as it does on main (absent ⇒ collider is root); re-importing it would pick up the decomposition.

🤖 Generated with Claude Code

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
vcad-mcp	Building	Preview	Aug 8, 2026 4:14pm

3 Skipped Deployments

Project	Deployment	Actions	Updated (UTC)
mecheval	Ignored		Aug 8, 2026 4:14pm
vcad	Ignored		Aug 8, 2026 4:14pm
vcad-docs	Ignored		Aug 8, 2026 4:14pm

[chojiai]

Choji review — Looks good

The implementation is sound: the IR extension, URDF reader, physics world, and validator all compose correctly, and the test suite pins every invariant the PR description claims. No correctness defects found.

No findings — looks good.

---

Reviewed bb83c08 · Choji updates this comment as you push · Mention @chojiai in a comment to discuss, re-review, or request a fix

[ecto]

Merged main (which landed the XLeRobot work mid-flight) and addressed both Choji nits.

Nit 1 — self-closing link tags swallow the reorder error. Fair, and fixed by deferring to main's implementation rather than mine. Main landed normalize_link_child_order + group_children_by_tag for the same quick-xml prerequisite while this branch was in flight. The two are equivalent in effect, but main's finds the <link> body bounds up front and returns Ok(link_xml) explicitly on Event::Eof for a self-closing tag — so a genuinely malformed link body still propagates its InvalidFormat instead of being caught and passed silently to serde. I dropped my reorder_children/locate_element_bounds generalization and kept main's.

Nit 2 — collision-only link renders a collision piece. This is intended, and unchanged from before this PR: the pre-existing reader already did visuals.first() else collisions.first(). When a link has no <visual>, its collision geometry is the only geometry it has — rendering nothing would be worse. Reusing that subtree as the root also avoids building a second identical copy.

The actionable half of the finding was right though: the path had no scene-entry assertion. collision_only_link_reuses_its_first_piece_as_the_render_root now pins it — exactly one scene entry, pointing at the root, with piece 1 confirmed absent so the other pieces can't leak in and render.

Bonus from the merge: third_party/xlerobot/xlerobot.urdf now actually exists, so the regression test the task originally asked for runs against the real asset instead of a hand-built fixture — crates/vcad-kernel-urdf/tests/xlerobot_collision_decomposition.rs. base_link keeps all 8 <collision> pieces and Moving_Jaw all 3, each resolving to its own _part*.ply convex mesh rather than the .STL the link renders as. The synthetic fixtures stay as unit-level coverage of placement and fallback cases.

Full workspace cargo test green (0 failures), cargo fmt --check clean, ir:check up to date, TS build clean. The vcad-eval/tests/torr_phantom_intersection.rs clippy findings came in with the merge and reproduce on main without this branch's changes.

🤖 Addressed by Claude Code

Dismissing prior approval to re-evaluate 7c099ee.

Dismissing prior approval to re-evaluate bb83c08.