search

deathkiller / jazz2-native

🎮 · Jazz² Resurrection: Native C++ reimplementation of Jazz Jackrabbit 2
https://deat.tk/jazz2/
GNU General Public License v3.0
552 stars 37 forks source link

Fix compilation against GLVND instead of legacy GLX #71

Closed vanfanel closed 4 months ago

vanfanel commented 4 months ago

Fix for modern GLVND OpenGL implementation detection. Legacy GLX is still used by default.

deathkiller commented 4 months ago

Is everyone using such a wild OpenGL import? I'm afraid to merge it, because I can't verify that it won't break any other unix targets. Do you have any reference to some other project where it's used like this? I would say that CMake's FindOpenGL.cmake is broken and needs to be fixed in the CMake repo instead of adding this workaround.

vanfanel commented 4 months ago

Is everyone using such a wild OpenGL import? I'm afraid to merge it, because I can't verify that it won't break any other unix targets. Do you have any reference to some other project where it's used like this? I would say that CMake's FindOpenGL.cmake is broken and needs to be fixed in the CMake repo instead of adding this workaround.

I called it a workaround, but it's not dangerous at all for other Unix-like targets: if you look at the PR, you will see it does what it did previously (ie: use GLX) and ONLY in the case that GLX is absent, it will try GLVND.

As for what others are doing, well, most people just ignores GLVND for now (doing so will bite their asses when GLX is deprecated...), others do what I did more or less, and others use find_library() which in the end is the same.

vanfanel commented 4 months ago

@deathkiller Another possible solution is what is done in the (incredible) wipeout-rewrite project: there's a USE_GLVND define that can be toggled at CMake configure time, and depending on that, GLVND or GLX is used:

https://github.com/phoboslab/wipeout-rewrite/blob/a372b51f59217da4a5208352123a4acca800783c/CMakeLists.txt#L206

But that doesn't imply autodetection.

deathkiller commented 4 months ago

there's a USE_GLVND define that can be toggled at CMake configure time, and depending on that, GLVND or GLX is used:

But it should already work this way, OpenGL::OpenGL should have priority. But for some reason it's not detected on your PC and we should probably investigate why, instead of adding a workaround. Because I see in GitHub builds that it works fine there - Using newer OpenGL::OpenGL target (GLVND) line is present in the build log and everything works fine.

vanfanel commented 4 months ago

You are right, sir: I have tried on a fresh Debian Testing install, and things work as expected without all this. I will investigate what's wrong with my other system, as in why doesn't cmake detect GLVND correctly there. GLVND/GLX detection was broken time ago and I though it still was, but apparently that's not the case.

Closing this, sorry for the noise.

vanfanel commented 4 months ago

For the record: the problem is that MESON/NINJA install the MESA libs into /usr/local/lib64 by default (I always like to have latest stable MESA installed on my systems, so I build and install it manually), and that's not an standard search path for CMAKE. So doing:

export CMAKE_LIBRARY_PATH=/usr/local/lib64
export PKG_CONFIG_PATH=/usr/local/lib64/pkgconfig

I do it in /etc/profile.d/custom.sh or similar. That way, both PKGCONFIG and CMAKE find the libs and everyone is happy.

Naturally, that doesn't happen on a fresh Debian install because managed lib installs end up in /usr/lib/x86_64-linux-gnu/ which CMAKE knows about by default.

I hope everyone learned a lesson today, boys and girls :)

deathkiller commented 4 months ago

Great! So the issue #69 can be closed.