Skip to content

Conversation

@N-Dekker
Copy link
Contributor

@N-Dekker N-Dekker commented Jun 19, 2025

Added three static member functions to ImageIOBase:

IsComponentTypeFloatingPoint(IOComponentEnum)
IsComponentTypeUnsigned(IOComponentEnum)
GetNumberOfBitsOfComponentType(IOComponentEnum)

Eases estimating whether a component type is floating point, whether the type is unsigned, and estimating its number of bits, respectively.

Removed the template specializations of MapPixelType<TPixel>, to avoid GCC compile errors saying error: specialization of 'itk::ImageIOBase::MapPixelType' after instantiation. Presented as "STYLE" commit, as it would originally just be a nicety 😃

@github-actions github-actions bot added type:Enhancement Improvement of existing methods or implementation type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct area:IO Issues affecting the IO module labels Jun 19, 2025
Directly initialized `MapPixelType::CType` to its intended value in the primary
template definition of `MapPixelType`, and removed its specializations.

Removed `IMAGEIOBASE_TYPEMAP`, following C++ Core Guidelines, May 8, 2025,
"Don’t use macros for program text manipulation",
https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es30-dont-use-macros-for-program-text-manipulation
@N-Dekker N-Dekker force-pushed the ComponentTypeTraits branch from 0f52915 to e498641 Compare June 20, 2025 16:12
@N-Dekker N-Dekker changed the title ENH: Ease getting type traits of pixel components from ImageIO ImageIO: Add member functions to get pixel component type traits, remove MapPixelType specializations Jun 20, 2025
@N-Dekker N-Dekker force-pushed the ComponentTypeTraits branch from e498641 to 9401fba Compare June 20, 2025 20:41
@github-actions github-actions bot removed the type:Enhancement Improvement of existing methods or implementation label Jun 20, 2025
@N-Dekker N-Dekker marked this pull request as ready for review June 21, 2025 08:57
Added three static member functions to ImageIOBase:

    IsComponentTypeFloatingPoint(IOComponentEnum)
    IsComponentTypeUnsigned(IOComponentEnum)
    GetNumberOfBitsOfComponentType(IOComponentEnum)

Eases estimating whether a component type is floating point,  whether the type
is unsigned, and estimating its number of bits, respectively.
@N-Dekker N-Dekker force-pushed the ComponentTypeTraits branch from 9401fba to 3604e0c Compare June 21, 2025 09:31
Copy link
Member

@thewtex thewtex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@thewtex thewtex merged commit 02c8b35 into InsightSoftwareConsortium:master Jun 23, 2025
16 checks passed
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jun 27, 2025
Follow-up to pull request InsightSoftwareConsortium#5421
commit b209c1a
"STYLE: Remove template specializations of `ImageIOBase::MapPixelType`"

Note that `MeshIOBase::MapComponentType` supports `long double` as pixel
component type, whereas `ImageIOBase::MapPixelType` does not. Moreover,
`MeshIOBase::MapComponentType` unconditionally maps `char` to `CHAR`, whereas
`ImageIOBase::MapPixelType` maps `char` to either `CHAR` or `UCHAR`, depending
on the signedness of `char`.
N-Dekker added a commit to N-Dekker/ITK that referenced this pull request Jul 10, 2025
`ImageIOBase::GetComponentTypeTraits` incorrectly assumed that the plain `char`
type is always mapped to `IOComponentEnum::CHAR`. However, that isn't the case
when the default `char` type is an unsigned type (as can be specified by GCC
option `-funsigned-char` and MSVC option `/J`).

This bug was introduced with pull request InsightSoftwareConsortium#5421
commit 3604e0c "ENH: Ease getting type traits of
pixel components from ImageIO" (merged to the master branch on June 23, 2025).

The bug caused incorrect return values from the following function calls:

    ImageIOBase::GetNumberOfBitsOfComponentType(IOComponentEnum::INT8);
    ImageIOBase::GetNumberOfBitsOfComponentType(IOComponentEnum::CHAR);
    ImageIOBase::GetComponentTypeFromTypeTraits(false, false, 0);
    ImageIOBase::GetComponentTypeFromTypeTraits(false, false, 8);

As was observed from the static_assert failures in "itkImageIOBaseTest.cxx",
when using the compiler option that makes the default `char` unsigned.
hjmjohnson pushed a commit that referenced this pull request Jul 11, 2025
`ImageIOBase::GetComponentTypeTraits` incorrectly assumed that the plain `char`
type is always mapped to `IOComponentEnum::CHAR`. However, that isn't the case
when the default `char` type is an unsigned type (as can be specified by GCC
option `-funsigned-char` and MSVC option `/J`).

This bug was introduced with pull request #5421
commit 3604e0c "ENH: Ease getting type traits of
pixel components from ImageIO" (merged to the master branch on June 23, 2025).

The bug caused incorrect return values from the following function calls:

    ImageIOBase::GetNumberOfBitsOfComponentType(IOComponentEnum::INT8);
    ImageIOBase::GetNumberOfBitsOfComponentType(IOComponentEnum::CHAR);
    ImageIOBase::GetComponentTypeFromTypeTraits(false, false, 0);
    ImageIOBase::GetComponentTypeFromTypeTraits(false, false, 8);

As was observed from the static_assert failures in "itkImageIOBaseTest.cxx",
when using the compiler option that makes the default `char` unsigned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:IO Issues affecting the IO module type:Testing Ensure that the purpose of a class is met/the results on a wide set of test cases are correct

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants