Skip to content

Fix unbounded memory growth when consecutive horizontal edges reverse direction - #36

Open
JuFrolov wants to merge 1 commit into
countertype:mainfrom
JuFrolov:fix/horizontal-direction
Open

JuFrolov wants to merge 1 commit into
countertype:mainfrom
JuFrolov:fix/horizontal-direction

Conversation

@JuFrolov

Copy link
Copy Markdown

Problem

union on a nearly flat ring whose consecutive horizontal edges reverse direction (a horizontal "spike") never finishes: the heap grows until the process aborts. Such rings are produced by difference on thin inputs, so they show up in real workloads, not only in synthetic data.

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

const ring = [
  { x: 761206, y: 2463086 }, { x: 761252, y: 2463086 }, { x: 761258, y: 2463086 },
  { x: 761209, y: 2463087 }, { x: 761213, y: 2463087 }, { x: 761281, y: 2463086 },
  { x: 761323, y: 2463086 }, { x: 761368, y: 2463086 },
];
union([ring], FillRule.NonZero); // 2.0.1-18: heap exhausted

The C++ library (clipper2-wasm 0.4.0) returns one ring of 4 vertices, area 13.5, in about 2 ms.

Cause

Both horizontal-processing routines (the general/open one and the closed-only one) destructure isLeftToRight from resetHorzDirection once, before the loop. When processing continues onto the next horizontal edge, resetHorzDirection is called again and leftX2/rightX2 are updated, but isLeftToRight keeps the direction of the first edge. If the next horizontal edge runs the other way, the sweep proceeds in the wrong direction and keeps allocating output points. The C++ library reassigns it on every call (is_left_to_right = ResetHorzDirection(horz, vertex_max, horz_left, horz_right); inside the loop of DoHorizontal), which is why it is not affected.

Fix

Make isLeftToRight mutable and update it together with the bounds in both routines.

Measurements

Node 24, Windows, i7-14700K. "main" is the published 2.0.1-18. Each union of the first two inputs runs in a separate process limited to a 256 MB heap.

Input main (2.0.1-18) This PR C++ (clipper2-wasm 0.4.0)
8-vertex ring above heap exhausted after 0.8 s 1.7 ms, 1 ring, 4 vertices, area 13.5 1.7 ms, 1 ring, 4 vertices, area 13.5
745-ring real input heap exhausted after 0.8 s 18.9 ms, 532 rings, 2 771 vertices 8.3 ms, 528 rings, 2 753 vertices
5 426-ring real input without spikes (median of 15) 80.1 ms 81.6 ms —

On the 745-ring input the area matches C++ to within 1·10⁻⁶ of the total (3 000 923 506 vs 3 000 920 455.5).

Tests

tests/horizontal-spikes.test.ts, 17 tests: the ring above, reversed winding, reflections, preserveCollinear, and inputs that reach both horizontal routines. Full suite: 354 passed (337 on main).

Independent of #35; both apply cleanly to main on their own.

@jpt

jpt commented Sep 26, 2026

Copy link
Copy Markdown
Contributor

thank you @JuFrolov ! I will evaluate shortly

This branch has not been deployed

No deployments
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