Title: [290138] trunk
Revision
290138
Author
ape...@igalia.com
Date
2022-02-18 10:12:06 -0800 (Fri, 18 Feb 2022)

Log Message

[CMake] Cannot find OpenGL when system provides opengl.pc instead of gl.pc
https://bugs.webkit.org/show_bug.cgi?id=236592

Reviewed by Michael Catanzaro.

.:

* Source/cmake/FindOpenGL.cmake: Rewrite to use imported targets, try the "opengl" and "glx"
pkg-config modules first, otherwise keep the existing logic that tried the "gl" pkg-config
module with fallbacks to find_path/find_library.
* Source/cmake/OptionsGTK.cmake: Check for the presence of the OpenGL::GLX target instead of
te GLX_FOUND variable.

Source/ThirdParty/ANGLE:

* PlatformGTK.cmake: Use the OpenGL::OpenGL imported target instead of the
OPENGL_LIBRARIES variable.
* PlatformWPE.cmake: Remove linking againt OpenGL libraries; the WPE port is not
supposed to link to any of them directly.

Source/WebCore:

* CMakeLists.txt: Use the OpenGL::OpenGL and OpenGL::GLX imported targets instead of variables.
* platform/graphics/GLContext.cpp: Move here includes for the GL headers.
* platform/graphics/GLContext.h: Remove includes for the GL headers, none of their
declarations are used here; instead include only the needed EGL headers to get the
definition of EGLNativeWindowType.

Source/WebKit:

* CMakeLists.txt: Remove usage of the OPENGL_ variables for the WebKit target, which is
uneeded because it links to WebCore, which transitively pulls the needed libraries now
that imported targets are being used.

Modified Paths

Diff

Modified: trunk/ChangeLog (290137 => 290138)


--- trunk/ChangeLog	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/ChangeLog	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1,3 +1,16 @@
+2022-02-18  Adrian Perez de Castro  <ape...@igalia.com>
+
+        [CMake] Cannot find OpenGL when system provides opengl.pc instead of gl.pc
+        https://bugs.webkit.org/show_bug.cgi?id=236592
+
+        Reviewed by Michael Catanzaro.
+
+        * Source/cmake/FindOpenGL.cmake: Rewrite to use imported targets, try the "opengl" and "glx"
+        pkg-config modules first, otherwise keep the existing logic that tried the "gl" pkg-config
+        module with fallbacks to find_path/find_library.
+        * Source/cmake/OptionsGTK.cmake: Check for the presence of the OpenGL::GLX target instead of
+        te GLX_FOUND variable.
+
 2022-02-17  Commit Queue  <commit-qu...@webkit.org>
 
         Unreviewed, reverting r289949.

Modified: trunk/Source/ThirdParty/ANGLE/ChangeLog (290137 => 290138)


--- trunk/Source/ThirdParty/ANGLE/ChangeLog	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/ThirdParty/ANGLE/ChangeLog	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1,3 +1,15 @@
+2022-02-18  Adrian Perez de Castro  <ape...@igalia.com>
+
+        [CMake] Cannot find OpenGL when system provides opengl.pc instead of gl.pc
+        https://bugs.webkit.org/show_bug.cgi?id=236592
+
+        Reviewed by Michael Catanzaro.
+
+        * PlatformGTK.cmake: Use the OpenGL::OpenGL imported target instead of the
+        OPENGL_LIBRARIES variable.
+        * PlatformWPE.cmake: Remove linking againt OpenGL libraries; the WPE port is not
+        supposed to link to any of them directly.
+
 2022-02-18  Zan Dobersek  <zdober...@igalia.com>
 
         [GTK][WPE] Don't use ANGLE's GBM-based display

Modified: trunk/Source/ThirdParty/ANGLE/PlatformGTK.cmake (290137 => 290138)


--- trunk/Source/ThirdParty/ANGLE/PlatformGTK.cmake	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/ThirdParty/ANGLE/PlatformGTK.cmake	2022-02-18 18:12:06 UTC (rev 290138)
@@ -33,7 +33,7 @@
     )
 
     if (USE_OPENGL)
-        list(APPEND ANGLEGLESv2_LIBRARIES ${OPENGL_LIBRARIES})
+        list(APPEND ANGLEGLESv2_LIBRARIES OpenGL::OpenGL)
     else ()
         list(APPEND ANGLEGLESv2_LIBRARIES OpenGL::GLES)
     endif ()

Modified: trunk/Source/ThirdParty/ANGLE/PlatformWPE.cmake (290137 => 290138)


--- trunk/Source/ThirdParty/ANGLE/PlatformWPE.cmake	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/ThirdParty/ANGLE/PlatformWPE.cmake	2022-02-18 18:12:06 UTC (rev 290138)
@@ -32,10 +32,4 @@
         ${CMAKE_DL_LIBS}
         Threads::Threads
     )
-
-    if (USE_OPENGL)
-        list(APPEND ANGLEGLESv2_LIBRARIES ${OPENGL_LIBRARIES})
-    else ()
-        list(APPEND ANGLEGLESv2_LIBRARIES ${OPENGLES_LIBRARIES})
-    endif ()
 endif ()

Modified: trunk/Source/WebCore/CMakeLists.txt (290137 => 290138)


--- trunk/Source/WebCore/CMakeLists.txt	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebCore/CMakeLists.txt	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1704,13 +1704,10 @@
     add_definitions(${LIBEPOXY_DEFINITIONS})
 else ()
     if (USE_OPENGL)
-        list(APPEND WebCore_SYSTEM_INCLUDE_DIRECTORIES
-            ${OPENGL_INCLUDE_DIRS}
-        )
-        list(APPEND WebCore_LIBRARIES
-            ${OPENGL_LIBRARIES}
-        )
-        add_definitions(${OPENGL_DEFINITIONS})
+        list(APPEND WebCore_LIBRARIES OpenGL::OpenGL)
+        if (TARGET OpenGL::GLX)
+            list(APPEND WebCore_LIBRARIES OpenGL::GLX)
+        endif ()
     elseif (USE_OPENGL_ES)
         list(APPEND WebCore_LIBRARIES OpenGL::GLES)
     endif ()

Modified: trunk/Source/WebCore/ChangeLog (290137 => 290138)


--- trunk/Source/WebCore/ChangeLog	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebCore/ChangeLog	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1,3 +1,16 @@
+2022-02-18  Adrian Perez de Castro  <ape...@igalia.com>
+
+        [CMake] Cannot find OpenGL when system provides opengl.pc instead of gl.pc
+        https://bugs.webkit.org/show_bug.cgi?id=236592
+
+        Reviewed by Michael Catanzaro.
+
+        * CMakeLists.txt: Use the OpenGL::OpenGL and OpenGL::GLX imported targets instead of variables.
+        * platform/graphics/GLContext.cpp: Move here includes for the GL headers.
+        * platform/graphics/GLContext.h: Remove includes for the GL headers, none of their
+        declarations are used here; instead include only the needed EGL headers to get the
+        definition of EGLNativeWindowType.
+
 2022-02-18  Peng Liu  <peng.l...@apple.com>
 
         [iOS][Modern Media Controls] Add an alternative overflow icon

Modified: trunk/Source/WebCore/platform/graphics/GLContext.cpp (290137 => 290138)


--- trunk/Source/WebCore/platform/graphics/GLContext.cpp	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebCore/platform/graphics/GLContext.cpp	2022-02-18 18:12:06 UTC (rev 290138)
@@ -25,6 +25,14 @@
 #include <wtf/ThreadSpecific.h>
 #include <wtf/text/StringToIntegerConversion.h>
 
+#if USE(LIBEPOXY)
+#include <epoxy/gl.h>
+#elif USE(OPENGL_ES)
+#include <GLES2/gl2.h>
+#else
+#include "OpenGLShims.h"
+#endif
+
 #if USE(EGL)
 #include "GLContextEGL.h"
 #endif

Modified: trunk/Source/WebCore/platform/graphics/GLContext.h (290137 => 290138)


--- trunk/Source/WebCore/platform/graphics/GLContext.h	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebCore/platform/graphics/GLContext.h	2022-02-18 18:12:06 UTC (rev 290138)
@@ -24,25 +24,21 @@
 #include "PlatformDisplay.h"
 #include <wtf/Noncopyable.h>
 
-#if USE(LIBEPOXY)
-#include <epoxy/gl.h>
-#elif USE(OPENGL_ES)
-#include <GLES2/gl2.h>
-#else
-#include "OpenGLShims.h"
-#endif
-
 #if USE(EGL) && !PLATFORM(GTK)
-#if PLATFORM(WPE)
 // FIXME: For now default to the GBM EGL platform, but this should really be
-// somehow deducible from the build configuration.
+// somehow deducible from the build configuration. This is needed with libepoxy
+// as it could have been configured with X11 support enabled, resulting in
+// transitive inclusions of headers with definitions that clash with WebCore.
 #define __GBM__ 1
-#endif // PLATFORM(WPE)
+#if USE(LIBEPOXY)
+#include <epoxy/egl.h>
+#else // !USE(LIBEPOXY)
 #include <EGL/eglplatform.h>
+#endif // USE(LIBEPOXY)
 typedef EGLNativeWindowType GLNativeWindowType;
 #else // !USE(EGL) || PLATFORM(GTK)
 typedef uint64_t GLNativeWindowType;
-#endif
+#endif // USE(EGL) && !PLATFORM(GTK)
 
 #if USE(CAIRO)
 typedef struct _cairo_device cairo_device_t;

Modified: trunk/Source/WebKit/CMakeLists.txt (290137 => 290138)


--- trunk/Source/WebKit/CMakeLists.txt	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebKit/CMakeLists.txt	2022-02-18 18:12:06 UTC (rev 290138)
@@ -373,18 +373,6 @@
         ANGLE::EGL
     )
 else ()
-    if (USE_OPENGL)
-        list(APPEND WebKit_SYSTEM_INCLUDE_DIRECTORIES
-            ${OPENGL_INCLUDE_DIRS}
-        )
-        list(APPEND WebKit_PRIVATE_LIBRARIES
-            ${OPENGL_LIBRARIES}
-        )
-        add_definitions(${OPENGL_DEFINITIONS})
-    elseif (USE_OPENGL_ES)
-        list(APPEND WebKit_PRIVATE_LIBRARIES OpenGL::GLES)
-    endif ()
-
     if (USE_EGL)
         list(APPEND WebKit_SYSTEM_INCLUDE_DIRECTORIES
             ${EGL_INCLUDE_DIRS}

Modified: trunk/Source/WebKit/ChangeLog (290137 => 290138)


--- trunk/Source/WebKit/ChangeLog	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/WebKit/ChangeLog	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1,3 +1,14 @@
+2022-02-18  Adrian Perez de Castro  <ape...@igalia.com>
+
+        [CMake] Cannot find OpenGL when system provides opengl.pc instead of gl.pc
+        https://bugs.webkit.org/show_bug.cgi?id=236592
+
+        Reviewed by Michael Catanzaro.
+
+        * CMakeLists.txt: Remove usage of the OPENGL_ variables for the WebKit target, which is
+        uneeded because it links to WebCore, which transitively pulls the needed libraries now
+        that imported targets are being used.
+
 2022-02-18  Tyler Wilcock  <tyle...@apple.com>
 
         AX: isAXAuthenticatedCallback must be handled on the main runloop

Modified: trunk/Source/cmake/FindOpenGL.cmake (290137 => 290138)


--- trunk/Source/cmake/FindOpenGL.cmake	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/cmake/FindOpenGL.cmake	2022-02-18 18:12:06 UTC (rev 290138)
@@ -1,13 +1,5 @@
-# - Try to Find OpenGL
-# Once done, this will define
+# Copyright (C) 2015, 2022 Igalia S.L.
 #
-#  OPENGL_FOUND - system has OpenGL installed.
-#  OPENGL_INCLUDE_DIRS - directories which contain the OpenGL headers.
-#  OPENGL_LIBRARIES - libraries required to link against OpenGL.
-#  OPENGL_DEFINITIONS - Compiler switches required for using OpenGL.
-#
-# Copyright (C) 2015 Igalia S.L.
-#
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
 # are met:
@@ -28,38 +20,125 @@
 # WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
 # OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
 # ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+#
+#[=======================================================================[.rst:
+FindOpenGL
+----------
 
+Find OpenGL headers and libraries.
+
+Imported Targets
+^^^^^^^^^^^^^^^^
+
+``OpenGL::OpenGL``
+  The main OpenGL library, if found. It must *not* be assumed that this
+  library provides symbols other than the base OpenGL set.
+``OpenGL::GLX``
+  The library containing the GL extension for the X11 windowing system,
+  if found.
+
+Result Variables
+^^^^^^^^^^^^^^^^
+
+This will define the following variables in your project:
+
+``OpenGL_FOUND``
+  True if the OpenGL library is available.
+``OpenGL_VERSION``
+  The version of the found OpenGL library, if possible to determine it at
+  build configuration time. The variable may be undefined.
+
+#]=======================================================================]
+
+# TODO:
+#  - Make GLX an optional component of the find-module.
+#  - Make EGL an optional component here, remove FindEGL.cmake.
+#  - Consider whether FindGLES2.cmake could be moved here as well.
+
 find_package(PkgConfig QUIET)
+pkg_check_modules(PC_OPENGL IMPORTED_TARGET opengl)
 
-pkg_check_modules(PC_OPENGL gl)
+if (PC_OPENGL_FOUND AND TARGET PkgConfig::PC_OPENGL)
+    #
+    # If the "opengl" module exists, it should be preferred; and the library
+    # name will typically be libOpenGL.so; in this case if GLX support is
+    # available a "glx" module will *also* be available. This is a modern
+    # Unix-ish convention and expected to be always provided by pkg-config
+    # modules, so there is no need to fall-back to manually using find_path()
+    # and find_library().
+    #
+    if (NOT TARGET OpenGL::OpenGL)
+        add_library(OpenGL::OpenGL INTERFACE IMPORTED GLOBAL)
+        set_property(TARGET OpenGL::OpenGL PROPERTY
+            INTERFACE_LINK_LIBRARIES PkgConfig::PC_OPENGL
+        )
+    endif ()
 
-if (PC_OPENGL_FOUND)
-    set(OPENGL_DEFINITIONS ${PC_OPENGL_CFLAGS_OTHER})
+    get_target_property(OpenGL_LIBRARY PkgConfig::PC_OPENGL INTERFACE_LINK_LIBRARIES)
+
+    pkg_check_modules(PC_GLX IMPORTED_TARGET glx)
+    if (TARGET PkgConfig::PC_GLX AND NOT TARGET OpenGL::GLX)
+        add_library(OpenGL::GLX INTERFACE IMPORTED GLOBAL)
+        set_property(TARGET OpenGL::GLX PROPERTY
+            INTERFACE_LINK_LIBRARIES PkgConfig::PC_GLX
+        )
+    endif ()
+else ()
+    # Otherwise, if an "opengl" pkg-config module does not exist, check for
+    # the "gl" one, which may or may not be present.
+    #
+    pkg_check_modules(PC_OPENGL gl)
+    find_path(OpenGL_INCLUDE_DIR
+        NAMES GL/gl.h
+        HINTS ${PC_OPENGL_INCLUDEDIR} ${PC_OPENGL_INCLUDE_DIRS}
+    )
+
+    find_library(OpenGL_LIBRARY
+        NAMES ${OpenGL_NAMES} gl GL
+        HINTS ${PC_OPENGL_LIBDIR} ${PC_OPENGL_LIBRARY_DIRS}
+    )
+
+    if (OpenGL_LIBRARY AND NOT TARGET OpenGL::OpenGL)
+        add_library(OpenGL::OpenGL UNKNOWN IMPORTED GLOBAL)
+        set_target_properties(OpenGL::OpenGL PROPERTIES
+            IMPORTED_LOCATION "${OpenGL_LIBRARY}"
+            INTERFACE_COMPILE_OPTIONS "${PC_OPENGL_CFLAGS_OTHER}"
+            INTERFACE_INCLUDE_DIRECTORIES "${OpenGL_INCLUDE_DIR}"
+        )
+    endif ()
+
+    if (TARGET OpenGL::OpenGL)
+        # We don't use find_package for GLX because it is part of -lGL, unlike EGL. We need to
+        # have OPENGL_INCLUDE_DIRS as part of the directories check_include_files() looks for in
+        # case OpenGL is installed into a non-standard location.
+        include(CMakePushCheckState)
+        CMAKE_PUSH_CHECK_STATE()
+        set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES} ${OpenGL_INCLUDE_DIR})
+        include(CheckIncludeFiles)
+        check_include_files("GL/glx.h" OpenGL_GLX_FOUND)
+        CMAKE_POP_CHECK_STATE()
+
+        if (OpenGL_GLX_FOUND)
+            # XXX: Should this actually check that the OpenGL library contains the GLX symbols?
+            add_library(OpenGL::GLX INTERFACE IMPORTED GLOBAL)
+            set_property(TARGET OpenGL::GLX PROPERTY
+                INTERFACE_LINK_LIBRARIES OpenGL::OpenGL
+            )
+        endif ()
+    endif ()
 endif ()
 
-find_path(OPENGL_INCLUDE_DIRS NAMES GL/gl.h
-    HINTS ${PC_OPENGL_INCLUDEDIR} ${PC_OPENGL_INCLUDE_DIRS}
-)
+set(OpenGL_VERSION "${PC_OPENGL_VERSION}")
 
-set(OPENGL_NAMES ${OPENGL_NAMES} gl GL)
-find_library(OPENGL_LIBRARIES NAMES ${OPENGL_NAMES}
-    HINTS ${PC_OPENGL_LIBDIR} ${PC_OPENGL_LIBRARY_DIRS}
+include(FindPackageHandleStandardArgs)
+find_package_handle_standard_args(OpenGL
+    REQUIRED_VARS OpenGL_LIBRARY
+    FOUND_VAR OpenGL_FOUND
 )
 
-include(FindPackageHandleStandardArgs)
-find_package_handle_standard_args(OpenGL REQUIRED_VARS OPENGL_INCLUDE_DIRS OPENGL_LIBRARIES
-                                  FOUND_VAR OPENGL_FOUND)
+mark_as_advanced(OpenGL_INCLUDE_DIR OpenGL_LIBRARY)
 
-mark_as_advanced(OPENGL_INCLUDE_DIRS OPENGL_LIBRARIES)
-
-if (OPENGL_FOUND)
-    # We don't use find_package for GLX because it is part of -lGL, unlike EGL. We need to
-    # have OPENGL_INCLUDE_DIRS as part of the directories check_include_files() looks for in
-    # case OpenGL is installed into a non-standard location.
-    include(CMakePushCheckState)
-    CMAKE_PUSH_CHECK_STATE()
-    set(CMAKE_REQUIRED_INCLUDES ${CMAKE_REQUIRED_INCLUDES} ${OPENGL_INCLUDE_DIRS})
-    include(CheckIncludeFiles)
-    check_include_files("GL/glx.h" GLX_FOUND)
-    CMAKE_POP_CHECK_STATE()
+if (OpenGL_FOUND)
+    set(OpenGL_LIBRARIES ${OpenGL_LIBRARY})
+    set(OpenGL_INCLUDE_DIRS ${OpenGL_INCLUDE_DIR})
 endif ()

Modified: trunk/Source/cmake/OptionsGTK.cmake (290137 => 290138)


--- trunk/Source/cmake/OptionsGTK.cmake	2022-02-18 18:10:35 UTC (rev 290137)
+++ trunk/Source/cmake/OptionsGTK.cmake	2022-02-18 18:12:06 UTC (rev 290138)
@@ -365,7 +365,7 @@
         SET_AND_EXPOSE_TO_BUILD(USE_OPENGL TRUE)
     endif ()
 
-    if (NOT EGL_FOUND AND (NOT ENABLE_X11_TARGET OR NOT GLX_FOUND))
+    if (NOT EGL_FOUND AND (NOT ENABLE_X11_TARGET OR NOT TARGET OpenGL::GLX))
         message(FATAL_ERROR "Either GLX or EGL is needed.")
     endif ()
 
@@ -373,7 +373,7 @@
 
     SET_AND_EXPOSE_TO_BUILD(USE_EGL ${EGL_FOUND})
 
-    if (ENABLE_X11_TARGET AND GLX_FOUND AND USE_OPENGL)
+    if (ENABLE_X11_TARGET AND USE_OPENGL AND TARGET OpenGL::GLX)
         SET_AND_EXPOSE_TO_BUILD(USE_GLX TRUE)
     endif ()
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to