Skip to content
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

X11: Don't override glxSwapInterval function pointers loaded by GLAD #68727

Merged
merged 1 commit into from
Nov 16, 2022

Conversation

akien-mga
Copy link
Member

Co-authored-by: @alcomposer

Tested on Intel HD 630 and AMD Radeon Vega M with Mesa 22.2.3.
And @alcomposer tested the same patch with Nvidia driver 520.56.06.


BTW there's more stuff in this file that could likely be cleaned up now that we use GLAD2's glx wrapper.
E.g. these overrides seem to duplicate what's already in glad/glx.h:

#define GLX_CONTEXT_MAJOR_VERSION_ARB 0x2091
#define GLX_CONTEXT_MINOR_VERSION_ARB 0x2092

Fixes godotengine#68722.

Co-authored-by: alcomposer <alex.w.mitchell@gmail.com>
@akien-mga akien-mga added this to the 4.0 milestone Nov 16, 2022
@akien-mga akien-mga requested a review from a team as a code owner November 16, 2022 09:08
@akien-mga akien-mga merged commit 89a33d2 into godotengine:master Nov 16, 2022
@akien-mga akien-mga deleted the GLES3-glx-swap-interval-fix branch November 16, 2022 09:21
@Riteo
Copy link
Contributor

Riteo commented Nov 16, 2022

My bad, this is my fault. This wasn't "an issue that has been around since the early GLES3 port" like @clayjohn said or a driver issue, this was the result of my GLX dynamic loading PR. I'm sorry.

I cleaned this code myself to use GLAD but I forgot to remove the local pointers, and not using X11 so much I didn't notice the crash.

I do agree that the whole GLManager_X11 class could be cleaned quite a bit and perhaps could be refactored to follow naming conventions more (starting from the class name). The EGLManager I wrote is pretty much a refactored/cleaned up version of this class ported to EGL.

@clayjohn
Copy link
Member

This is indeed the proper fix. Thanks to @alcomposer for pointing that out!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Crash GLManager_X11::set_use_vsync(bool)
3 participants