Simplify OSL test setup (#1868)

Use the OSL cmake exports to configure the variables needed to enable OSL testing. Also remove the need for specifying the standard OSL include path, an oslc install will already know where these are.
AcademySoftwareFoundation · Aug 3, 2024 · dd075c0 · dd075c0
1 parent 1775a6d
commit dd075c0
Show file tree

Hide file tree

Showing 5 changed files with 25 additions and 13 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -180,10 +180,23 @@ if (MATERIALX_BUILD_USE_CCACHE)
     endif()
 endif()
 
+# Attempt to configure OSL testing if it can be found by cmake.
+# This will not override any explicitly provided oslc and testrender
+if(MATERIALX_BUILD_RENDER AND MATERIALX_BUILD_GEN_OSL AND MATERIALX_BUILD_TESTS)
+    # We currently only need the actual OSL binaries if we're running tests with Render and GenOSL enabled.
+    find_package(OSL QUIET)
+    if(OSL_FOUND)
+        if(NOT MATERIALX_OSL_BINARY_OSLC)
+            set(MATERIALX_OSL_BINARY_OSLC $<TARGET_FILE:OSL::oslc>)
+        endif()
+        if(NOT MATERIALX_OSL_BINARY_TESTRENDER)
+            # currently OSL does not export a cmake target for testrender, but once that's added this can be simplified.
+            set(MATERIALX_OSL_BINARY_TESTRENDER $<TARGET_FILE_DIR:OSL::oslc>/testrender)
+        endif()
+    endif()
+endif()
+
 # Add global definitions
-add_definitions(-DMATERIALX_OSL_BINARY_OSLC=\"${MATERIALX_OSL_BINARY_OSLC}\")
-add_definitions(-DMATERIALX_OSL_BINARY_TESTRENDER=\"${MATERIALX_OSL_BINARY_TESTRENDER}\")
-add_definitions(-DMATERIALX_OSL_INCLUDE_PATH=\"${MATERIALX_OSL_INCLUDE_PATH}\")
 if (MATERIALX_OSL_LEGACY_CLOSURES)
     add_definitions(-DMATERIALX_OSL_LEGACY_CLOSURES)
 endif()

diff --git a/source/MaterialXRenderOsl/OslRenderer.cpp b/source/MaterialXRenderOsl/OslRenderer.cpp
@@ -48,10 +48,6 @@ void OslRenderer::setSize(unsigned int width, unsigned int height)
 
 void OslRenderer::initialize(RenderContextHandle)
 {
-    if (_oslIncludePath.isEmpty())
-    {
-        throw ExceptionRenderError("OSL validation include path is empty");
-    }
     if (_oslTestShadeExecutable.isEmpty() && _oslCompilerExecutable.isEmpty())
     {
         throw ExceptionRenderError("OSL validation executables not set");
@@ -61,7 +57,7 @@ void OslRenderer::initialize(RenderContextHandle)
 void OslRenderer::renderOSL(const FilePath& dirPath, const string& shaderName, const string& outputName)
 {
     // If command options missing, skip testing.
-    if (_oslTestRenderExecutable.isEmpty() || _oslIncludePath.isEmpty() ||
+    if (_oslTestRenderExecutable.isEmpty() ||
         _oslTestRenderSceneTemplateFile.isEmpty() || _oslUtilityOSOPath.isEmpty())
     {
         throw ExceptionRenderError("Command input arguments are missing");
@@ -218,7 +214,7 @@ void OslRenderer::renderOSL(const FilePath& dirPath, const string& shaderName, c
 void OslRenderer::shadeOSL(const FilePath& dirPath, const string& shaderName, const string& outputName)
 {
     // If no command and include path specified then skip checking.
-    if (_oslTestShadeExecutable.isEmpty() || _oslIncludePath.isEmpty())
+    if (_oslTestShadeExecutable.isEmpty())
     {
         return;
     }
@@ -279,7 +275,7 @@ void OslRenderer::shadeOSL(const FilePath& dirPath, const string& shaderName, co
 void OslRenderer::compileOSL(const FilePath& oslFilePath)
 {
     // If no command and include path specified then skip checking.
-    if (_oslCompilerExecutable.isEmpty() || _oslIncludePath.isEmpty())
+    if (_oslCompilerExecutable.isEmpty())
     {
         return;
     }
@@ -333,7 +329,7 @@ void OslRenderer::createProgram(const StageMap& stages)
         throw ExceptionRenderError("No shader code to validate");
     }
 
-    bool haveCompiler = !_oslCompilerExecutable.isEmpty() && !_oslIncludePath.isEmpty();
+    bool haveCompiler = !_oslCompilerExecutable.isEmpty();
     if (!haveCompiler)
     {
         throw ExceptionRenderError("No OSL compiler specified for validation");

diff --git a/source/MaterialXTest/MaterialXRenderOsl/CMakeLists.txt b/source/MaterialXTest/MaterialXRenderOsl/CMakeLists.txt
@@ -3,6 +3,11 @@ file(GLOB headers "${CMAKE_CURRENT_SOURCE_DIR}/*.h")
 
 target_sources(MaterialXTest PUBLIC ${source} ${headers})
 
+target_compile_definitions(MaterialXTest PRIVATE
+        MATERIALX_OSL_BINARY_OSLC=\"${MATERIALX_OSL_BINARY_OSLC}\"
+        MATERIALX_OSL_BINARY_TESTRENDER=\"${MATERIALX_OSL_BINARY_TESTRENDER}\"
+)
+
 add_tests("${source}")
 
 assign_source_group("Source Files" ${source})

diff --git a/source/MaterialXTest/MaterialXRenderOsl/GenReference.cpp b/source/MaterialXTest/MaterialXRenderOsl/GenReference.cpp
@@ -34,7 +34,6 @@ TEST_CASE("GenReference: OSL Reference", "[genreference]")
         oslRenderer = mx::OslRenderer::create();
         oslRenderer->setOslCompilerExecutable(MATERIALX_OSL_BINARY_OSLC);
         mx::FileSearchPath oslIncludePaths; 
-        oslIncludePaths.append(mx::FilePath(MATERIALX_OSL_INCLUDE_PATH));
         // Add in library include path for compile testing as the generated
         // shader's includes are not added with absolute paths.
         oslIncludePaths.append(searchPath.find("libraries/stdlib/genosl/include"));

diff --git a/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp b/source/MaterialXTest/MaterialXRenderOsl/RenderOsl.cpp
@@ -126,7 +126,6 @@ void OslShaderRenderTester::createRenderer(std::ostream& log)
     _renderer->setOslCompilerExecutable(oslcExecutable);
     const std::string testRenderExecutable(MATERIALX_OSL_BINARY_TESTRENDER);
     _renderer->setOslTestRenderExecutable(testRenderExecutable);
-    _renderer->setOslIncludePath(mx::FileSearchPath(MATERIALX_OSL_INCLUDE_PATH));
 
     try
     {