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

Don't duplicate points when connecting contours with zero length brid… #675

Merged
merged 1 commit into from Aug 14, 2020

Conversation

phkahler
Copy link
Member

This seems to fix issue #303 you may want to test it a bit more than I did.

@phkahler phkahler linked an issue Aug 14, 2020 that may be closed by this pull request
@phkahler
Copy link
Member Author

In testing this I may have stumbled on a simplified test case for 296. Verified it is not a regression with this PR.

@phkahler
Copy link
Member Author

The story: When combining contours into a single polygon, islands or loops inside the outer perimeter are connected by a bridge which goes from a point on the existing contour to a point on the interior one (hole) and then after going around the hole we go back across the bridge (between the same 2 points but in reverse) and continue with points on the existing contour. So if the outer is points A,B,C,D, ..... Z and the inner is points a,b,c,d,e .... n maybe the combined contour connects a bridge from C to k which would go: A,B,C, k,l,m,n,a,b,c,d...k,C,D,E, ..... Z notice that because of the bridge C and k are in the combined list twice. In the case with issue #303 the bridge is between points that are coincident. That means the polygon will have 2 points C,k on top of each other and then another pair k,C later. This would cause problems in ear triangulation, however there is a pass made over the contour to eliminate duplicate points. This patch just tweaks the bridge code to not insert point C at all, which eliminates the need to delete it later which apparently wasn't happening for reasons?!?!

In other words, the basis for this is sound and consistent with the other code so much so that it should not be needed. But for the same reasons it should be fine to use it.

@phkahler
Copy link
Member Author

Updated brace style.

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.

None yet

1 participant