Ported union, difference, and intersect to clipper2-ts to improve performance #2997

smallsaucepan · 2026-01-10T01:21:36Z

Addresses performance issues in these three packages introduced in v7.2.0.

Performance improvement summary:

function	improvement
difference	20x to 130x
intersect	55x to 60x
union	10x to 120x

Requires a custom rollup config to support es5 minified output (clipper2-ts requires bigint, which needs to be polyfilled with the JSBI library before bundling). This should only be a temporary measure until we move to a more modern target on a major version release.

Inclusion of JSBI into minified output results in approx 40% bundle size increase. Feel performance is causing more pain than bundle size though, so still consider worth while. And as mentioned above, it won't be forever.

Adds a non-published "internal" module for shared code. Can be used for other packages as a way to avoid copy-pasting code.

Resolves #2851
Resolves #2833
Resolves #2955

Please provide the following when creating a PR:

Meaningful title, including the name of the package being modified.
Summary of the changes.
Heads up if this is a breaking change.
Any issues this resolves.
Inclusion of your details in the contributors field of package.json - you've earned it! 👏
Confirmation you've read the steps for preparing a pull request.

Some input test data was incorrectly wound so fixed those, as well as truncated input coords to more realistic 6 decimal places. Split out some (hopefully) common code to a new, not to be published seperately turf-internals package. Implementation currently implies limit of 6 decimal places, so may need to make this configurable / more generous before release.

…st data.

…est data. Other issue specific test results adjusted.

…"linking" to the point @turf/turf could be built.

…th a non-default precision set). Reverted corrections to incorrect windings in input test to make sure we handle improperly wound geojson. Added runtime checks to force all windings to what clipper2 expects.

…rm misses. Trying to get away with a minimal internal package e.g. don't have an aggregating "export all" index.ts. Hence a custom tsup to cater for submodule only importing.

…published package, and avoid having to add an exception case to packages/turf/test.ts to skip that directory.

… for BigInt / JSBI handling.

bratter · 2026-01-10T22:41:23Z

This is awesome. Thank you!! It's going to help me for sure :-). Also, I guess the build size increase is before jsts is removed from the buffer change? Which will take the size back down again... then even better after a major version!

My one question is about the defensive winding checking. It certainly might be the best thing to do for robustness, BUT running Clipper2.AreaD on every input polygon adds quite a lot of overhead for a defensive transformation where if the polygon is correctly wound it will just work (note that Clipper and GeoJSON use the same winding direction). Given mfedderly's comments on defensive transformation in #2949, I am wondering if it is the right way to go. I do appreciate that if the tests passed before it wouldn't be possible to break them in a minor, but maybe at least worthwhile testing if rewind is more efficient that areaD?

IMHO, but feel free to ignore, the comments in the enforceOuterRing and enforceInnerRing functions are also a little unclear. I am almost certain (Per comment here: #2991 (comment)) that the winding order is the same. The reason why the rewind is needed in buffer is that the azimuthal transform in buffer flips the y which changes the winding order. The doc comment above those functions is implying the opposite.

The other question that this begs is should the test cases have improper winding? Several do at the moment, and some of them (@turf/union's not-overlapping test case for instance) even has one polygon wound one way and another the opposite in the same FeatureCollection.

smallsaucepan · 2026-01-12T11:57:21Z

My one question is about the defensive winding checking ... adds quite a lot of overhead for a defensive transformation

Long term I'm in the same camp. Turf will be simpler to maintain if we don't have to cater to sloppy geojson. If it's not compliant we should push back on bug reports, similar to what we're starting to do with unrealistically precise coords. That said, best to wait for a major release to put in big red letters "we're not going to be holding your hand anymore".

maybe at least worthwhile testing if rewind is more efficient that areaD

Great idea. I'll compare them.

The other question that this begs is should the test cases have improper winding?

Probably not (unless that's something we're testing for like in a linter). It's probably something that crept in over time and we should avoid it going forward. Would be an interesting audit!

I am almost certain (Per comment here: #2991 (comment)) that the winding order is the same.

Will double check this also. I recall reading somewhere that clipper2-ts (at least for union, etc) didn't strictly care about the order so long as you were consistent within the same invocation.

mfedderly · 2026-01-12T13:45:19Z

packages/internal/package.json

+{
+  "name": "@turf/internal",
+  "version": "7.3.1",
+  "private": true,


https://docs.npmjs.com/cli/v11/configuring-npm/package-json#private

You can't depend on this from other packages if you mark it as private, they won't be able to be work because one of their dependencies will be missing.

Ugh. Yes you're right, we can't drop tsup and keep this private. I think there's a valid need for a package where we can store shared code so will make public and document clearly it's not for direct usage, API may change at any time, etc.

@turf/clipper2-support or similar?

Makes more sense to me to keep it general purpose so we don't have to publish a new package for every bit of common code we want to share between packages.

mfedderly · 2026-01-12T13:59:19Z

packages/internal/clipper2/index.ts

+import { PolyPathD, PathD, PathsD, PolyTreeD, areaD } from "clipper2-ts";
+import { Polygon, MultiPolygon, Position } from "geojson";
+
+const DEFAULT_PRECISION = 8;


This probably needs a rename to make it clear that this is for coordinates in lng, lat. If you project the coords into something with meters as their unit, you probably want 2 instead.

mfedderly · 2026-01-12T14:05:11Z

packages/turf-difference/test/out/issue-#721-inverse.geojson

-            ]
-          ]
-        ]
-      }


Was this supposed to disappear? Did you check all of the output for visual sameness at least?

I don't use REGEN, so yes visually check and try to account for the differences. In this test (around since 2017) the removed results were three polygon slivers (effectively lines) that were being returned and causing errors downstream. They're regression tests for libraries we don't use any more, so would it be clearer to remove the test entirely?

Sounds good about the extra feature removal. I guess with the isobands, isolines, and buffer changes I was just lucky enough to have the same geojson shape before and after, it didn't really change the structure just the coordinates. If you think these tests will help us regress Turf in the future I'm happy to keep them, or drop them otherwise.

I swear GitHub used to have a nice visual diff for .geojson files that we could use in PRs, but it seems to have disappeared. We desperately need better tooling for situations like this where all of the output changes but not in a meaningful way. Its so much easier to just look visually than try to reason about if two polygons switched order, or a ring got rotated around by a few points.

mfedderly · 2026-01-12T14:07:13Z

packages/turf-intersect/test/out/Intersect1.geojson

-      "properties": {
-        "fill-opacity": 0.5,
-        "fill": "#00F"
-      },


Did you run prettier on the output geojson? I've been using pnpm prettier --write test/out/* test/in/* when I change test fixtures

Yep, at the editor level on save. And then again when lint-staged runs. Just ran manually then and didn't change anything. This is correct yes?

Interesting. I was surprised to see it go from multi-line to single line here. 🤷
My stuff doesn't run prettier on save at the moment and its driving me nuts.

mfedderly · 2026-01-12T14:08:35Z

packages/turf-intersect/test/out/issue-1394.geojson

-            [-85.42358073, 41.76885781]
-          ]
-        ]
-      }


Another suspiciously deleted output

Same as 721-inverse above. A wafer thin polygon being returned by a previous clipping lib that would blow up downstream turf functions.

mfedderly · 2026-01-12T14:56:08Z

packages/internal/clipper2/index.ts

+  }
+
+  return path;
+}


Agreed that we should start telling the users to get the winding right before passing them in, but that's likely a major version issue.

mfedderly · 2026-01-12T14:56:56Z

packages/internal/clipper2/index.ts

+function enforceOuterRing(path: PathD): PathD {
+  if (areaD(path) < 0) {
+    // Leave original array untouched.
+    return [...path].reverse();


You should be able to mutate path instead, because ringToPath already creates your own internal representation.

mfedderly · 2026-01-12T14:57:37Z

packages/internal/clipper2/index.ts

+  return {
+    type: "MultiPolygon",
+    coordinates: polygons,
+  };


I kind of want to just always return MultiPolygon's even if there's only 1 polygon, but again... major version.
We still have to consider the case where we get 0 polygons back and whether that should be a MultiPolygon.

Would make for a simpler interface. Plenty of options for what to return for 0, depending on how many liberties we're willing to take. MultiPolygon with empty coordinates array (not kosher from what I understand but used in the wild). Feature with null geometry (more correct, but back to where we started as far as return type conditional logic). Straight up null as we are now?

mfedderly · 2026-01-12T15:00:02Z

packages/internal/clipper2/index.ts

+      }
+
+      // Expectation is pools within islands within lakes within continents ...
+      // are handled as multipolygons. So further recursion is not required.


Have you tested this to make sure that it doesn't nest new outer rings inside holes?

mfedderly · 2026-01-12T15:01:21Z

packages/internal/clipper2/index.ts

+    if (first[0] !== last[0] || first[1] !== last[1]) {
+      coords.push([first[0], first[1]]);
+    }
+  }


I think this is fine, as long as we never do it with a LineString, but everything going through here should be Polygon rings.

smallsaucepan added 10 commits January 4, 2026 11:18

Ported intersect to clipper2. Fixed windings of some geojson input te…

e6c7431

…st data.

Retrofitting more compact implementation to union.

13bc9fc

Ported difference to clipper2. Fixed windings of some geojson input t…

ba8b43e

…est data. Other issue specific test results adjusted.

Added minimal package setup to @turf/internal to get it building and …

00c2074

…"linking" to the point @turf/turf could be built.

Converted to floating point version of clipper2 (works much better wi…

39035eb

…th a non-default precision set). Reverted corrections to incorrect windings in input test to make sure we handle improperly wound geojson. Added runtime checks to force all windings to what clipper2 expects.

Babel plugin to fix a few bigint replacements the third party transfo…

495bc0d

…rm misses. Trying to get away with a minimal internal package e.g. don't have an aggregating "export all" index.ts. Hence a custom tsup to cater for submodule only importing.

Relocating turf-internal to just internal. This indicates it's not a …

fc80ef3

…published package, and avoid having to add an exception case to packages/turf/test.ts to skip that directory.

Updated rollup config with custom transform to es5 to make allowances…

6b45ae3

… for BigInt / JSBI handling.

Missed reverting turf-union union3 test back to full input precision.

f9641d3

smallsaucepan mentioned this pull request Jan 12, 2026

WIP migrate @turf/buffer to TypeScript #2991

Draft

mfedderly reviewed Jan 12, 2026

View reviewed changes

Ported union, difference, and intersect to clipper2-ts to improve performance #2997

Are you sure you want to change the base?

Ported union, difference, and intersect to clipper2-ts to improve performance #2997

Conversation

smallsaucepan commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bratter commented Jan 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

smallsaucepan commented Jan 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

smallsaucepan Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

smallsaucepan commented Jan 10, 2026 •

edited

Loading

bratter commented Jan 10, 2026 •

edited

Loading

smallsaucepan Jan 13, 2026 •

edited

Loading