Skip to content

fix(boolean): an operand touching itself along two coincident edges is glued before the merge (WR-754, #3160) - #13576

Closed
bherbruck wants to merge 3 commits into
mainfrom
moved/solidcore-3167
Closed

bherbruck wants to merge 3 commits into
mainfrom
moved/solidcore-3167

Conversation

@bherbruck

@bherbruck bherbruck commented Nov 8, 2026 •

Copy link
Copy Markdown
Collaborator

Moved from tangency-labs/solidcore#3167

Fixes #13650 (solidcraft#12785).

The kernel's own results carry a solid that touches itself along a line (a hole's bottom meeting two pocket walls along a segment of their corner edge) as one edge of all four faces, and the merge handles that form. The record format writes such an edge as one edge per face pair (split_touching), and the body SolidCraft hands its next feature comes back with two coincident edges of two faces each; it passes the check, but the merge of a tool whose edge lies along that line gives a body that is not a closed solid (default fixes) or fails in topology_intersector::p1::relate with 1001 (SolidCraft's fix set).

New opt-in flag Kernel::fixes.boolean_glues_self_touching_edges: merge_boolean_by first glues, in each operand, the coincident edges that share both vertices, the curve kind and their points (three samples within the B-rep tolerance) and whose faces are all distinct, into one edge (GlueEdgesGeneral, no gap, with region nesting), in one shell or in two shells touching along the line (the round trip leaves the self-touching solid in two), restoring the kernel's own form. Edges whose faces repeat (the kernel's crack edges, which its check refuses glued) are left alone. Off by default: upstream merges the operand as given. Record: plan/upstream-issues/WR-754--boolean-operand-touching-itself-along-two-coincident-edges.md.

SolidCraft follow-up once this merges: add the flag to its fix list (crates/smlib/src/core.rs).

Changes a default: no

Tests: crates/solidcore-ops/tests/boolean_join_fills_pocket_with_hole_at_its_corner.rs (committed first, failing): the reporter's block, pocket and hole built with the ops crate; the join of the cut's own body (passes without the flag), the join after a record round trip with the flag (fails without it: not a closed solid), and the control without the hole. Also run, laptop: clippy on solidcore and solidcore-ops (all targets) clean, cargo fmt clean on the changed files, the boolean unit tests 76/76, the ops boolean tests 5/5. Pending: the full suites and the kernel cases on the remote host; results follow in a comment.

@dudedagger42, this is your PR moved here. If you can't push to moved/solidcore-3167, open a PR from your fork against kernel that says 'Replaces #13576' (see CONTRIBUTING, 'Updating a PR that was moved from solidcore').

dudedagger42 and others added 3 commits November 8, 2026 16:31
…fter a record round trip

solidcraft#12785: a block with a hexagonal pocket through it and a 2.5 mm hole
drilled from an end face that ends exactly at a corner of the pocket, so the
hole's bottom meets the two pocket walls along a segment of the corner edge:
a solid touching itself along a line. The cut leaves that segment as one edge
of four faces, and filling the pocket with the same hexagon as a join works.
After a record round trip (`to_record`, `from_record`: the body SolidCraft
hands its next feature) the segment is two coincident edges of two faces each,
the body still passes the check, and the join fails: with the default fixes
the result is not a closed solid, with SolidCraft's fix set
`topology_intersector::p1::relate` fails (status 1001).

The operands are built with the ops crate from the reporter's numbers (the
Opus trial's prejoin.sc). Controls: the cut's own body joins; the pocket
without the hole joins after the round trip.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s glued before the merge (WR-754, #3160)

The kernel's own results carry a solid that touches itself along a line as one
edge of all four faces, and the merge handles that form. The record format
writes such an edge as one edge per face pair (`split_touching`), and the body
SolidCraft hands its next feature comes back with two coincident edges of two
faces each: the merge of a tool whose edge lies along that line then gives a
body that is not a closed solid, or fails in
`topology_intersector::p1::relate` with 1001 (solidcraft#12785: a hexagon
pocket joined back where a hole ends at its corner).

New opt-in flag `Kernel::fixes.boolean_glues_self_touching_edges`:
`merge_boolean_by` first glues, in each operand, the coincident edges that
share both vertices, the curve kind and their points (three samples within
the B-rep tolerance) and whose faces are all distinct, into one edge
(`GlueEdgesGeneral`, no gap, with region nesting), in one shell or in two
shells touching along the line (the round trip leaves the solid in two). Edges whose faces repeat
(the kernel's crack edges) are left alone. Off by default: upstream merges the
operand as given.

Record: plan/upstream-issues/WR-754--boolean-operand-touching-itself-along-two-coincident-edges.md.
Test: crates/solidcore-ops/tests/boolean_join_fills_pocket_with_hole_at_its_corner.rs
(the round-trip case sets the flag; fails without it).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…review)

`glue_self_touching_edges` compared every edge pair and kept its glued edges
in a Vec, close to cubic on a body of thousands of edges, on every boolean
while the flag is on. The edges are now bucketed by their vertex pair, only
a bucket of two or more edges has its curves evaluated and its pairs
compared, and the glued edges are a HashSet. A body without self-touch costs
one pass over its edges.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 8, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

@github-actions github-actions Bot added the area:kernel B-rep kernel, booleans, blends, STEP/IGES label Nov 8, 2026
@dudedagger42

Copy link
Copy Markdown
Collaborator

On the solidcore tree before the move (0c222ff64; the same commits are on moved/solidcore-3167, rebased onto 9b559c4).

Kernel cases, whole corpus, with the harness's case budget raised to 1800 s (KERNEL_CASES_TIMEOUT_S), release, laptop, 5601 s: 1079 cases, 799 pass, 0 fail, 263 unsupported, 0 unported, 17 known-failing; fillets/cov-fillet-section-curvature-boss-root-r2 passes inside that budget. The harness still asserts, on one known-failing case whose result moved from the rig's recording: fillets/report-peg-front-and-concave-chamfers-together-d2, volume 9383.450962 against the rig's 9383.450660 (3e-8 relative). The glue code runs only behind boolean_glues_self_touching_edges, which the corpus's fixes.json does not set, so that movement is the base's (the solidcore main this branch sits on), not this PR's.

fillets/cov-fillet-section-curvature-boss-root-r2 alone (1800 s budget, release, laptop, back to back in one run): with boolean_glues_self_touching_edges off (the corpus's fixes.json as committed) 616 s; with the name appended to that list, on, 810 s. Both ran while another release build and a corpus run shared the laptop (the on run overlapped more of it), so the gap is not a measurement of the pre-pass; the release timing of the pre-pass on a 1600-sided prism (above) showed none. A quiet repeat on an idle machine is listed for the kernel lanes. The harness's fix set is the corpus's fixes.json (the rig's 297 names), so the flag is off in every corpus run as committed; the case's slowness is solidcore main's since 99ed57c (tangency-labs/solidcore#3238, filed before the move).

Base automatically changed from kernel to main November 9, 2026 07:51
brett19 added a commit to brett19/solidcraft that referenced this pull request Nov 14, 2026
…er the whole edge (tangency-labs#13650)

The glue of an operand's coincident self-touch edges before the merge (carried from
tangency-labs#13576) is now unconditional: the `boolean_glues_self_touching_edges` flag is gone.

Two edges are glued only when their curves are proved within the B-rep tolerance of
each other over their whole intervals, never on samples (`coincidence_bound`, rounding
included, a NaN refuses):
- two lines: the larger distance between their segments' ends (either pairing), each
  end widened by its evaluation error (`Line::segment_over`);
- two non-rational B-splines on the same interval inside both domains: the convex-hull
  bound of `nurbs::crv_deviation::certified_distance`;
- any other pair: no bound, not glued (the operand goes to the merge as given).

The buckets and the glued set are a BTreeMap and BTreeSet. The tangency-labs#13576 tests move into
the boolean area file (`tests/boolean.rs`, module `pocket_with_hole_at_its_corner`), the
round-trip case now runs with no fixes and with every fix, and new unit tests check
both bounds, including a bump between the old sample points that three samples
accepted and the bound refuses.

Carries tangency-labs#13576 by @bherbruck.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU
@brett19

brett19 commented Nov 14, 2026

Copy link
Copy Markdown
Contributor

@bherbruck, #16392 carries this PR's three commits, cherry-picked with their authorship and credited. We did this under the solidcore stall rule: we poked you on #13650 at 2026-11-13 22:34Z and had no reply within 15 minutes. On top of your commits, #16392 removes the boolean_glues_self_touching_edges flag, so the glue is always on. It also replaces the 3-sample coincidence test with a bound over each edge's whole span: an end-point bound for lines, certified_distance for non-rational B-splines, rounding included, and NaN refuses. Any other pair is not glued. Your test moves into solidcore-ops/tests/boolean.rs.

Both PRs close #13650, so the duplicate-PR check flags #16392. Could you close #13576 in favour of #16392? If you want both kept open, please add not-a-duplicate instead.

Written by Claude Code (Anthropic) on behalf of @brett19.

brett19 added a commit to brett19/solidcraft that referenced this pull request Nov 14, 2026
…er the whole edge (tangency-labs#13650)

The glue of an operand's coincident self-touch edges before the merge (carried from
tangency-labs#13576) is now unconditional: the `boolean_glues_self_touching_edges` flag is gone.

Two edges are glued only when their curves are proved within the B-rep tolerance of
each other over their whole intervals, never on samples (`coincidence_bound`, rounding
included, a NaN refuses):
- two lines: the larger distance between their segments' ends (either pairing), each
  end widened by its evaluation error (`Line::segment_over`);
- two non-rational B-splines on the same interval inside both domains: the convex-hull
  bound of `nurbs::crv_deviation::certified_distance`;
- any other pair: no bound, not glued (the operand goes to the merge as given).

The buckets and the glued set are a BTreeMap and BTreeSet. The tangency-labs#13576 tests move into
the boolean area file (`tests/boolean.rs`, module `pocket_with_hole_at_its_corner`), the
round-trip case now runs with no fixes and with every fix, and new unit tests check
both bounds, including a bump between the old sample points that three samples
accepted and the bound refuses.

Carries tangency-labs#13576 by @bherbruck.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU
@bherbruck

Copy link
Copy Markdown
Collaborator Author

Closing in favour of #16392, which carries these commits and finishes the fix (the glue is decided by a proof over the whole span, and there's no flag). Thanks @brett19.

@bherbruck bherbruck closed this Nov 14, 2026
xznhj8129 pushed a commit to xznhj8129/solidcraft that referenced this pull request Nov 14, 2026
…to one edge first (tangency-labs#13650) (tangency-labs#16392)

* test(boolean): a join filling a pocket whose corner a hole reaches, after a record round trip

solidcraft#12785: a block with a hexagonal pocket through it and a 2.5 mm hole
drilled from an end face that ends exactly at a corner of the pocket, so the
hole's bottom meets the two pocket walls along a segment of the corner edge:
a solid touching itself along a line. The cut leaves that segment as one edge
of four faces, and filling the pocket with the same hexagon as a join works.
After a record round trip (`to_record`, `from_record`: the body SolidCraft
hands its next feature) the segment is two coincident edges of two faces each,
the body still passes the check, and the join fails: with the default fixes
the result is not a closed solid, with SolidCraft's fix set
`topology_intersector::p1::relate` fails (status 1001).

The operands are built with the ops crate from the reporter's numbers (the
Opus trial's prejoin.sc). Controls: the cut's own body joins; the pocket
without the hole joins after the round trip.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(boolean): an operand touching itself along two coincident edges is glued before the merge (WR-754, tangency-labs#3160)

The kernel's own results carry a solid that touches itself along a line as one
edge of all four faces, and the merge handles that form. The record format
writes such an edge as one edge per face pair (`split_touching`), and the body
SolidCraft hands its next feature comes back with two coincident edges of two
faces each: the merge of a tool whose edge lies along that line then gives a
body that is not a closed solid, or fails in
`topology_intersector::p1::relate` with 1001 (solidcraft#12785: a hexagon
pocket joined back where a hole ends at its corner).

New opt-in flag `Kernel::fixes.boolean_glues_self_touching_edges`:
`merge_boolean_by` first glues, in each operand, the coincident edges that
share both vertices, the curve kind and their points (three samples within
the B-rep tolerance) and whose faces are all distinct, into one edge
(`GlueEdgesGeneral`, no gap, with region nesting), in one shell or in two
shells touching along the line (the round trip leaves the solid in two). Edges whose faces repeat
(the kernel's crack edges) are left alone. Off by default: upstream merges the
operand as given.

Record: plan/upstream-issues/WR-754--boolean-operand-touching-itself-along-two-coincident-edges.md.
Test: crates/solidcore-ops/tests/boolean_join_fills_pocket_with_hole_at_its_corner.rs
(the round-trip case sets the flag; fails without it).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* fix(boolean): the self-touch glue buckets edges by vertex pair (tangency-labs#3167 review)

`glue_self_touching_edges` compared every edge pair and kept its glued edges
in a Vec, close to cubic on a body of thousands of edges, on every boolean
while the flag is on. The edges are now bucketed by their vertex pair, only
a bucket of two or more edges has its curves evaluated and its pairs
compared, and the glued edges are a HashSet. A body without self-touch costs
one pass over its edges.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* solidcore: the self-touch glue is always on and proves coincidence over the whole edge (tangency-labs#13650)

The glue of an operand's coincident self-touch edges before the merge (carried from
tangency-labs#13576) is now unconditional: the `boolean_glues_self_touching_edges` flag is gone.

Two edges are glued only when their curves are proved within the B-rep tolerance of
each other over their whole intervals, never on samples (`coincidence_bound`, rounding
included, a NaN refuses):
- two lines: the larger distance between their segments' ends (either pairing), each
  end widened by its evaluation error (`Line::segment_over`);
- two non-rational B-splines on the same interval inside both domains: the convex-hull
  bound of `nurbs::crv_deviation::certified_distance`;
- any other pair: no bound, not glued (the operand goes to the merge as given).

The buckets and the glued set are a BTreeMap and BTreeSet. The tangency-labs#13576 tests move into
the boolean area file (`tests/boolean.rs`, module `pocket_with_hole_at_its_corner`), the
round-trip case now runs with no fixes and with every fix, and new unit tests check
both bounds, including a bump between the old sample points that three samples
accepted and the bound refuses.

Carries tangency-labs#13576 by @bherbruck.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU

* solidcore: the self-touch glue never glues a crack edge or a copy sharing a glued face (tangency-labs#13650)

Review of tangency-labs#16392: in a bucket of three or more coincident edges, each copy was checked
only against the first edge's faces, so two copies on one face could both glue in and
leave that face on both sides of the glued edge. `copy_pairs` (now a separate function)
checks each copy against the faces of the first edge and of every copy already glued.
An edge with a face on both sides (more than two of its uses on one face: the kernel's
crack edge) is never glued, as the doc already said. Unit test:
`copies_are_checked_against_every_face_already_glued`.

The errors of `GlueEdgesGeneral` still fail the boolean, now documented: the pairs meet
its precondition, so what is left are broken topology links or notification failures,
and a half-glued operand cannot be merged as given.

New ops test: two solids touching along an edge, after a record round trip, give the
exact volumes under a cut and an intersection across the edge and a union far from it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU

* solidcore: a self-touch gluing that fails leaves the operand as given (tangency-labs#13650)

Review of tangency-labs#16392: `GlueEdgesGeneral` can fail on a valid operand, before its first
topology change (notifications, `curve_max_distance_between` of the same-geometry test)
and after it (`set_tolerance`, `merge_shells`, `test_closure_split_region_attempt`,
whose statuses pass on under `brep_edit_statuses_pass_on`). The gluing is now all or
nothing: the operand is snapshotted before the first pair, and an ordinary failure
(`error::native_failure`) restores it exactly, so the merge runs on it as on main. Port
defects and cancellation still propagate; no gluing when no snapshot can be taken.
The doc lists every site and whether it comes before or after the first change.

WR-754 now points at the test's new home, `tests/boolean.rs`
`mod pocket_with_hole_at_its_corner`, and states the rollback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU

* solidcore: a failed self-touch gluing also puts back the kernel's rng and UB list (tangency-labs#13650)

Review of tangency-labs#16392: `test_closure_split_region_attempt` can draw from `k.rng`, so a gluing
that failed on a later pair left the merge running with an advanced rng, unlike main.
`glue_all_or_none` (the gluing loop, now its own function taking the glue step) saves
`k.rng` and `k.upstream_ub_passed` with the operand's snapshot and restores all three on
any failure. The doc's list of `GlueEdgesGeneral` sites after the first topology change
is completed (radial-sector `InvalidInput`s, `set_uv_trim_curve`, `set_orientation`, a
surviving wire's unlinking, `is_spine`).

Unit tests reach the restore on both branches: a second pair failing after the first
changed an edge and drew from the rng restores the edges, counts, rng and UB list, then
continues for an `InvalidInput` and returns a `NotFound`; and the real
`GlueEdgesGeneral` with non-shared vertices (0 glued) or a stale id (an error) leaves
the operand as given.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SdcgEtwSrdwZxqTQp5MfnU

---------

Co-authored-by: dudedagger42 <gsbulkley@gmail.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:kernel B-rep kernel, booleans, blends, STEP/IGES moved-from-solidcore Moved from tangency-labs/solidcore

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A boolean operand that touches itself along a line carried as two coincident edges fails (relate 1001 or not a closed solid)

3 participants