search

phetsims / dot

A math library with a focus on mutable and immutable linear algebra for 2D and 3D applications.
MIT License
13 stars 6 forks source link

line line intersection doesn't handle parallel lines #32

Closed samreid closed 2 months ago

samreid commented 9 years ago

While I was looking at the source for #31 I realized that line line intersection doesn't handle parallel lines. @jonathanolson I only see this used in Kite. Do we need to handle parallel lines? Why or why not?

samreid commented 9 years ago

Or perhaps the intersection would be (NaN,NaN)??

jonathanolson commented 8 years ago

Line-line intersections in general are either no points, one point, or infinite points (overlapping). Since the client is expecting a Vector2 (indicating a successful result), would we return null if there are either no intersections or infinitely many?

We'd want to threshold this (approximately collinear lines), and also handle cases where p1=p2 or p3=p4 presumably (where they don't properly define a line).

Thoughts?

samreid commented 8 years ago

How about returning Vector2(number,number) for successful single point intersection, and null for both cases of no intersection and infinitely many intersections. If the client wants to handle that, they can delve further and determine which case they are in.

jonathanolson commented 8 years ago

Sounds great. Self-assigning for implementation.

jonathanolson commented 8 years ago

Dot's lineLineIntersection (in Util.js) should detect if the lines are approximately parallel (to within an epsilon), and if so return null instead of a Vector2. Documentation should also be updated.

andrealin commented 8 years ago

The above commit returns null if the lines are parallel to within 1e-5 or if the p1=p2 or p3=p4. @jonathanolson let me know if anything needs to be changed.

andrealin commented 8 years ago

Some lines that are not parallel were treated as so because the denominator is negative. I fixed it in the above commit.

ariel-phet commented 8 years ago

Since @jonathanolson is on leave, assigning to @veillette if he or his team has time to take a look

samreid commented 8 years ago

@andrea-phet can you please add unit tests for these new situations in the file: dot/tests/qunit/js/simple-tests.js

The unit tests should be seen when launching http://localhost/dot/tests/qunit/unit-tests.html

And please remove console.log from Util.js

andrealin commented 7 years ago

lineLineIntersection done. Assigning to @jonathanolson to review.

jonathanolson commented 2 months ago

Looks good to me. Changed the initial equals() => equalsEpsilon() since we have an epsilon. Closing.