Make render delegate light filters setting consistent with the proced…

…ural (#1910) * remove quotes that were incorrectly inserted in the plugInfo.json * add test_1906 with light blocker * add test_1906 to the hydra group * update test_0245 to work with ARNOLD_PLUGIN_PATH containing a list of paths * update the cmake testsuite to correctly set the environment variables on unix (WIP) * test_0245: extract imaging from ndr function * fix a compilation warning "missing override" * move arnold ndr and imaging pluginfo creation in a dedicated function * indent code inside if(NOT BUILD_WITH_USD_STATIC) * link the hydra procedural with usdImagingArnold * use pluginfo creation functions in the hydra procedural * copy the generated schemas in the procedural build folder * test_0245 - look in the usd plugin paths to find ndrArnold and usdImagingArnold * fix cmake warning regarding cmake_minimum_version * suppress usdGenSchema missing warning when our internal one is set * add the plugin paths to the cmake tests to test the shared procedural * clear primvars:arnold:filters which highlights the bug * defer setting light filters to ProcessConnection * update changelog * update cmake configuration for the case where we only build the render delegate
Autodesk · May 10, 2024 · c3cd486 · c3cd486
1 parent 4390885
commit c3cd486
Show file tree

Hide file tree

Showing 19 changed files with 482 additions and 175 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 - [usd#1900](https://github.com/Autodesk/arnold-usd/issues/1900) - Fix transform hierarchies for Arnold non-xformable primitives
 - [usd#1908](https://github.com/Autodesk/arnold-usd/issues/1908) - Read deform_keys independently of the primvar interpolation
 - [usd#1903](https://github.com/Autodesk/arnold-usd/issues/1903) - USD Writer should skip materials when the shader mask is disabled
+- [usd#1906](https://github.com/Autodesk/arnold-usd/issues/1906) - Fix light filters assignment order in the render delegate to make it consistent with the procedural.
 
 ## [7.3.1.0] - 2024-03-27
 

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -1,7 +1,6 @@
-project(arnold-usd)
-
 cmake_minimum_required(VERSION 3.20)
 
+project(arnold-usd)
 
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/modules" 0)
 list(APPEND CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake/utils" 0)
@@ -41,7 +40,8 @@ if (BUILD_DOCS)
     add_subdirectory(docs)
 endif ()
 
-if (BUILD_TESTSUITE)
+# We are only testing the procedural
+if (BUILD_TESTSUITE AND BUILD_PROCEDURAL)
     enable_testing()
     add_subdirectory(testsuite)
 endif ()

diff --git a/cmake/utils/build.cmake b/cmake/utils/build.cmake
@@ -100,3 +100,32 @@ function(generate_plug_info_for_testsuite)
         NEWLINE_STYLE UNIX
     )
 endfunction()
+
+# Depending on the configuration we will have to install the ndr pluginfo with a different content and
+# at a different location, so we use this function to do so
+function(install_ndr_arnold_pluginfo LIB_PATH NDR_PLUGINFO CONFIG_ROOT)
+    # LIB_PATH is used in the plugInfo.json.in, do not rename
+    set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
+    configure_file(
+        "${NDR_PLUGINFO_SRC}"
+        "${NDR_PLUGINFO}"
+    )
+    install(FILES "${NDR_PLUGINFO}"
+        DESTINATION "${CONFIG_ROOT}/ndrArnold/resources")
+endfunction()
+
+set(ARNOLD_USDIMAGING_CLASSES Alembic Box Cone Curves Disk Implicit Nurbs Plane Points Polymesh Procedural Sphere Usd Volume VolumeImplicit)
+
+function(install_usdimaging_arnold_pluginfo LIB_PATH PLUGINFO CONFIG_ROOT)
+    # LIB_PATH is used in the plugInfo.json.in, do not rename
+    set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
+    foreach (each ${ARNOLD_USDIMAGING_CLASSES})
+        set(REGISTER_ARNOLD_TYPES "${REGISTER_ARNOLD_TYPES}\n\"UsdImagingArnold${each}Adapter\":{\"bases\":[\"UsdImagingGprimAdapter\"],\"primTypeName\":\"Arnold${each}\"},")
+    endforeach ()
+    configure_file(
+        "${USDIMAGINGARNOLD_PLUGINFO_SRC}"
+        "${PLUGINFO}"
+    )
+    install(FILES "${PLUGINFO}"
+        DESTINATION "${CONFIG_ROOT}/usdImagingArnold/resources")
+endfunction()
diff --git a/cmake/utils/clangdefaults.cmake b/cmake/utils/clangdefaults.cmake
@@ -1,4 +1,5 @@
 
-add_compile_options($<$<OR:$<CXX_COMPILER_ID:GNU>,$<CXX_COMPILER_ID:Clang>>:-Wno-deprecated>)
-
 add_compile_definitions(_LIBCPP_ENABLE_CXX17_REMOVED_UNARY_BINARY_FUNCTION)
+add_compile_options(-Wno-deprecated)
+add_compile_options(-Wno-deprecated-declarations)
+add_compile_options(-Wno-deprecated-builtins)
diff --git a/libs/render_delegate/light.cpp b/libs/render_delegate/light.cpp
@@ -638,7 +638,7 @@ void HdArnoldGenericLight::Sync(HdSceneDelegate* sceneDelegate, HdRenderParam* r
         }
         AiNodeSetDisabled(_light, !sceneDelegate->GetVisible(id));
 
-        SdfPath lightShaderPath = HdArnoldLight::ComputeLightShaders(sceneDelegate, id, TfToken("primvars:arnold:shaders"), _light);
+        SdfPath lightShaderPath = HdArnoldLight::ComputeLightShaders(sceneDelegate, _delegate, id, TfToken("primvars:arnold:shaders"), _light);
 
         // Get an eventual hydra instancer and rebuild the arnold instancer nodes.
         for (auto &instancerNode : _instancers) {
@@ -810,7 +810,7 @@ AtNode* GetLightNode(const HdLight* light)
     return static_cast<const HdArnoldGenericLight*>(light)->GetLightNode();
 }
 
-SdfPath ComputeLightShaders(HdSceneDelegate* sceneDelegate, const SdfPath &id, const TfToken &attr, AtNode *light )
+SdfPath ComputeLightShaders(HdSceneDelegate* sceneDelegate, HdArnoldRenderDelegate *renderDelegate, const SdfPath &id, const TfToken &attr, AtNode *light )
 {
     // get the sdf path for the light shader arnold node graph container
     SdfPath lightShaderPath;
@@ -836,8 +836,13 @@ SdfPath ComputeLightShaders(HdSceneDelegate* sceneDelegate, const SdfPath &id, c
 
             lightFilters = nodeGraph->GetTerminals(_tokens->filtersArray);
             if (!lightFilters.empty()) {
-                AiNodeSetArray(light, str::filters, AiArrayConvert(static_cast<uint32_t>(lightFilters.size()), 1,
-                                                                    AI_TYPE_NODE, lightFilters.data()));
+                VtArray<std::string> filtersNodeName;
+                for (const AtNode *node:lightFilters) {
+                    filtersNodeName.push_back(AiNodeGetName(node));
+                }
+                // Here we use HdArnoldSetParameter instead of AiNodeSetArray because it will defer connecting the filters
+                // to the ProcessConnection method which happens later in the process. This is how the procedural behaves.
+                HdArnoldSetParameter(light, AiNodeEntryLookUpParameter(AiNodeGetNodeEntry(light), str::filters), VtValue(filtersNodeName), renderDelegate);
             }
         }
     }

diff --git a/libs/render_delegate/light.h b/libs/render_delegate/light.h
@@ -104,7 +104,7 @@ HdLight* CreateGeometryLight(HdArnoldRenderDelegate* renderDelegate, const SdfPa
 /// @return Pointer to the Arnold Light, can be nullptr.
 AtNode* GetLightNode(const HdLight* light);
 
-SdfPath ComputeLightShaders(HdSceneDelegate* sceneDelegate, const SdfPath &id, const TfToken &attr, AtNode *light);
+SdfPath ComputeLightShaders(HdSceneDelegate* sceneDelegate, HdArnoldRenderDelegate *renderDelegate, const SdfPath &id, const TfToken &attr, AtNode *light);
 
 } // namespace HdArnoldLight
 

diff --git a/libs/render_delegate/mesh.cpp b/libs/render_delegate/mesh.cpp
@@ -394,7 +394,7 @@ void HdArnoldMesh::Sync(
                         primvarStr.erase(7, 6);
 
                         if (primvarStr == "arnold:shaders") {
-                            HdArnoldLight::ComputeLightShaders(sceneDelegate, id, 
+                            HdArnoldLight::ComputeLightShaders(sceneDelegate, _renderDelegate, id, 
                                 TfToken("primvars:arnold:light:shaders"), meshLight);
                         } else {
                             HdArnoldSetConstantPrimvar(

diff --git a/libs/render_delegate/node_graph.cpp b/libs/render_delegate/node_graph.cpp
@@ -78,7 +78,7 @@ class MaterialHydraReader : public MaterialReader
     // GetShaderInput is called to return a parameter value for a given shader
     // in the current network. It also returns the shaderId of the shader
     bool GetShaderInput(const SdfPath& shaderPath, const TfToken& param,
-        VtValue& value, TfToken& shaderId) 
+        VtValue& value, TfToken& shaderId) override
     {
         for (const auto& node : _network.nodes) {
             if (node.path != shaderPath) 

diff --git a/plugins/ndr/CMakeLists.txt b/plugins/ndr/CMakeLists.txt
@@ -33,22 +33,14 @@ if (NOT BUILD_WITH_USD_STATIC)
     target_compile_definitions(ndrArnold PRIVATE "NDRARNOLD_EXPORTS=1")
 
     # Configure plugInfo.json, it only need the library extension
-    set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
-    set(LIB_PATH ../ndrArnold)
-    set(NDR_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json")
-    configure_file(
-        "${NDR_PLUGINFO_SRC}"
-        "${NDR_PLUGINFO}"
-    )
+    install_ndr_arnold_pluginfo(../ndrArnold "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json" "${PREFIX_PLUGINS}")
+
     # We replicate the layout of the plugin installation inside the build dir for running the testsuite without installing.
     generate_plug_info_for_testsuite(TARGET_NAME ndrArnold TARGET_PLUGINFO "${NDR_PLUGINFO}")
 
     install(TARGETS ndrArnold
         DESTINATION "${PREFIX_PLUGINS}")
 
-    install(FILES "${NDR_PLUGINFO}"
-        DESTINATION "${PREFIX_PLUGINS}/ndrArnold/resources")
-
     # Why do we intall the headers ??
     #install(FILES ${HDR}
     #    DESTINATION "${PREFIX_HEADERS}/arnold_usd/ndr")

diff --git a/plugins/procedural/CMakeLists.txt b/plugins/procedural/CMakeLists.txt
@@ -39,7 +39,7 @@ endif()
 # LINK
 target_link_libraries(${PROC_NAME} PRIVATE translator common)
 if (ENABLE_HYDRA_IN_USD_PROCEDURAL)
-    target_link_libraries(${PROC_NAME} PRIVATE render_delegate ndrObjects)
+    target_link_libraries(${PROC_NAME} PRIVATE render_delegate ndrObjects usdImagingArnoldObjects)
 endif()
 
 if (APPLE OR LINUX)
@@ -61,27 +61,17 @@ if (BUILD_WITH_USD_STATIC)
             DESTINATION "${PREFIX_PROCEDURAL}")
 
     if (ENABLE_HYDRA_IN_USD_PROCEDURAL)
-        # install the ndr plugin
-        set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
-        set(LIB_PATH '../../usd_proc')
-        set(NDR_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/usd/ndrArnold/resources/plugInfo.json")
-        configure_file(
-            "${NDR_PLUGINFO_SRC}"
-            "${NDR_PLUGINFO}"
-        )
-        install(FILES "${NDR_PLUGINFO}"
-                DESTINATION "${PREFIX_PROCEDURAL}/usd/ndrArnold/resources")
-
-        # install the usd imaging plugin
-        set(LIB_PATH '../../usd_proc')
-        set(USDIMAGINGARNOLD_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/usd/usdImagingArnold/resources/plugInfo.json")
-        configure_file(
-            "${USDIMAGINGARNOLD_PLUGINFO_SRC}"
-            "${USDIMAGINGARNOLD_PLUGINFO}"
-        )
-
-        install(FILES "${USDIMAGINGARNOLD_PLUGINFO}"
-                DESTINATION "${PREFIX_PROCEDURAL}/usd/usdImagingArnold/resources")
+        # Install the ndr arnold pluginfo
+        install_ndr_arnold_pluginfo(
+            ../../usd_proc 
+            "${CMAKE_CURRENT_BINARY_DIR}/usd/ndrArnold/resources/plugInfo.json"
+            "${PREFIX_PROCEDURAL}/usd")
+
+        # install the usd imaging arnold pluginfo
+        install_usdimaging_arnold_pluginfo(
+            ../../usd_proc
+            "${CMAKE_CURRENT_BINARY_DIR}/usd/usdImagingArnold/resources/plugInfo.json"
+            "${PREFIX_PROCEDURAL}/usd")
 
         # Install the render delegate plugInfo.json
         set(HDARNOLD_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/usd/hdArnold/resources/plugInfo.json")

diff --git a/plugins/render_delegate/CMakeLists.txt b/plugins/render_delegate/CMakeLists.txt
@@ -1,44 +1,44 @@
 set(SRC
     renderer_plugin.cpp)
 if (NOT BUILD_WITH_USD_STATIC)
-add_library(hdArnold SHARED ${COMMON_SRC} ${SRC})
-if (BUILD_HEADERS_AS_SOURCES)
-    target_sources(hdArnold PRIVATE ${COMMON_HDR} ${HDR})
-endif ()
+    add_library(hdArnold SHARED ${COMMON_SRC} ${SRC})
+    if (BUILD_HEADERS_AS_SOURCES)
+        target_sources(hdArnold PRIVATE ${COMMON_HDR} ${HDR})
+    endif ()
 
-set(_usd_deps arch plug trace tf vt gf work sdf hf hd usdImaging usdLux pxOsd cameraUtil)
-if (${USD_VERSION} VERSION_LESS "0.20.5")
-    set(_usd_deps ${_usd_deps} hdx)
-endif ()
-add_common_dependencies(
-    TARGET_NAME hdArnold
-    USD_DEPENDENCIES ${_usd_deps})
-target_include_directories(hdArnold PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../../libs/render_delegate")
-target_compile_definitions(hdArnold PRIVATE "HDARNOLD_EXPORTS=1")
-target_link_libraries(hdArnold PRIVATE render_delegate)
+    set(_usd_deps arch plug trace tf vt gf work sdf hf hd usdImaging usdLux pxOsd cameraUtil)
+    if (${USD_VERSION} VERSION_LESS "0.20.5")
+        set(_usd_deps ${_usd_deps} hdx)
+    endif ()
+    add_common_dependencies(
+        TARGET_NAME hdArnold
+        USD_DEPENDENCIES ${_usd_deps})
+    target_include_directories(hdArnold PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/../../libs/render_delegate")
+    target_compile_definitions(hdArnold PRIVATE "HDARNOLD_EXPORTS=1")
+    target_link_libraries(hdArnold PRIVATE render_delegate)
 
-if (${USD_VERSION} VERSION_GREATER "0.19.7")
-    set(RENDERER_PLUGIN_BASE "HdRendererPlugin")
-else ()
-    set(RENDERER_PLUGIN_BASE "HdxRendererPlugin")
-endif ()
+    if (${USD_VERSION} VERSION_GREATER "0.19.7")
+        set(RENDERER_PLUGIN_BASE "HdRendererPlugin")
+    else ()
+        set(RENDERER_PLUGIN_BASE "HdxRendererPlugin")
+    endif ()
 
-# We replicate the layout of the plugin installation inside the build dir for running the testsuite without installing.
-# Since we need the same structure for each config, we need to use generator expressions to create the pluginfo
-# files inside the build directories. The current workaround for that is to use file(GENERATE).
-set(HDARNOLD_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json")
-set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
-configure_file(
-    plugInfo.json.in
-    "${HDARNOLD_PLUGINFO}"
-)
-generate_plug_info_for_testsuite(TARGET_NAME render_delegate TARGET_PLUGINFO "${HDARNOLD_PLUGINFO}")
+    # We replicate the layout of the plugin installation inside the build dir for running the testsuite without installing.
+    # Since we need the same structure for each config, we need to use generator expressions to create the pluginfo
+    # files inside the build directories. The current workaround for that is to use file(GENERATE).
+    set(HDARNOLD_PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json")
+    set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
+    configure_file(
+        plugInfo.json.in
+        "${HDARNOLD_PLUGINFO}"
+    )
+    generate_plug_info_for_testsuite(TARGET_NAME render_delegate TARGET_PLUGINFO "${HDARNOLD_PLUGINFO}")
 
-install(TARGETS hdArnold
-    DESTINATION "${PREFIX_PLUGINS}")
+    install(TARGETS hdArnold
+        DESTINATION "${PREFIX_PLUGINS}")
 
-install(FILES "${HDARNOLD_PLUGINFO}"
-    DESTINATION "${PREFIX_PLUGINS}/hdArnold/resources")
+    install(FILES "${HDARNOLD_PLUGINFO}"
+        DESTINATION "${PREFIX_PLUGINS}/hdArnold/resources")
 
 # Why do we install the headers ?
 #install(FILES ${HDR}

diff --git a/plugins/usd_imaging/CMakeLists.txt b/plugins/usd_imaging/CMakeLists.txt
@@ -17,20 +17,26 @@ set(HDR
     shape_adapter.h
     )
 
-if (NOT BUILD_WITH_USD_STATIC)
-    # ginstance and instancer is an exception
-    set(CREATE_ADAPTER_CLASSES "")
-    set(REGISTER_ADAPTER_CLASSES "")
-    set(REGISTER_ARNOLD_TYPES "")
-    foreach (each Alembic Box Cone Curves Disk Implicit Nurbs Plane Points Polymesh Procedural Sphere Usd Volume VolumeImplicit)
-        set(CREATE_ADAPTER_CLASSES "${CREATE_ADAPTER_CLASSES}\nCREATE_ADAPTER_CLASS(${each});")
-        set(REGISTER_ADAPTER_CLASSES "${REGISTER_ADAPTER_CLASSES}\nREGISTER_ADAPTER_CLASS(${each})")
-        set(REGISTER_ARNOLD_TYPES "${REGISTER_ARNOLD_TYPES}\n\"UsdImagingArnold${each}Adapter\":{\"bases\":[\"UsdImagingGprimAdapter\"],\"primTypeName\":\"Arnold${each}\"},")
-    endforeach ()
+# Create the shape adapter file
+set(CREATE_ADAPTER_CLASSES "")
+set(REGISTER_ADAPTER_CLASSES "")
+set(REGISTER_ARNOLD_TYPES "")
+foreach (each ${ARNOLD_USDIMAGING_CLASSES})
+    set(CREATE_ADAPTER_CLASSES "${CREATE_ADAPTER_CLASSES}\nCREATE_ADAPTER_CLASS(${each});")
+    set(REGISTER_ADAPTER_CLASSES "${REGISTER_ADAPTER_CLASSES}\nREGISTER_ADAPTER_CLASS(${each})")
+endforeach ()
+configure_file("${CMAKE_CURRENT_SOURCE_DIR}/shape_adapters.cpp.in" "${CMAKE_CURRENT_BINARY_DIR}/shape_adapters.cpp")
+
+# Add objects for the hydra procedural
+add_library(usdImagingArnoldObjects OBJECT ${SRC} "${CMAKE_CURRENT_BINARY_DIR}/shape_adapters.cpp")
+target_compile_definitions(usdImagingArnoldObjects PRIVATE "USDIMAGINGARNOLD_EXPORTS=1")
+add_common_includes(TARGET_NAME usdImagingArnoldObjects DEPENDENCIES common)
 
-    configure_file("${CMAKE_CURRENT_SOURCE_DIR}/shape_adapters.cpp.in" "${CMAKE_CURRENT_BINARY_DIR}/shape_adapters.cpp")
+# If we want a usdImagingArnold shared plugin
+if (NOT BUILD_WITH_USD_STATIC)
 
     add_library(usdImagingArnold SHARED ${SRC} "${CMAKE_CURRENT_BINARY_DIR}/shape_adapters.cpp")
+
     if (BUILD_HEADERS_AS_SOURCES)
         target_sources(usdImagingArnold PRIVATE ${HDR})
     endif ()
@@ -43,27 +49,19 @@ if (NOT BUILD_WITH_USD_STATIC)
     target_compile_definitions(usdImagingArnold PRIVATE "USDIMAGINGARNOLD_EXPORTS=1")
     # For the generated shape adapters to find headers here.
     target_include_directories(usdImagingArnold PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}")
+
+    install(TARGETS usdImagingArnold
+        DESTINATION "${PREFIX_PLUGINS}")
 
-    source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SRC} ${HDR} shape_adapters.cpp.in)
-    source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR}/../common PREFIX common FILES ${COMMON_SRC} ${COMMON_HDR})
-
-    set(LIB_EXTENSION ${CMAKE_SHARED_LIBRARY_SUFFIX})
-    set(LIB_PATH '../usdImagingArnold')
-    set(PLUGINFO "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json")
-    configure_file(
-        "${USDIMAGINGARNOLD_PLUGINFO_SRC}"
-        "${PLUGINFO}"
-    )
+    install_usdimaging_arnold_pluginfo(
+            ../usdImagingArnold
+            "${CMAKE_CURRENT_BINARY_DIR}/plug/plugInfo.json"
+            ${PREFIX_PLUGINS})
 
     # We replicate the layout of the plugin installation inside the build dir for running the testsuite without installing.
     generate_plug_info_for_testsuite(TARGET_NAME usdImagingArnold TARGET_PLUGINFO "${PLUGINFO}")
 
-    install(TARGETS usdImagingArnold
-        DESTINATION "${PREFIX_PLUGINS}")
-
-    install(FILES "${PLUGINFO}"
-        DESTINATION "${PREFIX_PLUGINS}/usdImagingArnold/resources")
-
-    #install(FILES ${HDR}
-    #    DESTINATION "${PREFIX_HEADERS}/arnold_usd/usd_imaging")
 endif()
+
+source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR} FILES ${SRC} ${HDR} shape_adapters.cpp.in)
+source_group(TREE ${CMAKE_CURRENT_SOURCE_DIR}/../common PREFIX common FILES ${COMMON_SRC} ${COMMON_HDR})