search

AcademySoftwareFoundation / OpenColorIO

A color management framework for visual effects and animation.
https://opencolorio.org
BSD 3-Clause "New" or "Revised" License
1.74k stars 430 forks source link

Fix CPU unit test failures on recent macOS ARM platforms #1950

Closed markreidvfx closed 3 months ago

markreidvfx commented 4 months ago

Fixes the macOS ARM CPU unit tests for https://github.com/AcademySoftwareFoundation/OpenColorIO/issues/1946.

For the string comparison failures I add a utility function called StringFloatVecClose that converts the string values to float and compares with a threshold.

There is still a metal unit test failure happening on my machine, haven't looked into the cause of it.

Re-created this PR and added macarm to the branch name.

markreidvfx commented 4 months ago

hmmm, I added macarm to the branch name and it doesn't seem to have kicked off the action.

doug-walker commented 4 months ago

Ah, you're right. Apparently there is a bug in our Action script. We had based our script on what Larry had done, so I assumed it would work (though it's not an exact copy, so we must have made a mistake in a different part). Apologies for causing you to create a new branch. :(

markreidvfx commented 4 months ago

I got the free arm runners working on my own repo. It was pretty easy to get working, I can add it to this PR if we want. Now that these are arm runners are free for OSS projects, do we want to add them to main CI workflow file or keep it separate?

doug-walker commented 4 months ago

Awesome, thanks Mark! I would vote to add the new runner to the main CI, if it doesn't cause any other issues.

markreidvfx commented 4 months ago

I enabled the mac arm runner to always run. Its still separate but can combine it into the main workflow file. I assume this job should be still not required to pass until the GPU stuff is stored out.

There are more GPU failure when using the action machine then on my physical machine. Looks like its using a software renderer.

Action

GL Vendor:    Apple Inc.
GL Renderer:  Apple Software Renderer
GL Version:   2.1 APPLE-21.0.19
GLSL Version: 1.20

My laptop

GL Vendor:    Apple
GL Renderer:  Apple M2 Max
GL Version:   2.1 Metal - 88
GLSL Version: 1.20

I hope there is a way to force the software locally renderer to replicate.

doug-walker commented 4 months ago

The main CI workflows don't run the GPU tests, so we could turn that off for macOS ARM too, temporarily at least.

I know our GPU tests don't pass on software OpenGL implementations such as Mesa, so I'm not surprised it is not passing in Apple's software GL either. It would be great to know more about what the issues are with the GitHub virtualization method and if it's possible to use the real GPU on that chip.

markreidvfx commented 4 months ago

I moved it all into the main CI workflow file now. I also added an universal arm variant that tests the build under rosetta. The GPU tests are disabled, so everything passes now.

markreidvfx commented 3 months ago

I moved OCIO_CHECK_STR_FLOAT_VEC_CLOSE_FROM and OCIO_CHECK_STR_FLOAT_VEC_CLOSE to testutils. It was a little tricky. The code is using one function from ParseUtils.cpp but the cpp gets included in the cpu tests here. This also adds a dependency on Platform.cpp which also included in a cpu test here. I tried just adding the sources to testutils/CMakeList.txt but It creates a bunch of complications when compiling with MSVC that I'm having trouble resolving. I think its because the cpp is essentially linking twice.

D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(102,1): error C2220: the following warning is treated as an error [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(102,1): warning C4273: 'OpenColorIO_v2_4dev::BoolToString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(680,32): message : see previous definition of 'BoolToString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(107,1): warning C4273: 'OpenColorIO_v2_4dev::BoolFromString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(681,24): message : see previous definition of 'BoolFromString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(114,1): warning C4273: 'OpenColorIO_v2_4dev::LoggingLevelToString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(683,32): message : see previous definition of 'LoggingLevelToString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(123,1): warning C4273: 'OpenColorIO_v2_4dev::LoggingLevelFromString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(684,32): message : see previous definition of 'LoggingLevelFromString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(134,1): warning C4273: 'OpenColorIO_v2_4dev::TransformDirectionToString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(686,32): message : see previous definition of 'TransformDirectionToString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(141,1): warning C4273: 'OpenColorIO_v2_4dev::TransformDirectionFromString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(688,38): message : see previous definition of 'TransformDirectionFromString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(154,1): warning C4273: 'OpenColorIO_v2_4dev::CombineTransformDirections': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(691,38): message : see previous definition of 'CombineTransformDirections' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(165,1): warning C4273: 'OpenColorIO_v2_4dev::GetInverseTransformDirection': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(690,38): message : see previous definition of 'GetInverseTransformDirection' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(172,1): warning C4273: 'OpenColorIO_v2_4dev::BitDepthToString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(694,32): message : see previous definition of 'BitDepthToString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(185,1): warning C4273: 'OpenColorIO_v2_4dev::BitDepthFromString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(695,28): message : see previous definition of 'BitDepthFromString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(200,1): warning C4273: 'OpenColorIO_v2_4dev::BitDepthIsFloat': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(696,24): message : see previous definition of 'BitDepthIsFloat' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(207,1): warning C4273: 'OpenColorIO_v2_4dev::BitDepthToInt': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(697,23): message : see previous definition of 'BitDepthToInt' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(219,1): warning C4273: 'OpenColorIO_v2_4dev::AllocationToString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(699,32): message : see previous definition of 'AllocationToString' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(226,1): warning C4273: 'OpenColorIO_v2_4dev::AllocationFromString': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(816,32): message : see previous definition of 'ROLE_COLOR_TIMING' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(516,41): warning C4273: 'OpenColorIO_v2_4dev::ROLE_TEXTURE_PAINT': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(821,32): message : see previous definition of 'ROLE_TEXTURE_PAINT' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(517,41): warning C4273: 'OpenColorIO_v2_4dev::ROLE_MATTE_PAINT': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(827,32): message : see previous definition of 'ROLE_MATTE_PAINT' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(518,41): warning C4273: 'OpenColorIO_v2_4dev::ROLE_RENDERING': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(834,32): message : see previous definition of 'ROLE_RENDERING' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(519,41): warning C4273: 'OpenColorIO_v2_4dev::ROLE_INTERCHANGE_SCENE': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(840,32): message : see previous definition of 'ROLE_INTERCHANGE_SCENE' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\ParseUtils.cpp(520,41): warning C4273: 'OpenColorIO_v2_4dev::ROLE_INTERCHANGE_DISPLAY': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO\OpenColorTypes.h(846,32): message : see previous definition of 'ROLE_INTERCHANGE_DISPLAY' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
  Platform.cpp
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\Platform.cpp(24,1): error C2220: the following warning is treated as an error [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\Platform.cpp(24,1): warning C4273: 'OpenColorIO_v2_4dev::GetEnvVariable': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO/OpenColorIO.h(195,32): message : see previous definition of 'GetEnvVariable' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\Platform.cpp(31,1): warning C4273: 'OpenColorIO_v2_4dev::SetEnvVariable': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO/OpenColorIO.h(197,24): message : see previous definition of 'SetEnvVariable' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\Platform.cpp(36,1): warning C4273: 'OpenColorIO_v2_4dev::UnsetEnvVariable': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO/OpenColorIO.h(199,24): message : see previous definition of 'UnsetEnvVariable' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\src\OpenColorIO\Platform.cpp(41,1): warning C4273: 'OpenColorIO_v2_4dev::IsEnvVariablePresent': inconsistent dll linkage [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]
D:\a\OpenColorIO\OpenColorIO\include\OpenColorIO/OpenColorIO.h(201,24): message : see previous definition of 'IsEnvVariablePresent' [D:\a\OpenColorIO\OpenColorIO\_build\tests\testutils\testutils.vcxproj]

I'm just using one function, it was easier to just copy that one function ParseUtils into into UnitTest.cpp instead. This is what I've currently done.