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

video/wayland: use EGL_EXT_present_opaque when available #4646

Merged
merged 2 commits into from
Aug 25, 2021
Merged

video/wayland: use EGL_EXT_present_opaque when available #4646

merged 2 commits into from
Aug 25, 2021

Conversation

1ace
Copy link
Contributor

@1ace 1ace commented Aug 14, 2021

Description

This should fix the issue with the games requesting alpha configs and not filling the alpha channel all the way to 100%

Existing Issue(s)

@flibitijibibo I couldn't find any GitHub issue for this issue; let me know if there was one and I'll add Closes #123 here :)

checklist:

@flibitijibibo
Copy link
Collaborator

Added a hint of top of this to support transparent windows, in case someone ever needs it:

flibitijibibo@a9d90f2

@1ace
Copy link
Contributor Author

1ace commented Aug 14, 2021

Good idea; I added your commit to the PR 👍

@flibitijibibo flibitijibibo self-assigned this Aug 15, 2021
@flibitijibibo flibitijibibo added this to the 2.0.18 milestone Aug 15, 2021
@flibitijibibo flibitijibibo added the enhancement New feature or request label Aug 16, 2021
#ifndef EGL_EXT_present_opaque
#define EGL_EXT_present_opaque 1
#define EGL_PRESENT_OPAQUE_EXT 0x31DF
#endif /* EGL_EXT_protected_content */
Copy link
Collaborator

Choose a reason for hiding this comment

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

EGL_EXT_protected_content

Only just realized this is copied from the extension below it... the Mesa patch has this too!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, guilty as charged 😅

Thanks for noticing that, I'll fix it 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Btw, this src/video/khronos/EGL/eglext.h change was only meant to reflect the real change that will be to pull this file from upstream once the extension lands (and when I do that, other things in this file will change, starting with the date in the header, and possibly other changes that hadn't been carried over yet).

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'll have to check and see how we do header updates here... sometimes we use the header and sometimes we define it in SDL_egl.c, I'm actually not sure what the rule is there. Ryan or Sam might know though.

@Cacodemon345
Copy link
Contributor

The hint documentation seems to be misleading for non-EGL platforms.

@flibitijibibo
Copy link
Collaborator

It's probably worth re-wording to ALLOW_TRANSPARENCY to fall more in line with the extension spec's wording - once the extension is finalized I'll redo the commit.

@flibitijibibo
Copy link
Collaborator

The EGL extension has been merged into the registry - with that in mind I've redone the hint patch to be worded more in line with what the extension suggests:

flibitijibibo@0c8947c

Which leaves one last question... @slouken, @icculus, how should we integrate the new attrib define? Is it enough to update the Khronos headers in our repo, or do we need to add it to SDL_egl.c as well to account for older EGL headers outside of the ones we provide?

@flibitijibibo
Copy link
Collaborator

Checked this on flibitBuild and it appears we will indeed need to define this in SDL_egl.c as well. @1ace, I believe this is all that's needed at this point:

  1. First commit can be an update of the official header
  2. Second commit needs the definition copied in SDL_egl.c
  3. Third commit can be replaced with flibitijibibo@0c8947c

Once that's all in place I'll get this in!

@1ace
Copy link
Contributor Author

1ace commented Aug 25, 2021

@flibitijibibo Thanks!

I just had a go at updating the local SDL copy of the EGL headers, and they're massively out of date, to the point where I'm not confident the update won't break something. I think I'll make a separate PR to update them, and only do your points 2 & 3 in this PR :)

@1ace
Copy link
Contributor Author

1ace commented Aug 25, 2021

Alright, PR updated 👍

@flibitijibibo
Copy link
Collaborator

Looks great, thanks for putting this whole extension together btw!

@flibitijibibo flibitijibibo merged commit 524964f into libsdl-org:main Aug 25, 2021
@1ace 1ace deleted the present-opaque branch August 25, 2021 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants