Page MenuHomePhabricator

ClippedSurfaceBoundsCalculator does not respect border-slices correctly
Closed, ResolvedPublic

Description

I have stumbled across a case in which the ClippedSurfaceBoundsCalculator produces false results as it does not consider the border-slices of the input-image correctly (rotated geometry).

The unit-test has not found this issue, so the test shall be extended and the issue fixed.

Event Timeline

User engelm has pushed new remote branch:

bug-19088-fix-clippedsurfaceboundscalculator

User engelm has pushed new remote branch:

bug-19088-fix-clippedsurfaceboundscalculator-rebased

Actual behavior:

ClippedSurfaceBoundsCalculator calculates the cut between a plane geometry and and an image geometry.
The outcome result is the first and the last slice which is cut by the plane.
The bug occur in rotated images.
When the image is rotated the calculated cut between the plane and the image is wrong.

Expected behavior:

The calculator should work properly for rotated images.

Cause of the bug:

Wrong calculated corner based origin of the bounding box.

Proposed solution:

The origin of the bounding box should computed with the image geometry to get the wright croner based origin.
Actuallay it is always calculated by the diagonl matrix form.

Affected classes:

ClippedSurfaceBoundsCalculator.cpp

How will the bugfix get tested?

It will be tested in ClippedSurfaceBoundsCalculatorTest.cpp. A new test will be added to the list. In this test the slice location of an imgae before and after the rotation will be calculated

Here you try to correct the bounding box:
http://mitk.org/git/?p=MITK.git;a=commitdiff;h=2a1eb4965fb5d2e97576457e1fb3f5773708a014

And here you undo your correction as it seems to be a mistake:
http://mitk.org/git/?p=MITK.git;a=commitdiff;h=bef2128b4395f8de4091b1ed60b039c1b9f8a339

But there's still a difference to the original code in which the directions were retrieved from the image axis vectors 1, 0, 2 instead of 0, 1, 2.

Hey Stefan,
You are right. But the unit test have perfomred successfully.
I look again on my calculation for this.
I inform when i am sure about this

I guess, well, I hope, there must be a difference. Otherwise the calculation would be kind of dispensable if the result doesn't matter. :)

It is suspicious that the unit test didn't reveal the difference. On the other side, we know that we have many unit tests that have room for improvement - a lot of room.

That said, we should figure out what should be the difference in theory, what's going on in practice, and how we can improve the unit test to cover it.

Hey Stefan,

i have checked the changing in the indeces.
The only thing which is done by the Vectors is to
create a cube.
Through changing the indices only the order of the lines of the cube
change, but the absolute coordinates do not change (important).

So it doesn't matter which to use. But I suggest to take my one, to prevent mistakes

0,1,2 -> x,y,z is more common to use

[291465]: COMP: Merge branch 'bug-19088-fix-clippedsurfaceboundscalculator-rebas

Merged commits:

2015-10-27 10:52:24 Markus Engel [8c1efd]
fixed memory leaks in mitkClippedSurfaceBoundsCalculatorTest