-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Improve OpenGL detection checks in CMake #14443
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| cmake_push_check_state() | ||
| FindOpenGLHeaders() | ||
| check_c_source_compiles(" | ||
| #include <GL/glx.h> | ||
| int main(int argc, char** argv) { return 0; }" HAVE_OPENGL_GLX) | ||
| cmake_pop_check_state() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good for detection during configuration time, but this won't add the GL paths to the compiler during build time.
Is that ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having a little trouble tracking down where the SDL_CMAKE_DEPENDS variable is set, which presumably has the declaration for OpenGL::GL, but regardless, the path is showing up in the generated makefiles and it does indeed build and link on my system without further changes.
I did notice a bug from the pipeline for some systems when it tried to use OPENGL_INCLUDE_DIR when it was missing, but I've fixed that now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if you configure with -DSDL_OPENGLES=OFF?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It builds and runs fine. The program I'm working on isn't using OpenGL ES though. The relevant log lines are
-- Performing Test HAVE_OPENGL_GLX
-- Performing Test HAVE_OPENGL_GLX - Success
-- Performing Test HAVE_OPENGL
-- Performing Test HAVE_OPENGL - Success
[...]
-- SDL_OPENGL (Wanted: ON): ON
-- SDL_OPENGLES (Wanted: OFF): OFF
No test appears for HAVE_OPENGLES.
The OpenGL headers are not always implicitly available, so this improves the check by calling `find_package` and using the `OPENGL_INCLUDE_DIRS` or `OPENGL_INCLUDE_DIR` var for the `check_c_source_compiles` test. The minimum CMake version currently set is 3.16, `OPENGL_INCLUDE_DIRS` was only added in 3.29, so the code is set to choose `OPENGL_INCLUDE_DIRS` if it exists. If the minimum CMake version is ever set to >= 3.29 this check can be removed and just the `OPENGL_INCLUDE_DIRS` variable can be chosen.
Fixed checks for the definition of `OPENGL_INCLUDE_DIRS` and `OPENGL_INCLUDE_DIR` existing and containing a path. Co-authored-by: Anonymous Maarten <[email protected]>
|
@madebr, is this something we should merge now? |
|
Merged, thanks! |
Adds code to automatically find the OpenGL headers for the
CheckGLX,CheckOpenGL, andCheckOpenGLEStests.Description
The OpenGL headers are not always implicitly available, so this improves the check by calling
find_packageand using theOPENGL_INCLUDE_DIRSorOPENGL_INCLUDE_DIRvar for thecheck_c_source_compilestest.The minimum CMake version currently set is 3.16,
OPENGL_INCLUDE_DIRSwas only added in 3.29, so the code is set to chooseOPENGL_INCLUDE_DIRSif it exists. If the minimum CMake version is ever set to >= 3.29 this check can be removed and just theOPENGL_INCLUDE_DIRSvariable can be used.I use Bazzite, which is built on top of Fedora Silverblue, an immutable Linux distro. As such, despite being Fedora,
dnfdoes not work, there are 3 alternatives: adddnfpackages as layers withrpm-ostree, use adistroboxcontainer for development, or usebrewto install the packages. The first goes against the purpose of the immutable OS, the second works, but the OpenGL driver ends up being llvmpipe, and I'm now using the third which places packages into/var/home/linuxbrew/.linuxbrew. I've set myCMAKE_PREFIX_PATHto include brew, so CMake is capable of finding the OpenGL package, but it is not implicitly in my path.This change allows for a cross platform improvement ensuring that so long as the user has setup their environment for CMake to find the requisite packages, their paths will be correctly included to make sure the OpenGL tests pass.
On my system, SDL builds with OpenGL support without any additional changes.
I've tested this change on Bazzite, and on Bazzite in a
distrobox(it's just a container that would ostensibly appear no different than regular Fedora). This change doesn't affect anything besides Unix that is not Apple, RISC OS or Haiku; that said, I've successfully tested it on macOS.