Skip to content

point_inside rescans the target AABB per query while both callers already hold it #3354

Description

@louistrue

Deferred out of #3341 deliberately, to keep that diff to the correctness change.

What

point_inside (rust/geometry/src/kernel/arrangement/classify.rs) calls aabb_of(tris), an O(N) scan over the target triangles, on every query — to get the box that sound_far needs to bound its parity segment.

Both callers already have that box:

  • BComponents::new computes and caches a per-component AABB in self.aabbs, then inside() and solid_side() route through the free point_inside, which rescans from scratch and ignores the cached box.
  • The B-side path already does it correctly: point_inside_bvh takes the box from Bvh::root_aabb(), which is O(1) because the BVH computes it during build anyway.

So the A-side pays an O(N) rescan the B-side avoids, using a box that is sitting on the same struct.

Cost, measured

Under the real release profile (opt-level 3, LTO, codegen-units 1):

triangles before after delta
12 361.1 ns 374.3 ns +3.7%
108 3366.9 ns 3509.8 ns +4.2%
972 28231.6 ns 30317.8 ns +7.4%
25392 728608 ns 755579 ns +3.7%

A second measurement put it at about 11% at 1000+ triangles. Constant-factor, no growth trend with N, because aabb_of is ~18 cheap compares per triangle against exact_seg_hits_tri's 2+ orient3d calls per triangle.

It is not a rare path. boolean_vids wraps every binary boolean — union, intersection and difference, not just batched difference — in a 1-element BComponents, so the rescan is paid on the hottest path in the kernel.

Fix

Change point_inside to accept the (lo, hi) box instead of recomputing it, and pass self.aabbs[k] from BComponents::inside / solid_side. No soundness risk: a cached or padded box is still a valid superset for the containment check sound_far does, and escalation counts were byte-identical in the measurement.

union_all in boolean.rs has the same shape: it hoists operand_extent once per mesh but not the box, so aabb_of reruns per sub-triangle probe against each other mesh instead of once.

Not urgent

This is cost, not correctness. CSG is already the load bottleneck in this codebase, so a few percent on the hottest kernel path is worth reclaiming, but it should be its own change with its own before/after numbers rather than riding along with a correctness fix.

Metadata

Metadata

Assignees

No one assigned

    Labels

    rustPull requests that update rust code

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions