diff --git a/Documentation/docs/migration_guides/itk_6_migration_guide.md b/Documentation/docs/migration_guides/itk_6_migration_guide.md index 6e228098aa6..51e4d4cd117 100644 --- a/Documentation/docs/migration_guides/itk_6_migration_guide.md +++ b/Documentation/docs/migration_guides/itk_6_migration_guide.md @@ -866,3 +866,52 @@ variable declared as `FilterType::OutputIterator` and then passed where an siblings in the iterator hierarchy, not subclasses, so such uses do not convert implicitly. Code that only iterates (`GoToBegin`/`IsAtEnd`/`Get`/`Set`) needs no change. + +## `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 an exception is thrown by default. Call + `SetFailOnAmbiguousOrdering(false)` to instead accept the legacy + `SerieHelper` heuristics (Instance Number when unique, else lexicographic + filename order) — a non-standards-conforming fallback retained only for + determinism and backward compatibility; do not trust its output. + 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. +- `SetLoadSequences`/`SetLoadPrivateTags` have no effect with the + `gdcm::Scanner` backend (the scan reads only the grouping/ordering tags). + They are retained for source compatibility but no longer alter enumeration. +- Non-image DICOM objects (structured reports, RTSTRUCT, DICOMDIR, + presentation states — anything without Rows `(0028,0010)`) are excluded + from enumeration, matching `SerieHelper`'s `ImageReader`-based acceptance. +- `GetUseSeriesDetails()` now defaults to `false`, consistent with the + documented opt-in ("you may want to try calling `SetUseSeriesDetails(true)`") + and with DICOM series identity: by default series are grouped by their raw + `SeriesInstanceUID` `(0020,000e)`, and `GetFileNames()` matches. + Grouping behavior is unchanged — the previous backend never applied the + detail tags until `SetUseSeriesDetails(true)` was called, even though the + getter misreported `true`. Only code that read `GetUseSeriesDetails()` + before calling the setter sees a different (now truthful) value. + +### 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. diff --git a/Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h b/Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h index b646a365d8a..b1c0659d6f4 100644 --- a/Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h +++ b/Modules/IO/GDCM/include/itkGDCMSeriesFileNames.h @@ -21,15 +21,11 @@ #include "itkProcessObject.h" #include "itkObjectFactory.h" #include "itkMacro.h" +#include +#include #include #include "ITKIOGDCMExport.h" -// forward declaration, to remove compile dependency on GDCM library -namespace gdcm -{ -class SerieHelper; -} - namespace itk { /** @@ -42,10 +38,11 @@ namespace itk * * 1. Extract Image Orientation & Image Position from DICOM images, and then * calculate the ordering based on the 3D coordinate of the slice. - * 2. If for some reason this information is not found or failed, another - * strategy is used: the ordering is based on 'Instance Number'. - * 3. If this strategy also failed, then the filenames are ordered by - * lexicographical order. + * 2. If the geometric ordering fails (duplicate positions, inconsistent + * orientation), an exception is thrown by default; see + * FailOnAmbiguousOrdering. When FailOnAmbiguousOrdering is false, the + * ordering falls back to 'Instance Number' when unique, else to + * lexicographic filename order. * * If multiple volumes are being grouped as a single series for your * DICOM objects, you may want to try calling SetUseSeriesDetails(true) @@ -148,7 +145,9 @@ class ITKIOGDCM_EXPORT GDCMSeriesFileNames : public ProcessObject /** Use additional series information such as ProtocolName * and SeriesName to identify when a single SeriesUID contains - * multiple 3D volumes - as can occur with perfusion and DTI imaging + * multiple 3D volumes - as can occur with perfusion and DTI imaging. + * Off by default: series are identified by their SeriesInstanceUID + * (0020,000e) alone, per the DICOM information model. */ void SetUseSeriesDetails(bool useSeriesDetails); @@ -166,15 +165,28 @@ class ITKIOGDCM_EXPORT GDCMSeriesFileNames : public ProcessObject /** Add more restriction on the selection of a Series. This follows the same * approach as SetUseSeriesDetails, but allows a user to add even more DICOM * tags to take into account for subrefining a set of DICOM files into multiple - * series. The tag format is "group|element" of a DICOM tag. + * series. The tag format is "group|element" of a DICOM tag. Call order + * relative to SetUseSeriesDetails does not matter. * \warning UseSeriesDetails needs to be set to true. */ void AddSeriesRestriction(const std::string & tag); - /** Parse any sequences in the DICOM file. Defaults to false - * to skip sequences. This makes loading DICOM files faster when - * sequences are not needed. + /** Throw an exception when a series cannot be ordered geometrically by + * gdcm::IPPSorter (duplicate ImagePositionPatient, inconsistent + * orientation). When false, fall back to the legacy SerieHelper + * heuristics: Instance Number when unique, else lexicographic filename + * order. The fallback is not DICOM-standards conforming and its output + * should not be trusted; it exists only for backward compatibility. */ + /** @ITKStartGrouping */ + itkSetMacro(FailOnAmbiguousOrdering, bool); + itkGetConstMacro(FailOnAmbiguousOrdering, bool); + itkBooleanMacro(FailOnAmbiguousOrdering); + /** @ITKEndGrouping */ + + /** No effect with the gdcm::Scanner backend (retained for source + * compatibility). Series enumeration reads only the grouping and + * ordering tags, so sequences are never parsed during the scan. */ /** @ITKStartGrouping */ itkSetMacro(LoadSequences, bool); @@ -182,9 +194,9 @@ class ITKIOGDCM_EXPORT GDCMSeriesFileNames : public ProcessObject itkBooleanMacro(LoadSequences); /** @ITKEndGrouping */ - /** Parse any private tags in the DICOM file. Defaults to false - * to skip private tags. This makes loading DICOM files faster when - * private tags are not needed. + /** No effect with the gdcm::Scanner backend (retained for source + * compatibility). Series enumeration reads only the grouping and + * ordering tags, so private tags are never parsed during the scan. */ /** @ITKStartGrouping */ itkSetMacro(LoadPrivateTags, bool); @@ -208,13 +220,42 @@ class ITKIOGDCM_EXPORT GDCMSeriesFileNames : public ProcessObject FileNamesContainerType m_InputFileNames{}; FileNamesContainerType m_OutputFileNames{}; - /** Internal structure to order series from one directory */ - std::unique_ptr m_SerieHelper; + /** Parse the input directory into the per-series file-name map. */ + void + BuildSeriesMap(); + + /** Files of one series; ordered lazily on the first GetFileNames call. */ + struct SeriesEntry + { + FileNamesContainerType Files{}; + bool Ordered{ false }; + }; + + /** Order one series geometrically, or apply the legacy fallback / throw + * per FailOnAmbiguousOrdering. */ + void + OrderSeries(SeriesEntry & entry); + + /** File names per distinct series identifier. */ + std::map m_SeriesFiles{}; + + /** Instance Number (0020,0013) per scanned file, for the legacy fallback. */ + std::map m_InstanceNumbers{}; + + /** User (group,element) tags from AddSeriesRestriction, appended to the + * series identifier (after the default detail tags) when UseSeriesDetails + * is enabled. */ + std::vector> m_UserRefineTags{}; + + /** 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{}; - bool m_UseSeriesDetails = true; + bool m_UseSeriesDetails = false; + bool m_FailOnAmbiguousOrdering = true; bool m_Recursive = false; bool m_LoadSequences = false; bool m_LoadPrivateTags = false; diff --git a/Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx b/Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx index a6697a36276..1aff70081ca 100644 --- a/Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx +++ b/Modules/IO/GDCM/src/itkGDCMSeriesFileNames.cxx @@ -18,17 +18,34 @@ #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 +#include +#include +#include +#include namespace itk { - -GDCMSeriesFileNames::GDCMSeriesFileNames() - : m_SerieHelper{ new gdcm::SerieHelper() } -{} +namespace +{ +// Default series-detail tags, matching +// gdcm::SerieHelper::CreateDefaultUniqueSeriesIdentifier. +constexpr std::pair DefaultDetailTags[] = { + { 0x0020, 0x0011 }, // Series Number + { 0x0018, 0x0024 }, // Sequence Name + { 0x0018, 0x0050 }, // Slice Thickness + { 0x0028, 0x0010 }, // Rows + { 0x0028, 0x0011 }, // Columns +}; +} // namespace + +GDCMSeriesFileNames::GDCMSeriesFileNames() = default; GDCMSeriesFileNames::~GDCMSeriesFileNames() = default; @@ -47,7 +64,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(std::stoul(tag.substr(0, bar), nullptr, 16)); + const auto element = static_cast(std::stoul(tag.substr(bar + 1), nullptr, 16)); + m_UserRefineTags.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 +105,207 @@ 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 + return; + } + m_SeriesUIDs.clear(); + m_SeriesFiles.clear(); + m_InstanceNumbers.clear(); - // Create its unique series ID - const std::string id = m_SerieHelper->CreateUniqueSeriesIdentifier(file).c_str(); + if (m_InputDirectory.empty()) + { + return; + } - m_SeriesUIDs.emplace_back(id.c_str()); - } - flist = m_SerieHelper->GetNextSingleSerieUIDFileSet(); + gdcm::Directory dir; + dir.Load(m_InputDirectory, m_Recursive); + const gdcm::Directory::FilenamesType & filenames = dir.GetFilenames(); + if (filenames.empty()) + { + return; } - if (m_SeriesUIDs.empty()) + + std::vector> refineTags; + if (m_UseSeriesDetails) { - itkWarningMacro("No Series were found"); + refineTags.assign(std::begin(DefaultDetailTags), std::end(DefaultDetailTags)); + refineTags.insert(refineTags.end(), m_UserRefineTags.begin(), m_UserRefineTags.end()); } - 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); + const gdcm::Tag instanceNumber(0x0020, 0x0013); + const gdcm::Tag rows(0x0028, 0x0010); + gdcm::Scanner scanner; + scanner.AddTag(seriesUID); + scanner.AddTag(instanceNumber); + scanner.AddTag(rows); + for (const auto & [group, element] : refineTags) { - itkWarningMacro("No Series can be found, make sure your restrictions are not too strong"); - return m_InputFileNames; + 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; + for (const auto & [group, element] : refineTags) { - if (!flist->empty()) // make sure we have at least one serie + const char * value = scanner.GetValue(fn, gdcm::Tag(group, element)); + const std::string s = (value != nullptr) ? value : ""; + if (id == uid && !s.empty()) { - gdcm::File * file = (*flist)[0]; // for example take the first one - const std::string id = m_SerieHelper->CreateUniqueSeriesIdentifier(file).c_str(); - - if (id == serie) - { - found = true; // we found a match - break; - } + id += '.'; } - flist = m_SerieHelper->GetNextSingleSerieUIDFileSet(); + id += s; } - 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; + }; + + 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 + } + if (scanner.GetValue(fn.c_str(), rows) == nullptr) + { + continue; // no Rows: not an image object (SR, RTSTRUCT, DICOMDIR, ...) + } + const std::string id = makeIdentifier(fn.c_str()); + SeriesEntry & entry = m_SeriesFiles[id]; + if (entry.Files.empty()) + { + m_SeriesUIDs.push_back(id); + } + entry.Files.push_back(fn); + if (const char * number = scanner.GetValue(fn.c_str(), instanceNumber)) + { + m_InstanceNumbers[fn] = number; } } - m_SerieHelper->OrderFileList(flist); - if (!flist->empty()) + m_CacheBuildTime.Modified(); +} + +void +GDCMSeriesFileNames::OrderSeries(SeriesEntry & entry) +{ + if (entry.Ordered || entry.Files.size() < 2) + { + entry.Ordered = true; + return; + } + // Geometric ordering: ImagePositionPatient projected on the slice normal. + // gdcm::IPPSorter is strict: it FAILS on duplicate IPP and gantry-tilt + // acquisitions (see issue #6468). + gdcm::IPPSorter sorter; + sorter.SetComputeZSpacing(false); + if (sorter.Sort(entry.Files)) + { + entry.Files = sorter.GetFilenames(); + entry.Ordered = true; + return; + } + if (m_FailOnAmbiguousOrdering) + { + itkExceptionMacro("Series cannot be ordered geometrically (duplicate ImagePositionPatient or inconsistent " + "orientation, see issue #6468). Set FailOnAmbiguousOrdering to false to accept the legacy " + "non-standard ordering heuristics."); + } + // Legacy SerieHelper heuristics (Instance Number, then lexicographic), + // kept only for determinism and backward compatibility: an untrustworthy, + // non-standard hack whose output should not be trusted. + std::map byInstanceNumber; + bool instanceNumbersUsable = true; + for (const std::string & fn : entry.Files) + { + const auto found = m_InstanceNumbers.find(fn); + try + { + const long number = std::stol(found != m_InstanceNumbers.end() ? found->second : std::string()); + instanceNumbersUsable = byInstanceNumber.emplace(number, fn).second; + } + catch (const std::exception &) + { + instanceNumbersUsable = false; + } + if (!instanceNumbersUsable) + { + break; + } + } + if (instanceNumbersUsable) { - ProgressReporter progress(this, 0, static_cast(flist->size()), 10); - for (auto & element : *flist) + entry.Files.clear(); + for (const auto & [number, fn] : byInstanceNumber) { - gdcm::FileWithName * header = element; - m_InputFileNames.push_back(header->filename); - progress.CompletedPixel(); + entry.Files.push_back(fn); } } else { - itkDebugMacro("No files were found"); + std::sort(entry.Files.begin(), entry.Files.end()); } + entry.Ordered = true; +} +const GDCMSeriesFileNames::SeriesUIDContainerType & +GDCMSeriesFileNames::GetSeriesUIDs() +{ + this->BuildSeriesMap(); + if (m_SeriesUIDs.empty()) + { + 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()) + { + SeriesEntry & entry = m_SeriesFiles[m_SeriesUIDs.front()]; + this->OrderSeries(entry); + m_InputFileNames = entry.Files; + } + else + { + itkWarningMacro("No Series can be found, make sure your restrictions are not too strong"); + } + return m_InputFileNames; + } + const auto it = m_SeriesFiles.find(serie); + if (it == m_SeriesFiles.end()) + { + itkWarningMacro("No Series were found"); + return m_InputFileNames; + } + this->OrderSeries(it->second); + m_InputFileNames = it->second.Files; return m_InputFileNames; } @@ -256,19 +408,10 @@ 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); + itkPrintSelfBooleanMacro(FailOnAmbiguousOrdering); itkPrintSelfBooleanMacro(Recursive); itkPrintSelfBooleanMacro(LoadSequences); itkPrintSelfBooleanMacro(LoadPrivateTags); @@ -277,8 +420,10 @@ GDCMSeriesFileNames::PrintSelf(std::ostream & os, Indent indent) const void GDCMSeriesFileNames::SetUseSeriesDetails(bool useSeriesDetails) { - m_UseSeriesDetails = useSeriesDetails; - m_SerieHelper->SetUseSeriesDetails(m_UseSeriesDetails); - m_SerieHelper->CreateDefaultUniqueSeriesIdentifier(); + if (m_UseSeriesDetails != useSeriesDetails) + { + m_UseSeriesDetails = useSeriesDetails; + this->Modified(); + } } } // namespace itk diff --git a/Modules/IO/GDCM/test/CMakeLists.txt b/Modules/IO/GDCM/test/CMakeLists.txt index 84fa55ebfa5..29319347357 100644 --- a/Modules/IO/GDCM/test/CMakeLists.txt +++ b/Modules/IO/GDCM/test/CMakeLists.txt @@ -519,6 +519,7 @@ set( ITKGDCMImageIOGTests itkGDCMImageIOGTest.cxx itkGDCMSeriesDirectionGTest.cxx + itkGDCMSeriesFileNamesContractGTest.cxx ) creategoogletestdriver(ITKGDCMImageIO "${ITKIOGDCM-Test_LIBRARIES}" "${ITKGDCMImageIOGTests}") diff --git a/Modules/IO/GDCM/test/itkGDCMSeriesFileNamesContractGTest.cxx b/Modules/IO/GDCM/test/itkGDCMSeriesFileNamesContractGTest.cxx new file mode 100644 index 00000000000..e3e9343dddb --- /dev/null +++ b/Modules/IO/GDCM/test/itkGDCMSeriesFileNamesContractGTest.cxx @@ -0,0 +1,371 @@ +/*========================================================================= + * + * Copyright NumFOCUS + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + *=========================================================================*/ + +// Characterization tests pinning the observable contract of +// itk::GDCMSeriesFileNames (issue #6467). These must remain green when the +// backend is migrated from the deprecated gdcm::SerieHelper to +// gdcm::Scanner + gdcm::IPPSorter: one series identifier for a single-series +// directory, deterministic geometric ordering that reconstructs a valid +// uniformly-spaced volume, GetInputFileNames == first series, and the +// Recursive flag controlling descent. + +#include "gtest/gtest.h" +#include "itkGDCMSeriesFileNames.h" +#include "itkGDCMImageIO.h" +#include "itkImageSeriesReader.h" +#include "itkImage.h" +#include "itkMath.h" +#include "itksys/SystemTools.hxx" +#include "itksys/Directory.hxx" +#include "gdcmReader.h" +#include "gdcmWriter.h" +#include "gdcmDataElement.h" +#include "gdcmStringFilter.h" +#include "gdcmTag.h" +#include +#include +#include +#include + +#define _STRING(s) #s +#define TOSTRING(s) std::string(_STRING(s)) + +namespace +{ +const std::string & +SeriesDir() +{ + static const std::string dir = TOSTRING(DICOM_SERIES_INPUT); + return dir; +} + +unsigned int +CopyDicomSlices(const std::string & srcDir, const std::string & dstDir) +{ + itksys::SystemTools::MakeDirectory(dstDir); + itksys::Directory directory; + directory.Load(srcDir.c_str()); + unsigned int copied = 0; + for (unsigned int i = 0; i < directory.GetNumberOfFiles(); ++i) + { + const std::string name = directory.GetFile(i); + if (itksys::SystemTools::GetFilenameLastExtension(name) != ".dcm") + { + continue; + } + if (itksys::SystemTools::CopyFileAlways(srcDir + '/' + name, dstDir + '/' + name)) + { + ++copied; + } + } + return copied; +} + +std::string +FirstDicomSlice(const std::string & srcDir) +{ + itksys::Directory directory; + directory.Load(srcDir.c_str()); + for (unsigned int i = 0; i < directory.GetNumberOfFiles(); ++i) + { + const std::string name = directory.GetFile(i); + if (itksys::SystemTools::GetFilenameLastExtension(name) == ".dcm") + { + return srcDir + '/' + name; + } + } + return {}; +} + +std::string +ReadSeriesInstanceUID(const std::string & file) +{ + gdcm::Reader reader; + reader.SetFileName(file.c_str()); + if (!reader.Read()) + { + return {}; + } + gdcm::StringFilter sf; + sf.SetFile(reader.GetFile()); + std::string uid = sf.ToString(gdcm::Tag(0x0020, 0x000e)); + while (!uid.empty() && (uid.back() == '\0' || uid.back() == ' ')) + { + uid.pop_back(); + } + return uid; +} + +// Copy one slice, overwriting Instance Number (0020,0013) so duplicate-IPP +// fixtures exercise the legacy ordering fallbacks. +bool +WriteWithInstanceNumber(const std::string & src, const std::string & dst, const char * instanceNumber) +{ + gdcm::Reader reader; + reader.SetFileName(src.c_str()); + if (!reader.Read()) + { + return false; + } + gdcm::DataElement de(gdcm::Tag(0x0020, 0x0013)); + de.SetVR(gdcm::VR::IS); + de.SetByteValue(instanceNumber, static_cast(std::strlen(instanceNumber))); + reader.GetFile().GetDataSet().Replace(de); + gdcm::Writer writer; + writer.SetFileName(dst.c_str()); + writer.SetFile(reader.GetFile()); + return writer.Write(); +} + +// Copy one slice, stripping Rows/Columns/PixelData so it mimics a non-image +// DICOM object (SR, RTSTRUCT, ...) sharing the series' SeriesInstanceUID; +// stripSeriesUID additionally drops SeriesInstanceUID (DICOMDIR-like). +bool +WriteNonImageObject(const std::string & src, const std::string & dst, bool stripSeriesUID = false) +{ + gdcm::Reader reader; + reader.SetFileName(src.c_str()); + if (!reader.Read()) + { + return false; + } + gdcm::DataSet & ds = reader.GetFile().GetDataSet(); + ds.Remove(gdcm::Tag(0x0028, 0x0010)); + ds.Remove(gdcm::Tag(0x0028, 0x0011)); + ds.Remove(gdcm::Tag(0x7fe0, 0x0010)); + if (stripSeriesUID) + { + ds.Remove(gdcm::Tag(0x0020, 0x000e)); + } + gdcm::Writer writer; + writer.SetCheckFileMetaInformation(false); + writer.SetFileName(dst.c_str()); + writer.SetFile(reader.GetFile()); + return writer.Write(); +} +} // namespace + +TEST(GDCMSeriesFileNamesContract, SingleSeriesEnumerationAndGrouping) +{ + auto sf = itk::GDCMSeriesFileNames::New(); + sf->SetInputDirectory(SeriesDir()); + + const std::vector uids = sf->GetSeriesUIDs(); + ASSERT_EQ(uids.size(), 1u) << "DicomSeries is a single series"; + + const std::vector byUid = sf->GetFileNames(uids.front()); + EXPECT_GT(byUid.size(), 1u) << "Series has multiple slices"; + + const std::vector input = sf->GetInputFileNames(); + EXPECT_EQ(input, byUid) << "GetInputFileNames must return the (single) series, ordered identically"; +} + +TEST(GDCMSeriesFileNamesContract, OrderingReconstructsValidVolume) +{ + using ImageType = itk::Image; + auto sf = itk::GDCMSeriesFileNames::New(); + sf->SetInputDirectory(SeriesDir()); + const std::vector files = sf->GetInputFileNames(); + ASSERT_GT(files.size(), 1u); + + auto reader = itk::ImageSeriesReader::New(); + reader->SetImageIO(itk::GDCMImageIO::New()); + reader->SetFileNames(files); + ASSERT_NO_THROW(reader->Update()); + + const ImageType::Pointer image = reader->GetOutput(); + EXPECT_EQ(image->GetLargestPossibleRegion().GetSize()[2], files.size()) << "Each ordered slice becomes one z-plane"; + for (unsigned int d = 0; d < 3; ++d) + { + const double spacing = image->GetSpacing()[d]; + EXPECT_TRUE(std::isfinite(spacing) && spacing > 0.0) << "Spacing[" << d << "] must be positive and finite"; + } +} + +TEST(GDCMSeriesFileNamesContract, RecursiveFlagHonored) +{ + const std::string root = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_recursive"; + itksys::SystemTools::RemoveADirectory(root); + itksys::SystemTools::MakeDirectory(root); + ASSERT_GT(CopyDicomSlices(SeriesDir(), root + "/nested"), 1u); + + // Recursive is read lazily during BuildSeriesMap(); order relative to + // SetInputDirectory does not matter. + auto flat = itk::GDCMSeriesFileNames::New(); + flat->SetRecursive(false); + flat->SetInputDirectory(root); + EXPECT_TRUE(flat->GetInputFileNames().empty()) << "No DICOM directly under root; flat scan finds none"; + + auto recursive = itk::GDCMSeriesFileNames::New(); + recursive->SetRecursive(true); + recursive->SetInputDirectory(root); + EXPECT_FALSE(recursive->GetInputFileNames().empty()) << "Recursive scan must descend into the subdirectory"; +} + +TEST(GDCMSeriesFileNamesContract, AmbiguousOrderingThrowsByDefault) +{ + const std::string root = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_ambiguous_throw"; + itksys::SystemTools::RemoveADirectory(root); + itksys::SystemTools::MakeDirectory(root); + const std::string slice = FirstDicomSlice(SeriesDir()); + ASSERT_FALSE(slice.empty()); + // Two copies of the same slice: duplicate ImagePositionPatient. + ASSERT_TRUE(itksys::SystemTools::CopyFileAlways(slice, root + "/copy_a.dcm")); + ASSERT_TRUE(itksys::SystemTools::CopyFileAlways(slice, root + "/copy_b.dcm")); + + auto sf = itk::GDCMSeriesFileNames::New(); + EXPECT_TRUE(sf->GetFailOnAmbiguousOrdering()); + sf->SetInputDirectory(root); + EXPECT_NO_THROW(sf->GetSeriesUIDs()) << "Enumeration does not order and must not throw"; + EXPECT_THROW(sf->GetInputFileNames(), itk::ExceptionObject); +} + +TEST(GDCMSeriesFileNamesContract, LegacyFallbackUsesInstanceNumberThenFilename) +{ + const std::string slice = FirstDicomSlice(SeriesDir()); + ASSERT_FALSE(slice.empty()); + + // Duplicate IPP with unique Instance Numbers: fallback must order by + // Instance Number, chosen here to be the reverse of lexicographic order. + const std::string byInstance = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_fallback_instance"; + itksys::SystemTools::RemoveADirectory(byInstance); + itksys::SystemTools::MakeDirectory(byInstance); + ASSERT_TRUE(WriteWithInstanceNumber(slice, byInstance + "/z_first.dcm", "1 ")); + ASSERT_TRUE(WriteWithInstanceNumber(slice, byInstance + "/a_second.dcm", "2 ")); + + auto sf = itk::GDCMSeriesFileNames::New(); + sf->FailOnAmbiguousOrderingOff(); + sf->SetInputDirectory(byInstance); + const std::vector instanceOrdered = sf->GetInputFileNames(); + ASSERT_EQ(instanceOrdered.size(), 2u); + EXPECT_NE(instanceOrdered[0].find("z_first"), std::string::npos); + EXPECT_NE(instanceOrdered[1].find("a_second"), std::string::npos); + + // Duplicate IPP and duplicate Instance Numbers: final fallback is + // lexicographic filename order. + const std::string byName = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_fallback_filename"; + itksys::SystemTools::RemoveADirectory(byName); + itksys::SystemTools::MakeDirectory(byName); + ASSERT_TRUE(itksys::SystemTools::CopyFileAlways(slice, byName + "/copy_b.dcm")); + ASSERT_TRUE(itksys::SystemTools::CopyFileAlways(slice, byName + "/copy_a.dcm")); + + auto lex = itk::GDCMSeriesFileNames::New(); + lex->FailOnAmbiguousOrderingOff(); + lex->SetInputDirectory(byName); + const std::vector nameOrdered = lex->GetInputFileNames(); + ASSERT_EQ(nameOrdered.size(), 2u); + EXPECT_LT(nameOrdered[0], nameOrdered[1]); +} + +TEST(GDCMSeriesFileNamesContract, NonImageObjectsExcluded) +{ + const std::string root = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_nonimage"; + itksys::SystemTools::RemoveADirectory(root); + itksys::SystemTools::MakeDirectory(root); + const unsigned int copied = CopyDicomSlices(SeriesDir(), root); + ASSERT_GT(copied, 1u); + const std::string slice = FirstDicomSlice(SeriesDir()); + ASSERT_FALSE(slice.empty()); + ASSERT_TRUE(WriteNonImageObject(slice, root + "/structured_report.dcm")); + + auto sf = itk::GDCMSeriesFileNames::New(); + sf->SetInputDirectory(root); + const std::vector uids = sf->GetSeriesUIDs(); + ASSERT_EQ(uids.size(), 1u) << "Non-image object must not surface as an extra series"; + const std::vector files = sf->GetFileNames(uids.front()); + EXPECT_EQ(files.size(), copied) << "Only image slices are enumerated"; + for (const std::string & fn : files) + { + EXPECT_EQ(fn.find("structured_report"), std::string::npos); + } +} + +TEST(GDCMSeriesFileNamesContract, DefaultGroupingUsesRawSeriesInstanceUID) +{ + const std::string slice = FirstDicomSlice(SeriesDir()); + ASSERT_FALSE(slice.empty()); + const std::string uid = ReadSeriesInstanceUID(slice); + ASSERT_FALSE(uid.empty()); + + auto sf = itk::GDCMSeriesFileNames::New(); + EXPECT_FALSE(sf->GetUseSeriesDetails()) << "Series details are a documented opt-in"; + sf->SetInputDirectory(SeriesDir()); + const std::vector uids = sf->GetSeriesUIDs(); + ASSERT_EQ(uids.size(), 1u); + EXPECT_EQ(uids.front(), uid) << "Default identifiers are plain SeriesInstanceUIDs (0020,000e)"; + EXPECT_FALSE(sf->GetFileNames(uid).empty()) << "A raw SeriesInstanceUID from other DICOM tooling must match"; +} + +TEST(GDCMSeriesFileNamesContract, SeriesRestrictionSurvivesSetUseSeriesDetails) +{ + // Instance Number is unique per slice, so restricting on it splits the + // single series into one series per file — easy to detect. + auto restrictionFirst = itk::GDCMSeriesFileNames::New(); + restrictionFirst->AddSeriesRestriction("0020|0013"); + restrictionFirst->SetUseSeriesDetails(true); + restrictionFirst->SetInputDirectory(SeriesDir()); + const std::vector uids = restrictionFirst->GetSeriesUIDs(); + EXPECT_GT(uids.size(), 1u) << "Restriction must take effect regardless of call order"; + + auto detailsFirst = itk::GDCMSeriesFileNames::New(); + detailsFirst->SetUseSeriesDetails(true); + detailsFirst->AddSeriesRestriction("0020|0013"); + detailsFirst->SetInputDirectory(SeriesDir()); + EXPECT_EQ(detailsFirst->GetSeriesUIDs(), uids) << "Call order must not change the series grouping"; +} + +TEST(GDCMSeriesFileNamesContract, MixedContentDirectoryEnumeratesOnlyImageSeries) +{ + const std::string root = TOSTRING(ITK_TEST_OUTPUT_DIR) + "/gdcmcontract_mixed"; + itksys::SystemTools::RemoveADirectory(root); + itksys::SystemTools::MakeDirectory(root); + const unsigned int copied = CopyDicomSlices(SeriesDir(), root); + ASSERT_GT(copied, 1u); + const std::string slice = FirstDicomSlice(SeriesDir()); + ASSERT_FALSE(slice.empty()); + // SR-like object sharing the series' SeriesInstanceUID. + ASSERT_TRUE(WriteNonImageObject(slice, root + "/report_in_series.dcm")); + // DICOMDIR-like object with no SeriesInstanceUID at all. + ASSERT_TRUE(WriteNonImageObject(slice, root + "/dicomdir_like.dcm", true)); + // Plain non-DICOM file. + { + std::ofstream readme(root + "/README.txt"); + readme << "not a DICOM file\n"; + } + + auto sf = itk::GDCMSeriesFileNames::New(); + sf->SetInputDirectory(root); + const std::vector uids = sf->GetSeriesUIDs(); + ASSERT_EQ(uids.size(), 1u) << "Only the image series is enumerated"; + const std::vector ordered = sf->GetInputFileNames(); + ASSERT_EQ(ordered.size(), copied); + for (const std::string & fn : ordered) + { + EXPECT_EQ(fn.find("report_in_series"), std::string::npos); + EXPECT_EQ(fn.find("dicomdir_like"), std::string::npos); + EXPECT_EQ(fn.find("README"), std::string::npos); + } + + // The mixed content must not disturb geometric ordering of the series. + using ImageType = itk::Image; + auto reader = itk::ImageSeriesReader::New(); + reader->SetImageIO(itk::GDCMImageIO::New()); + reader->SetFileNames(ordered); + ASSERT_NO_THROW(reader->Update()); + EXPECT_EQ(reader->GetOutput()->GetLargestPossibleRegion().GetSize()[2], ordered.size()); +}