Image IO follow-ups deferred from PR #1585

[imikejackson]

PR #1585 added a new format-agnostic image-IO subsystem (IImageIO + StbImageIO / TiffImageIO backends) and three new SimplnxCore filters (ReadImageFilter, WriteImageFilter, ReadImageStackFilter). Two automated review passes (cross-cutting code quality + design validation) flagged a number of items that were intentionally deferred from that PR to keep the diff focused and to land the merge sooner. This issue tracks them.

Each item links back to the relevant comment thread on #1585 for the original rationale.

Correctness / API discipline

• Replace throw std::runtime_error in GetDataTypeSize and the ImageIOUtilities choice helpers with Result<T>.
  simplnx/Common/TypesUtility.hpp (GetDataTypeSize) and simplnx/Utilities/ImageIO/ImageIOUtilities.hpp (ChoiceToImageDataType, ImageDataTypeToChoice) currently throw on unexpected enum values. The codebase convention is Result<T> / MakeErrorResult. The throws are unreachable in practice today (callers come from validated ChoicesParameter indexes / known DataType enum values), but a future caller from an unvalidated path would crash any pipeline runner that does not catch. Should be done as a coordinated pass that also touches the existing GetNumericTypeSize for consistency.

• TiffOpenOptions first-error vs last-error semantics.
  simplnx/Utilities/ImageIO/TiffImageIO.cpp — when libtiff produces multiple errors during a single operation, the per-handle error handler currently keeps only the last one. The first error is usually the most diagnostic ("unknown tag 700" → "cannot read scanline N" — the first one is the actual cause). Change to first-write-wins.

• Add a focused regression test for the ITK multi-component ConvertImageToDataStoreAsType fix.
  PR ENH: Add Image Reader/Writer that depend on Tiff and Stb libraries. #1585 fixed a bug in ITKImageProcessing/Common/ReadImageUtils.hpp where multi-component pixel images were getting only their first 1/N scalars filled in (pixelContainer->Size() returns the pixel count, not the scalar element count). Add a test that reads a 3-channel RGB TIFF with type conversion to uint16 and asserts every channel of every pixel is populated, not just the first 1/3.

Performance

• In-core memcpy fast path on CopyPixelDataFunctor / ExtractSliceFunctor.
  Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadImage.cpp and WriteImage.cpp currently walk every pixel through AbstractDataStore::operator[] / setValue — a virtual call per scalar component. For a 4096×4096 RGBA image that is ~67M virtual calls per slice. When (a) no cropping, (b) srcType == destType, and (c) the destination is in-core, the whole row (or the whole image) can be a single std::memcpy. Depends on the planned bulk copyIntoBuffer / copyFromBuffer API on AbstractDataStore — file a separate issue for that API if it doesn't already exist.

• Tiled-TIFF chunked read.
  simplnx/Utilities/ImageIO/TiffImageIO.cpp readPixelData falls back to TIFFReadRGBAImageOriented for tiled TIFFs and allocates std::vector<uint32_t>(width * height) (1 GiB for a 16384² image). Switch to TIFFReadRGBAStrip / TIFFReadRGBATile to read in chunks. At minimum, reject upfront with a clear error if the implied scratch buffer exceeds some threshold rather than failing inside std::vector::vector with std::bad_alloc.

• Per-slice sub-filter construction in ReadImageStack.
  Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadImageStack.cpp constructs a fresh ReadImageFilter, Arguments, and scratch DataStructure per slice (and another ResampleImageGeomFilter / ConvertColorToGrayScaleFilter per slice when those modes are on). For a 1000-slice stack that is 1000 sub-filter setups. Hoist the Arguments outside the slice loop and only insertOrAssign the per-slice file path.

• Skip per-slice double-preflight in ReadImageStack.
  ReadImageFilter::execute re-runs preflight, which itself opens the file to read metadata. Each slice file is therefore opened twice. Consider an algorithm-level entry point (ReadImage::operator() directly) the stack code can use without going through the filter wrapper.

• FlipAboutXAxis row-swap via std::swap_ranges.
  Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadImageStack.cpp does element-by-element scalar swap with getValue / operator[]. FlipAboutYAxis already uses iterator-based row buffer copies; FlipAboutXAxis should swap whole rows via std::swap_ranges on the iterators. Same algorithmic complexity, real constant-factor savings.

• Throttled progress messaging in slice loops.
  Both Algorithms/ReadImageStack.cpp and Algorithms/WriteImage.cpp fmt::format a per-slice "Importing/Writing X/Y" message every iteration. For thousands of slices this is wasted work behind a probably-no-op handler. Use ThrottledMessenger per the progress-messaging skill.

• Streaming ReadImage instead of whole-image scratch buffer.
  Algorithms/ReadImage.cpp allocates std::vector<uint8> tempBuffer of one image's worth of bytes — bounded for typical inputs but a 65535×65535 RGBA float32 image is 64 GiB. Add a scanline-streaming code path that materializes one row at a time when the source format supports it (libtiff scanline reads do).

• IImageIO::beginStream() / writeRow() / endStream() extension.
  Algorithms/WriteImage.cpp per-slice scratch buffer is correct and bounded today, but a streaming write API would let TIFF and other row-oriented formats skip the intermediate slice buffer entirely. Only worth doing if profiling shows the slice buffer matters.

Architecture / API

• Format registry instead of an if/else chain in CreateImageIO.
  simplnx/Utilities/ImageIO/ImageIOFactory.cpp hard-codes the supported extension list inside an if/else. Same list is duplicated in WriteImageFilter::parameters() (currently ExtensionListType{} — empty). Add a static const std::map<std::string, std::function<std::unique_ptr<IImageIO>()>> plus a GetSupportedReadExtensions() / GetSupportedWriteExtensions() helper that the filter parameter UI can call. Important once the third backend lands.

• Populate WriteImageFilter extension dropdown from the same source as CreateImageIO.
  Plugins/SimplnxCore/src/SimplnxCore/Filters/WriteImageFilter.cpp declares ExtensionListType{} (empty) for the output FileSystemPathParameter — the user can type any extension and only preflightImpl validates. Tied to the previous registry item.

• Drop SIMPLNXCORE_EXPORT from BuildReadImageFilterArgs.
  Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/ReadImageStack.hpp exports a sub-filter argument helper across the DLL boundary. If only the algorithms call it, drop the export and put it in an anonymous namespace.

• Decouple ReadImageStack algorithm from ReadImageFilter header.
  Algorithms/ReadImageStack.cpp currently includes SimplnxCore/Filters/ReadImageFilter.hpp to construct a ReadImageFilter per slice. If we expose an algorithm-level entry point on ReadImage (see the double-preflight item above), the algorithm file can depend only on data + IO and not on a sibling filter header.

• ConvertColorToGrayScaleFilter in-place mode.
  Algorithms/ReadImageStack.cpp does an awkward "convert to grayscale → get array named gray<original> → delete the original → rename the gray one back" dance because ConvertColorToGrayScaleFilter does not support in-place. Long-term fix is on the grayscale filter.

Defensive

• WriteImageFilter::preflightImpl should validate totalDigits >= 1.
  Negative or zero totalDigits is currently passed straight to CreateIndexString which throws an fmt::format exception out of preflight. Bounds-check in preflight and MakeErrorResult instead.

• fs::create_directories use std::error_code overload.
  Algorithms/WriteImage.cpp fs::create_directories failure currently produces a generic "Error creating output directory" message. The std::error_code overload would let us surface the OS-level reason.

• Surface override conflicts in ReadImageFilter::preflightImpl.
  When the file specifies an origin/spacing and the user also overrides one in the filter, emit a PreflightValue warning ("file specifies origin X but you are overriding to Y") so the conflict is visible up front. Currently silent.

• TIFFGetField(XPOSITION/YPOSITION) partial-presence.
  simplnx/Utilities/ImageIO/TiffImageIO.cpp readMetadata sets hasOrigin = true if either X or Y position tag is present; if only X is present, yPosition stays 0.0f and is silently written into metadata.origin[1]. Either require both, or comment the partial-presence behavior.

Documentation

• Filter docs need a "use this filter for these formats" section.
  Both the new ReadImageFilter / WriteImageFilter / ReadImageStackFilter and the existing ITK-based equivalents are now equally findable in the filter palette (after PR ENH: Add Image Reader/Writer that depend on Tiff and Stb libraries. #1585 dropped the per-format tags from the ITK reader). Add a "Use this for PNG/JPEG/BMP/TIFF; use ITK for everything else" note to both filter pairs' documentation so users know which to reach for first.

• Replace placeholder doc comments on ReadImageFilter.hpp and ReadImageStackFilter.hpp.
  Both /** @brief This filter will .... */ comments are placeholders. Replace with a one-sentence description of what the filter does.

Source

• Cross-cutting code review
• Design-validation review
• Round-3 fixes already landed in #1585