diff --git a/.github/workflows/macos-arm.yml b/.github/workflows/arm.yml similarity index 89% rename from .github/workflows/macos-arm.yml rename to .github/workflows/arm.yml index 0648012274a..1d0b9b42051 100644 --- a/.github/workflows/macos-arm.yml +++ b/.github/workflows/arm.yml @@ -1,4 +1,4 @@ -name: ITK.macOS.Arm64 +name: ITK.Arm64 on: push: @@ -36,12 +36,24 @@ env: jobs: build: runs-on: ${{ matrix.os }} - timeout-minutes: 0 + timeout-minutes: 600 strategy: fail-fast: false matrix: include: + - os: ubuntu-24.04-arm + name: "Ubuntu-24.04-arm" + cmake-build-type: "Release" + cmake-generator: "Ninja" + python-version: "" + ctest-cache: | + BUILD_SHARED_LIBS:BOOL=OFF + BUILD_EXAMPLES:BOOL=OFF + ITK_WRAP_PYTHON:BOOL=OFF + ITK_USE_CLANG_FORMAT:BOOL=OFF + ctest-options: "" + - os: macos-15 name: "x86_64-rosetta" cmake-build-type: "Release" @@ -67,7 +79,7 @@ jobs: ITK_USE_CLANG_FORMAT:BOOL=OFF ctest-options: "-E itkPyBufferMemoryLeakTest" - name: macOS-${{ matrix.name }} + name: ARMBUILD-${{ matrix.name }} steps: - name: Checkout diff --git a/Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx b/Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx index da70101e0ae..2bf349764b3 100644 --- a/Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx +++ b/Modules/Core/Common/test/itkMetaDataDictionaryGTest.cxx @@ -17,9 +17,11 @@ *=========================================================================*/ #include "itkGTest.h" +#include "itkGTestPredicate.h" #include "itkMetaDataDictionary.h" #include "itkMetaDataObject.h" +#include "itkTestVerifyMetaData.h" #include #include // For iota. @@ -44,6 +46,62 @@ createMetaDataDictionary() return metaDataDictionary; } + +template +static void +CheckMetaData(itk::MetaDataDictionary & metaDict, const std::string & key, const T & knownValue) +{ + itk::EncapsulateMetaData(metaDict, key, knownValue); + ITK_EXPECT_METADATA_VALUE(metaDict, key, knownValue); +} + + +static void +doExposeMetaDatas() +{ + // Simplified version of tests found in HDF5 reading/writing + // that are broken out here to improve localization of bugs + // found during linux-arm building + itk::MetaDataDictionary metaDict; + + CheckMetaData(metaDict, "TestBool", false); +#if !defined(ITK_FUTURE_LEGACY_REMOVE) + CheckMetaData(metaDict, "TestChar", 'c'); +#endif + CheckMetaData(metaDict, "TestUChar", 'u'); + CheckMetaData(metaDict, "TestShort", 1); + CheckMetaData(metaDict, "TestUShort", 3); + CheckMetaData(metaDict, "TestInt", 5); + CheckMetaData(metaDict, "TestUInt", 7); + CheckMetaData(metaDict, "TestLong", 5); + CheckMetaData(metaDict, "TestULong", 7); + CheckMetaData(metaDict, "TestLLong", -5); + CheckMetaData(metaDict, "TestULLong", 7ull); + CheckMetaData(metaDict, "TestFloat", 1.23456f); + CheckMetaData(metaDict, "TestDouble", 1.23456); + +#if !defined(ITK_FUTURE_LEGACY_REMOVE) + itk::Array metaDataCharArray(5); + metaDataCharArray[0] = 'h'; + metaDataCharArray[1] = 'e'; + metaDataCharArray[2] = 'l'; + metaDataCharArray[3] = 'l'; + metaDataCharArray[4] = 'o'; + CheckMetaData>(metaDict, "TestCharArray", metaDataCharArray); +#endif + + itk::Array metaDataDoubleArray(5); + metaDataDoubleArray[0] = 3.0; + metaDataDoubleArray[1] = 1.0; + metaDataDoubleArray[2] = 4.0; + metaDataDoubleArray[3] = 5.0; + metaDataDoubleArray[4] = 2.0; + CheckMetaData>(metaDict, "TestDoubleArray", metaDataDoubleArray); + + CheckMetaData(metaDict, "StdString", "Test std::string"); +} + + template itk::MetaDataObjectBase::Pointer createMetaDataObject(const T & invalue) @@ -56,6 +114,9 @@ createMetaDataObject(const T & invalue) TEST(MetaDataDictionary, Basic) { + // Isolate + doExposeMetaDatas(); + // This test exercises and checks the non-constant interface itk::MetaDataDictionary dic = createMetaDataDictionary(); diff --git a/Modules/Core/TestKernel/include/itkGTestPredicate.h b/Modules/Core/TestKernel/include/itkGTestPredicate.h index 5d82a37f288..95a8177ee60 100644 --- a/Modules/Core/TestKernel/include/itkGTestPredicate.h +++ b/Modules/Core/TestKernel/include/itkGTestPredicate.h @@ -23,6 +23,8 @@ #include "gtest/gtest.h" #include "itkNumericTraits.h" +#include "itkMetaDataDictionary.h" +#include "itkMetaDataObject.h" #include @@ -33,6 +35,11 @@ #define ITK_EXPECT_VECTOR_NEAR(val1, val2, rmsError) \ EXPECT_PRED_FORMAT3(itk::GTest::Predicate::VectorDoubleRMSPredFormat, val1, val2, rmsError) +/** A custom GTest macro for verifying a value in a MetaDataDictionary. + */ +#define ITK_EXPECT_METADATA_VALUE(metaDict, key, knownValue) \ + EXPECT_PRED_FORMAT3(itk::GTest::Predicate::CheckMetaDataPredFormat, metaDict, key, knownValue) + /** \namespace itk::GTest::Predicate * \brief The Predicate namespace contains functions used to @@ -41,6 +48,120 @@ namespace itk::GTest::Predicate { +/** Implements GTest Predicate Formatter for ITK_EXPECT_METADATA_VALUE + * macro. + */ +template +inline ::testing::AssertionResult +CheckMetaDataPredFormat(const char * metaDictExpr, + const char * keyExpr, + const char * knownValueExpr, + itk::MetaDataDictionary & metaDict, + const std::string & key, + const T & knownValue) +{ + T exposedValue{}; + +#if defined ITK_FUTURE_LEGACY_REMOVE + static_assert( + !std::is_same_v, T>, + "Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent."); + static_assert( + !std::is_same_v, + "Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent."); + if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + return ::testing::AssertionFailure() << "Failure ExposeMetaData for key '" << key << "' (" << keyExpr << ") in " + << metaDictExpr; + } +#else + if constexpr (std::is_same_v, T>) + { + if (!itk::ExposeMetaData>(metaDict, key, exposedValue)) + { + if constexpr (std::is_signed_v) + { + itk::Array temp_value{}; + if (!itk::ExposeMetaData>(metaDict, key, temp_value)) + { + return ::testing::AssertionFailure() + << "Failure ExposeMetaData '" << key << "' (" << keyExpr << ") in " << metaDictExpr; + } + exposedValue = temp_value; + } + else + { + itk::Array temp_value{}; + if (!itk::ExposeMetaData>(metaDict, key, temp_value)) + { + return ::testing::AssertionFailure() + << "Failure ExposeMetaData '" << key << "' (" << keyExpr << ") in " << metaDictExpr; + } + exposedValue = temp_value; + } + } + } + else if constexpr (std::is_same_v) + { + if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + if constexpr (std::is_signed_v) + { + signed char temp_value{}; + if (!itk::ExposeMetaData(metaDict, key, temp_value)) + { + return ::testing::AssertionFailure() + << "Failure ExposeMetaData '" << key << "' (" << keyExpr << ") in " << metaDictExpr; + } + exposedValue = static_cast(temp_value); + } + else + { + unsigned char temp_value{}; + if (!itk::ExposeMetaData(metaDict, key, temp_value)) + { + return ::testing::AssertionFailure() + << "Failure ExposeMetaData '" << key << "' (" << keyExpr << ") in " << metaDictExpr; + } + exposedValue = static_cast(temp_value); + } + } + } + else if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + return ::testing::AssertionFailure() << "Failure ExposeMetaData for key '" << key << "' (" << keyExpr << ") in " + << metaDictExpr; + } +#endif + + bool match = false; + if constexpr (std::is_floating_point_v) + { + match = itk::Math::AlmostEquals(exposedValue, knownValue); + } + else + { + match = (exposedValue == knownValue); + } + + if (match) + { + return ::testing::AssertionSuccess(); + } + + ::testing::AssertionResult failure = ::testing::AssertionFailure(); + failure << "Incorrect meta value read in for " << keyExpr << " ('" << key << "') in " << metaDictExpr << "\n" + << " Actual: " << exposedValue << "\n" + << " Expected: " << knownValue << " (" << knownValueExpr << ")\n"; + failure << "========================================\n"; + std::stringstream ss; + metaDict.Print(ss); + failure << ss.str(); + failure << "========================================"; + return failure; +} + + /** Implements GTest Predicate Formatter for ITK_EXPECT_VECTOR_NEAR * macro. This macro and formatter work with any combination of ITK * array-like objects which are supported by itk::NumericTraits. The diff --git a/Modules/Core/TestKernel/include/itkTestVerifyMetaData.h b/Modules/Core/TestKernel/include/itkTestVerifyMetaData.h new file mode 100644 index 00000000000..aff70f1af6b --- /dev/null +++ b/Modules/Core/TestKernel/include/itkTestVerifyMetaData.h @@ -0,0 +1,150 @@ +/*========================================================================= + * + * 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. + * + *=========================================================================*/ +#ifndef itkTestVerifyMetaData_h +#define itkTestVerifyMetaData_h + +#include "itkMetaDataDictionary.h" +#include "itkMetaDataObject.h" +namespace itk +{ +/* A utility function used for testing, this is not intended to be part of the public interface */ +/* Used only to avoid duplicate code in itkMetaDictionaryGTest.cxx and itkHDF5ImageIOTest.cxx */ +template +int +VerifyMetaDataPrivateTestingUtility(const itk::MetaDataDictionary & metaDict, + const std::string & key, + const T & knownValue) +{ + int status = EXIT_SUCCESS; + T exposedValue{}; + +#if defined ITK_FUTURE_LEGACY_REMOVE + static_assert( + !std::is_same_v, T>, + "Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent."); + static_assert( + !std::is_same_v, + "Should not use the ambiguous 'char' stored in meta data, because it is not-cross platform consistent."); + if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + std::cerr << "Failure ExposeMetaData for key '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } +#else + if constexpr (std::is_same_v, T>) + { + // If Encapsulate and Expose Metadata is all in core memory operations, + // the type of char may be preserved. + if (!itk::ExposeMetaData>(metaDict, key, exposedValue)) + { + // If Encapsulate and Expose Metadata is written to disk and + // possibly shared across platforms, then 'char' may have been + // stored in an intermediate format (aka file on disk) with explicit + // signed or unsigned characteristics + if constexpr (std::is_signed_v) + { + itk::Array temp_value{}; + if (!itk::ExposeMetaData>(metaDict, key, temp_value)) + { + std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } + exposedValue = temp_value; + } + else + { + itk::Array temp_value{}; + if (!itk::ExposeMetaData>(metaDict, key, temp_value)) + { + std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } + exposedValue = temp_value; + } + } + } + else if constexpr (std::is_same_v) + { + // If Encapsulate and Expose Metadata is all in core memory operations, + // the type of char may be preserved. + if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + // If Encapsulate and Expose Metadata is written to disk and + // possibly shared across platforms, then 'char' may have been + // stored in an intermediate format (aka file on disk) with explicit + // signed or unsigned characteristics + if constexpr (std::is_signed_v) + { + signed char temp_value{}; + + if (!itk::ExposeMetaData(metaDict, key, temp_value)) + { + std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } + exposedValue = static_cast(temp_value); + } + else + { + unsigned char temp_value{}; + if (!itk::ExposeMetaData(metaDict, key, temp_value)) + { + std::cerr << "Failure ExposeMetaData '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } + exposedValue = static_cast(temp_value); + } + } + } + else if (!itk::ExposeMetaData(metaDict, key, exposedValue)) + { + std::cerr << "Failure ExposeMetaData of boolean type '" << key << "'" << std::endl; + status = EXIT_FAILURE; + } +#endif + + + if constexpr (std::is_floating_point_v) + { + if (itk::Math::NotAlmostEquals(exposedValue, knownValue)) + { + std::cerr << "Incorrect meta value read in for " << key << " '" << exposedValue << "' != '" << knownValue << "'" + << std::endl; + status = EXIT_FAILURE; + } + } + else + { + if (exposedValue != knownValue) + { + std::cerr << "Incorrect meta value read in for " << key << " '" << exposedValue << "' != '" << knownValue << "'" + << std::endl; + status = EXIT_FAILURE; + } + } + if (status == EXIT_FAILURE) + { + std::cerr << "========================================" << std::endl; + metaDict.Print(std::cerr); + std::cerr << "========================================" << std::endl; + } + return status; +} +} // namespace itk + +#endif // itkTestVerifyMetaData_h diff --git a/Modules/IO/HDF5/src/itkHDF5ImageIO.cxx b/Modules/IO/HDF5/src/itkHDF5ImageIO.cxx index 512149258bf..c4ba06801ea 100644 --- a/Modules/IO/HDF5/src/itkHDF5ImageIO.cxx +++ b/Modules/IO/HDF5/src/itkHDF5ImageIO.cxx @@ -24,6 +24,7 @@ #include "itkMakeUniqueForOverwrite.h" #include +#include // For is_signed_v. namespace itk { @@ -83,9 +84,18 @@ GetType() }; \ ITK_MACROEND_NOOP_STATEMENT -GetH5TypeSpecialize(float, H5::PredType::NATIVE_FLOAT); -GetH5TypeSpecialize(double, H5::PredType::NATIVE_DOUBLE); +#if !defined ITK_FUTURE_LEGACY_REMOVE +/* The char type is intended to be storage of ASCII values + * where ASCII is only valid between 0-127 (aka 7-bit), so + * it should not matter if it is signed or unsigned + * Use signed for persistent cross-platform interpretation. + * + * Prefer int8_t (signed char) for signed numeric 8bit values. + * Prefer uint8_t (unsigned char) for unsigned numeric 8-bit values. + */ GetH5TypeSpecialize(char, H5::PredType::NATIVE_CHAR); +#endif +GetH5TypeSpecialize(signed char, H5::PredType::NATIVE_SCHAR); GetH5TypeSpecialize(unsigned char, H5::PredType::NATIVE_UCHAR); GetH5TypeSpecialize(short, H5::PredType::NATIVE_SHORT); GetH5TypeSpecialize(short unsigned int, H5::PredType::NATIVE_USHORT); @@ -95,10 +105,12 @@ GetH5TypeSpecialize(long, H5::PredType::NATIVE_LONG); GetH5TypeSpecialize(long unsigned int, H5::PredType::NATIVE_ULONG); GetH5TypeSpecialize(long long, H5::PredType::NATIVE_LLONG); GetH5TypeSpecialize(unsigned long long, H5::PredType::NATIVE_ULLONG); +GetH5TypeSpecialize(float, H5::PredType::NATIVE_FLOAT); +GetH5TypeSpecialize(double, H5::PredType::NATIVE_DOUBLE); -/* The following types are not implemented. This comment serves - * to indicate that the full complement of possible H5::PredType - * types are not implemented int the ITK IO reader/writer +/* The following bool type is not implemented. This comment serves + * to indicate that not all possible values of H5::PredType + * types are implemented in the ITK IO reader/writer * GetH5TypeSpecialize(bool, H5::PredType::NATIVE_HBOOL) */ @@ -111,9 +123,13 @@ PredTypeToComponentType(H5::DataType & type) { return IOComponentEnum::UCHAR; } - if (type == H5::PredType::NATIVE_CHAR) + else if (type == H5::PredType::NATIVE_CHAR) { - return IOComponentEnum::CHAR; + return std::is_signed_v ? IOComponentEnum::SCHAR : IOComponentEnum::UCHAR; + } + else if (type == H5::PredType::NATIVE_SCHAR) + { + return IOComponentEnum::SCHAR; } else if (type == H5::PredType::NATIVE_USHORT) { @@ -166,7 +182,7 @@ ComponentToPredType(IOComponentEnum cType) case IOComponentEnum::UCHAR: return H5::PredType::NATIVE_UCHAR; case IOComponentEnum::SCHAR: - return H5::PredType::NATIVE_CHAR; + return H5::PredType::NATIVE_SCHAR; case IOComponentEnum::USHORT: return H5::PredType::NATIVE_USHORT; case IOComponentEnum::SHORT: @@ -262,7 +278,7 @@ HDF5ImageIO::WriteScalar(const std::string & path, const bool value) { constexpr hsize_t numScalars(1); const H5::DataSpace scalarSpace(1, &numScalars); - const H5::PredType scalarType = H5::PredType::NATIVE_HBOOL; + const H5::PredType scalarType = H5::PredType::STD_U8LE; H5::DataSet scalarSet = m_H5File->createDataSet(path, scalarType, scalarSpace); // // HDF5 can't distinguish @@ -274,7 +290,7 @@ HDF5ImageIO::WriteScalar(const std::string & path, const bool value) bool trueVal(true); isBool.write(scalarType, &trueVal); isBool.close(); - auto tempVal = static_cast(value); + auto tempVal = static_cast(value); scalarSet.write(&tempVal, scalarType); scalarSet.close(); } @@ -550,6 +566,27 @@ HDF5ImageIO::StoreMetaData(MetaDataDictionary * metaDict, const std::string & name, unsigned long numElements) { +#ifdef ITK_FUTURE_LEGACY_REMOVE + static_assert(!std::is_same_v, + "HDF5 meta-data should not use the ambiguous 'char' to be stored" + "in HDF5 meta data, because it is not-cross platform consistent."); +#else + if (std::is_same_v) + { + // Make best attempt, and preserve backward compatibility + // Be explicit about the storage type requested for persistence to be read back on any platform. + if constexpr (std::is_signed_v) + { + this->StoreMetaData(metaDict, HDFPath, name, numElements); + } + else + { + this->StoreMetaData(metaDict, HDFPath, name, numElements); + } + return; + } +#endif + if (numElements == 1) { auto val = this->ReadScalar(HDFPath); @@ -756,16 +793,7 @@ HDF5ImageIO::ReadImageInformation() // work around bool/int confusion on disk. if (metaDataType == H5::PredType::NATIVE_INT) { - if (doesAttrExist(metaDataSet, "isBool")) - { - // itk::Array apparently can't - // happen because vnl_vector isn't - // instantiated - auto tmpVal = this->ReadScalar(localMetaDataName); - bool val = tmpVal != 0; - EncapsulateMetaData(metaDict, name, val); - } - else if (doesAttrExist(metaDataSet, "isLong")) + if (doesAttrExist(metaDataSet, "isLong")) { auto val = this->ReadScalar(localMetaDataName); EncapsulateMetaData(metaDict, name, val); @@ -782,16 +810,39 @@ HDF5ImageIO::ReadImageInformation() } else if (metaDataType == H5::PredType::NATIVE_CHAR) { - this->StoreMetaData(&metaDict, localMetaDataName, name, metaDataDims[0]); + if constexpr (std::numeric_limits::is_signed) + { + this->StoreMetaData(&metaDict, localMetaDataName, name, metaDataDims[0]); + } + else + { + if (doesAttrExist(metaDataSet, "isBool")) + { + // itk::Array apparently can't + // happen because vnl_vector isn't + // instantiated + auto tmpVal = this->ReadScalar(localMetaDataName); + bool val = tmpVal != 0; + EncapsulateMetaData(metaDict, name, val); + } + else + { + this->StoreMetaData(&metaDict, localMetaDataName, name, metaDataDims[0]); + } + } + } + else if (metaDataType == H5::PredType::NATIVE_SCHAR) + { + this->StoreMetaData(&metaDict, localMetaDataName, name, metaDataDims[0]); } - else if (metaDataType == H5::PredType::NATIVE_UCHAR) + else if (metaDataType == H5::PredType::NATIVE_UCHAR || metaDataType == H5::PredType::STD_U8LE) { if (doesAttrExist(metaDataSet, "isBool")) { // itk::Array apparently can't // happen because vnl_vector isn't // instantiated - auto tmpVal = this->ReadScalar(localMetaDataName); + auto tmpVal = this->ReadScalar(localMetaDataName); bool val = tmpVal != 0; EncapsulateMetaData(metaDict, name, val); } @@ -964,12 +1015,41 @@ template bool HDF5ImageIO::WriteMeta(const std::string & name, MetaDataObjectBase * metaObjBase) { - auto * metaObj = dynamic_cast *>(metaObjBase); - if (metaObj == nullptr) +#ifdef ITK_FUTURE_LEGACY_REMOVE + static_assert(!std::is_same_v, + "HDF5 meta-data should not use the ambiguous 'char' to be stored" + "in HDF5 meta data, because it is not-cross platform consistent."); +#else + if (std::is_same_v) + { + auto charObj = dynamic_cast *>(metaObjBase); + if (charObj == nullptr || charObj->GetMetaDataObjectTypeInfo() != typeid(char)) + { + return false; + } + auto charValue = charObj->GetMetaDataObjectValue(); + // Make the best attempt and preserve backward compatibility + // Be explicit about the storage type requested for persistence to be read back on any platform. + if constexpr (std::is_signed_v) + { + auto tempMetaDataObject = MetaDataObject::New(); + tempMetaDataObject->SetMetaDataObjectValue(charValue); + return this->WriteMeta(name, tempMetaDataObject); + } + else + { + auto tempMetaDataObject = MetaDataObject::New(); + tempMetaDataObject->SetMetaDataObjectValue(charValue); + return this->WriteMeta(name, tempMetaDataObject); + } + } +#endif + if (metaObjBase == nullptr || metaObjBase->GetMetaDataObjectTypeInfo() != typeid(TType)) { return false; } - TType val = metaObj->GetMetaDataObjectValue(); + auto * metaObj = dynamic_cast *>(metaObjBase); + TType val = metaObj->GetMetaDataObjectValue(); this->WriteScalar(name, val); return true; } @@ -978,12 +1058,46 @@ template bool HDF5ImageIO::WriteMetaArray(const std::string & name, MetaDataObjectBase * metaObjBase) { +#ifdef ITK_FUTURE_LEGACY_REMOVE + static_assert(!std::is_same_v, + "HDF5 meta-data should not use the ambiguous 'char' to be stored" + "in HDF5 meta data, because it is not-cross platform consistent."); +#else + if (std::is_same_v) + { + + auto charArrayObject = dynamic_cast> *>(metaObjBase); + if (charArrayObject == nullptr || charArrayObject->GetMetaDataObjectTypeInfo() != typeid(itk::Array)) + { + return false; + } + auto charArrayValue = charArrayObject->GetMetaDataObjectValue(); + + // Make the best attempt and preserve backward compatibility + // Be explicit about the storage type requested for persistence to be read back on any platform. + if constexpr (std::is_signed_v) + { + Array signedCharArray = charArrayValue; + auto tempMetaDataObject = MetaDataObject>::New(); + tempMetaDataObject->SetMetaDataObjectValue(signedCharArray); + return this->WriteMetaArray(name, tempMetaDataObject); + } + else + { + Array unsignedCharArray = charArrayValue; + auto tempMetaDataObject = MetaDataObject>::New(); + tempMetaDataObject->SetMetaDataObjectValue(unsignedCharArray); + return this->WriteMetaArray(name, tempMetaDataObject); + } + } +#endif + using MetaDataArrayObject = MetaDataObject>; - auto * metaObj = dynamic_cast(metaObjBase); - if (metaObj == nullptr) + if (metaObjBase == nullptr || metaObjBase->GetMetaDataObjectTypeInfo() != typeid(Array)) { return false; } + auto * metaObj = dynamic_cast(metaObjBase); Array val = metaObj->GetMetaDataObjectValue(); std::vector vecVal(val.GetSize()); for (unsigned int i = 0; i < val.size(); ++i) @@ -1104,10 +1218,18 @@ HDF5ImageIO::WriteImageInformation() { continue; } +#if !defined ITK_FUTURE_LEGACY_REMOVE + // Writing the ambiguous type is not supported. + // Use unsigned char, signed char, std::uint8_t or int8_t if (this->WriteMeta(objName, metaObj)) { continue; } +#endif + if (this->WriteMeta(objName, metaObj)) + { + continue; + } if (this->WriteMeta(objName, metaObj)) { continue; @@ -1152,10 +1274,18 @@ HDF5ImageIO::WriteImageInformation() { continue; } +#ifndef ITK_FUTURE_LEGACY_REMOVE + // Make the best attempt and preserve backward compatibility + // Be explicit about the storage type requested for persistence to be read back on any platform. if (this->WriteMetaArray(objName, metaObj)) { continue; } +#endif + if (this->WriteMetaArray(objName, metaObj)) + { + continue; + } if (this->WriteMetaArray(objName, metaObj)) { continue; diff --git a/Modules/IO/HDF5/test/itkHDF5ImageIOTest.cxx b/Modules/IO/HDF5/test/itkHDF5ImageIOTest.cxx index ed3fea13a08..bf85cae5a7f 100644 --- a/Modules/IO/HDF5/test/itkHDF5ImageIOTest.cxx +++ b/Modules/IO/HDF5/test/itkHDF5ImageIOTest.cxx @@ -19,10 +19,11 @@ #include "itkHDF5ImageIO.h" #include "itkHDF5ImageIOFactory.h" #include "itkIOTestHelper.h" -#include "itkMetaDataObject.h" +#include "itkTestVerifyMetaData.h" #include "itkMath.h" #include "itkTestingMacros.h" + template int HDF5ReadWriteTest(const char * fileName) @@ -55,45 +56,26 @@ HDF5ReadWriteTest(const char * fileName) // // add some unique metadata itk::MetaDataDictionary & metaDict(im->GetMetaDataDictionary()); - constexpr bool metaDataBool(false); - itk::EncapsulateMetaData(metaDict, "TestBool", metaDataBool); - - constexpr char metaDataChar('c'); - itk::EncapsulateMetaData(metaDict, "TestChar", metaDataChar); - - constexpr unsigned char metaDataUChar('u'); - itk::EncapsulateMetaData(metaDict, "TestUChar", metaDataUChar); - - constexpr short metaDataShort{ 1 }; - itk::EncapsulateMetaData(metaDict, "TestShort", metaDataShort); - - constexpr unsigned short metaDataUShort{ 3 }; - itk::EncapsulateMetaData(metaDict, "TestUShort", metaDataUShort); - - constexpr int metaDataInt{ 5 }; - itk::EncapsulateMetaData(metaDict, "TestInt", metaDataInt); - - constexpr unsigned int metaDataUInt{ 7 }; - itk::EncapsulateMetaData(metaDict, "TestUInt", metaDataUInt); - - constexpr long metaDataLong{ 5 }; - itk::EncapsulateMetaData(metaDict, "TestLong", metaDataLong); - - constexpr unsigned long metaDataULong{ 7 }; - itk::EncapsulateMetaData(metaDict, "TestULong", metaDataULong); - - constexpr long long metaDataLLong(-5); - itk::EncapsulateMetaData(metaDict, "TestLLong", metaDataLLong); - constexpr unsigned long long metaDataULLong(7ull); - itk::EncapsulateMetaData(metaDict, "TestULLong", metaDataULLong); - - constexpr float metaDataFloat(1.23456); - itk::EncapsulateMetaData(metaDict, "TestFloat", metaDataFloat); - - constexpr double metaDataDouble(1.23456); - itk::EncapsulateMetaData(metaDict, "TestDouble", metaDataDouble); + itk::EncapsulateMetaData(metaDict, "TestBool", false); +#ifndef ITK_FUTURE_LEGACY_REMOVE + // char is not portable across platforms + itk::EncapsulateMetaData(metaDict, "TestChar", 'c'); +#endif + itk::EncapsulateMetaData(metaDict, "TestSChar", 's'); + itk::EncapsulateMetaData(metaDict, "TestUChar", 'u'); + itk::EncapsulateMetaData(metaDict, "TestShort", 1); + itk::EncapsulateMetaData(metaDict, "TestUShort", 3); + itk::EncapsulateMetaData(metaDict, "TestInt", 5); + itk::EncapsulateMetaData(metaDict, "TestUInt", 7); + itk::EncapsulateMetaData(metaDict, "TestLong", 5); + itk::EncapsulateMetaData(metaDict, "TestULong", 7); + itk::EncapsulateMetaData(metaDict, "TestLLong", -5); + itk::EncapsulateMetaData(metaDict, "TestULLong", 7ull); + itk::EncapsulateMetaData(metaDict, "TestFloat", 1.23456f); + itk::EncapsulateMetaData(metaDict, "TestDouble", 1.23456); +#ifndef ITK_FUTURE_LEGACY_REMOVE itk::Array metaDataCharArray(5); metaDataCharArray[0] = 'h'; metaDataCharArray[1] = 'e'; @@ -101,6 +83,7 @@ HDF5ReadWriteTest(const char * fileName) metaDataCharArray[3] = 'l'; metaDataCharArray[4] = 'o'; itk::EncapsulateMetaData>(metaDict, "TestCharArray", metaDataCharArray); +#endif itk::Array metaDataDoubleArray(5); metaDataDoubleArray[0] = 3.0; @@ -110,8 +93,7 @@ HDF5ReadWriteTest(const char * fileName) metaDataDoubleArray[4] = 2.0; itk::EncapsulateMetaData>(metaDict, "TestDoubleArray", metaDataDoubleArray); - const std::string metaDataStdString("Test std::string"); - itk::EncapsulateMetaData(metaDict, "StdString", metaDataStdString); + itk::EncapsulateMetaData(metaDict, "StdString", "Test std::string"); // // fill image buffer @@ -134,6 +116,7 @@ HDF5ReadWriteTest(const char * fileName) std::cout << "itkHDF5ImageIOTest" << std::endl << "Exception Object caught: " << std::endl << err << std::endl; return EXIT_FAILURE; } + std::cout << "Writing image with metadata values to " << fileName << " : SUCCEEDED." << std::endl; if (im->GetOrigin() != im2->GetOrigin()) { @@ -152,146 +135,103 @@ HDF5ReadWriteTest(const char * fileName) << std::endl; return EXIT_FAILURE; } + std::cout << "Checking MetaDataDictionary Values:" << std::endl; // // Check MetaData const itk::MetaDataDictionary & metaDict2(im2->GetMetaDataDictionary()); - bool metaDataBool2(false); + int missingKeysCount = 0; + for (const auto & key : metaDict.GetKeys()) + { + if (metaDict2.Find(key) == metaDict2.End()) + { + std::cout << "Missing key in the metadata dictionary extracted from image read in from disk: " << key + << std::endl; + ++missingKeysCount; + } + } + if (missingKeysCount != 0) + { + return EXIT_FAILURE; + } + - if (!itk::ExposeMetaData(metaDict2, "TestBool", metaDataBool2) || metaDataBool != metaDataBool2) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestBool", false) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestBool " << metaDataBool << ' ' << metaDataBool2 << std::endl; + metaDict2.Print(std::cerr); success = EXIT_FAILURE; } - - char metaDataChar2(0); - if (!itk::ExposeMetaData(metaDict2, "TestChar", metaDataChar2) || metaDataChar2 != metaDataChar) +#ifndef ITK_FUTURE_LEGACY_REMOVE + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestChar", 'c') != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestChar " << metaDataChar2 << ' ' << metaDataChar << std::endl; success = EXIT_FAILURE; } - - unsigned char metaDataUChar2(0); - if (!itk::ExposeMetaData(metaDict2, "TestUChar", metaDataUChar2) || metaDataUChar2 != metaDataUChar) +#endif + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestUChar", 'u') != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestUChar " << metaDataUChar2 << ' ' << metaDataUChar << std::endl; success = EXIT_FAILURE; } - - short metaDataShort2(-1); - if (!itk::ExposeMetaData(metaDict2, "TestShort", metaDataShort2) || metaDataShort2 != metaDataShort) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestSChar", 's') != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestShort " << metaDataShort2 << ' ' << metaDataShort << std::endl; success = EXIT_FAILURE; } - - unsigned short metaDataUShort2(0); - if (!itk::ExposeMetaData(metaDict2, "TestUShort", metaDataUShort2) || - metaDataUShort2 != metaDataUShort) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestShort", 1) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestUShort " << metaDataUShort2 << ' ' << metaDataUShort << std::endl; success = EXIT_FAILURE; } - - int metaDataInt2(1234); - if (!itk::ExposeMetaData(metaDict2, "TestInt", metaDataInt2) || metaDataInt2 != metaDataInt) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestUShort", 3) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestInt " << metaDataInt2 << ' ' << metaDataInt << std::endl; success = EXIT_FAILURE; } - - unsigned int metaDataUInt2(1234); - if (!itk::ExposeMetaData(metaDict2, "TestUInt", metaDataUInt2) || metaDataUInt2 != metaDataUInt) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestInt", 5) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestUInt " << metaDataUInt2 << ' ' << metaDataUInt << std::endl; success = EXIT_FAILURE; } - - long metaDataLong2(0); - if (!itk::ExposeMetaData(metaDict2, "TestLong", metaDataLong2) || metaDataLong2 != metaDataLong) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestUInt", 7) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestLong " << metaDataLong2 << ' ' << metaDataLong << std::endl; success = EXIT_FAILURE; } - - unsigned long metaDataULong2(0); - if (!itk::ExposeMetaData(metaDict2, "TestULong", metaDataULong2) || metaDataULong2 != metaDataULong) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestLong", 5) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestULong " << metaDataULong2 << ' ' << metaDataULong << std::endl; success = EXIT_FAILURE; } - - long long metaDataLLong2(0); - if (!itk::ExposeMetaData(metaDict2, "TestLLong", metaDataLLong2) || metaDataLLong2 != metaDataLLong) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestULong", 7) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestLLong " << metaDataLLong2 << ' ' << metaDataLLong << std::endl; success = EXIT_FAILURE; } - - unsigned long long metaDataULLong2(0); - if (!itk::ExposeMetaData(metaDict2, "TestULLong", metaDataULLong2) || - metaDataULLong2 != metaDataULLong) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestLLong", -5) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestULLong " << metaDataULLong2 << ' ' << metaDataULLong << std::endl; success = EXIT_FAILURE; } - - float metaDataFloat2(0.0f); - if (!itk::ExposeMetaData(metaDict2, "TestFloat", metaDataFloat2) || - itk::Math::NotAlmostEquals(metaDataFloat2, metaDataFloat)) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestULLong", 7ull) != EXIT_SUCCESS) { - std::cerr << "Failure Reading metaData " - << "TestFloat " << metaDataFloat2 << ' ' << metaDataFloat << std::endl; success = EXIT_FAILURE; } - - double metaDataDouble2(0.0); - if (!itk::ExposeMetaData(metaDict2, "TestDouble", metaDataDouble2) || - itk::Math::NotAlmostEquals(metaDataDouble2, metaDataDouble)) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestFloat", 1.23456f) != EXIT_SUCCESS) { - std::cerr << "Failure reading metaData " - << "TestDouble " << metaDataDouble2 << ' ' << metaDataDouble << std::endl; success = EXIT_FAILURE; } - - itk::Array metaDataCharArray2{}; - if (!itk::ExposeMetaData>(metaDict2, "TestCharArray", metaDataCharArray2) || - metaDataCharArray2 != metaDataCharArray) + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "TestDouble", 1.23456) != EXIT_SUCCESS) { - std::cerr << "Failure reading metaData " - << "TestCharArray" << metaDataCharArray2 << ' ' << metaDataCharArray2 << std::endl; success = EXIT_FAILURE; } - - itk::Array metaDataDoubleArray2{}; - if (!itk::ExposeMetaData>(metaDict2, "TestDoubleArray", metaDataDoubleArray2) || - metaDataDoubleArray2 != metaDataDoubleArray) +#ifndef ITK_FUTURE_LEGACY_REMOVE + if (itk::VerifyMetaDataPrivateTestingUtility>(metaDict2, "TestCharArray", metaDataCharArray) != + EXIT_SUCCESS) { - std::cerr << "Failure reading metaData " - << "TestDoubleArray " << metaDataDoubleArray2 << ' ' << metaDataDoubleArray << std::endl; success = EXIT_FAILURE; } - - std::string metaDataStdString2(""); - if (!itk::ExposeMetaData(metaDict2, "StdString", metaDataStdString2) || - metaDataStdString2 != metaDataStdString) +#endif + if (itk::VerifyMetaDataPrivateTestingUtility>(metaDict2, "TestDoubleArray", metaDataDoubleArray) != + EXIT_SUCCESS) + { + success = EXIT_FAILURE; + } + if (itk::VerifyMetaDataPrivateTestingUtility(metaDict2, "StdString", std::string("Test std::string")) != + EXIT_SUCCESS) { - std::cerr << "Failure reading metaData " - << "StdString " << metaDataStdString2 << ' ' << metaDataStdString << std::endl; success = EXIT_FAILURE; } - itk::ImageRegionIterator it2(im2, im2->GetLargestPossibleRegion()); for (it.GoToBegin(), it2.GoToBegin(); !it.IsAtEnd() && !it2.IsAtEnd(); ++it, ++it2) { @@ -347,7 +287,7 @@ itkHDF5ImageIOTest(int argc, char * argv[]) ITK_EXERCISE_BASIC_OBJECT_METHODS(imageio, HDF5ImageIO, StreamingImageIOBase); int result(0); - + result += HDF5ReadWriteTest("SCharImage.hdf5"); result += HDF5ReadWriteTest("UCharImage.hdf5"); result += HDF5ReadWriteTest("FloatImage.hdf5"); result += HDF5ReadWriteTest("ULongLongImage.hdf5"); diff --git a/Modules/IO/MINC/src/itkMINCImageIO.cxx b/Modules/IO/MINC/src/itkMINCImageIO.cxx index 3f5e74f0ca4..07c5c5bf0bb 100644 --- a/Modules/IO/MINC/src/itkMINCImageIO.cxx +++ b/Modules/IO/MINC/src/itkMINCImageIO.cxx @@ -711,7 +711,7 @@ MINCImageIO::ReadImageInformation() switch (volume_data_type) { case MI_TYPE_BYTE: - EncapsulateMetaData(thisDic, "storage_data_type", typeid(char).name()); + EncapsulateMetaData(thisDic, "storage_data_type", typeid(int8_t).name()); break; case MI_TYPE_UBYTE: EncapsulateMetaData(thisDic, "storage_data_type", typeid(unsigned char).name()); @@ -1394,7 +1394,7 @@ MINCImageIO::Write(const void * buffer) break; case IOComponentEnum::SCHAR: volume_data_type = MI_TYPE_BYTE; - get_buffer_min_max(buffer, buffer_length, buffer_min, buffer_max); + get_buffer_min_max(buffer, buffer_length, buffer_min, buffer_max); break; case IOComponentEnum::USHORT: volume_data_type = MI_TYPE_USHORT; diff --git a/Modules/IO/Meta/test/testMetaImage.cxx b/Modules/IO/Meta/test/testMetaImage.cxx index c67124f3966..953364480d0 100644 --- a/Modules/IO/Meta/test/testMetaImage.cxx +++ b/Modules/IO/Meta/test/testMetaImage.cxx @@ -250,7 +250,11 @@ testMetaImage(int, char *[]) { return EXIT_FAILURE; } - if (ReadWriteCompare(static_cast(-8), "char")) + if (ReadWriteCompare(static_cast(-8), "int8_t")) + { + return EXIT_FAILURE; + } + if (ReadWriteCompare(static_cast(-8), "signed char")) { return EXIT_FAILURE; } diff --git a/README.md b/README.md index 631abbffc07..6469a7000e4 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ ITK: The Insight Toolkit | Azure Pipelines | macOS | [![Build Status](https://dev.azure.com/itkrobotmacos/ITK.macOS/_apis/build/status/ITK.macOS?branchName=main)](https://dev.azure.com/itkrobotmacos/ITK.macOS/_build/latest?definitionId=2&branchName=main) | [![Build Status](https://dev.azure.com/itkrobotmacospython/ITK.macOS.Python/_apis/build/status/ITK.macOS.Python?branchName=main)](https://dev.azure.com/itkrobotmacospython/ITK.macOS.Python/_build/latest?definitionId=2&branchName=main) | | GitHub Actions | Linux, Windows, macOS | [![ITK.Pixi](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/pixi.yml/badge.svg)](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/pixi.yml) | | | GitHub Actions | macOS (Apple Silicon)| [![ITK.macOS.Arm64](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/macos-arm.yml/badge.svg)](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/macos-arm.yml)| | +| GitHub Actions | Linux, macOS ARM64| [![ITK.Arm64](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/arm.yml/badge.svg)](https://github.com/InsightSoftwareConsortium/ITK/actions/workflows/arm.yml)| | | Azure Pipelines | Linux (Code coverage)| [![Build Status](https://dev.azure.com/itkrobotbatch/ITK.Coverage/_apis/build/status/ITK.Coverage?branchName=main)](https://dev.azure.com/itkrobotbatch/ITK.Coverage/_build/latest?definitionId=3&branchName=main) | | Links