Hide PugiXML symbols via cmake (#1944)

In response to #1941. Updating cmake to hide the PugiXML symbols. Creates separate static library for PugiXML and hide the symbols using cmake, so it should be portable.
AcademySoftwareFoundation · Aug 6, 2024 · cfa6f7b · cfa6f7b
1 parent e98d05f
commit cfa6f7b
Show file tree

Hide file tree

Showing 12 changed files with 86 additions and 43 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -300,7 +300,8 @@ function(mx_add_library MATERIALX_MODULE_NAME)
             SOURCE_FILES
             HEADER_FILES
             INLINED_FILES
-            LIBRARIES)
+            LIBRARIES
+            PRIVATE_LIBRARIES)
     cmake_parse_arguments(args
             "${options}"
             "${oneValueArgs}"
@@ -328,11 +329,15 @@ function(mx_add_library MATERIALX_MODULE_NAME)
             target_sources(${TARGET_NAME} PRIVATE ${CMAKE_CURRENT_BINARY_DIR}/version.rc)
         endif()
 
-        target_link_libraries(
-                ${TARGET_NAME}
+        target_link_libraries(${TARGET_NAME}
+            PUBLIC
                 ${args_LIBRARIES}
                 ${CMAKE_DL_LIBS})
 
+        target_link_libraries(${TARGET_NAME}
+             PRIVATE
+                ${args_PRIVATE_LIBRARIES})
+
         target_include_directories(${TARGET_NAME}
                 PUBLIC
                 $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/../>
@@ -357,6 +362,9 @@ function(mx_add_library MATERIALX_MODULE_NAME)
         set_property(GLOBAL APPEND PROPERTY MATERIALX_MODULES ${MATERIALX_MODULE_NAME})
     endif()
 
+    set_target_properties(${TARGET_NAME} PROPERTIES CXX_VISIBILITY_PRESET hidden)
+    set_target_properties(${TARGET_NAME} PROPERTIES CMAKE_VISIBILITY_INLINES_HIDDEN 1)
+
     target_sources(${TARGET_NAME}
         PRIVATE
             ${args_SOURCE_FILES}

diff --git a/source/CMakeLists.txt b/source/CMakeLists.txt
@@ -5,6 +5,9 @@ if (MATERIALX_BUILD_MONOLITHIC)
     # such that the individual module would have been built if not in monolithic build mode
     add_library(${MATERIALX_MODULE_NAME} "" "" )
 
+    set_target_properties(${MATERIALX_MODULE_NAME} PROPERTIES CXX_VISIBILITY_PRESET hidden)
+    set_target_properties(${MATERIALX_MODULE_NAME} PROPERTIES CMAKE_VISIBILITY_INLINES_HIDDEN 1)
+
     # Create version resource
     if(MATERIALX_BUILD_SHARED_LIBS AND MSVC)
         configure_file(${CMAKE_SOURCE_DIR}/cmake/modules/MaterialXVersion.rc.in ${CMAKE_CURRENT_BINARY_DIR}/version.rc)

diff --git a/source/MaterialXCore/Library.h b/source/MaterialXCore/Library.h
@@ -32,12 +32,10 @@
         #define MATERIALX_EXPORT_EXTERN_TEMPLATE(...) template class MATERIALX_SYMBOL_EXPORT __VA_ARGS__
         #define MATERIALX_IMPORT_EXTERN_TEMPLATE(...) extern template class MATERIALX_SYMBOL_IMPORT __VA_ARGS__
     #else
-        // Presently non-Windows platforms just export all symbols from
-        // shared libraries rather than using the explicit declarations.
-        #define MATERIALX_SYMBOL_EXPORT
-        #define MATERIALX_SYMBOL_IMPORT
-        #define MATERIALX_EXPORT_EXTERN_TEMPLATE(...)
-        #define MATERIALX_IMPORT_EXTERN_TEMPLATE(...)
+        #define MATERIALX_SYMBOL_EXPORT __attribute__((__visibility__("default")))
+        #define MATERIALX_SYMBOL_IMPORT __attribute__((__visibility__("default")))
+        #define MATERIALX_EXPORT_EXTERN_TEMPLATE(...) template class MATERIALX_SYMBOL_EXPORT __VA_ARGS__
+        #define MATERIALX_IMPORT_EXTERN_TEMPLATE(...) extern template class MATERIALX_SYMBOL_IMPORT __VA_ARGS__
     #endif
 #else
     #define MATERIALX_SYMBOL_EXPORT

diff --git a/source/MaterialXFormat/CMakeLists.txt b/source/MaterialXFormat/CMakeLists.txt
@@ -1,5 +1,7 @@
-file(GLOB_RECURSE materialx_source "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp")
-file(GLOB_RECURSE materialx_headers "${CMAKE_CURRENT_SOURCE_DIR}/*.h*")
+add_subdirectory(External/PugiXML)
+
+file(GLOB materialx_source "${CMAKE_CURRENT_SOURCE_DIR}/*.cpp")
+file(GLOB materialx_headers "${CMAKE_CURRENT_SOURCE_DIR}/*.h*")
 
 mx_add_library(MaterialXFormat
     SOURCE_FILES
@@ -8,5 +10,10 @@ mx_add_library(MaterialXFormat
         ${materialx_headers}
     LIBRARIES
         MaterialXCore
+    PRIVATE_LIBRARIES
+        "$<BUILD_INTERFACE:pugixml::pugixml>"
     EXPORT_DEFINE
         MATERIALX_FORMAT_EXPORTS)
+
+# we need to use BUILD_INTERFACE here to hide the use of pugixml from the cmake export when we're building
+# MaterialX as a static library - feels like maybe a cmake bug?
diff --git a/source/MaterialXFormat/External/PugiXML/CMakeLists.txt b/source/MaterialXFormat/External/PugiXML/CMakeLists.txt
@@ -0,0 +1,13 @@
+add_library(pugixml STATIC)
+add_library(pugixml::pugixml ALIAS pugixml)
+
+target_sources(pugixml
+    PRIVATE
+        pugixml.cpp
+    PUBLIC
+        pugixml.hpp
+        pugiconfig.hpp
+)
+
+set_target_properties(pugixml PROPERTIES CXX_VISIBILITY_PRESET hidden)
+set_target_properties(pugixml PROPERTIES CMAKE_VISIBILITY_INLINES_HIDDEN 1)
diff --git a/source/MaterialXFormat/External/PugiXML/readme.txt b/source/MaterialXFormat/External/PugiXML/readme.txt
@@ -50,3 +50,11 @@ HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
 WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
 FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
 OTHER DEALINGS IN THE SOFTWARE.
+
+----------------------------------------------------------------
+
+These files are taken from https://github.com/zeux/pugixml/tree/v1.9
+
+The following MaterialX-specific modifications have been applied the original source:
+   * Adding tracking for new-line characters
+   * Adding support for serialization of `<` and `>` instead of `&lt` and `&gt`
diff --git a/source/MaterialXGenShader/Util.h b/source/MaterialXGenShader/Util.h
@@ -90,8 +90,8 @@ MX_GENSHADER_API bool hasElementAttributes(OutputPtr output, const StringVec& at
 // These are deprecated wrappers for older versions of the function interfaces in this module.
 // Clients using these interfaces should update them to the latest API.
 //
-MX_GENSHADER_API [[deprecated]] void findRenderableMaterialNodes(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool, std::unordered_set<ElementPtr>&);
-MX_GENSHADER_API [[deprecated]] void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool includeReferencedGraphs = false);
+[[deprecated]] MX_GENSHADER_API void findRenderableMaterialNodes(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool, std::unordered_set<ElementPtr>&);
+[[deprecated]] MX_GENSHADER_API void findRenderableElements(ConstDocumentPtr doc, vector<TypedElementPtr>& elements, bool includeReferencedGraphs = false);
 
 MATERIALX_NAMESPACE_END
 

diff --git a/source/MaterialXRenderGlsl/CMakeLists.txt b/source/MaterialXRenderGlsl/CMakeLists.txt
@@ -53,25 +53,25 @@ endif()
 
 if(WIN32)
     if(MSVC)
-        target_link_libraries(
-                ${TARGET_NAME}
+        target_link_libraries(${TARGET_NAME}
+            PUBLIC
                 Opengl32)
     elseif(MINGW)
-        target_link_libraries(
-                ${TARGET_NAME}
+        target_link_libraries(${TARGET_NAME}
+                PUBLIC
                 Opengl32
                 gdi32)
     endif()
 elseif(APPLE)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             "-framework OpenGL"
             "-framework Foundation"
             "-framework Cocoa"
             "-framework Metal")
 elseif(UNIX)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             OpenGL::GL
             X11::X11
             X11::Xt)

diff --git a/source/MaterialXRenderHw/CMakeLists.txt b/source/MaterialXRenderHw/CMakeLists.txt
@@ -27,19 +27,19 @@ mx_add_library(MaterialXRenderHw
     ADD_OBJECTIVE_C_CODE)
 
 if(APPLE)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             "-framework Foundation"
             "-framework Metal")
     if (NOT MATERIALX_BUILD_IOS)
-        target_link_libraries(
-                ${TARGET_NAME}
+        target_link_libraries(${TARGET_NAME}
+                PUBLIC
                 "-framework Cocoa"
         )
     endif()
 elseif(UNIX)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             X11::X11
             X11::Xt)
 endif()
diff --git a/source/MaterialXRenderMsl/CMakeLists.txt b/source/MaterialXRenderMsl/CMakeLists.txt
@@ -46,23 +46,23 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang")
 endif()
 
 if(MSVC)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             Opengl32)
 elseif(APPLE)
     if(NOT MATERIALX_BUILD_IOS)
-        target_link_libraries(
-                ${TARGET_NAME}
+        target_link_libraries(${TARGET_NAME}
+                PUBLIC
                 "-framework Cocoa"
                 "-framework OpenGL")
     endif()
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             "-framework Foundation"
             "-framework Metal")
 elseif(UNIX)
-    target_link_libraries(
-            ${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME}
+            PUBLIC
             OpenGL::GL
             X11::X11
             X11::Xt)

diff --git a/source/MaterialXRenderMsl/MetalState.h b/source/MaterialXRenderMsl/MetalState.h
@@ -14,14 +14,14 @@
 #import <Metal/Metal.h>
 
 #include <MaterialXCore/Generated.h>
+#include <MaterialXRenderMsl/Export.h>
 
 MATERIALX_NAMESPACE_BEGIN
 class MetalFramebuffer;
 // Shared pointer to a MetalFramebuffer
 using MetalFramebufferPtr = std::shared_ptr<class MetalFramebuffer>;
-MATERIALX_NAMESPACE_END
 
-struct MetalState
+struct MX_RENDERMSL_API MetalState
 {
     static MetalState* getSingleton()
     {
@@ -67,11 +67,13 @@ struct MetalState
     std::atomic<int> inFlightCommandBuffers;
 };
 
-#define MTL(a) (MetalState::getSingleton()->a)
-#define MTL_DEPTHSTENCIL_STATE(a) (MetalState::getSingleton()->a##DepthStencilState)
-#define MTL_TRIGGER_CAPTURE MetalState::getSingleton()->triggerProgrammaticCapture()
-#define MTL_STOP_CAPTURE MetalState::getSingleton()->stopProgrammaticCapture()
-#define MTL_PUSH_FRAMEBUFFER(a) MetalState::getSingleton()->framebufferStack.push(a)
-#define MTL_POP_FRAMEBUFFER(a) MetalState::getSingleton()->framebufferStack.pop()
+MATERIALX_NAMESPACE_END
+
+#define MTL(a) (MaterialX::MetalState::getSingleton()->a)
+#define MTL_DEPTHSTENCIL_STATE(a) (MaterialX::MetalState::getSingleton()->a##DepthStencilState)
+#define MTL_TRIGGER_CAPTURE MaterialX::MetalState::getSingleton()->triggerProgrammaticCapture()
+#define MTL_STOP_CAPTURE MaterialX::MetalState::getSingleton()->stopProgrammaticCapture()
+#define MTL_PUSH_FRAMEBUFFER(a) MaterialX::MetalState::getSingleton()->framebufferStack.push(a)
+#define MTL_POP_FRAMEBUFFER(a) MaterialX::MetalState::getSingleton()->framebufferStack.pop()
 
 #endif // MATERIALXVIEW_METALSTATE_H
diff --git a/source/MaterialXRenderMsl/MetalState.mm b/source/MaterialXRenderMsl/MetalState.mm
@@ -8,6 +8,8 @@
 
 #include <MaterialXRenderMsl/MetalFramebuffer.h>
 
+MATERIALX_NAMESPACE_BEGIN
+
 std::unique_ptr<MetalState> MetalState::singleton = nullptr;
 
 MetalState::MetalState()
@@ -239,3 +241,5 @@
 {
     return framebufferStack.top();
 }
+
+MATERIALX_NAMESPACE_END