Merge pull request InsightSoftwareConsortium#4668 from blowekamp/bug_…

…label_unique

Fix Unique LabelMap filters producing non-unique labels and causing concurrent pixel writes.

Modules/Filtering/LabelMap/ITKKWStyleOverwrite.txt

@@ -2,4 +2,5 @@ test/*\.cxx Namespace Disable
 test/itkAttributePositionLabelMapFilterTest1.cxx Namespace Disable
 test/itkShapeLabelMapFilterGTest.cxx Namespace Disable
 test/itkStatisticsLabelMapFilterGTest.cxx Namespace Disable
+test/itkUniqueLabelMapFiltersGTest.cxx Namespace Disable
 test/itkShapeLabelObjectAccessorsTest1.cxx SemicolonSpace Disable

Modules/Filtering/LabelMap/include/itkAttributeUniqueLabelMapFilter.hxx

@@ -111,6 +111,8 @@ AttributeUniqueLabelMapFilter<TImage, TAttributeAccessor>::GenerateData()
  }
  }
 
+ assert(newMainLine || (idx[0] >= prevIdx[0]));
+
  if (newMainLine)
  {
  // just push the line
@@ -121,7 +123,7 @@ AttributeUniqueLabelMapFilter<TImage, TAttributeAccessor>::GenerateData()
  OffsetValueType prevLength = prev.line.GetLength();
  OffsetValueType length = l.line.GetLength();
 
- if (prevIdx[0] + prevLength >= idx[0])
+ if (prevIdx[0] + prevLength > idx[0])
  {
  // the lines are overlapping. We need to choose which line to keep.
  // the label, the only "attribute" to be guaranteed to be unique, is
@@ -193,7 +195,7 @@ AttributeUniqueLabelMapFilter<TImage, TAttributeAccessor>::GenerateData()
  // keep the previous one. If the previous line fully overlap the
  // current one,
  // the current one is fully discarded.
- if (prevIdx[0] + prevLength > idx[0] + length)
+ if (prevIdx[0] + prevLength >= idx[0] + length)
  {
  // discarding the current line - just do nothing
  }
@@ -202,9 +204,15 @@ AttributeUniqueLabelMapFilter<TImage, TAttributeAccessor>::GenerateData()
  IndexType newIdx = idx;
  newIdx[0] = prevIdx[0] + prevLength;
  OffsetValueType newLength = idx[0] + length - newIdx[0];
- l.line.SetIndex(newIdx);
- l.line.SetLength(newLength);
- lines.push_back(l);
+
+ if (newLength > 0)
+ {
+ l.line.SetIndex(newIdx);
+ l.line.SetLength(newLength);
+ // The front of this line is trimmed, it may occur after a line in the queue
+ // so the queue is used for the proper ordering.
+ pq.push(l);
+ }
  }
  }
  }

Modules/Filtering/LabelMap/include/itkShapeUniqueLabelMapFilter.h

@@ -177,6 +177,8 @@ class ITK_TEMPLATE_EXPORT ShapeUniqueLabelMapFilter : public InPlaceLabelMapFilt
  }
  }
 
+ assert(newMainLine || (idx[0] >= prevIdx[0]));
+
  if (newMainLine)
  {
  // just push the line
@@ -187,7 +189,7 @@ class ITK_TEMPLATE_EXPORT ShapeUniqueLabelMapFilter : public InPlaceLabelMapFilt
  OffsetValueType prevLength = prev.line.GetLength();
  OffsetValueType length = l.line.GetLength();
 
- if (prevIdx[0] + prevLength >= idx[0])
+ if (prevIdx[0] + prevLength > idx[0])
  {
  // the lines are overlapping. We need to choose which line to keep.
  // the label, the only "attribute" to be guaranteed to be unique, is
@@ -244,6 +246,7 @@ class ITK_TEMPLATE_EXPORT ShapeUniqueLabelMapFilter : public InPlaceLabelMapFilt
  prevLength = idx[0] - prevIdx[0];
  if (prevLength != 0)
  {
+ assert(prevIdx[0] <= idx[0]);
  lines.back().line.SetLength(idx[0] - prevIdx[0]);
  }
  else
@@ -259,7 +262,7 @@ class ITK_TEMPLATE_EXPORT ShapeUniqueLabelMapFilter : public InPlaceLabelMapFilt
  // keep the previous one. If the previous line fully overlap the
  // current one,
  // the current one is fully discarded.
- if (prevIdx[0] + prevLength > idx[0] + length)
+ if (prevIdx[0] + prevLength >= idx[0] + length)
  {
  // discarding the current line - just do nothing
  }
@@ -268,9 +271,14 @@ class ITK_TEMPLATE_EXPORT ShapeUniqueLabelMapFilter : public InPlaceLabelMapFilt
  IndexType newIdx = idx;
  newIdx[0] = prevIdx[0] + prevLength;
  OffsetValueType newLength = idx[0] + length - newIdx[0];
- l.line.SetIndex(newIdx);
- l.line.SetLength(newLength);
- lines.push_back(l);
+ if (newLength > 0)
+ {
+ l.line.SetIndex(newIdx);
+ l.line.SetLength(newLength);
+ // The front of this line is trimmed, it may occur after a line in the queue
+ // so the queue is used for the proper ordering.
+ priorityQueue.push(l);
+ }
  }
  }
  }

Modules/Filtering/LabelMap/test/CMakeLists.txt

@@ -1569,14 +1569,10 @@ itk_add_test(
  --compare
  DATA{Baseline/cthead1Label-label-statistics-unique-labelmap-baseline1.png}
  ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterTest1.png
- --compare
- DATA{Baseline/cthead1Label-label-statistics-unique-labelmap-dilate-baseline1.png}
- ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterDilationStability1.png
  itkStatisticsUniqueLabelMapFilterTest1
  DATA{${ITK_DATA_ROOT}/Input/cthead1Label.png}
  DATA{${ITK_DATA_ROOT}/Input/cthead1.png}
  ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterTest1.png
- ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterDilationStability1.png
  0
  100)
 itk_add_test(
@@ -1587,19 +1583,17 @@ itk_add_test(
  --compare
  DATA{Baseline/cthead1Label-label-statistics-unique-labelmap-baseline2.png}
  ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterTest2.png
- --compare
- DATA{Baseline/cthead1Label-label-statistics-unique-labelmap-dilate-baseline2.png}
- ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterDilationStability2.png
  --compareNumberOfPixelsTolerance
  35
  itkStatisticsUniqueLabelMapFilterTest1
  DATA{${ITK_DATA_ROOT}/Input/cthead1Label.png}
  DATA{${ITK_DATA_ROOT}/Input/cthead1.png}
  ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterTest2.png
- ${ITK_TEST_OUTPUT_DIR}/itkStatisticsUniqueLabelMapFilterDilationStability2.png
  1
  100)
 
-set(ITKLabelMapGTests itkShapeLabelMapFilterGTest.cxx itkStatisticsLabelMapFilterGTest.cxx)
+set(ITKLabelMapGTests itkShapeLabelMapFilterGTest.cxx
+ itkStatisticsLabelMapFilterGTest.cxx
+ itkUniqueLabelMapFiltersGTest.cxx)
 
 creategoogletestdriver(ITKLabelMap "${ITKLabelMap-Test_LIBRARIES}" "${ITKLabelMapGTests}")

Modules/Filtering/LabelMap/test/itkStatisticsUniqueLabelMapFilterTest1.cxx

@@ -29,26 +29,56 @@
 #include "itkGrayscaleDilateImageFilter.h"
 #include "itkObjectByObjectLabelMapFilter.h"
 
+template <typename TLabelMap>
+int
+CheckLabelMapOverlap(TLabelMap * labelMap)
+{
+ int exitCode = EXIT_SUCCESS;
+
+ for (auto & labelObject : labelMap->GetLabelObjects())
+ {
+ // Manually check each label object against all other label objects, to ensure that no two label objects share an
+ // index.
+ for (itk::SizeValueType lineNumber = 0; lineNumber < labelObject->GetNumberOfLines(); ++lineNumber)
+ {
+ auto line = labelObject->GetLine(lineNumber);
+ auto idx = line.GetIndex();
+ ITK_TEST_EXPECT_TRUE(line.GetLength() <= labelObject->GetNumberOfPixels());
+ for (itk::SizeValueType lengthIndex = 0; lengthIndex < line.GetLength(); ++lengthIndex)
+ {
+ for (auto & checkObject : labelMap->GetLabelObjects())
+ {
+ if (checkObject != labelObject && checkObject->HasIndex(idx))
+ {
+ std::cerr << "Label: " << int(labelObject->GetLabel()) << " and " << int(checkObject->GetLabel())
+ << " has index " << idx << std::endl;
+ exitCode = EXIT_FAILURE;
+ }
+ }
+ ++idx[0];
+ }
+ }
+ }
+ return exitCode;
+}
+
 int
 itkStatisticsUniqueLabelMapFilterTest1(int argc, char * argv[])
 {
- // ToDo: remove dilationOutput once the JIRA issue 3370 has been solved
- // Then, argc != 6
- if (argc != 7)
+ if (argc != 6)
  {
  std::cerr << "Missing parameters." << std::endl;
  std::cerr << "Usage: " << itkNameOfTestExecutableMacro(argv);
- std::cerr << " input feature output dilationOutput";
+ std::cerr << " input feature output";
  std::cerr << " reverseOrdering attribute";
  std::cerr << std::endl;
  return EXIT_FAILURE;
  }
  const char * inputImage = argv[1];
  const char * featureImage = argv[2];
  const char * outputImage = argv[3];
- const char * dilationOutput = argv[4];
- const bool reverseOrdering = std::stoi(argv[5]);
- const unsigned int attribute = std::stoi(argv[6]);
+ const bool reverseOrdering = std::stoi(argv[4]);
+ const unsigned int attribute = std::stoi(argv[5]);
 
  constexpr unsigned int Dimension = 2;
 
@@ -123,6 +153,15 @@ itkStatisticsUniqueLabelMapFilterTest1(int argc, char * argv[])
 
  itk::SimpleFilterWatcher watcher(unique, "filter");
 
+ ITK_TRY_EXPECT_NO_EXCEPTION(unique->Update());
+
+ int exitCode = CheckLabelMapOverlap(unique->GetOutput());
+
+ if (exitCode == EXIT_FAILURE)
+ {
+ std::cerr << "Overlap detected in the label map." << std::endl;
+ }
+
  using LabelMapToImageFilterType = itk::LabelMapToLabelImageFilter<LabelMapType, ImageType>;
  auto labelMapToImageFilter = LabelMapToImageFilterType::New();
  labelMapToImageFilter->SetInput(unique->GetOutput());
@@ -135,14 +174,7 @@ itkStatisticsUniqueLabelMapFilterTest1(int argc, char * argv[])
 
  ITK_TRY_EXPECT_NO_EXCEPTION(writer->Update());
 
-
- // WARNING: TEMPORARY: JIRA ISSUE 3370
- // Writing an additional output of just the dilated label
- writer->SetInput(grayscaleDilateFilter->GetOutput());
- writer->SetFileName(dilationOutput);
- writer->UseCompressionOn();
-
  ITK_TRY_EXPECT_NO_EXCEPTION(writer->Update());
 
- return EXIT_SUCCESS;
+ return exitCode;
 }