Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clip: test out use of polyclip-ts instead of polygon-clipping library #379

Open
twelch opened this issue Nov 27, 2024 · 2 comments
Open
Milestone

Comments

@twelch
Copy link
Contributor

twelch commented Nov 27, 2024

It is refactored to use Typescript, uses big integer to solve floating point accuracy causing errors. However it is likely quite a bit slower, unclear how much. Performance testing needed.

Suggest looking at use of setPrecision - luizbarboza/polyclip-ts#5

@twelch
Copy link
Contributor Author

twelch commented Nov 27, 2024

If polyclip-ts doesn't make sense to use, then we can at least better make use of polygon-clipping methods to prevent infinite loop errors. Right now we naively do chunking of features in user space. This might be better controlled through available environment variables.

Global settings are set via environment variables. POLYGON_CLIPPING_MAX_QUEUE_SIZE and POLYGON_CLIPPING_MAX_SWEEPLINE_SEGMENTS: Aims to prevent infinite loops - usually caused by floating-point math round-off errors. Defaults are 1,000,000.

@twelch twelch added this to the 7.1 milestone Dec 13, 2024
@twelch twelch closed this as completed Dec 31, 2024
@twelch
Copy link
Contributor Author

twelch commented Dec 31, 2024

As of Turf 7.2.0, polyclip-ts is now used in all of its clip functions instead of the polygon-clipping library. This supports more precision in floating point numbers which should blow up less, but likely at the expense of speed. It's unclear if this is worth using, need to test using real world large examples in SeaSketch.

The current geoprocessing clip and clipMultiMerge don't actually use turf. They use polygon-clipping directly. At the very least, it could be documented that the turf clip functions (union, difference) exist that the report developer could try. Both are viable options

@twelch twelch reopened this Dec 31, 2024
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

No branches or pull requests

1 participant