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 extra EGL extensions #1668

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

chrisduerr
Copy link
Contributor

This patch adds multiple new EGL extensions which are useful for
implementing Wayland applications. These have been mostly implemented
manually, since gl_generator lacks support for these newer EGL
extensions (except for EGL_KHR_image_base).

The following extensions were added:

  • EGL_KHR_image_base
  • EGL_WL_bind_wayland_display
  • EGL_WL_create_wayland_buffer_from_image

Currently this PR doesn't really implement a good way to access this new functionality, instead it just exposes the EGL static publicly. Ideally there would be some abstraction that retrieves the Egl object, while automatically making sure it is initialized.

So I see two options, feedback appreciated:

  • Expose method that basically just does EGL.as_ref()? (does not ensure initialization)
  • Add method to Display (requires display creation)

This patch adds multiple new EGL extensions which are useful for
implementing Wayland applications. These have been mostly implemented
manually, since gl_generator lacks support for these newer EGL
extensions (except for `EGL_KHR_image_base`).

The following extensions were added:
 - EGL_KHR_image_base
 - EGL_WL_bind_wayland_display
 - EGL_WL_create_wayland_buffer_from_image
@MarijnS95
Copy link
Member

Fwiw for #1658 I'd also like to use updated EGL bindings from gl_generator, which is very trivial to achieve: brendanzab/gl-rs#546

Unfortunately it seems that efforts to transition ownership of gl-rs (which is still an important cornerstone in the Rust ecosystem) have stalled: brendanzab/gl-rs#524

@kchibisov
Copy link
Member

I'm not sure how we should expose the ref to EGL and similar types, since we should have it for all the platforms where we use loader (so, except macOS).

Maybe we should have a method to manually load(which will return Option) and just a method to get a &'static Egl from the Display(this will always work)? And the manual loading will be present only for EGL, because the rest is a bit special and shouldn't be used anyway in the context of device extensions.

I'm also interested in solving the generator issue upstream, but I'll see what we can do

@chrisduerr
Copy link
Contributor Author

I added some functions to access the APIs for EGL and GLX. This works for my usecase at least, so maybe that's enough to just merge it?

Let me know if you want a cleaner/more elaborate solution.

Copy link
Member

@kchibisov kchibisov left a comment

Choose a reason for hiding this comment

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

Not sure what to do with wgl...

Comment on lines +73 to +76
egl::BindWaylandDisplayWL::load_with(loader);
egl::UnbindWaylandDisplayWL::load_with(loader);
egl::QueryWaylandBufferWL::load_with(loader);
egl::CreateWaylandBufferFromImageWL::load_with(loader);
Copy link
Member

Choose a reason for hiding this comment

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

Could we maybe make it somehow done externally? So the user will have to load them if they want to, otherwise you'd have to add all the extensions here, which might be not great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well this does load all the extensions which were added in this PR?

It seems odd to generate extensions, but then not load them?

Copy link
Member

Choose a reason for hiding this comment

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

No, I mean, that you can add extension to egl_sys but then don't update glutin to load it, so only bump to egl_sys will be required...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah but then the functions will still be exposed, but when you try to call them it won't work? That just seems inconvenient to me from a user perspective.

IF you use the sys crate directly, it's more obvious that you need to load stuff.

glutin/src/api/egl/mod.rs Outdated Show resolved Hide resolved
glutin/src/display.rs Outdated Show resolved Hide resolved
glutin_egl_sys/src/egl.rs Show resolved Hide resolved
@chrisduerr chrisduerr requested a review from kchibisov May 4, 2024 17:21
@chrisduerr chrisduerr mentioned this pull request May 23, 2024
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants