Repository navigation
Conversation
Contributor
|
thank you @JuFrolov ! I will evaluate shortly |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
unionon 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 bydifferenceon thin inputs, so they show up in real workloads, not only in synthetic data.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
isLeftToRightfromresetHorzDirectiononce, before the loop. When processing continues onto the next horizontal edge,resetHorzDirectionis called again andleftX2/rightX2are updated, butisLeftToRightkeeps 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 ofDoHorizontal), which is why it is not affected.Fix
Make
isLeftToRightmutable 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.
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
mainon their own.