[X11] Add support for using EGL/GLES instead of GLX. #82101
Conversation
|
Tested locally on Mageia 9, it seems to work, though I'm getting a weird crash. Works fine:
Crashes:
Steps to reproduce:
|
|
I think we need to make some changes to |
I do not see any options for GLES except for Android, so I guess it's not supported by OpenXR (but devices like Raspberry Pi are likely not XR capable anyway). |
There's an OpenXR extension for it from Monado ( See: https://registry.khronos.org/OpenXR/specs/1.0/html/xrspec.html#XR_MNDX_egl_enable |
Probably the same is true for ANGLE on Windows, and we should use underlying D3D11 device instead as well. |
|
I'd like to get this merged for 4.2 if possible, but I'm puzzled by the crash I ran into. Can anyone reproduce it? |
|
I just tested this PR on Ubuntu 22.04, opening the editor for an existing Godot project using Full stack trace |
I'd seriously like to see what that (GDB) frame looks like. The failing line of code is related to... Size calculation of the autoplay icon in the animation player editor. The failing method in question is: Size2i Image::get_size() const {
return Size2i(width, height);
}What on earth could be failing here?
inline Vector2i(const int32_t p_x, const int32_t p_y) {
x = p_x;
y = p_y;
}The rest of that class is some interesting union {
struct {
union {
int32_t x;
int32_t width;
};
union {
int32_t y;
int32_t height;
};
};
int32_t coord[2] = { 0 };
};It looks like I'm really baffled. The only things that come to mind to me is either some extremely obscure memory corruption (stack bit me once, but why would it now?) or some UB with unions. Regarding the memory corruption, a memory sanitizer could help (although I don't recall it directly reporting a stack overflow as such) Regarding the UB I'd like to point out that cppreference's union page says that "It is undefined behavior to read from the member of the union that wasn't most recently written. Many compilers implement, as a non-standard language extension, the ability to read inactive members of a union." Perhaps this one might be biting us out of the blue? Due to the randomness of this crash, honestly both cases look equally probable to the best of my knowledge, unless there's (hopefully) some way simpler explanation. Again, I'm seriously baffled as, from what I can see, this PR is both touching stuff that has nothing to do with that code path and pretty much just plumbing the EGL driver, which works in each single supported desktop platform if we include the Wayland branch. |
It's a segfault, so it probably isn't really about the size calculation. It's probably that the It could be an issue in loading the icon? Perhaps it's failing to load and returning EDIT: I just ran it in GDB, and assuming it's not lying to me (which it sometimes does, unfortunately), it looks like |
Oh god I completely missed that dereference operator :gdyeet:
There, much better than UB or stack overflows! Thanks for checking!
Perhaps, but I don't think that a value like this is expected by the API. That's a
Indeed that method has a lot of places where it can fetch the texture.
Ref<Texture2D> Control::get_theme_icon(const StringName &p_name, const StringName &p_theme_type) const {
ERR_READ_THREAD_GUARD_V(Ref<Texture2D>());
if (!data.initialized) {
WARN_PRINT_ONCE(vformat("Attempting to access theme items too early in %s; prefer NOTIFICATION_POSTINITIALIZE and NOTIFICATION_THEME_CHANGED", this->get_description()));
}
if (p_theme_type == StringName() || p_theme_type == get_class_name() || p_theme_type == data.theme_type_variation) {
const Ref<Texture2D> *tex = data.theme_icon_override.getptr(p_name);
if (tex) {
return *tex;
}
}
if (data.theme_icon_cache.has(p_theme_type) && data.theme_icon_cache[p_theme_type].has(p_name)) {
return data.theme_icon_cache[p_theme_type][p_name];
}
List<StringName> theme_types;
data.theme_owner->get_theme_type_dependencies(this, p_theme_type, &theme_types);
Ref<Texture2D> icon = data.theme_owner->get_theme_item_in_types(Theme::DATA_TYPE_ICON, p_name, theme_types);
data.theme_icon_cache[p_theme_type][p_name] = icon;
return icon;
}Honestly the weirdest of them all is that That said, this requires a thorough debug which I suppose would just be easier to do in loco instead than back and forth on a GitHub thread. |
|
Crash should be fixed, I have missed a define and |
There was a problem hiding this comment.
Tested locally, works great!
I could run https://github.com/team-godog/NGJ2023_Godog successfully with --rendering-driver opengl3_es.
With VSync disabled, I get in average (laptop on battery, not thorough test, high fluctuations):
- OpenGL: 210-320 FPS
- OpenGL ES: 140-190 FPS
I guess it's something rendering team might want to take a look at. I would expect almost the same performance on the same hardware, so something in the GLES pipeline might be less optimized. |
Yeah, my test project isn't the best for proper benchmarking but now that we have GLES on desktop with this PR and with ANGLE for Windows/macOS, we should be able to do some comparisons and optimizations. Much easier to figure out potential performance issues when comparing the two methods on the same hardware :) |
|
Such great work! Really appreciated. |
|
A quick test with an empty 3d scene 👍
Btw. https://hackaday.com/2023/09/28/a-raspberry-pi-5-is-better-than-two-pi-4s/ :) |
|
Thanks! |
Follow up to #72831
Allow using EGL/GLES on X11 (using project setting or
--rendering-driver opengl3_esargument). GLES should have better compatibility with various single board computers (see godotengine/godot-proposals#988).Note:
Currently, it's still using GLX for desktop GL and PRIME detection, but it might be better to switch it to EGL as well (but I'm not sure if EGL is always available).