search

mathandy / svgpathtools

A collection of tools for manipulating and analyzing SVG Path objects and Bezier curves.
MIT License
557 stars 142 forks source link

Improve Arc/Line intersections #36

Closed SebKuzminsky closed 6 years ago

SebKuzminsky commented 6 years ago

I've found two problems with Arc.intersect():

  1. Arc.intersect() returns all intersections between the specified Line and the complete ellipse implied by the Arc, including intersection points that do not lie between the start and end of the Arc.
  2. Arc.intersect() returns parameterized t values that do not fall in the expected [0.0, 1.0] range. When evaluated by Arc.point(), these out-of-range t values do specify cartesian/complex points that correspond with the actual intersection points (except fot the spurious t values returned that correspond with intersections of the ellipse outside the start to end range).

This PR fixes the first of the two problems by filtering out from the list of proposed intersection points the ones that do not lie between start and end on the Arc.

It also and adds unit tests for both problems, so it adds unit tests that fail, which I'm not sure if you want. I'd be happy to change the branch so that it only tests for the problem that it fixes, so that the net number of failing unit tests doesn't go up. (I'd put the tests for the second problem on a different branch.) Just let me know what you want.

SebKuzminsky commented 6 years ago

@mathandy did you click Approve when you intended to click Merge, or is the merge waiting for something?

mathandy commented 6 years ago

Checked through your test cases. Didn't go through your fixes yet.

On Wed, Dec 27, 2017 at 12:06 AM, Sebastian Kuzminsky < notifications@github.com> wrote:

@mathandy https://github.com/mathandy did you click Approve when you intended to click Merge, or is the merge waiting for something?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mathandy/svgpathtools/pull/36#issuecomment-354053056, or mute the thread https://github.com/notifications/unsubscribe-auth/ADB0Pb4Z1cx8YmiD0S3-w0HP08nz-XSVks5tEdBYgaJpZM4RMcba .

SebKuzminsky commented 6 years ago

I added another failing test that reproduces an arc/arc intersection problem I ran into. The Arcs look like this (note that the big arc is open at the bottom):

arcs

SebKuzminsky commented 6 years ago

I'm closing this PR because I have a better arc/line intersection fix coming in a different PR, and because I ill-advisedly added those arc/arc intersection tests that confuse things.

I'll deal with arc/arc intersections later.