Skip to content

Fix Bentley-Ottmann polygon walk at shared vertices#382

Open
aldernero wants to merge 2 commits into
tdewolff:masterfrom
aldernero:master
Open

Fix Bentley-Ottmann polygon walk at shared vertices#382
aldernero wants to merge 2 commits into
tdewolff:masterfrom
aldernero:master

Conversation

@aldernero

Copy link
Copy Markdown
Contributor

Summary

  • Fix panics in bentleyOttmann when building result polygons at vertices where many segments meet (e.g. after Path.StrokeSettle on dense, shared-edge geometry such as merged Voronoi cell outlines).
  • Search both directions around the snap-square event ring for the next result segment, with a fallback to another left endpoint at the vertex.
  • Close partial contours to the other endpoint instead of panicking when no neighbor is found.
  • Add regression tests for merged grid stroke/settle (path_intersection_voronoi_test.go).

Problem

Path.Stroke calls Settle(Positive) on the offset outline. At tight junctions, polygon walking only searched one direction for the next segment at a vertex. When no match was found, the code panicked:

next node for result polygon is nil, probably buggy intersection code

Test plan

  • go test ./... in this repo
  • Reproduced originally via merged Voronoi cell stroke in gaul (TestVoronoiCanvasMergedStroke_denseGrid)

Made with Cursor

When building result polygons, the next segment at a vertex was only
searched in one direction around the snap-square event ring. Offset stroke
outlines (e.g. dense adjacent cells) could leave no match and panic.

Search both directions, fall back to another left endpoint in the result,
and close the contour to the other endpoint instead of panicking when no
neighbor exists. Add regression tests for merged grid stroke/settle.

Co-authored-by: Cursor <cursoragent@cursor.com>
@tdewolff

Copy link
Copy Markdown
Owner

Thank you for your contribution, but I can't get your test to panic like it says it should. In fact, we go from 500 path commands to 5 (MoveTo + 3 LineTo + Close) which is exactly right. Also when drawing it looks correct.

The panic is actually there for a reason, it means something else went wrong in another part of the code. I still wonder if you could reproduce the original bug you encountered?

anaelorlinski added a commit to anaelorlinski/canvas that referenced this pull request Jun 19, 2026
Imports upstream PR tdewolff#382. The previous one-direction-only search at
shared vertices panicked on dense geometry (e.g. stroked outlines
clipped against rects whose edges align). Searches both directions
around the snap-square event ring with a fallback to another
left-endpoint at the vertex.
…the panic

The previous tests used a manually-crafted exact unit-square grid, which does
not trigger the bentleyOttmann polygon-walk panic on the pre-fix code because
all vertices land exactly on the snap-rounding grid.

The real reproduction requires floating-point imprecision: actual Voronoi
cell boundaries (computed in github.com/aldernero/gaul) accumulate tiny errors
(e.g. 1.4e-17 instead of 0.0) that create near-coincident vertices. When these
vertices are in the stroke outline of 100 merged cells and settled with the
Positive fill rule, the one-directional event-ring search fails to find the
next result segment and panics:

  next node for result polygon is nil, probably buggy intersection code

Add testdata/voronoi_stroke_pre_settle.gob: the gob-encoded pre-Settle stroke
outline generated from that Voronoi geometry. TestSettleVoronoiDenseGridStrokeOutline
loads it and calls Settle(Positive); it panics on the pre-fix code and passes
with the fix applied.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aldernero

Copy link
Copy Markdown
Contributor Author

You are correct — the original tests I added used a manually-crafted exact unit-square grid, which does not reproduce the panic. I've pushed a fix.

Why the original tests didn't panic

The bug requires floating-point imprecision in the vertex coordinates. With a manually-crafted grid all coordinates land exactly on the snap-rounding grid (BentleyOttmannEpsilon = 1e-8), so the event ring is well-behaved and the one-directional search always succeeds.

The actual reproduction comes from the Voronoi algorithm: even though the sites are on a regular grid (so the cells should be perfect axis-aligned squares), the floating-point arithmetic accumulates tiny errors — e.g. a vertex that should be exactly 0.0 comes out as 1.3877787807814457e-17. After stroking 100 merged cells at width 0.2, the pre-Settle stroke outline has 1487 commands instead of the expected 1440; those 47 extra near-coincident segments create the vertex configuration where the CCW search finds nothing and the one-directional search panics.

What's in the updated PR

I replaced the four synthetic tests (which all passed on the pre-fix code) with:

  1. TestSettleMergedVoronoiLikeGrid — keeps the basic "settle the merged grid" smoke-test (NonZero rule, always passed, still passes).
  2. TestSettleVoronoiDenseGridStrokeOutline — the canonical regression test. It loads testdata/voronoi_stroke_pre_settle.gob, which is the gob-encoded stroke outline from 100 Voronoi cells (generated by github.com/aldernero/gaul and captured before the internal Settle call). Calling p.Settle(Positive) on this fixture panics on the pre-fix code with next node for result polygon is nil and passes with the fix.

The fixture was generated by:

// sites on a 10×10 regular grid, VoronoiWithRect gives near-exact square cells
merged := paths.Merge()
canvas.FastStroke = true
preSettle := merged.Stroke(0.2, canvas.ButtCap, canvas.MiterJoin, 0.05)
// encode preSettle to testdata/voronoi_stroke_pre_settle.gob

Let me know if you'd prefer a different approach for the fixture (e.g. inlining a minimal failing path as a string constant, or referencing the gaul test as the external reproducer).

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.

2 participants