Thank you for reporting this, unfortunately I won't be able to look after this bug before June 15th.

New remote branch pushed: bug-15093-fix-itkimport

I tried to reproduce the error from the reported commit, but without success.

I have added new test for the ItkImageImport which generates an itk::Image for a given PixelType, imports it into an mitk::Image and compares the meta information and also the buffer values (voxel-wise).

@Felix: could you please checkout the remote branch (bug-15093-fix-itkimport), build the MitkTestDriver and then execute the mitkImportItkImageTest?

The test image

@Jan: The test you wrote passes, but the problem is still present. Here's the code causing problem on our side:

  template<typename TPixel, unsigned int VDimensions>
  void InRangeOverlay::InternalThreshold(
    const itk::Image<TPixel, VDimensions>* image,
    mitk::Image::Pointer& output,
    const double th[])
  {
    typedef itk::Image<TPixel, VDimensions> InputImageType;
    typedef itk::Image<unsigned char, VDimensions> OutputImageType;
    typedef itk::BinaryThresholdImageFilter<
      InputImageType, OutputImageType> BinaryThresholdFilterType;

    typename BinaryThresholdFilterType::Pointer thresholder =
      BinaryThresholdFilterType::New();
    thresholder->SetInput(image);
    thresholder->SetLowerThreshold(th[0]);
    thresholder->SetUpperThreshold(th[1]);
    thresholder->SetInsideValue(255);
    thresholder->SetOutsideValue(0);
    thresholder->Update();

    typedef itk::SimpleMemberCommand<InRangeOverlay> ITKCommandType;
    ITKCommandType::Pointer progressListener = ITKCommandType::New();
    progressListener->SetCallbackFunction(this,
      &InRangeOverlay::UpdateProgress);
    unsigned long observerTag = thresholder->AddObserver(
      itk::ProgressEvent(), progressListener);

    try
    {
      output = mitk::ImportItkImage(thresholder->GetOutput());
    }
    catch(itk::ExceptionObject&)
    {
      throw ThresholdingComputationFail();
    }

    thresholder->RemoveObserver(observerTag);
  }
}

Thanks for the test image, however even so I cannot reproduce the bug. I have tested two things:

(1) Load your test image through ITK, import to MITK and compare (as in the test)

(2) Load your test image through ITK, execute the binary threshold filter with the same code as pasted by you, but without the observer and with exact threshold values: 130=lower and 140=upper. I then imported the result to MITK and also saved it from the mitk::Image afterwards.

Both worked perfectly. The threshold produced correct image

• the midpoint (was 251) has the correct background value 0
• both lines on the right side have the foreground value 255

@Felix: As my attempts to reproduce the bug failed, could you provide us with an unit test for the erroneous behaviour along with a test image, so we can execute the code directly on our machines?

But anyhow, the test I wrote to reproduce the bug should be merged into master. Requesting Core Modification.

Overlay ground truth #1

Overlay ground truth #2

The unit test

The unit test files.cmake

This is where the fun begins.

1. If you run the tests I uploaded "as-is", the tests will pass. I found that suspicious so I decided to investigate

2. If you uncomment the line
   
   mitk::IOUtil::SaveImage(output, "/tmp/output_InternalThreshold.nii");
   
   and run the tests again, the tests will continue to pass and the produced image looks correct.

3. If you uncomment the line (keeping the line in 2) uncommented)
   
   mitk::IOUtil::SaveImage(overlayImage, "/tmp/overlayImage_TestOverlay.nii");
   
   and run the tests again, the tests will continue to pass, but if you look at the produced image, it is garbage. This is what I am seeing and why my other (and more complex) test fails

4. If you uncomment the line (keeping the lines in 2) and 3) uncommented)
   
   mitk::IOUtil::SaveImage(truth, "/tmp/truth_TestOverlay.nii");
   
   and run the tests again, the test "DirectOverlayTest999" will fail. The image from 2) is still garbage.

5. If you uncomment the lines (keeping the lines in 2), 3) and 4) uncommented)
   
   typedef itk::ImageFileWriter< InputImageType > WriterType; WriterType::Pointer writer = WriterType::New(); writer->SetFileName("/tmp/overlayITK_TestOverlay.nii"); writer->SetInput(overlayItk); writer->Update();
   
   and run the tests again, the test "DirectOverlayTest999" will fail. The image from 2) is still garbage.

6. Now, keep only
   
   mitk::IOUtil::SaveImage(truth, "/tmp/truth_TestOverlay.nii");
   
   and
   
   typedef itk::ImageFileWriter< InputImageType > WriterType; WriterType::Pointer writer = WriterType::New(); writer->SetFileName("/tmp/overlayITK_TestOverlay.nii"); writer->SetInput(overlayItk); writer->Update();
   
   uncommented. Run the tests again and both tests should fails.

Felix, thank you for the unit test + test cases. I will test it, by Wednesday at the latest.

I was able reproduce the bug with the attached test. The ImportItkImage seems not to be directly responsible for the bug - this is also indicated by the working tests.

But still, the data gets somehow invalidated, and only the data, the geometry of the saved images remains correct. Unfortunately we did not find out yet why...

New findings:

• the m_Data pointer gets invalidated, as soon as the end of the InternalThreshold function is reached, the output ( passed in as reference ) has this pointer itself as m_Data => invalid data

It is possibly freed somewhere inbetween, as the

: mitk::ImageWriter::New()

line from IOUtil::SaveImage have altered exactly the same position ( this would also explain why it worked before ? )

• using an altered macro ( itkImage is passed in, Imported outside the macro )
  
  : AccessFixedDimensionByItk_2( original, InternalThreshold2, 3, itkOverlayImage, th ); : overlayImage2 = mitk::ImportItkImage( itkOverlayImage );

is passing the test independent of (un-)commenting the SaveImage command.

I have enhanced the new mitkImportItkImageTest so it reproduces the reported buggy behavior. It now tests the ImportItkImage in combination with the AccessByItk call. The changes are available also in the remote branch.

As the import fails only when called within the AccessByItk, the bug is most probable located in the ImageToItk filter called when accessing an mitk image. Somehow the itk::Image ( or its associated PixelContainer) created inside of AccessByItk manages the imported data pointer itself and deletes it as soon as it leaves the AccessByItk macro's scope.

I investigated this issue today using Jan's expanded unit test. It seems like the memory management is corrupted. Whit using mitk::ItkImageImport the memory is managed as a reference of the filter. So the image did not allocate its own memory field. Once the methods gets out of scope, the reference on the filter is lost and so are the data.
This problem can be solved by using the method mitk::GrabItkImageMemory(...) which is also implemented in the mitkITKImageImport class.
This method takes care of the allocated memory regardless of the output from the threshold filter.

@Felix: I adapted the test a little bit (I used itk::ThresholdImageFilter instead of itk::BinaryThresholdImageFilter), please make sure your thresholding code works correctly.

@Jan: Can you please doublecheck the branch to make sure everything works fine. Branch is bug-15093-FixITKImageImport.
Thanks

Thanks for working on that!

Out of curiosity, why did you change the filter type? Also, why replace ImportItkImage with GrabItkImageMemory? I mean, ImportItkImage is still broken right?

Finally, I think the test should be renamed if the GrabItkImageMemory change is desired.

(In reply to comment #19)

[...] Also, why replace
ImportItkImage with GrabItkImageMemory? I mean, ImportItkImage is still
broken right?

The problem is selecting the method appropriate for the particular use case.

The ImportItkImage works perfectly if both source (the itk image) and target (the mitk image) share the same scope as demonstrated by the test. It is first a minimal solution measured by memory overhead and second allows for concatenating an MITK filter pipeline to an ITK filter pipeline.

If the scope-condition is broken, as in your case, the GrabItkImageMemory has to be used instead.

I will enhance the class documentation so the usage is clearly formulated there.

Finally, I think the test should be renamed if the GrabItkImageMemory change
is desired.

Thanks for the suggestion. But we should split the test, instead of renaming, since the remaining parts are testing the ImportItkImage.

Thanks for pointing that out.

Do you know why it worked before with the ITK3-based MITK version?

FYI, I can confirm that our code works when using GrabItkImageMemory instead of ImportItkImage. Again, thanks for working on that.

[31b1fe]: Merge branch 'bug-15093-fix-itkimport'

Merged commits:

2013-06-04 14:50:32 Jan Hering [a3bc33]
Enhancing documentation

• added doxygen snippets to GrabItkImageMemoryTest
• made a note on ImportItkImage usage

2013-06-04 13:51:44 Jan Hering [5083ac]
Splitting ItkImageImport Test

• into mitkItkImageImportTest
• and mitkGrabItkImageMemoryTest

2013-06-03 18:22:26 Sven Mersmann [1d27c0]
Adapted the unit test to make it run. One method needed to be modified to guarantee correct memory management.

2013-06-01 10:11:18 Jan Hering [f02378]
Changed test output

• printout of index and different values on check failure

2013-06-01 09:22:53 Jan Hering [178653]
Enhancing Import Test to reproduce the bug

• added ITK-based threshold filter, executed through AccessByItk
• added testing of ImportItkImage within an AccessByItk macro call

2013-05-10 13:52:30 Jan Hering [4ebb3d]
Added new test for ItkImageImport

[293b39]: Merge branch 'bug-15093-fix-itkimport-COMP'

Merged commits:

2013-06-04 15:05:49 Jan Hering [ec0c39]
COMP Fixed test macro usage

[b6948c]: Merge branch 'bug-15093-fix-itkimport-COMP'

Merged commits:

2013-06-04 15:53:27 Jan Hering [157658]
COMP Fixing main declaration

The documentation was built successfully, the tests are running on cont. and nightly clients. Closing bug.