Keep a reversed split loop that lies inside the remaining path (union could fill a hole)

[JuFrolov]

Problem

union can return a region that is in none of the inputs:

import { union, FillRule } from 'clipper2-ts';

const paths = [
  [{ x: 91, y: 7 }, { x: -145, y: -11 }, { x: -141, y: -15 }],
  [{ x: -28, y: 75 }, { x: -33, y: 76 }, { x: 1, y: 0 }],
  [{ x: -22, y: 51 }, { x: -39, y: 76 }, { x: -25, y: -2 }],
];
const out = union(paths, FillRule.NonZero);
// Point (-7, 7) is not inside any input, but is inside `out`.

The output is a single ring of area 1 865; 649 square units of it lie outside the inputs. The C++ library returns the same ring, so this is inherited from upstream: AngusJohnson/Clipper2#1109.

Cause

In exact arithmetic triangles 2 and 3 do not touch — there is a narrow crack between them. With integer intersection points, (0.95, 0.13) and (0.94, 0.13) become (0, 0), which moves the left edge of triangle 2 across the right edge of triangle 3 and closes the crack into a small hole.

Before cleanup the output ring is still correct at (−7, 7): the main part winds +1 there and a small loop of opposite orientation winds −1. fixSelfIntersects → doSplitOp splits that loop off; because it is smaller than the remaining path and of opposite orientation, it is dropped as a spurious twist. The −1 is lost and the remaining path fills the hole.

Dropping a reversed loop that lies outside the remaining path only removes a sliver. Dropping one that lies inside fills a hole.

Fix

Before dropping a smaller reversed loop, test whether its centroid (computed in doubles, so it is not rounded outside a thin triangle) has a non-zero winding number relative to the remaining path. If it does, keep the loop as its own path (a hole). Loops outside the remaining path are still dropped.

The minimal case now returns the same outer ring plus the hole (0,0) (-24,-1) (-22,50), total area 1 254.

Measurements

Node 24, Windows, i7-14700K. "main" is the published 2.0.1-18. Area outside the input is measured with difference(output, input).

Input	main (2.0.1-18)	This PR
3 triangles above: winding at (−7, 7)	1	0
3 triangles: area outside input	649	38 (truncation slivers)
3 rings, 15 vertices (test fixture): area outside input	522 841 250.5	51 531.5
5 426-ring real input: area outside input (union and PolyTree)	523 779 700	961 222.5
5 426-ring real input: union time, median of 11	89.2 ms	91.9 ms
Random stress: 40 000 sets of 3–8 thin triangles/quads, 1.8 M probe points more than 2 units from any input edge — points covered outside the input / input points lost	2 / 0	0 / 0

The real inputs are unions of many visibility polygons, which produce exactly these thin slivers; the 5 426-ring input was cleaned with simplifyPaths(…, 0) first.

Tests

tests/union-pocket.test.ts: the three triangles, a 3-ring fixture reduced from the real input, and the same fixture through PolyTree. All three fail on main and pass with this change. Full suite: 340 passed (337 on main).

[jpt]

@JuFrolov thank you for this one as well! I will take a look, although I am more hesitant about this than #36 as I would prefer issues shared between this port and upstream be fixed in the same way, so that I do not diverge further than I have already. regardless, I will take a look. thanks again

[JuFrolov]

AngusJohnson/Clipper2#1109

[jpt]

thanks - I did see that PR and I'm hoping angus merges it. in some ways clipper2-ts is "ahead" of clipper2, but I worry this might cause the need for larger refactoring in the future, so I am being somewhat conservative here - there are many issues and PRs upstream that I would like to fix in clipper2-ts but I don't want to become incompatible. on the other hand, clipper2 has been fairly inactive lately, so I may just accept this PR once I've had a moment to evaluate. I will keep you updated, and appreciate your work both here and upstream!