From bb28982dedc0c05314d27c4cfdd8b9a2231cf24d Mon Sep 17 00:00:00 2001 From: Patrick Hodoul Date: Thu, 20 Jan 2022 09:16:43 -0500 Subject: [PATCH] Adsk Contrib - Improve the GPU unit test framework (#1562) (#1587) Signed-off-by: Patrick Hodoul --- .../ops/gradingtone/GradingToneOpGPU.cpp | 6 +- src/libutils/oglapphelpers/glsl.cpp | 19 +- tests/gpu/CMakeLists.txt | 3 + tests/gpu/GPUUnitTest.cpp | 163 +++++++++++++----- tests/gpu/GPUUnitTest.h | 10 +- tests/gpu/GradingToneOp_test.cpp | 6 +- 6 files changed, 139 insertions(+), 68 deletions(-) diff --git a/src/OpenColorIO/ops/gradingtone/GradingToneOpGPU.cpp b/src/OpenColorIO/ops/gradingtone/GradingToneOpGPU.cpp index 75d7b30888..3c64b5046e 100644 --- a/src/OpenColorIO/ops/gradingtone/GradingToneOpGPU.cpp +++ b/src/OpenColorIO/ops/gradingtone/GradingToneOpGPU.cpp @@ -84,7 +84,7 @@ void AddBoolUniform(GpuShaderCreatorRcPtr & shaderCreator, } } -static const std::string opPrefix{ "grading_tone" }; +const std::string opPrefix{ "grading_tone" }; void AddGTProperties(GpuShaderCreatorRcPtr & shaderCreator, GpuShaderText & st, @@ -1301,7 +1301,7 @@ void Add_SContrastBottomPre_Shader(GpuShaderText & st, GradingStyle style) // Bottom end st.newLine() << "{"; // establish scope so local variable names won't conflict - st.setIndent(4); + st.indent(); st.newLine() << st.floatKeywordConst() << " x0 = " << bottomPoint << ";"; st.newLine() << st.floatKeywordConst() << " y0 = " << bottomPoint << ";"; st.newLine() << st.floatKeywordConst() << " y3 = pivot - (pivot - y0) * 0.25;"; @@ -1562,7 +1562,7 @@ void GetGradingToneGPUShaderProgram(GpuShaderCreatorRcPtr & shaderCreator, st.newLine() << "{"; st.indent(); - // Properties holds shader variables names and is initialized with undecorated names suitable + // Properties hold shader variables names and are initialized with undecorated names suitable // for local variables. GTProperties properties; AddGTProperties(shaderCreator, st, gtData, properties, dyn); diff --git a/src/libutils/oglapphelpers/glsl.cpp b/src/libutils/oglapphelpers/glsl.cpp index 70c386f255..fb1b8b9dfe 100644 --- a/src/libutils/oglapphelpers/glsl.cpp +++ b/src/libutils/oglapphelpers/glsl.cpp @@ -479,21 +479,20 @@ unsigned OpenGLBuilder::buildProgram(const std::string & clientShaderProgram, bo glDeleteShader(m_fragShader); } - std::ostringstream os; - os << getGLSLVersionString() << std::endl - << (!standaloneShader ? m_shaderDesc->getShaderText() : "") << std::endl - << clientShaderProgram << std::endl; + std::ostringstream oss; + oss << getGLSLVersionString() << std::endl + << (!standaloneShader ? m_shaderDesc->getShaderText() : "") << std::endl + << clientShaderProgram << std::endl; if(m_verbose) { - std::cout << std::endl; - std::cout << "GPU Shader Program:" << std::endl; - std::cout << std::endl; - std::cout << os.str() << std::endl; - std::cout << std::endl; + std::cout << "\nGPU Shader Program:\n\n" + << oss.str() + << "\n\n" + << std::flush; } - m_fragShader = CompileShaderText(GL_FRAGMENT_SHADER, os.str().c_str()); + m_fragShader = CompileShaderText(GL_FRAGMENT_SHADER, oss.str().c_str()); LinkShaders(m_program, m_fragShader); m_shaderCacheID = shaderCacheID; diff --git a/tests/gpu/CMakeLists.txt b/tests/gpu/CMakeLists.txt index 52e4f9d504..aeeb66f7f2 100644 --- a/tests/gpu/CMakeLists.txt +++ b/tests/gpu/CMakeLists.txt @@ -47,7 +47,10 @@ target_link_libraries(test_gpu_exec PRIVATE OpenColorIO oglapphelpers + pystring::pystring unittest_data + utils::strings + testutils ) add_test(NAME test_gpu COMMAND test_gpu_exec) diff --git a/tests/gpu/GPUUnitTest.cpp b/tests/gpu/GPUUnitTest.cpp index cc623b9148..c6c1bd0715 100644 --- a/tests/gpu/GPUUnitTest.cpp +++ b/tests/gpu/GPUUnitTest.cpp @@ -2,19 +2,20 @@ // Copyright Contributors to the OpenColorIO Project. -#include -#include +#include +#include +#include #include #include - -#include -#include -#include -#include +#include #include #include "GPUUnitTest.h" + +#include "apputils/argparse.h" +#include "utils/StringUtils.h" + #include "oglapp.h" #if __APPLE__ #include "metalapp.h" @@ -26,12 +27,12 @@ namespace OCIO = OCIO_NAMESPACE; namespace Shader { // Default error threshold - const float defaultErrorThreshold = 1e-7f; + constexpr float defaultErrorThreshold = 1e-7f; // In some occasions, MAX_FLOAT will be "rounded" to infinity on some GPU renderers. // In order to avoid this issue, consider all number over/under a given threshold as // equal for testing purposes. - const float largeThreshold = std::numeric_limits::max(); + constexpr float largeThreshold = std::numeric_limits::max(); enum LimitsDiff { @@ -95,7 +96,7 @@ namespace Shader inline bool RelativeDifference(float x1, float x2, float min_x1, float & diff) { const float absx1 = fabs(x1); - float div = std::max(absx1, min_x1); + const float div = std::max(absx1, min_x1); const float thisDiff = fabs(x1 - x2) / div; if (thisDiff > diff) { @@ -122,8 +123,8 @@ namespace Shader } -OCIOGPUTest::OCIOGPUTest(const std::string& testgroup, - const std::string& testname, +OCIOGPUTest::OCIOGPUTest(const std::string & testgroup, + const std::string & testname, OCIOTestFuncCallback test) : m_group(testgroup) , m_name(testname) @@ -132,10 +133,6 @@ OCIOGPUTest::OCIOGPUTest(const std::string& testgroup, { } -OCIOGPUTest::~OCIOGPUTest() -{ -} - void OCIOGPUTest::setProcessor(OCIO::TransformRcPtr transform) { OCIO::ConfigRcPtr config = OCIO::Config::Create(); @@ -151,7 +148,7 @@ void OCIOGPUTest::setProcessor(OCIO::ConstConfigRcPtr config, void OCIOGPUTest::setProcessor(OCIO::ConstProcessorRcPtr processor) { - if(m_processor.get()!=0x0) + if (m_processor.get() != nullptr) { throw OCIO::Exception("GPU Unit test already exists"); } @@ -192,9 +189,9 @@ AddTest::AddTest(OCIOGPUTestRcPtr test) namespace { - static constexpr unsigned g_winWidth = 256; - static constexpr unsigned g_winHeight = 256; - static constexpr unsigned g_components = 4; + constexpr unsigned g_winWidth = 256; + constexpr unsigned g_winHeight = 256; + constexpr unsigned g_components = 4; void AllocateImageTexture(OCIO::OglAppRcPtr & app) { @@ -381,7 +378,7 @@ namespace } } - static constexpr size_t invalidIndex = std::numeric_limits::max(); + constexpr size_t invalidIndex = std::numeric_limits::max(); // Validate the GPU processing against the CPU one. void ValidateImageTexture(OCIO::OglAppRcPtr & app, OCIOGPUTestRcPtr & test) @@ -471,8 +468,8 @@ namespace } if (idxInf != invalidIndex) { - size_t componentIdx = idxInf % 4; - size_t pixelIdx = idxInf / 4; + componentIdx = idxInf % 4; + pixelIdx = idxInf / 4; err << std::setprecision(10) << "\nLarge number error: " << diff << " at pixel: " << pixelIdx << " on component " << componentIdx @@ -488,8 +485,8 @@ namespace } if (idxNan != invalidIndex) { - size_t componentIdx = idxNan % 4; - size_t pixelIdx = idxNan / 4; + componentIdx = idxNan % 4; + pixelIdx = idxNan / 4; err << std::setprecision(10) << "\nNAN error: " << diff << " at pixel: " << pixelIdx << " on component " << componentIdx @@ -512,19 +509,70 @@ namespace } }; -int main(int argc, char ** argv) +int main(int argc, const char ** argv) { - // Step 1: Initialize the graphic library engines. - OCIO::OglAppRcPtr app; - + +#if !defined(NDEBUG) && defined(_WIN32) + // Disable the 'assert' dialog box in debug mode. + _CrtSetReportMode(_CRT_ASSERT, _CRTDBG_MODE_DEBUG); +#endif + + bool printHelp = false; bool useMetalRenderer = false; - for(int i = 0; i < argc; ++i) + bool verbose = false; + bool stopOnFirstError = false; + + // Note that empty strings mean to run all the unit tests. + std::string filter, utestGroupAllowed, utestNameAllowed; + + ArgParse ap; + ap.options("\nCommand line arguments:\n", + "--help", &printHelp, "Print help message", + "--metal", &useMetalRenderer, "Run the GPU unit test with Metal", + "-v", &verbose, "Output the GPU shader program", + "--stop_on_error", &stopOnFirstError, "Stop on the first error", + "--run_only %s", &filter, "Run only some unit tests\n" + "\tex: --run_only ExponentOp/forward i.e. run only \"ExponentOp/forward\"\n" + "\tex: --run_only ExponentOp i.e. run \"ExponentOp/*\"\n" + "\tex: --run_only /forward i.e. run \"*/forward\"\n", + nullptr); + + if (ap.parse(argc, argv) < 0) + { + std::cerr << ap.geterror() << std::endl; + ap.usage(); + return 1; + } + + if (printHelp) + { + ap.usage(); + return 1; + } + + if (!filter.empty()) { - if(strcmp(argv[i], "-metal") == 0) + const std::vector results = StringUtils::Split(filter, '/'); + if (!results.empty()) { - useMetalRenderer = true; + utestGroupAllowed = StringUtils::Lower(StringUtils::Trim(results[0])); + if (results.size() >= 2) + { + utestNameAllowed = StringUtils::Lower(StringUtils::Trim(results[1])); + + if (results.size() >= 3) + { + std::cerr << "Invalid value for the argument '--run_only'." << std::endl; + ap.usage(); + return 1; + } + } } } + + // Step 1: Initialize the graphic library engines. + OCIO::OglAppRcPtr app; + try { if(useMetalRenderer) @@ -561,7 +609,7 @@ int main(int argc, char ** argv) unsigned failures = 0; - std::cerr << "\n OpenColorIO_Core_GPU_Unit_Tests\n\n"; + std::cout << "\n OpenColorIO_Core_GPU_Unit_Tests\n\n"; UnitTests & tests = GetUnitTests(); const size_t numTests = tests.size(); @@ -570,7 +618,32 @@ int main(int argc, char ** argv) const unsigned curr_failures = failures; OCIOGPUTestRcPtr test = tests[idx]; - + + // Is that a unit test to run ? + + const std::string utestGroup = test->group(); + const std::string utestName = test->name(); + + bool utestAllowed = true; + + if (!utestGroupAllowed.empty() && StringUtils::Lower(utestGroup)!=utestGroupAllowed) + { + utestAllowed = false; + } + + if (!utestNameAllowed.empty() && StringUtils::Lower(utestName)!=utestNameAllowed) + { + utestAllowed = false; + } + + if (!utestAllowed) + { + continue; + } + + // Prepare the unit test. + + test->setVerbose(verbose); test->setShadingLanguage( #if __APPLE__ useMetalRenderer ? @@ -582,6 +655,7 @@ int main(int argc, char ** argv) try { test->setup(); + enabledTest = test->isEnabled(); constexpr size_t maxCharToDisplay = 49; @@ -594,7 +668,7 @@ int main(int argc, char ** argv) name.resize(maxCharToDisplay); } - std::cerr << "[" + std::cout << "[" << std::right << std::setw(3) << (idx+1) << "/" << numTests << "] [" << std::left << std::setw(maxCharToDisplay+1) @@ -610,12 +684,12 @@ int main(int argc, char ** argv) const size_t numRetest = test->getNumRetests(); // Need to run once and for each retest. - for (size_t idx = 0; idx <= numRetest; ++idx) + for (size_t idxRetest = 0; idxRetest <= numRetest; ++idxRetest) { - if (idx != 0) // Skip first run. + if (idxRetest != 0) // Skip first run. { // Call the retest callback. - test->retestSetup(idx - 1); + test->retestSetup(idxRetest - 1); } // Process the image texture into the rendering buffer. @@ -640,15 +714,15 @@ int main(int argc, char ** argv) if (!enabledTest) { - std::cerr << "DISABLED" << std::endl; + std::cout << "DISABLED" << std::endl; } else if(curr_failures==failures && test->isValid()) { - size_t idx = test->getMaxDiffIndex(); - size_t componentIdx = idx % 4; - size_t pixelIdx = idx / 4; + const size_t idxMaxDiff = test->getMaxDiffIndex(); + const size_t componentIdx = idxMaxDiff % 4; + const size_t pixelIdx = idxMaxDiff / 4; - std::cerr << "PASSED - (MaxDiff: " << test->getMaxDiff() + std::cout << "PASSED - (MaxDiff: " << test->getMaxDiff() << " at pix[" << pixelIdx << "][" << componentIdx << "])" << std::endl; } @@ -660,9 +734,8 @@ int main(int argc, char ** argv) // Get rid of the test. tests[idx] = nullptr; - } - std::cerr << std::endl << failures << " tests failed" << std::endl << std::endl; + std::cout << std::endl << failures << " tests failed" << std::endl << std::endl; return failures; } diff --git a/tests/gpu/GPUUnitTest.h b/tests/gpu/GPUUnitTest.h index e4e4b781b9..00bd4f89e5 100644 --- a/tests/gpu/GPUUnitTest.h +++ b/tests/gpu/GPUUnitTest.h @@ -31,17 +31,13 @@ class OCIOGPUTest // Keeping the original input value size allows // to avoid manipulating padded values added to fit // the predefined GPU texture size. - size_t m_originalInputValueSize; - - CustomValues() - : m_originalInputValueSize(0) - {} + size_t m_originalInputValueSize{ 0 }; }; public: OCIOGPUTest(const std::string& testgroup, const std::string& testname, OCIOTestFuncCallback test); - ~OCIOGPUTest(); + ~OCIOGPUTest() = default; inline const std::string& group() const { return m_group; } inline const std::string& name() const { return m_name; } @@ -161,7 +157,7 @@ class OCIOGPUTest bool m_legacyShader{ false }; unsigned m_legacyShaderLutEdge{ 32 }; CustomValues m_values; - OCIO_NAMESPACE::GpuLanguage m_gpuShadingLanguage; + OCIO_NAMESPACE::GpuLanguage m_gpuShadingLanguage {OCIO_NAMESPACE::GPU_LANGUAGE_GLSL_1_2}; std::vector m_retests; diff --git a/tests/gpu/GradingToneOp_test.cpp b/tests/gpu/GradingToneOp_test.cpp index bb4d068c87..8fa660bf48 100644 --- a/tests/gpu/GradingToneOp_test.cpp +++ b/tests/gpu/GradingToneOp_test.cpp @@ -389,7 +389,7 @@ class GTRetest public: GTRetest() = delete; - GTRetest(OCIOGPUTest & test) + explicit GTRetest(OCIOGPUTest & test) : m_test(test) { OCIO::ConstProcessorRcPtr & processor = test.getProcessor(); @@ -437,7 +437,7 @@ OCIO_ADD_GPU_TEST(GradingTone, style_log_dynamic_retests) class MyGTRetest : public GTRetest { public: - MyGTRetest(OCIOGPUTest & test) : GTRetest(test) + explicit MyGTRetest(OCIOGPUTest & test) : GTRetest(test) { } @@ -519,7 +519,7 @@ OCIO_ADD_GPU_TEST(GradingTone, two_transforms_retests) class MyGTRetest : public GTRetest { public: - MyGTRetest(OCIOGPUTest & test) : GTRetest(test) + explicit MyGTRetest(OCIOGPUTest & test) : GTRetest(test) { }