First full working implementation

[sheerun]

This PR builds on top of #1 but totall changes algorithm that relies on union and difference of polygon-clipping library (it turned out that martinez-polygon-clipping that you currently use has bugs that prevent correct union). The performance is more or less on par with JSTS but there is some room for improvement..

The change of algorithm is because it's really hard to handle cases where buffer is more wide than feature that is buffered (e.g. buffering series of small angles with buffer 5x of their size). Your approach of finding intersection of elongated segments simpley doesn't work in such cases which you might be already aware. The approach I've selected doesn't have any issues except bug within polygon-clipping itself: mfogel/polygon-clipping#60 (it can be reproduces on Warsaw example I've added to demo app).

I'm sending this PR because it's mostly working implementation and I won't have much more time this week to polish this and fix all tests. I've improved debugger so I'm pretty sure it works is most cases.

The polygons I'm unioning have shape of santa claus staff because I've found it's minimal shape that I can draw for each angle of polygon that covers all the cases.

Here's video how it currently works: https://v.usetapes.com/4py5dUu9Wc

[rowanwins]

Wow @sheerun I'm really looking forward to seeing what you've done, I'll spend some time this evening checking it out!

[rowanwins]

Hi @sheerun

I've just updated the polygon-clipping dependency to v0.14.0 and I haven't hit any bugs with the warsaw example since then (I did prior to the upgrade) so hopefully that has resolved that.

I also have a branch called class-basedwhich has some additional ideas and might be worth a poke around.

On the downside running npm run bench shows that this lib is quite a bit slower than jsts at the moment.

// Simple polygon
geojson-buffer x 765 ops/sec ±10.58% (81 runs sampled)
polygon-offset x 354 ops/sec ±1.28% (90 runs sampled)
jsts x 13,313 ops/sec ±4.49% (80 runs sampled)

// Warsaw
geojson-buffer x 1.20 ops/sec ±10.37% (8 runs sampled)
polygon-offset x 2.15 ops/sec ±1.23% (10 runs sampled)
jsts x 49.12 ops/sec ±8.06% (54 runs sampled)

My general experience is that this is due to the union-ing step, from what I can see your approach potentially calls union twice which isn't ideal. I was hoping I might be able to use shamos-hoey to detect if there were any self-intersections before performing a union.

Anyway I'll need to do some further digging into what might be causing the slowness.

Thanks for a massive contribution!

[sheerun]

@rowanwins I think at this point most performance improvement, including shamos-hoey, can go directly to polygon-clipping repository (even if only as a flag), so other people can also use it.

As for calling union twice it's only appears so. In reality underneath union is called for each pair of polygons anyway, so there are N union calls, not 2 of them.

[rowanwins]

Hi @sheerun

I think at this point most performance improvement, including shamos-hoey, can go directly to polygon-clipping repository (even if only as a flag), so other people can also use it.

I'm not quite sure I understand this comment sorry. Are you basically saying that effort needs to go to speeding up the polygon-clipping library and not this one? And that we should accept a working solution even if it's slow?

Using my class-based branch (which isn't yet finished) I get the following sorts of benchmarks

// Warsaw
geojson-buffer x 7.15 ops/sec ±6.94% (21 runs sampled)
polygon-offset x 2.06 ops/sec ±6.16% (10 runs sampled)
jsts x 41.16 ops/sec ±8.19% (55 runs sampled)
- Fastest is jsts

We're taking a big performance hit by calling polygon-clipping, but I haven't found a way around it yet. If I remove that single call we get the raw offset curve which includes self-intersections

// Warsaw
geojson-buffer x 70.27 ops/sec ±6.39% (61 runs sampled)
polygon-offset x 2.08 ops/sec ±1.22% (10 runs sampled)
jsts x 39.75 ops/sec ±8.33% (56 runs sampled)
- Fastest is geojson-buffer

Even detecting the self-intersections is quick with shamos-hoey, it's adding next to nothing to the performance time

geojson-buffer x 73.89 ops/sec ±6.13% (55 runs sampled)
polygon-offset x 2.02 ops/sec ±5.45% (9 runs sampled)
jsts x 41.07 ops/sec ±7.19% (57 runs sampled)
- Fastest is geojson-buffer

However it's working out how to remedy the self-intersections that's the hard part.... unfortunately thats the part that most journal papers are a bit vague on (eg this paper references bentley-otmann but that's it, this paper leverages the c++ GLU tessellator to handle it). Perhaps the simplest work I've come across is here which makes it look simple but I'm sure the devil is in the detail....

I suspect part of the benefit that jsts has is that all the data structures are in the one place, whereas we're passing data from library to library which comes with overhead...

I'm not sure if @dr-jts has any helpful advice at this stage - I guess the basic question is do I need to use a full blown union algorithm (eg polygon-clipping) to get a useful result or is there a simpler way if I have the offset edges and the self-intersection points?

[sheerun]

I'm not quite sure I understand this comment sorry. Are you basically saying that effort needs to go to speeding up the polygon-clipping library and not this one? And that we should accept a working solution even if it's slow?

That's pretty much what I mean :)

I don't really know how polygon-clipping is working internally but I think some improvements could happen e.g. if polygons were being "unionised" by selecting each time polygons with 2 least amount of edges. It's because according to the authors:

The Martinez-Rueda-Feito polygon clipping algorithm is used to compute the result in O((n+k)*log(n)) time, where n is the total number of edges in all polygons involved and k is the number of intersections between edges.

Maybe polygon-clipping could also skip unionizing in some cases e.g. if bounding boxes of some polygons are not overlapping. Maybe also by clustering polygons and skipping unionizing in some cases, hard to tell...

I suspect part of the benefit that jsts has is that all the data structures are in the one place, whereas we're passing data from library to library which comes with overhead...

I believe slowdown might not be because of passing references but because of algorithm itself which seems that needs to be very complicated to handle all edge cases. It's better to measrure this first before optimizing. Especially difficult case is big buffer with small polygon where there are countless possibilities to cover..

I think it could take many months to develop correct algorithm that doesn't depend on .union call (e.g. the dissolving one from the paper you've linked, but it's unclear it's faster as it reports worst time complexity of O (n^2 log n). which is slower than Martinez-Rueda-Feito and average time complexity of O (n log n) which is pretty much the same as Martinez-Rueda-Feito)

[sheerun]

In short I believe the correct way forward is to:

1. Remove extra call to .union in this PR (but I think merging biggest and most intersecting polygon last is good strategy so I'm not sure it's good idea to put everything into single .union call)
2. Add shamos-hoey to polygon-clipping lib to improve its performance
3. Maybe change order in which polygons are merged, again in polygon-clipping repository

I also believe currently the only fool-proof way to remedy detected intersections is to use Martinez-Rueda-Feito which seems to have the same performance as dissolving algorithm you're linking.

[sheerun]

Another optimizatin can come from reducing number of "parts" that we call .union with post-processing resulting parts (e.g. if angle is in 90-180 degrees angle range we can probably assume that current polygon is not intersecting with previous one and just write algorithm that manually "unions" them even faster than Martinez-Rueda-Feito.

In short there are many ways to improve performance, but current implementation is good enough to ship

[anastely]

I'm just using @turfjs/nearest-point " https://github.com/Turfjs/turf/tree/master/packages/turf-nearest-point " to get nearby users
and I have the same error
attempting to a configurable attribute of a configurable property turf
but when I run the debugger the app work very well :/
any idea?
sorry about this

[rowanwins]

Hi @anastely

I'm not sure why you're asking your question in this repo? If you've got an issue with turf open it over in the turf repo.

Thanks

[anastely]

@rowanwins I'm confused, so sorry, I was think he have a answer or something

[sheerun]

@rowanwins May I suggest to incorporate this implemetation as part of Turf 7 just so you could publish stable version of it without JSTS, and then publish separate version of turf-buffer-jsts that uses JSTS underneath if someone wishes to trade performance for bundle size? (after publishing of Turf 7 you could still replace algorithms to faster one if you manage to implement it)