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

Some quadratic intersections with horizontal lines don't work #70

Closed
glenn-allen opened this issue Mar 1, 2017 · 1 comment
Closed

Comments

@glenn-allen
Copy link

Similar to #15 quadratic bezier intersection seems to struggle with straight lines.

One issue appears to be with the utils' "roots" function which I think is missing parenthesis on the divisor:
I.e. return [ (2*b-c)/2*(b-c) ].filter(reduce);
I think should be: return [ (2*b-c)/(2*(b-c)) ].filter(reduce);
Though my understanding of the maths isn't great so you'd want to double check that.

A second problem appears to be rounding issues again within the "utils.align" method.
My test data shows that the discriminant should be 0, however the "b" value is becoming -84.99999... instead of -85 which means d is no longer 0 and a different code path is taken.

I also wrote some tests to validate the scenarios, and I tried to push all this to open a pull request but wasn't able to (I'm new to this, do I need permissions??).

// Quadratic intersection
  var test_line = { p1: { x: 20, y: -50 }, p2: { x: 20, y: 99.5 } };
  [
    [{ x: -30, y: 0 }, { x: 105, y: 10 }, { x: 240, y: 0 }],
    [{ x: -30, y: 0 }, { x: 105, y: 18 }, { x: 240, y: 0 }],
    [{ x: -30, y: 0 }, { x: 105, y: 33 }, { x: 240, y: 0 }],
    [{ x: -30, y: 0 }, { x: 105, y: 34 }, { x: 240, y: 0 }]
  ].forEach(function(points, index) {
    var bezier = Bezier.quadraticFromPoints.apply(Bezier, points);
    assert(bezier.intersects(test_line).length !== 0, `Intersection ${index + 1} should exist`);
  });

The first 2 sets of points failed prior to the divisor fix, the fourth set of points still fails due to the rounding error.

Either way, thanks so much for the library, it's helped us a lot!

@glenn-allen
Copy link
Author

After a long time, I've updated the library and tested these scenarios, I can confirm they're fixed. Thanks!

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

No branches or pull requests

1 participant