Page MenuHomePhabricator

mitk::Line doesn't compile
Closed, ResolvedPublic

Description

In the current version the class mitk::Line doesn't compile. This is not detected by the dart clients because the class is only a header file with inline methods and is used nowhere.

The problem is, that in line 83 a itk::point is subtracted from a itk::vector which is not possible in the current itk version. It is simple to fix by replacing line 79 to 83:

itk::Vector<TCoordRep,NPointDimension> point2;
point2 = m_Point + m_Direction;

m_Point = point1;
m_Direction = point2 - point1;

by the following code:

itk::Point<TCoordRep,NPointDimension> point2;
point2 = m_Point + m_Direction;

m_Point = point1;
m_Direction = point2.GetVectorFromOrigin() - point1.GetVectorFromOrigin();

... in addition to the fix, a simple test of this class would be helpful to automatically detect bugs in the future.

Event Timeline

User franza has pushed new remote branch:

bug-17915-mitkLineCompileError

fixed the bug as described in the new branch. Should be merged as soon as possible (core change request is still needed).

A simple test to this class would also be nice. It shouldn't be much effort to implement such a test.

Can we move the inline methods into a cpp file?

User franza has pushed new remote branch:

bug-17915-MitkLineBugIntegrationBranch

[2b5084]: Merge branch 'bug-17915-mitkLineCompileError'

Merged commits:

2014-07-23 15:29:34 Alfred Franz [be85cd]
added a cpp file for mitk::Line


2014-07-23 15:28:44 Alfred Franz [0acc5c]
added a unit test for mitk::Line


2014-07-23 15:27:51 Alfred Franz [a2a52c]
fixed another bug


2014-07-17 13:47:32 Alfred Franz [0e99bb]
fixed compile error of mitk line

fixed the bug and added a unit test. Also created a cpp file for this class. However, the inline methods where not moved to the cpp file yet because this is a templated class.

If anyone knows how to make a cpp implementation for templated classes, show me or feel free to add this. There are some tutorials on doing this (e.g. http://www.parashift.com/c++-faq-lite/templates-defn-vs-decl.html) but I'm not quite sure if this is the right way and will work within MITK.

[1d37a6]: COMP: Merge branch 'bug-17915-SmallFixes'

Merged commits:

2014-07-23 16:02:59 Alfred Franz [3b9e91]

COMP (#17915): fixed another small bug

User franza has pushed new remote branch:

bug-17915-SmallFixes

[334ec5]: COMP: Merge branch 'bug-17915-SmallFixes'

Merged commits:

2014-07-23 16:36:10 Alfred Franz [d3e374]

COMP: Fixed a warning under linux

I didn't know that it is a template.

What about a unit test containing an instantiation of mitk::Line and testing the basic features? Then the code would be instantiated and compiled for the unit test.

User wirkert has pushed new remote branch:

bug-17915-WriteTestsForLineClass

Thomas Kirchner and Sebastian Wirkert analyzed Line class and made a few changes to eliminate some bugs. Still TODO:

  • Line with length 0 should be checked and exception should be thrown in these cases.
  • Rectangle intersection and Box intersection methods could be formulated more generic. A simple helper method to check if for line/line intersection point / line/(hyper)plane could be used to check for all corners. Currently, especially the rectangle intersection code is not very neat.
  • Lots of methods are still not test (e.g. IsPartOfLineSegment)

the critical bugs are fixed, the remaining todos are not that imporant. So adapting the importance to medium.

Updating target milestone to upcoming release

A problem of this class is, that it is not properly tested and already several bugs showed up doing very basic testing. So I would not recommend using it.

Reset target milestone to Unspecified as it is not crucial for the upcoming release. Also lowered importance as the original bug seems to be fixed now.

@Sebastian: In comment #11 you pushed a test and in comment #15 you say it is not properly tested. :-) Please consider pushing the test and enhance it if necessary.

kislinsk claimed this task.

The original issue of this task was fixed a long time ago.