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

Add support for EGL 1.4 #83930

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

underdoeg
Copy link
Contributor

Debian 10 and (I think) 11 only have libEGL Version 1.4. Those OS are still widely used in integrated hardware like Rock PI and similar ARM devices.
This patch checks for the EGL version and calls older methods if necessary.

@AThousandShips AThousandShips changed the title add support for EGL 1.4 Add support for EGL 1.4 Oct 25, 2023
@AThousandShips AThousandShips requested a review from a team October 25, 2023 08:37
Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1.5 was assumed as default when EGL was only used with ANGLE on macOS/Windows, for X11 in should be fine to 1.4 (not sure about Wayland).

drivers/egl/egl_manager.cpp Outdated Show resolved Hide resolved
drivers/egl/egl_manager.cpp Outdated Show resolved Hide resolved
@underdoeg
Copy link
Contributor Author

Trying to fix the formatting.

@bruvzg This change should theoretically not affect newer systems with EGL 1.5 in place

@AThousandShips
Copy link
Member

I'd recommend using clang-format, see here

@AThousandShips
Copy link
Member

Please amend your commits instead of making new ones, with git commit --amend

@underdoeg
Copy link
Contributor Author

too late to ammend but I haves squashed them now

Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit surprised that EGL 1.5 is still not widely supported (from what I can tell, it's a 9+ years old spec), that said, improving compatibility is very good! :)

Still, the way the version is checked is a bit redundant in some places and outright wrong in the others, so some changes are due.

drivers/egl/egl_manager.cpp Outdated Show resolved Hide resolved
drivers/egl/egl_manager.cpp Outdated Show resolved Hide resolved
drivers/egl/egl_manager.h Outdated Show resolved Hide resolved
@underdoeg
Copy link
Contributor Author

I'm a bit surprised that EGL 1.5 is still not widely supported (from what I can tell, it's a 9+ years old spec), that said, improving compatibility is very good! :)

That is sadly very often the case with embedded arm devices. lots of manufacturers have their own OS or push Android and Debian is just an afterthought.

drivers/egl/egl_manager.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@Riteo Riteo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, great job! Thanks for your patience!

@Riteo
Copy link
Contributor

Riteo commented Oct 25, 2023

For posterity: since we're just checking two versions a normal if/else is fine, but if we want to check multiple versions in the future we will need to switch to an if/else if and check for the individual versions.

Sound trivial but I can imagine forgetting about it in the distant future.

@akien-mga akien-mga modified the milestones: 4.x, 4.2 Oct 25, 2023
@akien-mga akien-mga merged commit 1b3e00d into godotengine:master Oct 26, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks!

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.

5 participants