Skip to content

Polygon clipping fix#101

Merged
artemp merged 4 commits intomasterfrom
polygon-clipping-fix
Aug 14, 2019
Merged

Polygon clipping fix#101
artemp merged 4 commits intomasterfrom
polygon-clipping-fix

Conversation

@artemp
Copy link
Contributor

@artemp artemp commented Aug 9, 2019

No description provided.

@artemp artemp requested a review from a team as a code owner August 9, 2019 14:48
@artemp artemp requested review from mapsam and springmeyer August 9, 2019 14:49
@flippmoke
Copy link
Member

@artemp or @ericfischer could you produce an image of the results here compared to #100 (comment)

I would love to visually see the differences again.

@e-n-f
Copy link
Contributor

e-n-f commented Aug 9, 2019

I'll try to build both versions to compare the output, unless @artemp gets there before I do.

@e-n-f
Copy link
Contributor

e-n-f commented Aug 9, 2019

As far as I can tell, both implementations are now producing visually identical output:

Screen Shot 2019-08-09 at 12 39 22 PM

@flippmoke
Copy link
Member

flippmoke commented Aug 12, 2019

After thinking about this more two situations are concerns here for me, first can this turn the following into two exterior rings after clipping?

image

Next, would this resolve to a single exterior ring and single hole.

image

These are two common failure cases for many clipping algorithms and things I would be concerned that Boost may not resolve correctly.

@artemp
Copy link
Contributor Author

artemp commented Aug 12, 2019

@flippmoke - thanks for providing pics ^ - it's always easier to see the issue. Lets establish what is a correct expected result for both cases. Could you post some more info, pls!

@flippmoke
Copy link
Member

The expected solution for the first one is two polygons each with no interior rings, one small triangle and one polygon with a larger square with a triangle cut out of it.

The expected solution for the second one is one polygon with one interior ring with a hole cut out of it. The interior ring is a triangle that touches the square exterior ring at one point.

@artemp
Copy link
Contributor Author

artemp commented Aug 13, 2019

  • boost-1.64 (earliest I have installed)

One polygon with interior ring touches exterior at the tangent (which is valid according to OGC/ISO standards)

one-polygon-with-hole

POLYGON((2560 3072,1024 3072,1024 1024,2560 1024,2560 3072),(2560 2048,2048 1536,1536 2048,2048 2560,2560 2048))
 Is valid:1
Is simple:1

two separate polygons

test

POLYGON((2048 3072,1024 3072,1024 1024,2048 1024,2048 1536,1536 2048,2048 2560,2048 3072))
 Is valid:1
Is simple:1
POLYGON((2048 2560,1792 2048,2048 1536,2048 2560))
 Is valid:1
Is simple:1

I'm yet to incorporate these tests into vector tiles but here is the start. Results are as expected ^
/cc @flippmoke @ericfischer @springmeyer

Copy link
Contributor

@e-n-f e-n-f left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think either of these PRs would be an improvement over the current situation. Maybe fuzzing will reveal a case that one of them can handle that the other can't.

@flippmoke
Copy link
Member

LGTM, lets ship it.

@artemp artemp merged commit a0c567a into master Aug 14, 2019
@millzpaugh millzpaugh mentioned this pull request Aug 14, 2019
@mapsam mapsam mentioned this pull request Aug 14, 2019
@springmeyer springmeyer deleted the polygon-clipping-fix branch October 21, 2019 01:33
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

Successfully merging this pull request may close these issues.

4 participants