GLES3: Split EGL includes in platform_egl.h#115632
Draft
akien-mga wants to merge 1 commit intogodotengine:masterfrom
Draft
GLES3: Split EGL includes in platform_egl.h#115632akien-mga wants to merge 1 commit intogodotengine:masterfrom
platform_egl.h#115632akien-mga wants to merge 1 commit intogodotengine:masterfrom
Conversation
akien-mga
commented
Jan 30, 2026
Comment on lines
+40
to
+43
| #ifdef MACOS_ENABLED | ||
| // FIXME: macOS relies on GL being included at this stage for some reason. | ||
| #include "platform_gl.h" | ||
| #endif |
Member
Author
There was a problem hiding this comment.
Without including GL here, macOS build breaks: https://github.com/akien-mga/godot/actions/runs/21516923985/job/61997594479
clang++ -o platform/macos/display_server_macos.macos.editor.x86_64.o -c -std=gnu++17 -fno-exceptions -Wctor-dtor-privacy -Wnon-virtual-dtor -arch x86_64 -mmacosx-version-min=10.13 -ffp-contract=off -fobjc-arc -fvisibility=hidden -isysroot /Applications/Xcode_26.0.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.0.sdk -msse4.2 -mpopcnt -fcolor-diagnostics -O2 -Wall -Wextra -Wwrite-strings -Wno-unused-parameter -Wshadow-field-in-constructor -Wshadow-uncaptured-local -Wno-ordered-compare-function-pointers -Wenum-conversion -Wimplicit-fallthrough -Werror -DTOOLS_ENABLED -DDEBUG_ENABLED -DNDEBUG -DENGINE_UPDATE_CHECK_ENABLED -DNO_EDITOR_SPLASH -DSTRICT_CHECKS -DACCESSKIT_ENABLED -DSDL_ENABLED -DMACOS_ENABLED -DUNIX_ENABLED -DCOREAUDIO_ENABLED -DCOREMIDI_ENABLED -DGLES3_ENABLED -DVULKAN_ENABLED -DRD_ENABLED -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES -DMINIZIP_ENABLED -DBROTLI_ENABLED -DOVERRIDE_ENABLED -DOVERRIDE_PATH_ENABLED -DTHREADS_ENABLED -DCLIPPER2_ENABLED -DZSTD_STATIC_LINKING_ONLY -DVK_USE_PLATFORM_MACOS_MVK -DVK_USE_PLATFORM_METAL_EXT -DGLAD_ENABLED -DEGL_ENABLED -DTESTS_ENABLED -Ithirdparty/freetype/include -Ithirdparty/libpng -Ithirdparty/glad -Ithirdparty/vulkan -Ithirdparty/vulkan/include -Ithirdparty/zstd -Ithirdparty/zlib -Ithirdparty/clipper2/include -Ithirdparty/brotli/include -Ithirdparty/angle/include -Iplatform/macos -Iaccesskit-c-0.18.0/include -I. platform/macos/display_server_macos.mm
In file included from platform/macos/display_server_macos.mm:71:
In file included from ./drivers/gles3/rasterizer_gles3.h:35:
In file included from ./drivers/gles3/effects/copy_effects.h:35:
In file included from ./drivers/gles3/shaders/effects/copy.glsl.gen.h:35:
In file included from ./drivers/gles3/shader_gles3.h:43:
In file included from platform/macos/platform_gl.h:41:
Error: ./thirdparty/glad/glad/gl.h:38:4: error: OpenGL (gl.h) header already included (API: gl), remove previous include!
38 | #error OpenGL (gl.h) header already included (API: gl), remove previous include!
| ^
Error: ./thirdparty/glad/glad/gl.h:40:9: error: '__gl_h_' macro redefined [-Werror,-Wmacro-redefined]
40 | #define __gl_h_ 1
| ^
/Applications/Xcode_26.0.1.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX26.0.sdk/System/Library/Frameworks/OpenGL.framework/Headers/gl.h:2:9: note: previous definition is here
2 | #define __gl_h_
| ^
I don't know why, but it doesn't happen on Windows and Linux, so I suspect it's pointing at some bad include order for GL stuff in platform/macos.
akien-mga
commented
Jan 30, 2026
| #include "core/templates/local_vector.h" | ||
| #include "servers/display/display_server.h" | ||
|
|
||
| // EGL includes platform-specific headers, so should come last. |
Member
Author
There was a problem hiding this comment.
Doesn't seem to make a difference if first or last here, but I think the correct practice is to put it last?
akien-mga
commented
Jan 30, 2026
Comment on lines
+81
to
+90
| #if defined(EGL_ENABLED) || defined(ANDROID_ENABLED) | ||
| // We include EGL below to get a debug callback, but EGL is not | ||
| // available on iOS or the web. | ||
| #define EGL_DEBUG_CALLBACK | ||
|
|
||
| #ifdef _WIN32 | ||
| #define strcpy strcpy_s | ||
| #endif | ||
|
|
||
| #include "platform_egl.h" |
Member
Author
There was a problem hiding this comment.
I refactored this a bit as the previous logic was to conditionally use EGL for debug callbacks on desktop platforms (EGL_ENABLED) and Android (doesn't define it, but it's an EGL platform).
And the strcpy isn't compiler but stdlib specific on Windows AFAIK, and only needed when using the EGL debug callback.
Member
Author
|
So Windows + ANGLE fails linking (both with MSVC and MinGW), and I'm not really sure why: Even this minimal change fails: diff --git a/drivers/egl/egl_manager.h b/drivers/egl/egl_manager.h
index 7fe5f916cc..164b7ad738 100644
--- a/drivers/egl/egl_manager.h
+++ b/drivers/egl/egl_manager.h
@@ -33,6 +33,7 @@
#ifdef EGL_ENABLED
// These must come first to avoid windows.h mess.
+#include "platform_egl.h"
#include "platform_gl.h"
#include "core/templates/local_vector.h"
diff --git a/drivers/gles3/rasterizer_gles3.cpp b/drivers/gles3/rasterizer_gles3.cpp
index 1df41493d5..971ae64bfc 100644
--- a/drivers/gles3/rasterizer_gles3.cpp
+++ b/drivers/gles3/rasterizer_gles3.cpp
@@ -82,6 +82,7 @@
#define CAN_DEBUG
#endif
+#include "platform_egl.h"
#include "platform_gl.h"
#if defined(MINGW_ENABLED) || defined(_MSC_VER)
diff --git a/platform/windows/platform_egl.h b/platform/windows/platform_egl.h
new file mode 100644
index 0000000000..48584878a2
--- /dev/null
+++ b/platform/windows/platform_egl.h
@@ -0,0 +1,10 @@
+#pragma once
+
+#ifdef EGL_STATIC
+#define KHRONOS_STATIC 1
+#include "thirdparty/angle/include/EGL/egl.h"
+#include "thirdparty/angle/include/EGL/eglext.h"
+#undef KHRONOS_STATIC
+#else
+#include "thirdparty/glad/glad/egl.h"
+#endif
diff --git a/platform/windows/platform_gl.h b/platform/windows/platform_gl.h
index 3d4a05c0db..3dd1257cea 100644
--- a/platform/windows/platform_gl.h
+++ b/platform/windows/platform_gl.h
@@ -38,13 +38,4 @@
#define GLES_API_ENABLED // Allow using GLES (ANGLE).
#endif
-#ifdef EGL_STATIC
-#define KHRONOS_STATIC 1
-#include "thirdparty/angle/include/EGL/egl.h"
-#include "thirdparty/angle/include/EGL/eglext.h"
-#undef KHRONOS_STATIC
-#else
-#include "thirdparty/glad/glad/egl.h"
-#endif
-
#include "thirdparty/glad/glad/gl.h"So I guess with ANGLE we have to include ANGLE EGL whenever we include GL too? |
Member
ANGLE should not include any headers, but we are using modified GLAD, to be capable of loading both GL and EGL, and all of these changes are in |
akien-mga
force-pushed
the
gles3-split-platform-egl
branch
from
30a8b3e to
c9680d4
Compare
EGL brings in platform-specific headers such as the dreaded `windows.h`, and `platform_gl.h` is used throughout `drivers/gles3` for basic OpenGL types such as `GLuint`. We don't want `windows.h` pollution there. Note for Android: EGL seems used explicitly only via `rasterizer_gles3.cpp` to enable GL debug printing, and some custom stuff in `config.cpp`.
akien-mga
force-pushed
the
gles3-split-platform-egl
branch
from
c9680d4 to
8c11f4d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
EGL brings in platform-specific headers such as the dreaded
windows.h, andplatform_gl.his used throughoutdrivers/gles3for basic OpenGL types such asGLuint. We don't wantwindows.hpollution there.Note for Android: EGL seems used explicitly only via
rasterizer_gles3.cppto enable GL debug printing, and some custom stuff inconfig.cpp.Split from #115623, it helps solves an include order issue there.
Seems to work ok on Linux from a quick test, but this needs testing on Windows and macOS, with and without ANGLE.