-
Notifications
You must be signed in to change notification settings - Fork 23
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
polygon clipping errors #30
Comments
Can you reproduce the inversion from the screenshot alone? Or would it help if I tried to create a reduced test case? |
It's just a question of fiddling with the mouse and then printing projection.rotate()… A bad case:
|
And
creates an inversion of land and ocean |
Another inversion: https://observablehq.com/d/303582dda96b0649 |
I've updated the notebook to only show the offending polygon (and added the ability to step through all polygons). |
Very useful! Now it looks like a cause is that this polygon is degenerate:
by bisecting I found this point, which is appearing twice:
[34.9226, 29.5013]
filtering it out solves the inversion. (And if I load the usual topojson
from Mike's world-atlas, there's no inversion… so I guess that's one bug in
visionscarto-world-atlas, and a fragility n d3-geo-polygon.)
In both cases, Antarctica is still bleeding, so we're not done. But:
progress! 👍
|
I've fixed the problematic point in https://unpkg.com/[email protected]/ |
Looks like I finally nailed that bug. There is a case where we would rejoin with a line if the segment's first and last points were coincident. But the test for coincidence used a very tolerant pointEqual, and two points seperated by 1e-6 were considered equal, and they were rejoined directly, creating this "bleeding". Fixed at a08a09e Test here: https://observablehq.com/d/7c5b4f1ff5a76024 (EDIT - Dec. 2019: the bug is still here — looks like I forgot to merge the change) |
We still get some inversions on the example, but I'm having a hard time isolating them. |
What's your process and intuition to narrow them down? |
To find the problematic rotations I have no method, I just rotate the airocean globe until I see a flash of black (if such a thing exists). If I'm lucky to find one, I can just type To find the faulty code, my method in this case is a bit like what you did in the notebook: I remove as much "matter" as I can from the polygons, and try to get to (ideally) a faulty triangle or a polygon with very few edges. Then I write two test cases in node, one with the faulty rotation, the other with a different rotation, which is ideally very close to the faulty one, but does not break. After that, I start to edit the src code, adding console.warn() where I suspect there might be a difference, and run the two test cases side by side. The numbers change a bit, but I'm looking for places where the structure is different. In the case of a08a09e, I ended up at this specific line of code, and saw that the pointEqual test was suddenly saying true and triggered the bug. |
What assumptions can be made about the inversions so far? E.g.:
|
I wish I knew. An idea might be to use the faulty GeoJSON and track down
the part of the code where the inversion occurs — maybe we'll see what
happens and find a way to make it more robust.
|
Not sure if related or relevant: compareIntersection appears to apply some fuzz via epsilon. Related issue: #3 |
Closing for now, as bleeding issues are solved. If we can find an inversion we'll open a new issue. |
while these first two could be solved by clipping to the sphere, the last one shows we still sometimes have inversions:
(reported by @mootari @ https://imgur.com/a/Km2wK6m)
The text was updated successfully, but these errors were encountered: