-
Notifications
You must be signed in to change notification settings - Fork 62
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
"Unable to complete output ring staring at" #91
Comments
@betovelandia There were a bunch of fixes made in master https://github.com/mfogel/polygon-clipping/commits/master that haven't yet been included in a release. Could you check these against master? When I install this in a library I then do: cd node_modules/polygon-clipping && yarnpkg install && yarnpkg run build to ensure I'm using master and the dist/build files which need to be built as well. Otherwise hoping @mfogel can do a new release with the latest fixes. |
Checked against master, the error is still there, any pointers of what it might be and how we might help to fix it? |
@andrewharvey I went ahead released v0.14.3 just now with the latest on master @betovelandia thanks for the failing coordinates. If you want a quick fix, I would try trimming the number of digits after the decimal place before running the coords through the library. |
I ran into this as well. Since the thrown error was specifically because there is no last matching segment yet there is a first segment. |
I am running into this error as well, using turf's difference function (which uses polygon-clipping's difference function) to get the difference between a polygon and a multipolygon |
Seeing this as well on const polygonClipping = require('polygon-clipping');
const poly1 = [
[
[-89.6913437618266, 32.5917775294804],
[-89.6913420772531, 32.5863246737333],
[-89.6932638650491, 32.5863115360137],
[-89.6913437618266, 32.5917775294804], // Technically not needed, will fail still without this line
],
];
const poly2 = [
[
[-89.6871254313303, 32.5917917879984],
[-89.6913437618266, 32.5917775294802],
[-89.6956011358121, 32.591762995718], // Technically not needed, will fail still without this line
[-89.6871254313303, 32.5917917879984], // Technically not needed, will fail still without this line
],
];
console.log(polygonClipping.difference(poly1, poly2)); Full non-reduced JS version in this gist: https://gist.github.com/twolfson/bfd03b9e4b254374c32a6d91427f36df |
My suggestion, given the issues with floating point issues, is scale those number by 1000. That might work. |
Oooh, that's a great idea! I got so caught up in truncation/rounding, I'd forgot about that as an option =) |
Unfortunately, that doesn't resolve the issue =/ At first I thought it was possibly my implementation but even moving the numbers to just ints or shifting the decimal continues to encounter the issue: const polygonClipping = require('polygon-clipping');
const poly1 = [
[
[-896913437618266, 325917775294804],
[-896913420772531, 325863246737333],
[-896932638650491, 325863115360137],
[-896913437618266, 325917775294804], // Technically not needed, will fail still without this line
],
];
const poly2 = [
[
[-896871254313303, 325917917879984],
[-896913437618266, 325917775294802],
[-896956011358121, 32591762995718], // Technically not needed, will fail still without this line
[-896871254313303, 325917917879984], // Technically not needed, will fail still without this line
],
];
console.log(polygonClipping.difference(poly1, poly2)); In case anyone would like the code for scaling, here's what I was working on -- but it's more likely rounding is still the workaround for now =/ import assert from "assert"
import cloneDeep from "lodash/cloneDeep"
import difference from "@turf/difference"
import { getGeom } from "@turf/invariant"
// Wrapped `@turf/difference` (`polygon-clipping`) with magnitude scaling to avoid factor issues
// https://github.com/mfogel/polygon-clipping/issues/91#issuecomment-974614399
const FEATURE_DIFFERENCE_MULTIPLY_FACTOR = 10e3 // Alternatively, consider using 4096 or other 2^n to avoid float issues
function _scaleFeatureCoordinates(feature, factor) {
// WARNING: THIS WILL MUTATE OUR FEATURE IN-PLACE
// https://github.com/Turfjs/turf/blob/v6.5.0/packages/turf-difference/index.js#L39-L47
// https://github.com/Turfjs/turf/blob/v6.5.0/packages/turf-invariant/index.ts#L236-L243
// DEV: Coordinates are always an array of arrays, https://github.com/mfogel/polygon-clipping/blob/v0.15.3/src/geom-in.js#L4-L77
const geom = getGeom(feature)
geom.coordinates = geom.coordinates.map((ring) =>
ring.map(([x, y]) => [x * factor, y * factor])
)
return geom
}
export function featureDifference(_featureA, _featureB) {
// Deep clone our features
const featureA = cloneDeep(_featureA)
const featureB = cloneDeep(_featureB)
// Sanity check we won't lose any precision
// DEV: Disabled due to encountering some precision loss =/
// _scaleFeatureCoordinates(featureA, FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
// _scaleFeatureCoordinates(featureA, 1 / FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
// _scaleFeatureCoordinates(featureB, FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
// _scaleFeatureCoordinates(featureB, 1 / FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
// assert.deepEqual(JSON.stringify(getGeom(featureA).coordinates), JSON.stringify(getGeom(_featureA).coordinates))
// assert.deepEqual(JSON.stringify(getGeom(featureB).coordinates), JSON.stringify(getGeom(_featureB).coordinates))
// Scale up our features for-real, clip, and scale back down
// DEV: During our testing, we do encounter occasional precision loss at 1e-12 or 1e-13 due to the nature of numbers in JS
_scaleFeatureCoordinates(featureA, FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
_scaleFeatureCoordinates(featureB, FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
const result = difference(featureA, featureB)
if (result) {
_scaleFeatureCoordinates(result, 1 / FEATURE_DIFFERENCE_MULTIPLY_FACTOR)
}
return result
} |
Yeah. Probably not. The whole library suffers from floating point rounding issues. As part of JSCAD, we are calculating an 'epsilon' of precision based on the scale of the shape. FYI, the EPS from constants.js is that for shapes scaled around 1.0. Not sure that will help, but might give you some more food for thought when 'rounding' those points. |
I was experiencing many issues with the // Utility to scale/round coords so we don't have rounding issues
const SCALE_FACTOR = 1e4;
const scaleFeature = function(feature, factor, doRounding) {
const round = (num)=>(doRounding ? Math.round(num) : num);
const geom = turf.invariant.getGeom(feature);
const recursiveScaler = function(arg) {
if (typeof arg[0]=='object') //array so recurse
return arg.map(recursiveScaler);
let [x,y] = arg;
return [round(x*factor), round(y*factor)];
};
geom.coordinates = geom.coordinates.map(recursiveScaler);
return geom;
};
// Scale a cloned copy so we don't modify original
let feature1Cloned = cloneDeep(feature1.geometry);
let feature2Cloned = cloneDeep(feature2.geometry);
scaleFeature(feature1Cloned, SCALE_FACTOR, true);
scaleFeature(feature2Cloned, SCALE_FACTOR, true);
// Ok, try the intersection again; if there is an intersection, then undo scaling on the intersection
intersectionFeature = turf.intersect(simplified, placeCloned);
if (intersectionFeature)
scaleFeature(intersectionFeature, 1/SCALE_FACTOR); |
I'm running into this using In my case, truncating to 6 or so significant digits is also fine, so truncating during internal calculations also helps, that would work for me, too. |
I've run into this error specifically when dealing with shapes that are practically on top of each other or overlapping. For example, here are all the bus stops in Bloomington, MN. I tried to build isochrones for each and union them, but for bus stops across the street from each other, the isochrones would be practically identical. Depending on what you are using the data for, you may be able to pre-process the data before attempting a |
My issue was solved by switching to the |
I was wondering if there is any progress on this issue. I am running into the same problem. |
Like @markstos, I was able to resolve my issue by switching to polyclip. While it does appear that polygon-clipping and polyclip are written by the same people, for some reason polyclip is more reliable. |
just fix coordinates to 8~10 decimals |
@puxiao Why is that range magical? Why not 7-11? And why a range? Why not exactly 8 or 10? The comment you link to just says "use 8 to 10 decimals", so it doesn't really explain. My guess is this: When floating point math goes wrong, you end up with a case where you should get 1==1, but instead you get "1=0.9999999999999999999", so some equality check fails, like checking that the end point of a ring matches the beginning. I suspect the "8-10" suggestion is really the idea that if you truncate values enough, then you don't trigger the floating problems with small differences and end up with numbers that actually match. But if you search the source code of the |
Your are right. 0.1+0.2= 0.30000000000000004 Such contingencies are common. Emm.. only this option is available for the time being. |
I am getting the following error:
Here are the arguments:
This is how it looks:
Any idea why this might be happening?
The text was updated successfully, but these errors were encountered: