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

getWinding()-fix for #819 #848

Closed
wants to merge 1 commit into from
Closed

getWinding()-fix for #819 #848

wants to merge 1 commit into from

Conversation

iconexperience
Copy link
Contributor

Handle cases when point y is equal to y of horizontal curve in path. Fixes #819

@iconexperience
Copy link
Contributor Author

This changes was a bit complicated, so I decided to rework part of getWinding() completely. Here are some remarks regarding the changes:

  • in the new code we first detect the start and end of each loop and then run through each loop separately. This way we do not have to check if we are on the same loop all the time.
  • horizonal curves (these are absolutely horizontal curves) are usually ignored. Only if the point is on the curve and the winding before the curve is different from and after the curve we treat this as a "touch point".
  • Since we ignore horizontal curves now, an intersection at the end of one curve, which is again found at the beginning of a following curve, is now counted only once, even if any number of horizontal curves are between these two curves.

@iconexperience
Copy link
Contributor Author

What I do not fully understand (and what I did not dare to touch) are the cases when horizontal == true. Can you tell me when this is used?

Another thing that I do not fully understand is the calculation of the tangents at the previous and next curve to find touch points (lines 371 to 375 in the original code). Since all curve are monotone in y direction, wouldn't it be possible to simply use the winding of the curve instead of calculating the tangent?

@iconexperience
Copy link
Contributor Author

And here is a little more background on the problem that this patch is supposed to fix. If the point happens to have the same y value as the end of one curve, the same intersection is found twice because the start of the next curve has the same y value. This was handled previously by checking if the intersections found at the beginning of a curve is the same as the one at the end of a curve. So cases like these were not a problem:
image
But the old code failed if there were horizontal curves (absolutely horizontal lines) between two curves. In this case the curves sharing the intersection were not adjacent. So in cases like this, the two intersections were counted:
image
There can be cases when several horizontal curves are between two intersection points, but things get really funny if the first and last curve of a path are horizontal, like in this case:
image
The new code should solve all thes cases. I will try to provide some more tests.

@iconexperience
Copy link
Contributor Author

I have created a test for a path consisting of only horizontal curves. Since the new code finds the first and last non-horizontal curve in a loop, this could potentially cause problems (but it doesnt 😉 ). Here is the sketch

@iconexperience
Copy link
Contributor Author

Here is another test case where the path's first and last curve are horizontal. Seems to work well with the new code.

@iconexperience
Copy link
Contributor Author

@lehni
This seems to be still buggy. Please do not merge, I will try to fix it and issue a new pull request.

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.

1 participant