ihmcrobotics / euclid

Vector math, geometry, reference frame, and shapes 2D & 3D
Apache License 2.0
31 stars 9 forks source link

EuclidGeometryTools.percentageOfIntersectionBetweenTwoLine2Ds loses information about colinear edge case #24

Closed calvertdw closed 4 years ago

calvertdw commented 5 years ago

For the colinear case, it would be nice if the return value could be identified as such. Since a return of 0.0 is already a successful return value for non-parallel lines, 0.0 doesn't feel right as a return value. I suggest Double.POSITIVE_INFINITY since the return value is Double.NaN for the parallel non-intersecting case.

What do you think?

public static double percentageOfIntersectionBetweenTwoLine2Ds(double pointOnLine1x, double pointOnLine1y, double lineDirection1x, double lineDirection1y,
                                                               double pointOnLine2x, double pointOnLine2y, double lineDirection2x, double lineDirection2y)
{
   //      We solve for x the problem of the form: A * x = b
   //            A      *     x     =      b
   //      / lineDirection1x -lineDirection2x \   / alpha \   / pointOnLine2x - pointOnLine1x \
   //      |                                  | * |       | = |                               |
   //      \ lineDirection1y -lineDirection2y /   \ beta  /   \ pointOnLine2y - pointOnLine1y /
   // Here, only alpha or beta is needed.
   double determinant = -lineDirection1x * lineDirection2y + lineDirection1y * lineDirection2x;
   double dx = pointOnLine2x - pointOnLine1x;
   double dy = pointOnLine2y - pointOnLine1y;
   if (Math.abs(determinant) < EuclidGeometryTools.ONE_TRILLIONTH)
   { // The lines are parallel
      // Check if they are collinear
      double cross = dx * lineDirection1y - dy * lineDirection1x;
      if (Math.abs(cross) < EuclidGeometryTools.ONE_TRILLIONTH)
      {
         /*
          * The two lines are collinear. There's an infinite number of intersection. Let's just set the
          * result to pointOnLine1, i.e. alpha = 0.0.
          */
         return Double.POSITIVE_INFINITY;
      }
      else
      {
         return Double.NaN;
      }
   }
   else
   {
      double oneOverDeterminant = 1.0 / determinant;
      double AInverse00 = oneOverDeterminant * -lineDirection2y;
      double AInverse01 = oneOverDeterminant * lineDirection2x;
      double alpha = AInverse00 * dx + AInverse01 * dy;
      return alpha;
   }
}
SylvainBertrand commented 4 years ago

Sorry, I haven't looked at this yet. I think that sounds like a good idea, I'll need to see the implementation details, but I think that'd be a good change.

SylvainBertrand commented 4 years ago

Ok I've pushed on develop.