-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Add an EGL driver and port X11 OpenGL handling to it #68188
Conversation
This should be all, but I'll only have the confirmation when it'll get merged by looking at the final diff.
Thanks for splitting out this PR!
AFAIK, Android is already using EGL.
I'll paraphrase what I wrote over on #57025: I'm working on a OpenXR with OpenGL PR (#67775), and the only way in the core standard to initialize OpenXR on X11 is using the However, if we can't rely on that extension, then for OpenXR we'll need to stick with GLX for X11. There's perhaps a way to use GLX, but load libGL.so dynamically to prevent the dependency for folks who only have Wayland (without xwayland)? @BastiaanOlij What are your thoughts? @clayjohn Are there any other considerations with switching from GLX to EGL on X11? |
Oh, I had no idea. Perhaps we could let it use this driver (in another PR) if all goes well. |
0e74dc5
to
729c9fb
Compare
I just (blindly) ported the GPU detection code, but I have no way of testing it. Please let me know if it works. |
@dsnopek I guess for OpenXR we can simply detect that extension and fail initialization if that extension is not available and EGL is used. Hopeful OpenXR runtimes on Linux will implement the extension. |
@BastiaanOlij so, your inclination is to go for the switch to EGL on X11, and not worry that we'd be incompatible with SteamVR? As I said on RocketChat, my inclination would be to stick with GLX for now (but dynamically load libGL.so somehow) to retain compatibility. But I'm fine to go with whatever the consensus is! |
0288ba5
to
99d52cd
Compare
@dsnopek I agree with bastiaan mainly due to the incentivization argument, followed by the other things I said. BTW if this gets merged I'd be glad to help you transition to the EGLManager on your OpenGL VR code :) |
Marking this PR as ready for review as I tested it a bit through a modified version of Calinou's 4.0 version of the sponza demo (https://github.com/Calinou/godot-sponza/tree/4.0-dev) and it seems to work fine. Now, if the consensus is ok with the possible OpenXR runtime issues abundantly discussed previously, I think that this can be merged. (For the reference I'd be ok, but I'm not an XR user, so I'm not sure how much my opinion is worth in this regard) Also, as I said before, if this PR gets merged I'll have no issues trying to give my hand (sadly not in VR) at helping anyone who might be affected by this change, such as @dsnopek :) |
This uncouples Godot from GLX, effectively allowing other `DisplayServer`s like Wayland to be implemented.
So apparently, mostly due to issues with nvidia and OpenXR compatibility, it may still be wanted to fallback to GLX. This means that I'll have to revisit how these interfaces are loaded. Expect another PR which loads GLX dynamically. Edit: this has been done. Once I figure out a clean way of letting the |
Thinking about it, like @clayjohn noted, adding EGL support for X11 while still fallbacking to GLX wouldn't have that many pros compared to just GLX, so this PR will switch to just implementing an EGL driver that the Wayland PR will use. Wayland will depend on this one, this won't be supposed to be merged as-is. |
You know what, I'm closing this one and creating a fresh PR, it wouldn't make much sense to overwrite everything, making the discussion above disconnected from this other approach. |
Superceded by #68944. |
This uncouples Godot from GLX, effectively allowing other
DisplayServer
s like Wayland to use OpenGL with the new display server agnostic glvnd stack.This is done by replacing the
libGL
dependency with libglvnd's ownlibEGL
(which shouldn't be a problem since it has been around for quite a while now), adding a newEGL_ENABLED
define (defined indrivers/egl/SCsub
) and a newEGLManager
driver, which wraps the EGL API in a platform-agnostic way.This new driver is based almost completely on the original
GLManager_X11
(the creators of which I thank a lot), cleaning it up and making it follow more closely the coding conventions I learned while working on the rest of the engine. Most notably it removes all window size handling logic (as it's not needed and already did nothing) and, for the sake of being platform-agnostic, moves all X11VisualInfo
handling to itsDisplayServer
. Other platforms using it will have to implement their own specific quirks on their own, like Wayland having to manipulate its ownwayland_egl
handles. Note that this driver currently asks only for OpenGL, but it should be easy to adapt to also ask GLES, depending on the platform.To use it, all platforms that wish to do so must implement a platform-specific child class which extends its two
_get_platform_extension_*
methods and make all calls through it.All OpenGL methods get loaded by GLAD through a code path selected by the
EGL_ENABLED
define inRasterizerGLES3
.Discrete GPU detection has been blindly ported (I can't test it) and moved to the EGL driver directory (although it uses OpenGL stuff to get the renderer name).
Note that, although AFAIK EGL is supported (or may even be already used) on Android, I haven't ported anything other than X11 stuff to this driver.
I got also told that currently VR handling depends on GLX, but as I got no VR equipment I'm unable to work on this this myself. I'd be glad to assist any VR developer that wishes to help me with this problem.