ENH: Reimplement GDCMSeriesFileNames on gdcm::IPPSorter/Scanner (drop deprecated SerieHelper)

Documentation/docs/migration_guides/itk_6_migration_guide.md

@@ -734,3 +734,34 @@ removed. The default (non-FFTW) FFT backend is now PocketFFT
   or upstreamed on the next vnl pin update.
 - `vnl_fft_prime_factors`, `gpfa`-family symbols, and the vxl
   `vnl_algo_test_fft*`/`test_convolve` tests are gone from the vendored tree.
+
+## `GDCMSeriesFileNames` reimplemented on `gdcm::Scanner`/`IPPSorter`
+
+`itk::GDCMSeriesFileNames` no longer uses the GDCM-deprecated
+`gdcm::SerieHelper`. It now enumerates with `gdcm::Directory`, groups series
+with `gdcm::Scanner` (replicating `SerieHelper::CreateUniqueSeriesIdentifier`),
+and orders slices geometrically with `gdcm::IPPSorter`. The public API is
+unchanged; consumers compile without modification.
+
+### What you need to do
+
+- Nothing for the common case: single-series directories enumerate, group, and
+  order as before.
+- If you relied on file ordering for **duplicate-`ImagePositionPatient`** or
+  **gantry-tilted** acquisitions, re-check it. `gdcm::IPPSorter` is strict and
+  fails to sort those; on failure the input (filesystem) order is left
+  unchanged rather than fabricated. First-class support is tracked in
+  [#6468](https://github.com/InsightSoftwareConsortium/ITK/issues/6468).
+- Tags passed to `AddSeriesRestriction` now refine the series identifier
+  (sub-dividing a `SeriesInstanceUID`, the documented intent, e.g.
+  `"0008|0021"`) instead of `SerieHelper`'s largely-inert file-restriction
+  list. Malformed tags are warned-and-ignored rather than honored.
+
+### Concerns
+
+- Repeated `GetSeriesUIDs`/`GetFileNames`/`GetInputFileNames` calls no longer
+  re-scan the directory (lazy parse cached by a `TimeStamp`); a directory
+  mutated on disk between calls without an intervening `Modified()` is not
+  re-read.
+- The vendored `gdcm::SerieHelper` is untouched (GDCM still uses it
+  internally); it may be removed once upstream GDCM drops it.

Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h

@@ -21,15 +21,11 @@
 #include "itkProcessObject.h"
 #include "itkObjectFactory.h"
 #include "itkMacro.h"
+#include <map>
+#include <utility>
 #include <vector>
 #include "ITKIOGDCMExport.h"
 
-// forward declaration, to remove compile dependency on GDCM library
-namespace gdcm
-{
-class SerieHelper;
-}
-
 namespace itk
 {
 /**
@@ -208,8 +204,21 @@ class ITKIOGDCM_EXPORT GDCMSeriesFileNames : public ProcessObject
   FileNamesContainerType m_InputFileNames{};
   FileNamesContainerType m_OutputFileNames{};
 
-  /** Internal structure to order series from one directory */
-  std::unique_ptr<gdcm::SerieHelper> m_SerieHelper;
+  /** Parse the input directory into the per-series file-name map. */
+  void
+  BuildSeriesMap();
+
+  /** Ordered file names per distinct series identifier. */
+  std::map<std::string, FileNamesContainerType> m_SeriesFiles{};
+
+  /** (group,element) tags appended to the series identifier when
+   * UseSeriesDetails is enabled; seeded with the GDCM default detail tags and
+   * extended by AddSeriesRestriction. */
+  std::vector<std::pair<unsigned short, unsigned short>> m_RefineTags{};
+
+  /** Modified time of the last directory parse; the cache is rebuilt only
+   * when the object has been Modified() since. */
+  TimeStamp m_CacheBuildTime{};
 
   /** Internal structure to keep the list of series UIDs */
   SeriesUIDContainerType m_SeriesUIDs{};

Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx

@@ -18,17 +18,48 @@
 
 #include "itkGDCMSeriesFileNames.h"
 #include "itksys/SystemTools.hxx"
-#include "itkProgressReporter.h"
 #include "itkPrintHelper.h"
-#include "gdcmSerieHelper.h"
+#include "gdcmDirectory.h"
+#include "gdcmScanner.h"
+#include "gdcmIPPSorter.h"
+#include "gdcmTag.h"
+#include <algorithm>
+#include <cctype>
+#include <map>
+#include <stdexcept>
+#include <vector>
 
 namespace itk
 {
 
+namespace
+{
+// Order one series geometrically with gdcm::IPPSorter (ImagePositionPatient
+// projected on the slice normal). IPPSorter is strict: it FAILS on duplicate
+// IPP and gantry-tilt acquisitions (see issue #6468). On failure the input
+// order is left unchanged rather than fabricating an order.
+std::vector<std::string>
+OrderSeriesGeometrically(const std::vector<std::string> & files)
+{
+  if (files.size() < 2)
+  {
+    return files;
+  }
+  gdcm::IPPSorter sorter;
+  sorter.SetComputeZSpacing(false);
+  if (sorter.Sort(files))
+  {
+    return sorter.GetFilenames();
+  }
+  return files;
+}
+} // namespace
+
 
 GDCMSeriesFileNames::GDCMSeriesFileNames()
-  : m_SerieHelper{ new gdcm::SerieHelper() }
-{}
+{
+  this->SetUseSeriesDetails(true); // seeds the default series-detail tags
+}
 
 GDCMSeriesFileNames::~GDCMSeriesFileNames() = default;
 
@@ -47,7 +78,27 @@ GDCMSeriesFileNames::SetInputDirectory(const char * name)
 void
 GDCMSeriesFileNames::AddSeriesRestriction(const std::string & tag)
 {
-  m_SerieHelper->AddRestriction(tag);
+  // Parse a "group|element" tag (hex) and add it to the series-identifier
+  // criteria so it sub-refines a SeriesInstanceUID into multiple series, as
+  // documented and as used by the ITK examples (e.g. "0008|0021").
+  const std::string::size_type bar = tag.find('|');
+  if (bar == std::string::npos)
+  {
+    itkWarningMacro("Ignoring malformed series restriction tag '" << tag << "' (expected \"group|element\")");
+    return;
+  }
+  try
+  {
+    const auto group = static_cast<unsigned short>(std::stoul(tag.substr(0, bar), nullptr, 16));
+    const auto element = static_cast<unsigned short>(std::stoul(tag.substr(bar + 1), nullptr, 16));
+    m_RefineTags.emplace_back(group, element);
+  }
+  catch (const std::exception &)
+  {
+    itkWarningMacro("Ignoring malformed series restriction tag '" << tag << "' (expected hex \"group|element\")");
+    return;
+  }
+  this->Modified();
 }
 
 void
@@ -68,92 +119,133 @@ GDCMSeriesFileNames::SetInputDirectory(const std::string & name)
     return;
   }
   m_InputDirectory = name;
-  m_SerieHelper->Clear();
-  m_SerieHelper->SetUseSeriesDetails(m_UseSeriesDetails);
-  m_SerieHelper->SetLoadMode((m_LoadSequences ? 0 : gdcm::LD_NOSEQ) | (m_LoadPrivateTags ? 0 : gdcm::LD_NOSHADOW));
-  m_SerieHelper->SetDirectory(name, m_Recursive);
-  // as a side effect it also execute
   this->Modified();
 }
 
-const GDCMSeriesFileNames::SeriesUIDContainerType &
-GDCMSeriesFileNames::GetSeriesUIDs()
+void
+GDCMSeriesFileNames::BuildSeriesMap()
 {
-  m_SeriesUIDs.clear();
-  // Accessing the first serie found (assume there is at least one)
-  gdcm::FileList * flist = m_SerieHelper->GetFirstSingleSerieUIDFileSet();
-  while (flist)
+  // Reuse the previous parse unless the object has been modified since.
+  if (m_CacheBuildTime.GetMTime() > this->GetMTime())
   {
-    if (!flist->empty()) // make sure we have at least one serie
-    {
-      gdcm::File * file = (*flist)[0]; // for example take the first one
-
-      // Create its unique series ID
-      const std::string id = m_SerieHelper->CreateUniqueSeriesIdentifier(file).c_str();
+    return;
+  }
+  m_SeriesUIDs.clear();
+  m_SeriesFiles.clear();
 
-      m_SeriesUIDs.emplace_back(id.c_str());
-    }
-    flist = m_SerieHelper->GetNextSingleSerieUIDFileSet();
+  if (m_InputDirectory.empty())
+  {
+    return;
   }
-  if (m_SeriesUIDs.empty())
+
+  gdcm::Directory dir;
+  dir.Load(m_InputDirectory, m_Recursive);
+  const gdcm::Directory::FilenamesType & filenames = dir.GetFilenames();
+  if (filenames.empty())
   {
-    itkWarningMacro("No Series were found");
+    return;
   }
-  return m_SeriesUIDs;
-}
 
-const GDCMSeriesFileNames::FileNamesContainerType &
-GDCMSeriesFileNames::GetFileNames(const std::string serie)
-{
-  m_InputFileNames.clear();
-  // Accessing the first serie found (assume there is at least one)
-  gdcm::FileList * flist = m_SerieHelper->GetFirstSingleSerieUIDFileSet();
-  if (!flist)
+  const gdcm::Tag seriesUID(0x0020, 0x000e);
+  gdcm::Scanner   scanner;
+  scanner.AddTag(seriesUID);
+  if (m_UseSeriesDetails)
   {
-    itkWarningMacro("No Series can be found, make sure your restrictions are not too strong");
-    return m_InputFileNames;
+    for (const auto & [group, element] : m_RefineTags)
+    {
+      scanner.AddTag(gdcm::Tag(group, element));
+    }
   }
-  if (!serie.empty()) // user did not specify any sub selection based on UID
+  if (!scanner.Scan(filenames))
   {
-    bool found = false;
-    while (flist && !found)
+    itkWarningMacro("Failed to scan DICOM tags in " << m_InputDirectory);
+    return;
+  }
+
+  // Build the unique series identifier per file, replicating
+  // gdcm::SerieHelper::CreateUniqueSeriesIdentifier.
+  auto makeIdentifier = [&](const char * fn) -> std::string {
+    const char *      uidValue = scanner.GetValue(fn, seriesUID);
+    std::string       id = (uidValue != nullptr) ? uidValue : "";
+    const std::string uid = id;
+    if (m_UseSeriesDetails)
     {
-      if (!flist->empty()) // make sure we have at least one serie
+      for (const auto & [group, element] : m_RefineTags)
       {
-        gdcm::File *      file = (*flist)[0]; // for example take the first one
-        const std::string id = m_SerieHelper->CreateUniqueSeriesIdentifier(file).c_str();
-
-        if (id == serie)
+        const char *      value = scanner.GetValue(fn, gdcm::Tag(group, element));
+        const std::string s = (value != nullptr) ? value : "";
+        if (id == uid && !s.empty())
         {
-          found = true; // we found a match
-          break;
+          id += '.';
         }
+        id += s;
       }
-      flist = m_SerieHelper->GetNextSingleSerieUIDFileSet();
     }
-    if (!found)
+    // Eliminate all non-alphanumeric characters (keep '.').
+    id.erase(std::remove_if(id.begin(), id.end(), [](unsigned char c) { return c != '.' && std::isalnum(c) == 0; }),
+             id.end());
+    return id;
+  };
+
+  std::map<std::string, FileNamesContainerType> grouped;
+  for (const std::string & fn : filenames)
+  {
+    if (!scanner.IsKey(fn.c_str()))
     {
-      itkWarningMacro("No Series were found");
-      return m_InputFileNames;
+      continue; // not a DICOM file the scanner could read
     }
+    const std::string id = makeIdentifier(fn.c_str());
+    if (grouped.find(id) == grouped.end())
+    {
+      m_SeriesUIDs.push_back(id);
+    }
+    grouped[id].push_back(fn);
+  }
+
+  for (auto & [id, files] : grouped)
+  {
+    m_SeriesFiles[id] = OrderSeriesGeometrically(files);
   }
-  m_SerieHelper->OrderFileList(flist);
 
-  if (!flist->empty())
+  m_CacheBuildTime.Modified();
+}
+
+const GDCMSeriesFileNames::SeriesUIDContainerType &
+GDCMSeriesFileNames::GetSeriesUIDs()
+{
+  this->BuildSeriesMap();
+  if (m_SeriesUIDs.empty())
   {
-    ProgressReporter progress(this, 0, static_cast<itk::SizeValueType>(flist->size()), 10);
-    for (auto & element : *flist)
+    itkWarningMacro("No Series were found");
+  }
+  return m_SeriesUIDs;
+}
+
+const GDCMSeriesFileNames::FileNamesContainerType &
+GDCMSeriesFileNames::GetFileNames(const std::string serie)
+{
+  this->BuildSeriesMap();
+  m_InputFileNames.clear();
+  if (serie.empty())
+  {
+    // Return the first series encountered (single-series assumption).
+    if (!m_SeriesUIDs.empty())
     {
-      gdcm::FileWithName * header = element;
-      m_InputFileNames.push_back(header->filename);
-      progress.CompletedPixel();
+      m_InputFileNames = m_SeriesFiles[m_SeriesUIDs.front()];
     }
+    else
+    {
+      itkWarningMacro("No Series can be found, make sure your restrictions are not too strong");
+    }
+    return m_InputFileNames;
   }
-  else
+  const auto it = m_SeriesFiles.find(serie);
+  if (it == m_SeriesFiles.end())
   {
-    itkDebugMacro("No files were found");
+    itkWarningMacro("No Series were found");
+    return m_InputFileNames;
   }
-
+  m_InputFileNames = it->second;
   return m_InputFileNames;
 }
 
@@ -256,16 +348,6 @@ GDCMSeriesFileNames::PrintSelf(std::ostream & os, Indent indent) const
   os << indent << "InputFileNames: " << m_InputFileNames << std::endl;
   os << indent << "OutputFileNames: " << m_OutputFileNames << std::endl;
 
-  os << indent << "SerieHelper: ";
-  if (m_SerieHelper.get() != nullptr)
-  {
-    os << m_SerieHelper.get() << std::endl;
-  }
-  else
-  {
-    os << "(null)" << std::endl;
-  }
-
   os << indent << "SeriesUIDs: " << m_SeriesUIDs << std::endl;
 
   itkPrintSelfBooleanMacro(UseSeriesDetails);
@@ -278,7 +360,16 @@ void
 GDCMSeriesFileNames::SetUseSeriesDetails(bool useSeriesDetails)
 {
   m_UseSeriesDetails = useSeriesDetails;
-  m_SerieHelper->SetUseSeriesDetails(m_UseSeriesDetails);
-  m_SerieHelper->CreateDefaultUniqueSeriesIdentifier();
+  m_RefineTags.clear();
+  if (m_UseSeriesDetails)
+  {
+    // Default detail tags, matching gdcm::SerieHelper::CreateDefaultUniqueSeriesIdentifier.
+    m_RefineTags.emplace_back(0x0020, 0x0011); // Series Number
+    m_RefineTags.emplace_back(0x0018, 0x0024); // Sequence Name
+    m_RefineTags.emplace_back(0x0018, 0x0050); // Slice Thickness
+    m_RefineTags.emplace_back(0x0028, 0x0010); // Rows
+    m_RefineTags.emplace_back(0x0028, 0x0011); // Columns
+  }
+  this->Modified();
 }
 } // namespace itk

Modules/IO/GDCM/test/CMakeLists.txt

@@ -519,6 +519,7 @@ set(
   ITKGDCMImageIOGTests
   itkGDCMImageIOGTest.cxx
   itkGDCMSeriesDirectionGTest.cxx
+  itkGDCMSeriesFileNamesContractGTest.cxx
 )
 creategoogletestdriver(ITKGDCMImageIO "${ITKIOGDCM-Test_LIBRARIES}" "${ITKGDCMImageIOGTests}")