Skip to content

Modernize and bring on par with Earcut v3.2.3#132

Merged
mourner merged 15 commits into
masterfrom
modernize
Jul 3, 2026
Merged

Modernize and bring on par with Earcut v3.2.3#132
mourner merged 15 commits into
masterfrom
modernize

Conversation

@mourner

@mourner mourner commented Jul 2, 2026

Copy link
Copy Markdown
Member

Brings the C++ port up to date with the latest earcut.js and modernizes the whole build/test/dev setup. Net: +1,487 / −11,105 lines across 83 files (most deletions are unvendored deps and hand-transcribed fixtures). Supercedes and closes #131. Closes #117. Closes #124.

Algorithm parity

  • Caught up to earcut.js v3.2.3 (from the old v2.2.x baseline), stepping through v3.0.1 / v3.0.2 / v3.1.x parity along the way, with bit-identical correctness against the JS fixtures.
  • Ported the optional mapbox::refine() Delaunay post-pass (legalizes interior edges in place, same triangle count/index format).
  • Perf work: replaced the z-order merge sort with std::sort, added the block-index optimization, plus assorted microoptimizations.

Testing & benchmarking

  • Switched from hand-transcribed C++ fixtures to reading the JS repo's JSON fixtures at runtime (fetched via CMake FetchContent, pinned to the matching tag) — deletes ~5k lines of generated fixture code and keeps us in lockstep automatically.
  • Added a 120k-polygon MVT benchmark (real-world production workload) plus a correctness net over the full fixture set.

Build & CI

  • Modernized the CMake config, removed git submodules / vendored deps (googletest, benchmark, glfw, libtess2 now fetched on demand) — plain non-recursive clone now builds.
  • Reworked CI: dependency caching, Ninja, sanitizer + coverage jobs, a clang-format check, a viz-compiles job, and -Werror (EARCUT_WARNING_IS_ERROR) across all jobs. Fixed a g++ bitfield-narrowing warning surfaced by it.

Docs & dev experience

  • Rewrote the README (v3.2.3 status, clearer usage/custom-type examples, new Performance / Robustness / Refinement / Visualizer sections, borrowing wording from the JS readme), refreshed the logo.
  • Visualizer: added an earcut-refine mode and defaulted it to the earcut fixture to match the JS demo.
  • Removed the stale CHANGELOG.md (GitHub Releases is the source of truth).

@mourner mourner marked this pull request as ready for review July 3, 2026 12:00
@mourner mourner requested a review from a team as a code owner July 3, 2026 12:00
@mourner mourner merged commit c68c883 into master Jul 3, 2026
9 checks passed
@mourner mourner deleted the modernize branch July 3, 2026 12:07
@mrgreywater

mrgreywater commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the update! Great to see progress :)

I noticed the refiner API appears to use a flat array of vertices as the second argument, which may force the user to make a flattened copy of the whole polygon data and is inconsistent with the normal earcut function. Could we change the signature to refine(indices, polygon) instead?

@mourner

mourner commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

@mrgreywater I was also mulling over this but decided to go with the flat version because we have to flatten the polygon either way, either before or internally in refine, because indices Earcut outputs can only reference flat coords (they don't index into nested arrays). We could maybe add an overload for this to make things easier, but I'd keep the flat version because the user might have flattened coords anyway, no reason to make the roundtrip to nested and back (e.g. when decoding MVT).

@mourner

mourner commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Maybe we could try the polygonPoint approach from the PR and see how it affects benchmarks. If it doesn't bring much overhead than yeah, maybe we should consider switching over for consistency.

@mrgreywater

Copy link
Copy Markdown
Collaborator

I'll see if I can come up with a clean overload that accepts both options when I'm back home. I'd really prefer if the interface stayed consistent.

@mourner

mourner commented Jul 3, 2026

Copy link
Copy Markdown
Member Author

Got Claude to investigate this, here's its response:

@mrgreywater Before you sink time into the overload — I benchmarked the nested-indexing approach and it turns out to be a real regression, so I want to save you the exploration.

I ran three variants of refine on the MVT fixture (pure refine cost, /all bucket, 3 reps):

Variant Time vs flat
flat coords (current) 95 ms 1.0×
nested + offset table + upper_bound 254 ms 2.7× slower
nested + polygonPoint linear scan (this PR) 496 ms 5.2× slower

Two things stack up: the global→(ring, local) mapping runs on every vertex access inside the hot inCircle loop, and vector<vector<Point>> scatters rings across the heap so we lose the cache locality of a contiguous buffer. Even swapping the linear scan for an offset table + binary search only recovers half of it. And the linear scan is O(rings) per access, so it degrades further on holey polygons — exactly the water tiles.

That said, I fully agree the interface should stay consistent, and I think there's a clean way to get there. Instead of indexing the nested structure in place, a convenience overload can flatten once and delegate to the flat path:

template <typename N, typename Polygon>
void refine(std::vector<N>& triangles, const Polygon& polygon) {
    using Point = typename Polygon::value_type::value_type;
    std::vector<Point> coords;
    for (const auto& ring : polygon) coords.insert(coords.end(), ring.begin(), ring.end());
    refine(triangles, coords); // flat fast path
}

The flatten is a single O(verts) copy — negligible next to refine's work — and it runs at the flat 95 ms, not 254/496. So users with a nested polygon get the consistent refine(indices, polygon) signature, and users who already have flat coords (MVT decode, etc.) skip the roundtrip. The only wrinkle is disambiguating the two overloads (SFINAE on whether the element is a point or a ring), which is straightforward.

Happy to draft that overload if you agree with the direction — didn't want you to keep polishing the in-place version given the numbers.

@mrgreywater

Copy link
Copy Markdown
Collaborator

I think having a convenience overload that does the flat copy is a good solution. If we find a faster solution, we can always improve it later without breaking the code for the users of the library.

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.

Performance degrades with number of holes Timeline for next release

2 participants