From eeaa8916fd259c80ef7a2900a1e754a4ce6db1a3 Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Thu, 7 Sep 2023 23:46:33 -0700 Subject: [PATCH 1/6] Fix CPUProcessor/optimizations on arm64 Arm64 has fma instructions that can lead to different results Break the manual computaion into multiple steps Signed-off-by: Mark Reid --- tests/cpu/CPUProcessor_tests.cpp | 29 +++++++++++++++++++++-------- 1 file changed, 21 insertions(+), 8 deletions(-) diff --git a/tests/cpu/CPUProcessor_tests.cpp b/tests/cpu/CPUProcessor_tests.cpp index d5f092e48d..c2fed39022 100644 --- a/tests/cpu/CPUProcessor_tests.cpp +++ b/tests/cpu/CPUProcessor_tests.cpp @@ -2772,14 +2772,27 @@ void ComputeImage(unsigned width, unsigned height, unsigned nChannels, for(size_t idx=0; idx<(width*height);) { // Manual computation of the results. - - const float pxl[4]{ (float(inValues[idx+0]) * inScale + (float)offset4[0]) * outScale, - (float(inValues[idx+1]) * inScale + (float)offset4[1]) * outScale, - (float(inValues[idx+2]) * inScale + (float)offset4[2]) * outScale, - nChannels==4 - ? ((float(inValues[idx+3]) * inScale + (float)offset4[3]) * outScale) - : 0.0f - }; + // Break operations into steps similar to cpu processor + // to avoid potential fma compiler optimizations + const float in_scale[4]{ float(inValues[idx+0]) * inScale, + float(inValues[idx+1]) * inScale, + float(inValues[idx+2]) * inScale, + nChannels==4 + ? float(inValues[idx+3]) * inScale + : 0.0f + }; + + const float operation[4]{ in_scale[0] + (float)offset4[0], + in_scale[1] + (float)offset4[1], + in_scale[2] + (float)offset4[2], + in_scale[3] + (float)offset4[3], + }; + + const float pxl[4]{ operation[0] * outScale, + operation[1] * outScale, + operation[2] * outScale, + operation[3] * outScale, + }; // Validate all the results. From fae111d5d8104933e311553681accdd0b804aa4e Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Sun, 11 Feb 2024 15:12:26 -0800 Subject: [PATCH 2/6] Compare strings by float value Signed-off-by: Mark Reid --- tests/cpu/UnitTestUtils.cpp | 23 +++++++++++ tests/cpu/UnitTestUtils.h | 11 ++++++ tests/cpu/fileformats/FileFormatCTF_tests.cpp | 38 +++++++++++++++++-- tests/cpu/fileformats/FileFormatHDL_tests.cpp | 20 +++++++++- .../FileFormatResolveCube_tests.cpp | 27 +++++++++++-- .../cpu/fileformats/FileFormatSpi1D_tests.cpp | 9 ++++- 6 files changed, 118 insertions(+), 10 deletions(-) diff --git a/tests/cpu/UnitTestUtils.cpp b/tests/cpu/UnitTestUtils.cpp index f298db0f3e..66f57a7bdf 100644 --- a/tests/cpu/UnitTestUtils.cpp +++ b/tests/cpu/UnitTestUtils.cpp @@ -6,6 +6,7 @@ #include "Logging.h" #include "OpBuilders.h" +#include "ParseUtils.h" #include "UnitTestUtils.h" #include "utils/StringUtils.h" @@ -71,6 +72,28 @@ ConstProcessorRcPtr GetFileTransformProcessor(const std::string & fileName) return config->getProcessor(fileTransform); } +bool StringFloatVecClose(std::string value, std::string expected, float eps) +{ + std::vector a; + std::vector b; + if (StringVecToFloatVec(a, StringUtils::SplitByWhiteSpaces(value)) && + StringVecToFloatVec(b, StringUtils::SplitByWhiteSpaces(expected))) + { + if (a.size() == b.size()) + { + for(unsigned int i = 0; i < a.size(); i++) + { + if (std::abs(a[i] - b[i]) >= eps) + { + return false; + } + } + return true; + } + } + return false; +} + std::string CreateTemporaryDirectory(const std::string & name) { int nError = 0; diff --git a/tests/cpu/UnitTestUtils.h b/tests/cpu/UnitTestUtils.h index bbdac35549..c4281c57d3 100644 --- a/tests/cpu/UnitTestUtils.h +++ b/tests/cpu/UnitTestUtils.h @@ -93,6 +93,17 @@ inline bool EqualWithSafeRelError(T value, / div) <= eps; } +bool StringFloatVecClose(std::string value, std::string expected, float eps); + +#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,line) \ + (OCIO::StringFloatVecClose(x,y,tol)) ? ((void)0) \ + : ((std::cout << __FILE__ << ":" << line << ":\n" \ + << "FAILED: " << FIELD_STR(x) << " == " << FIELD_STR(y) << "\n" \ + << "\tvalues were '" << (x) << "' and '" << (y) << "'\n"), \ + (void)++unit_test_failures) + +#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE(x,y,tol) OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,__LINE__) + // C++20 introduces new strongly typed, UTF-8 based, char8_t and u8string types // which are not implicitly convertible to char and std::string respectively. // Here we simply choose to ignore these new types for unit tests while the diff --git a/tests/cpu/fileformats/FileFormatCTF_tests.cpp b/tests/cpu/fileformats/FileFormatCTF_tests.cpp index 7a7ab67b7f..fd7273e200 100644 --- a/tests/cpu/fileformats/FileFormatCTF_tests.cpp +++ b/tests/cpu/fileformats/FileFormatCTF_tests.cpp @@ -6934,8 +6934,23 @@ OCIO_ADD_TEST(CTFTransform, grading_rgbcurve_lin_ctf) )" }; - OCIO_CHECK_EQUAL(expected.size(), outputTransform.str().size()); - OCIO_CHECK_EQUAL(expected, outputTransform.str()); + const StringUtils::StringVec osvec = StringUtils::SplitByLines(outputTransform.str()); + const StringUtils::StringVec resvec = StringUtils::SplitByLines(expected); + OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); + for(unsigned int i = 0; i < resvec.size(); ++i) + { + if ( (i >= 5 && i <= 7) || + (i >= 12 && i <= 15) || + (i == 18)) + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } + else + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + + } } // All curves are default curves, no curve is saved. @@ -8326,8 +8341,23 @@ R"( )" }; - OCIO_CHECK_EQUAL(expectedCLF.size(), output1.str().size()); - OCIO_CHECK_EQUAL(expectedCLF, output1.str()); + + const StringUtils::StringVec osvec = StringUtils::SplitByLines(output1.str()); + const StringUtils::StringVec resvec = StringUtils::SplitByLines(expectedCLF); + OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); + for(unsigned int i = 0; i < resvec.size(); ++i) + { + if ( (i >= 10 && i <= 19) || + (i >= 24 && i <= 31)) + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } + else + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + + } } OCIO_ADD_TEST(FileFormatCTF, lut_interpolation_option) diff --git a/tests/cpu/fileformats/FileFormatHDL_tests.cpp b/tests/cpu/fileformats/FileFormatHDL_tests.cpp index d5ecf39bb7..0b78d72282 100644 --- a/tests/cpu/fileformats/FileFormatHDL_tests.cpp +++ b/tests/cpu/fileformats/FileFormatHDL_tests.cpp @@ -5,6 +5,7 @@ #include "fileformats/FileFormatHDL.cpp" #include "testutils/UnitTest.h" +#include "UnitTestUtils.h" namespace OCIO = OCIO_NAMESPACE; @@ -325,8 +326,23 @@ OCIO_ADD_TEST(FileFormatHDL, bake_1d_shaper) "\t16.291878\n" "}\n"; - OCIO_CHECK_EQUAL(expectedHDL.size(), outputHDL.str().size()); - OCIO_CHECK_EQUAL(expectedHDL, outputHDL.str()); + const StringUtils::StringVec osvec = StringUtils::SplitByLines(expectedHDL); + const StringUtils::StringVec resvec = StringUtils::SplitByLines(outputHDL.str()); + OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); + for(unsigned int i = 0; i < resvec.size(); ++i) + { + if ( (i >= 10 && i <= 19) || + (i >= 22 && i <= 31) || + (i >= 34 && i <= 43)) + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } + else + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + + } } } diff --git a/tests/cpu/fileformats/FileFormatResolveCube_tests.cpp b/tests/cpu/fileformats/FileFormatResolveCube_tests.cpp index c8c6a2ad48..6888d63d87 100644 --- a/tests/cpu/fileformats/FileFormatResolveCube_tests.cpp +++ b/tests/cpu/fileformats/FileFormatResolveCube_tests.cpp @@ -384,7 +384,14 @@ OCIO_ADD_TEST(FileFormatResolveCube, bake_1d_shaper) OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); for(unsigned int i = 0; i < resvec.size(); ++i) { - OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + if (i <= 0) + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + else + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } } } } @@ -445,7 +452,14 @@ OCIO_ADD_TEST(FileFormatResolveCube, bake_3d) OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); for(unsigned int i = 0; i < resvec.size(); ++i) { - OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + if (i <= 3) + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + else + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } } } @@ -522,7 +536,14 @@ OCIO_ADD_TEST(FileFormatResolveCube, bake_1d_3d) OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); for(unsigned int i = 0; i < resvec.size(); ++i) { - OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + if (i <= 2) + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + else + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } } } diff --git a/tests/cpu/fileformats/FileFormatSpi1D_tests.cpp b/tests/cpu/fileformats/FileFormatSpi1D_tests.cpp index c9e98c2779..17f840e7a7 100644 --- a/tests/cpu/fileformats/FileFormatSpi1D_tests.cpp +++ b/tests/cpu/fileformats/FileFormatSpi1D_tests.cpp @@ -482,7 +482,14 @@ OCIO_ADD_TEST(FileFormatSpi1D, bake_1d_shaper) OCIO_CHECK_EQUAL(osvec.size(), resvec.size()); for(unsigned int i = 0; i < resvec.size(); ++i) { - OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + if (i <= 5 || i >= 15) + { + OCIO_CHECK_EQUAL(osvec[i], resvec[i]); + } + else + { + OCIO_CHECK_STR_FLOAT_VEC_CLOSE(osvec[i], resvec[i], 1e-5f); + } } } } From 31bf61fcaaf1e6bbdbe5708b0320e8baf5b902ee Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Tue, 13 Feb 2024 22:13:02 -0800 Subject: [PATCH 3/6] Enable macarm runners by default Signed-off-by: Mark Reid --- .github/workflows/ci-macarm.yml | 58 +++++++++++++++------------------ 1 file changed, 26 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci-macarm.yml b/.github/workflows/ci-macarm.yml index 7009fbf2db..f9b725cda0 100644 --- a/.github/workflows/ci-macarm.yml +++ b/.github/workflows/ci-macarm.yml @@ -7,9 +7,7 @@ name: CI-MacARM # This workflow is for CI for ARM-based MacOS. In an ideal world, this would -# just be another matrix entry in the main CI workflow, but MacOS ARM runners -# too expensive to run on every PR push. So we break into a separate workflow -# to have more fine-grained control over when it runs. +# just be another matrix entry in the main CI workflow. # This section controls when the workflow runs. on: @@ -17,25 +15,22 @@ on: # contains the substrings "macarm", "-alpha", "-beta", or is called # "release". push: - tags: - - v** - branches: - - '*macarm*' - - '*alpha*' - - '*beta*' - - 'release' - # Run for pull requests whose branch name includes "macarm" (this allows - # us to test specific PRs that we think need ARM verification). + # Versioned branches and tags are ignored for OCIO <= 1.x.x + branches-ignore: + - RB-0.* + - RB-1.* + - gh-pages + tags-ignore: + - v0.* + - v1.* pull_request: - branches: - - '*macarm*' - # Run monthly on the 27th at 8:00AM to make sure we haven't broken - # anything among the many PRs that didn't test on ARM. - schedule: - - cron: "0 8 27 * *" - if: github.repository == 'AcademySoftwareFoundation/OpenColorIO' - -permissions: read-all + branches-ignore: + - RB-0.* + - RB-1.* + - gh-pages + tags-ignore: + - v0.* + - v1.* jobs: # --------------------------------------------------------------------------- @@ -43,19 +38,18 @@ jobs: # --------------------------------------------------------------------------- macos-arm: - name: 'macOS 13 arm - ' - # Needs special runners, only run on the main repo, - if: github.repository == 'AcademySoftwareFoundation/OpenColorIO' - runs-on: macos-13-xlarge + + runs-on: macos-14 strategy: matrix: build: [1] From b7a23387b71f36a123e9171c9f95fb8d3b828fa7 Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Wed, 14 Feb 2024 21:18:27 -0800 Subject: [PATCH 4/6] Move mac arm runner to main workflow Signed-off-by: Mark Reid --- .github/workflows/ci-macarm.yml | 148 ------------------------------ .github/workflows/ci_workflow.yml | 137 ++++++++++++++++++++++++++- 2 files changed, 134 insertions(+), 151 deletions(-) delete mode 100644 .github/workflows/ci-macarm.yml diff --git a/.github/workflows/ci-macarm.yml b/.github/workflows/ci-macarm.yml deleted file mode 100644 index f9b725cda0..0000000000 --- a/.github/workflows/ci-macarm.yml +++ /dev/null @@ -1,148 +0,0 @@ -# SPDX-License-Identifier: BSD-3-Clause -# Copyright Contributors to the OpenColorIO Project. -# -# GitHub Actions workflow file -# https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions - -name: CI-MacARM - -# This workflow is for CI for ARM-based MacOS. In an ideal world, this would -# just be another matrix entry in the main CI workflow. - -# This section controls when the workflow runs. -on: - # Run upon push, but only for tagged versions, or if the branch name - # contains the substrings "macarm", "-alpha", "-beta", or is called - # "release". - push: - # Versioned branches and tags are ignored for OCIO <= 1.x.x - branches-ignore: - - RB-0.* - - RB-1.* - - gh-pages - tags-ignore: - - v0.* - - v1.* - pull_request: - branches-ignore: - - RB-0.* - - RB-1.* - - gh-pages - tags-ignore: - - v0.* - - v1.* - -jobs: - # --------------------------------------------------------------------------- - # macOS - # --------------------------------------------------------------------------- - - macos-arm: - name: 'macOS 14 arm - ' - - runs-on: macos-14 - strategy: - matrix: - build: [1] - include: - - build: 1 - arch-type: "arm64" - build-type: Release - build-shared: 'ON' - build-docs: 'OFF' - build-openfx: 'OFF' - use-simd: 'ON' - use-oiio: 'OFF' - cxx-standard: 11 - python-version: '3.11' - steps: - - name: Setup Python - uses: actions/setup-python@v5 - with: - python-version: ${{ matrix.python-version }} - - name: Checkout - uses: actions/checkout@v4 - - name: Install docs env - run: share/ci/scripts/macos/install_docs_env.sh - if: matrix.build-docs == 'ON' - - name: Install tests env - run: share/ci/scripts/macos/install_tests_env.sh - - name: Create build directories - run: | - mkdir _install - mkdir _build - - name: Configure - run: | - cmake ../. \ - -DCMAKE_INSTALL_PREFIX=../_install \ - -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} \ - -DCMAKE_CXX_STANDARD=${{ matrix.cxx-standard }} \ - -DBUILD_SHARED_LIBS=${{ matrix.build-shared }} \ - -DOCIO_BUILD_DOCS=${{ matrix.build-docs }} \ - -DOCIO_BUILD_OPENFX=${{ matrix.build-openfx }} \ - -DOCIO_BUILD_GPU_TESTS=ON \ - -DOCIO_USE_SIMD=${{ matrix.use-simd }} \ - -DOCIO_USE_OIIO_FOR_APPS=${{ matrix.use-oiio }} \ - -DOCIO_INSTALL_EXT_PACKAGES=ALL \ - -DOCIO_WARNING_AS_ERROR=ON \ - -DPython_EXECUTABLE=$(which python) \ - -DCMAKE_OSX_ARCHITECTURES="${{ matrix.arch-type }}" - working-directory: _build - - name: Build - run: | - cmake --build . \ - --target install \ - --config ${{ matrix.build-type }} \ - -- -j$(sysctl -n hw.ncpu) - echo "ocio_build_path=$(pwd)" >> $GITHUB_ENV - working-directory: _build - - name: Test - run: ctest -V -C ${{ matrix.build-type }} - working-directory: _build - - name: Test CMake Consumer with shared OCIO - if: matrix.build-shared == 'ON' - run: | - cmake . \ - -DCMAKE_PREFIX_PATH=../../../_install \ - -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} - cmake --build . \ - --config ${{ matrix.build-type }} - ./consumer - working-directory: _build/tests/cmake-consumer-dist - - name: Test CMake Consumer with static OCIO - if: matrix.build-shared == 'OFF' - # The yaml-cpp_VERSION is set below because Findyaml-cpp.cmake needs it but is unable to - # extract it from the headers, like the other modules. - # - # Prefer the static version of each dependencies by using _STATIC_LIBRARY. - # Alternatively, this can be done by setting _LIBRARY and _INCLUDE_DIR to - # the static version of the package. - run: | - cmake . \ - -DCMAKE_PREFIX_PATH=../../../_install \ - -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} \ - -Dexpat_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -Dexpat_STATIC_LIBRARY=ON \ - -DImath_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -DImath_STATIC_LIBRARY=ON \ - -Dpystring_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -Dyaml-cpp_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -Dyaml-cpp_STATIC_LIBRARY=ON \ - -Dyaml-cpp_VERSION=0.7.0 \ - -DZLIB_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -DZLIB_STATIC_LIBRARY=ON \ - -Dminizip-ng_ROOT=${{ env.ocio_build_path }}/ext/dist \ - -Dminizip-ng_STATIC_LIBRARY=ON - cmake --build . \ - --config ${{ matrix.build-type }} - ./consumer - working-directory: _build/tests/cmake-consumer-dist diff --git a/.github/workflows/ci_workflow.yml b/.github/workflows/ci_workflow.yml index cd076af1ac..93ad0b17a9 100644 --- a/.github/workflows/ci_workflow.yml +++ b/.github/workflows/ci_workflow.yml @@ -445,9 +445,140 @@ jobs: # The yaml-cpp_VERSION is set below because Findyaml-cpp.cmake needs it but is unable to # extract it from the headers, like the other modules. # - # Prefer the static version of each dependencies by using _STATIC_LIBRARY. - # Alternatively, this can be done by setting _LIBRARY and _INCLUDE_DIR to - # the static version of the package. + # Prefer the static version of each dependencies by using _STATIC_LIBRARY. + # Alternatively, this can be done by setting _LIBRARY and _INCLUDE_DIR to + # the static version of the package. + run: | + cmake . \ + -DCMAKE_PREFIX_PATH=../../../_install \ + -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} \ + -Dexpat_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -Dexpat_STATIC_LIBRARY=ON \ + -DImath_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -DImath_STATIC_LIBRARY=ON \ + -Dpystring_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -Dyaml-cpp_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -Dyaml-cpp_STATIC_LIBRARY=ON \ + -Dyaml-cpp_VERSION=0.7.0 \ + -DZLIB_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -DZLIB_STATIC_LIBRARY=ON \ + -Dminizip-ng_ROOT=${{ env.ocio_build_path }}/ext/dist \ + -Dminizip-ng_STATIC_LIBRARY=ON + cmake --build . \ + --config ${{ matrix.build-type }} + ./consumer + working-directory: _build/tests/cmake-consumer-dist + + # --------------------------------------------------------------------------- + # macOS Arm + # --------------------------------------------------------------------------- + + macos-arm: + name: 'macOS 14 arm + ' + + runs-on: macos-14 + strategy: + matrix: + build: [1, 2] + include: + - build: 1 + arch-type: "arm64" + test-rosseta: "OFF" + build-type: Release + build-shared: 'ON' + build-docs: 'OFF' + build-openfx: 'OFF' + use-simd: 'ON' + use-oiio: 'OFF' + cxx-standard: 11 + python-version: '3.11' + - build: 2 + arch-type: "x86_64;arm64" + test-rosseta: "ON" + build-type: Release + build-shared: 'ON' + build-docs: 'OFF' + build-openfx: 'OFF' + use-simd: 'ON' + use-oiio: 'OFF' + cxx-standard: 11 + python-version: '3.11' + steps: + - name: Setup Python + uses: actions/setup-python@v4 + with: + python-version: ${{ matrix.python-version }} + - name: Checkout + uses: actions/checkout@v3 + - name: Install docs env + run: share/ci/scripts/macos/install_docs_env.sh + if: matrix.build-docs == 'ON' + - name: Install tests env + run: share/ci/scripts/macos/install_tests_env.sh + - name: Create build directories + run: | + mkdir _install + mkdir _build + - name: Configure + run: | + cmake ../. \ + -DCMAKE_INSTALL_PREFIX=../_install \ + -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} \ + -DCMAKE_CXX_STANDARD=${{ matrix.cxx-standard }} \ + -DBUILD_SHARED_LIBS=${{ matrix.build-shared }} \ + -DOCIO_BUILD_DOCS=${{ matrix.build-docs }} \ + -DOCIO_BUILD_OPENFX=${{ matrix.build-openfx }} \ + -DOCIO_BUILD_GPU_TESTS=OFF \ + -DOCIO_USE_SIMD=${{ matrix.use-simd }} \ + -DOCIO_USE_OIIO_FOR_APPS=${{ matrix.use-oiio }} \ + -DOCIO_INSTALL_EXT_PACKAGES=ALL \ + -DOCIO_WARNING_AS_ERROR=ON \ + -DPython_EXECUTABLE=$(which python) \ + -DCMAKE_OSX_ARCHITECTURES="${{ matrix.arch-type }}" + working-directory: _build + - name: Build + run: | + cmake --build . \ + --target install \ + --config ${{ matrix.build-type }} \ + -- -j$(sysctl -n hw.ncpu) + echo "ocio_build_path=$(pwd)" >> $GITHUB_ENV + working-directory: _build + - name: Test + run: ctest -V -C ${{ matrix.build-type }} + working-directory: _build + - name: Test Rosetta + if: matrix.test-rosseta == 'ON' + run: | + /usr/bin/arch -x86_64 /bin/zsh -c "ctest -V -C ${{ matrix.build-type }}" + working-directory: _build + - name: Test CMake Consumer with shared OCIO + if: matrix.build-shared == 'ON' + run: | + cmake . \ + -DCMAKE_PREFIX_PATH=../../../_install \ + -DCMAKE_BUILD_TYPE=${{ matrix.build-type }} + cmake --build . \ + --config ${{ matrix.build-type }} + ./consumer + working-directory: _build/tests/cmake-consumer-dist + - name: Test CMake Consumer with static OCIO + if: matrix.build-shared == 'OFF' + # The yaml-cpp_VERSION is set below because Findyaml-cpp.cmake needs it but is unable to + # extract it from the headers, like the other modules. + # + # Prefer the static version of each dependencies by using _STATIC_LIBRARY. + # Alternatively, this can be done by setting _LIBRARY and _INCLUDE_DIR to + # the static version of the package. run: | cmake . \ -DCMAKE_PREFIX_PATH=../../../_install \ From df6bfc0bdc1b15b6fc536d087d5f40e2fb8f05c6 Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Mon, 18 Mar 2024 21:37:36 -0700 Subject: [PATCH 5/6] fix rosetta typo Signed-off-by: Mark Reid --- .github/workflows/ci_workflow.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci_workflow.yml b/.github/workflows/ci_workflow.yml index 93ad0b17a9..3eb0008998 100644 --- a/.github/workflows/ci_workflow.yml +++ b/.github/workflows/ci_workflow.yml @@ -492,7 +492,7 @@ jobs: include: - build: 1 arch-type: "arm64" - test-rosseta: "OFF" + test-rosetta: "OFF" build-type: Release build-shared: 'ON' build-docs: 'OFF' @@ -503,7 +503,7 @@ jobs: python-version: '3.11' - build: 2 arch-type: "x86_64;arm64" - test-rosseta: "ON" + test-rosetta: "ON" build-type: Release build-shared: 'ON' build-docs: 'OFF' @@ -557,7 +557,7 @@ jobs: run: ctest -V -C ${{ matrix.build-type }} working-directory: _build - name: Test Rosetta - if: matrix.test-rosseta == 'ON' + if: matrix.test-rosetta == 'ON' run: | /usr/bin/arch -x86_64 /bin/zsh -c "ctest -V -C ${{ matrix.build-type }}" working-directory: _build From dd25530fb33b198da90393fe9da76295fadb6544 Mon Sep 17 00:00:00 2001 From: Mark Reid Date: Mon, 18 Mar 2024 22:20:59 -0700 Subject: [PATCH 6/6] move StringFloatVecClose to testutils Signed-off-by: Mark Reid --- tests/cpu/UnitTestUtils.cpp | 23 ------------------- tests/cpu/UnitTestUtils.h | 11 --------- tests/testutils/UnitTest.cpp | 44 ++++++++++++++++++++++++++++++++++++ tests/testutils/UnitTest.h | 11 +++++++++ 4 files changed, 55 insertions(+), 34 deletions(-) diff --git a/tests/cpu/UnitTestUtils.cpp b/tests/cpu/UnitTestUtils.cpp index 66f57a7bdf..f298db0f3e 100644 --- a/tests/cpu/UnitTestUtils.cpp +++ b/tests/cpu/UnitTestUtils.cpp @@ -6,7 +6,6 @@ #include "Logging.h" #include "OpBuilders.h" -#include "ParseUtils.h" #include "UnitTestUtils.h" #include "utils/StringUtils.h" @@ -72,28 +71,6 @@ ConstProcessorRcPtr GetFileTransformProcessor(const std::string & fileName) return config->getProcessor(fileTransform); } -bool StringFloatVecClose(std::string value, std::string expected, float eps) -{ - std::vector a; - std::vector b; - if (StringVecToFloatVec(a, StringUtils::SplitByWhiteSpaces(value)) && - StringVecToFloatVec(b, StringUtils::SplitByWhiteSpaces(expected))) - { - if (a.size() == b.size()) - { - for(unsigned int i = 0; i < a.size(); i++) - { - if (std::abs(a[i] - b[i]) >= eps) - { - return false; - } - } - return true; - } - } - return false; -} - std::string CreateTemporaryDirectory(const std::string & name) { int nError = 0; diff --git a/tests/cpu/UnitTestUtils.h b/tests/cpu/UnitTestUtils.h index c4281c57d3..bbdac35549 100644 --- a/tests/cpu/UnitTestUtils.h +++ b/tests/cpu/UnitTestUtils.h @@ -93,17 +93,6 @@ inline bool EqualWithSafeRelError(T value, / div) <= eps; } -bool StringFloatVecClose(std::string value, std::string expected, float eps); - -#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,line) \ - (OCIO::StringFloatVecClose(x,y,tol)) ? ((void)0) \ - : ((std::cout << __FILE__ << ":" << line << ":\n" \ - << "FAILED: " << FIELD_STR(x) << " == " << FIELD_STR(y) << "\n" \ - << "\tvalues were '" << (x) << "' and '" << (y) << "'\n"), \ - (void)++unit_test_failures) - -#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE(x,y,tol) OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,__LINE__) - // C++20 introduces new strongly typed, UTF-8 based, char8_t and u8string types // which are not implicitly convertible to char and std::string respectively. // Here we simply choose to ignore these new types for unit tests while the diff --git a/tests/testutils/UnitTest.cpp b/tests/testutils/UnitTest.cpp index ea7ea444db..3269f56b22 100644 --- a/tests/testutils/UnitTest.cpp +++ b/tests/testutils/UnitTest.cpp @@ -11,8 +11,10 @@ #include "apputils/argparse.h" #include "UnitTest.h" +#include "utils/NumberUtils.h" #include "utils/StringUtils.h" +namespace OCIO = OCIO_NAMESPACE; UnitTests & GetUnitTests() { @@ -20,6 +22,48 @@ UnitTests & GetUnitTests() return oiio_unit_tests; } +static bool StringVecToFloatVec(std::vector &floatArray, const StringUtils::StringVec &lineParts) +{ + floatArray.resize(lineParts.size()); + + for(unsigned int i=0; i a; + std::vector b; + if (StringVecToFloatVec(a, StringUtils::SplitByWhiteSpaces(value)) && + StringVecToFloatVec(b, StringUtils::SplitByWhiteSpaces(expected))) + { + if (a.size() == b.size()) + { + for(unsigned int i = 0; i < a.size(); i++) + { + if (std::abs(a[i] - b[i]) >= eps) + { + return false; + } + } + return true; + } + } + return false; +} + + int unit_test_failures{ 0 }; int UnitTestMain(int argc, const char ** argv) diff --git a/tests/testutils/UnitTest.h b/tests/testutils/UnitTest.h index 5d7d52daac..f4bb339ee4 100644 --- a/tests/testutils/UnitTest.h +++ b/tests/testutils/UnitTest.h @@ -220,6 +220,17 @@ int UnitTestMain(int argc, const char ** argv); ++unit_test_failures; \ } +bool StringFloatVecClose(std::string value, std::string expected, float eps); + +#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,line) \ + (StringFloatVecClose(x,y,tol)) ? ((void)0) \ + : ((std::cout << __FILE__ << ":" << line << ":\n" \ + << "FAILED: " << FIELD_STR(x) << " == " << FIELD_STR(y) << "\n" \ + << "\tvalues were '" << (x) << "' and '" << (y) << "'\n"), \ + (void)++unit_test_failures) + +#define OCIO_CHECK_STR_FLOAT_VEC_CLOSE(x,y,tol) OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM(x,y,tol,__LINE__) + /// Check that an exception E is thrown and that what() contains W /// When a function can throw different exceptions this can be used /// to verify that the right one is thrown.