Ryan Schultz
42fdfcf318
also added 'architectural doc'... This document is a dense knowledge-transfer reference for AI agents or developers who need to continue work on the "Join Coplanar Surfaces" feature for Bonsai's SVG drawing generation. It covers the full decision history — every guard, heuristic, and edge case that was discovered and resolved — so that a new contributor can understand *why* the code is shaped the way it is before proposing changes.
316 lines
14 KiB
Markdown
316 lines
14 KiB
Markdown
# fix-3742-coplanar-bonsai-projection_2 — Architectural Decisions
|
||
|
||
## Purpose
|
||
|
||
This document is a dense knowledge-transfer reference for AI agents or
|
||
developers who need to continue work on the "Join Coplanar Surfaces" feature
|
||
for Bonsai's SVG drawing generation. It covers the full decision history —
|
||
every guard, heuristic, and edge case that was discovered and resolved —
|
||
so that a new contributor can understand *why* the code is shaped the way
|
||
it is before proposing changes.
|
||
|
||
---
|
||
|
||
## What the Feature Does
|
||
|
||
Bonsai generates plan/section/elevation SVGs by projecting 3D IFC elements
|
||
through a camera. When two adjacent, same-material elements share a face
|
||
plane, the projection renders the shared boundary as a visible line **in
|
||
both** SVG groups — a duplicate that should not appear in the final drawing.
|
||
|
||
`remove_coplanar_boundary_lines(root)` in
|
||
`src/bonsai/bonsai/bim/module/drawing/operator.py` (method of `CreateDrawing`)
|
||
post-processes the SVG to detect and remove those duplicate line segments.
|
||
|
||
The feature is opt-in: it only runs when **both**
|
||
`cprops.generate_material_layers` and `cprops.join_coplanar_surfaces` are
|
||
enabled (the second toggle lives under "Generate Material Layers" in the
|
||
`BIM_PT_camera` panel, defaults to off).
|
||
|
||
---
|
||
|
||
## Overall Pipeline
|
||
|
||
```
|
||
remove_coplanar_boundary_lines(root)
|
||
│
|
||
├── Parse SVG <g> groups → collect (group_el, mat_ids, face_mat, style, segs, guid)
|
||
│
|
||
└── For every pair (i, j) of groups in the same SVG layer:
|
||
1. mat_keys_match(mat_i, mat_j, face_i, face_j) — same styled face?
|
||
2. style_i == style_j — same SVG stroke style?
|
||
3. are_coplanar_and_adjacent(guid_i, guid_j) — adjacent in 3D?
|
||
4. Bilateral segment matching — find shared SVG segments
|
||
5. Remove shared segments; add back split remnants
|
||
```
|
||
|
||
---
|
||
|
||
## `are_coplanar_and_adjacent` — Decision Pipeline
|
||
|
||
The function returns one of several values (all truthy except `False`):
|
||
|
||
| Return value | Meaning |
|
||
|-----------------|----------------------------------------------------------------------|
|
||
| `False` | Not adjacent or not coplanar — skip pair |
|
||
| `True` | Adjacent, coplanar, on **adjacent parallel** surfaces |
|
||
| `"same_surface"`| Adjacent, coplanar, on the **exact same** surface (identical planes) |
|
||
| `"contained_b"` | Pair passes via AABB containment — `b` (j-element) is inner |
|
||
| `"contained_a"` | Pair passes via AABB containment — `a` (i-element) is inner |
|
||
|
||
### Step-by-step guards
|
||
|
||
#### 1. Object lookup / non-mesh fallback
|
||
If either GUID resolves to no Blender object, or if either object is not a
|
||
MESH, assume adjacent (`True`). Non-mesh objects have no geometry to test.
|
||
|
||
#### 2. AABB guard (fast rejection)
|
||
For each world axis, if the two bounding boxes are separated (one element's
|
||
min > the other's max + tol), return `False` immediately. Avoids expensive
|
||
vertex iteration for elements that are clearly far apart.
|
||
|
||
`tol = 0.01 m` (3D geometry tolerance, separate from SVG tolerance).
|
||
|
||
#### 3. Proximity check (vertex–vertex then vertex–edge)
|
||
|
||
**Why two stages?** Vertex-to-vertex handles the common case (shared corner).
|
||
Vertex-to-edge handles *containment*: when a smaller element (e.g. a pilaster)
|
||
is embedded inside a larger one, the inner element's corners lie on the
|
||
**edges** of the outer element — never at its corner vertices. Checking
|
||
only vertex–vertex would incorrectly reject these pairs.
|
||
|
||
```python
|
||
# Fast path
|
||
has_shared = any((va - vb).length_squared < tol_sq for va in verts_a for vb in verts_b)
|
||
# Containment fallback
|
||
if not has_shared:
|
||
has_shared = vertex_near_edges(verts_b, obj_a, tol_sq) \
|
||
or vertex_near_edges(verts_a, obj_b, tol_sq)
|
||
```
|
||
|
||
`point_to_seg_dist_sq(p, a, b)` projects `p` onto segment `a–b`, clamps `t`
|
||
to `[0, 1]`, returns squared distance.
|
||
|
||
#### 4. AABB containment — skip normal check
|
||
|
||
**Why?** The dominant-normal heuristic (step 5) picks the normal of the
|
||
largest polygon. For a short, flat inner element (e.g. a thin wall stub, or a
|
||
layer plate), the largest polygon is the *top face* → normal points in Z,
|
||
not along the wall face direction. The dot-product check then fails even
|
||
though the elements genuinely share a coplanar face.
|
||
|
||
**Guard:** Before the normal check, test whether one AABB fully encloses the
|
||
other (within tol). If so, return the appropriate `"contained_a"` or
|
||
`"contained_b"` sentinel and skip steps 5–7.
|
||
|
||
```python
|
||
def aabb_contains(outer, inner):
|
||
for axis in range(3):
|
||
if min(c[axis] for c in inner) < min(c[axis] for c in outer) - tol: return False
|
||
if max(c[axis] for c in inner) > max(c[axis] for c in outer) + tol: return False
|
||
return True
|
||
```
|
||
|
||
`"contained_b"` = `aabb_contains(corners_a, corners_b)` → b is inside a
|
||
`"contained_a"` = `aabb_contains(corners_b, corners_a)` → a is inside b
|
||
|
||
**Safety:** Two elements at different depth layers (which the normal check
|
||
was protecting against) can never have one AABB fully nested inside the
|
||
other — so this shortcut is safe to take.
|
||
|
||
#### 5. Dominant normal check
|
||
|
||
```python
|
||
dot = abs(n_a.dot(n_b))
|
||
if dot <= 1.0 - 3.8e-5: # ~0.5° tolerance
|
||
return False
|
||
```
|
||
|
||
Uses the normal of the **largest polygon** (not area-weighted average).
|
||
Area-weighted average fails for slabs because top and bottom faces cancel.
|
||
|
||
#### 6. Face-plane check (parallel offset walls)
|
||
|
||
**Why?** Parallel normals alone don't guarantee the elements share a plane.
|
||
Two walls at an L-corner may both face +Y but be at different Y positions.
|
||
|
||
**Fix:** Project all vertices of *both* elements onto `n_a` (always `n_a`,
|
||
not each onto its own normal — see below). Require at least one scalar
|
||
position from A to be within `tol` of one from B.
|
||
|
||
```python
|
||
plane_pos_a = {round(v.dot(n_a), 5) for v in verts_a}
|
||
plane_pos_b = {round(v.dot(n_a), 5) for v in verts_b}
|
||
same_plane = any(abs(pa - pb) < tol for pa in plane_pos_a for pb in plane_pos_b)
|
||
```
|
||
|
||
**Critical detail — both sets projected onto `n_a`:** If the two normals
|
||
are anti-parallel (+Y vs −Y), projecting each onto its *own* normal produces
|
||
values with opposite signs that never match, even though both faces are on
|
||
the same plane. Projecting both onto `n_a` collapses the sign difference.
|
||
|
||
#### 7. Depth check (same floor level)
|
||
|
||
Projects all vertices onto the camera look-direction (`_cam_look`). Requires
|
||
the depth ranges to overlap by more than `tol`. This catches same-wall-type
|
||
elements on different floor levels whose X–Y positions happen to coincide.
|
||
|
||
#### 8. Same-surface vs adjacent-surface sentinel
|
||
|
||
After all guards pass, the return value encodes *how* coplanar the pair is:
|
||
|
||
```python
|
||
coplanar_result = "same_surface" if plane_pos_a == plane_pos_b else True
|
||
```
|
||
|
||
- `plane_pos_a == plane_pos_b` (identical sets) → elements are on the **exact
|
||
same surface** — same material-layer plane positions along the normal.
|
||
Example: two wall panels side-by-side on the same face.
|
||
- Otherwise → elements are on **adjacent parallel surfaces** — they share one
|
||
plane position at their interface (e.g. the outer face of wall A = inner face
|
||
of wall B at their junction). Example: back-to-back walls at a corner.
|
||
|
||
This distinction drives the `ivs_equal` guard decision in the bilateral loop.
|
||
|
||
---
|
||
|
||
## Bilateral Segment Matching
|
||
|
||
### Segment grouping
|
||
|
||
Segments are grouped by **axis-aligned line key** using `seg_line_key`:
|
||
|
||
```python
|
||
key = ("h", round(y / TOL)) # horizontal
|
||
key = ("v", round(x / TOL)) # vertical
|
||
# Diagonal → None (ignored)
|
||
```
|
||
|
||
`TOL = 0.01` SVG units. All segments within `TOL/2` of the same line get the
|
||
same key. Intervals along the running axis are merged with `ivs_union` before
|
||
comparison.
|
||
|
||
### Matched pair flow
|
||
|
||
For each line key shared by both groups:
|
||
1. Compute `shared = ivs_intersect(union_i, union_j)`.
|
||
2. If no intersection → skip.
|
||
3. Apply `ivs_equal` guard (see below).
|
||
4. Remove **all** original paths at that key from both groups.
|
||
5. Add back `rem_i = ivs_subtract(union_i, shared)` and
|
||
`rem_j = ivs_subtract(union_j, shared)` as new `<path>` elements.
|
||
|
||
### `ivs_equal` guard — offset wall protection
|
||
|
||
**Problem solved:** Two walls meeting at an L-corner, back-to-back on
|
||
**adjacent** parallel planes. In SVG they each have a segment on the shared
|
||
horizontal line (the building corner), but their running-direction extents are
|
||
**offset** — one starts where the other ends, or they partially overlap.
|
||
Without a guard, the overlapping portion would be removed, incorrectly
|
||
eliminating a real architectural boundary line.
|
||
|
||
**Rule:** Only remove the shared portion if it covers the **full running-direction
|
||
extent** of at least one of the two elements on that line.
|
||
|
||
```python
|
||
if not (ivs_equal(shared, union_i) or ivs_equal(shared, union_j)):
|
||
continue # offset overlap — real boundary, keep it
|
||
```
|
||
|
||
**When bypassed:**
|
||
- `_is_contained` (`"contained_a"` / `"contained_b"` pairs): the partial
|
||
overlap between an outer element's long edge and a physically-contained inner
|
||
element's short edge is a genuine shared interface, not an offset-wall corner.
|
||
- `_is_same_surface` (`"same_surface"` pairs): wall panels on the exact same
|
||
surface may have SVG segments that are offset in the running direction
|
||
(due to panel arrangement) yet are still a genuine shared interface.
|
||
|
||
### `_bilateral_had_explicit_keys` flag
|
||
|
||
**Problem solved:** After bilateral finds common line keys but skips them all
|
||
(offset-overlap guard), `matched` remains 0. The unilateral fallback then
|
||
fires with `matched == 0` and removes the same boundary segments the bilateral
|
||
correctly rejected.
|
||
|
||
**Fix:** Set `_bilateral_had_explicit_keys = True` whenever a shared key
|
||
has a non-empty intersection (regardless of whether `ivs_equal` passes).
|
||
Gate the unilateral fallback on `not _bilateral_had_explicit_keys`.
|
||
|
||
### Unilateral bbox fallback
|
||
|
||
When no bilateral key match is found **and** no explicit common keys were
|
||
found, fall back to checking whether any segment of either element lies on
|
||
the bbox boundary of the other element *and* the other element extends past
|
||
that edge. This handles cases where one element has an implicit shared edge
|
||
(no explicit path at that position).
|
||
|
||
Not applied to contained pairs (`_is_contained`) — the bilateral overlap
|
||
check is sufficient there, and the unilateral would over-remove.
|
||
|
||
---
|
||
|
||
## Known Unresolved Case: `2NR8CMK21CwO / 0XMcotQ9nDsx`
|
||
|
||
This AABB-contained pair has one bilateral match (0.099 SVG units) but the
|
||
user sees a remaining visible internal boundary. Analysis showed:
|
||
|
||
- Element i (2NR8CMK21CwO) is in the **lower-left quadrant** of j's plan area.
|
||
- Shared keys `v/11339` and `h/8659` have perfectly **adjacent** (non-overlapping)
|
||
intervals — correctly not removed.
|
||
- i has two **ONLY-I** horizontal segments (`h/8649` at y≈86.49 and `h/9534` at
|
||
y≈95.34) spanning i's full x-width within j's plan area. These are internal
|
||
dividing lines that bilateral cannot find because j has no segment there.
|
||
|
||
To fully resolve this, a "segment enclosed in outer bbox" removal pass would be
|
||
needed for contained pairs. This was not implemented because removing all of
|
||
i's enclosed segments also removes i's left-edge segment at `v/11339` (the
|
||
shared outer boundary), creating visual gaps where j does not have a
|
||
corresponding covering segment. The correct fix requires knowing which of i's
|
||
segments are on the outer boundary of the combined element vs which are purely
|
||
internal — this requires either filled-area reasoning or explicit face topology,
|
||
neither of which is currently available in the SVG post-processing pass.
|
||
|
||
---
|
||
|
||
## Prop / UI Wiring
|
||
|
||
| File | Change |
|
||
|------|--------|
|
||
| `prop.py` | `join_coplanar_surfaces: BoolProperty(default=False)` on camera props |
|
||
| `ui.py` | Toggle shown indented under `generate_material_layers` (only when layers enabled) |
|
||
| `operator.py` | Both BISECT and OPENCASCADE code paths gate on `generate_material_layers and join_coplanar_surfaces` |
|
||
|
||
---
|
||
|
||
## Helper Function Reference
|
||
|
||
| Function | Purpose |
|
||
|----------|---------|
|
||
| `ivs_union(ivs)` | Merge overlapping `(lo, hi)` intervals into sorted non-overlapping list |
|
||
| `ivs_intersect(a, b)` | Pairwise intersection of two interval lists |
|
||
| `ivs_subtract(ivs, sub)` | Remove `sub` intervals from `ivs`, returning remainder |
|
||
| `ivs_equal(a, b)` | True if two interval lists are equal within `TOL` |
|
||
| `seg_line_key(seg)` | Returns `("h", int_key)` or `("v", int_key)` for axis-aligned segments |
|
||
| `seg_interval(seg)` | Returns `(lo, hi)` running-direction interval of a segment |
|
||
| `seg_axis_coord(seg, kind)` | Returns fixed coordinate (x for vertical, y for horizontal) |
|
||
| `segs_bbox(segs)` | AABB `(min_x, max_x, min_y, max_y)` of a parsed segment list |
|
||
| `seg_on_boundary_of(seg, bbox_self, bbox_other)` | True if seg lies on the shared interface edge between two bbox'd elements |
|
||
| `make_seg(kind, coord, lo, hi)` | Construct `((x0,y0),(x1,y1))` from axis/coord/interval |
|
||
| `parse_line(d)` | Parse SVG path `d` attribute in `"Mx,y Lx,y"` format |
|
||
| `dominant_world_normal(obj)` | World-space normal of the largest polygon in a mesh |
|
||
| `point_to_seg_dist_sq(p, a, b)` | Squared perpendicular distance from point to segment |
|
||
| `vertex_near_edges(verts, obj, tol_sq)` | True if any vertex in `verts` lies within `tol_sq` of any edge of `obj` |
|
||
| `aabb_contains(outer, inner)` | True if `inner` bbox fits entirely within `outer` bbox (with tol) |
|
||
|
||
---
|
||
|
||
## Test Cases Resolved on This Branch
|
||
|
||
| Pair | View | Expected | Root cause | Fix |
|
||
|------|------|----------|------------|-----|
|
||
| `08yv0FO / 39Iwvbn` | Plan | No join | Offset unions on adjacent-plane walls | `ivs_equal` guard |
|
||
| `1dM2d4kqjFH8 / 2lwj_pzhrADf` | Plan | Join | Full union match; was broken by `ivs_equal` overshoot | Verify guard passes when unions are equal |
|
||
| `08yv0FO / 39Iwvbn` (unilateral) | Plan | No join | After bilateral skip, unilateral fallback fired | `_bilateral_had_explicit_keys` suppression |
|
||
| `2CH9svq8r2gfCQ / 2u$gvViprDhe` | Plan | Join | Contained pair; `ivs_equal` blocked partial overlap | `_is_contained` bypass |
|
||
| `2NR8CMK21CwO / 0XMcotQ9nDsx` | Plan | Partial join | Contained pair; only 1 of 5 keys has overlapping intervals (0.099 SVG units) | Partial (see unresolved case above) |
|
||
| `2Op$pYptHFmf / 3nXW_ntDPAPv` | Plan | Join | Same-surface panels; `ivs_equal` blocked offset-but-genuine overlap | `_is_same_surface` bypass |
|