Page MenuHomePhabricator

PlaneGeometry has misleading definition of "parallel"
Closed, ResolvedPublic

Description

PlaneGeometry provides the method IsParallel() which is used internally by other methods and can be used from outside to check the parallelity of two planes.

However, the method only returns true if the normal vector of both planes points into the same direction, i.e., for parallel planes with flipped orientations the method returns false.

Should be fixed.

Event Timeline

I just searched through all .cpp and .h files of MITK to check the usage of the affected mitk::PlaneGeometry methods.

  • IsParallel() is only used in mitk::PlanarFigureMapper2D (where the bug was actually found)
  • IsOnPlane() is not used at all outside of mitk::PlaneGeometry

I'm not entirely sure about the usage of operator== and operator!= for planes, because this is a bit harder to find out, but on a first look I didn't find any usage either.

The following definition of parallelity is currently used in PlanarFigureMapper2D as a workaround, maybe this helps:

mitk::Vector3D planeNormal1 = ...
mitk::Vector3D planeNormal2 = ...
mitk::Vector3D crossProduct = itk::CrossProduct( planeNormal1, planeNormal2 );

if ( (crossProduct.GetNorm() < mitk::eps) )
{

// Planes are parallel

}

Status:

  • IsParallel() method is fixed. Can now handle both cases.
  • moving operator==, operator!= into private didn't cause any compile errors/warnigs
  • documenting the methods

Still to be done:

  • adapt the unit tests

[SVN revision 22741]
FIX (#3965): fixing the IsParallel method, making operator== and != private and extending the unit test.

Still not working, still relevant and urgent. Mathias is working on a fix.

[SVN revision 23212]
FIX (#3965): Reduce accuracy when checking for parallelism

[SVN revision 23214]
COMP (#3965): Adapt test to reduced accuracy for parallelism check