Fixing EGL Context Sharing, Windowless Buffers, BadDisplay

[alextechcc]

I've got two SoCs with proprietary drivers that seem impossible to run on Vulkan but show promise with opengl. Unfortunately they both cannot handle the current hacks for the context. I want to try to kick off fixing this but my background is absolutely not here. So, I think the best way forward is to try and summarize everything I could find on the issue and suggestions, as it sprawls several projects and issues and comments.

I plan to try and clean this up over time and suggest ways forward.

Big history and background

Suggestion to implement surfman: gfx-rs/gfx#3133

Seems a big reason that surfman was originally added was to get surfaces independent of a context.

Just spoke to @pcwalton about https://github.com/pcwalton/surfman, and I'm wondering if we can use this for implementing the true surfaces on GL context, that don't require GL device. Seems like a direction worth exploring.

... But then part of the implementation relied on luck

@zicklag now I don't understand how and why #3151 worked, given our recent discovery of the lack of sharing in pcwalton/surfman#65 . Do you know?

I'm not sure, but I suspect that the context that we crated for the surface that is bound to the window, ended up being the current context for the whole duration of the rendering pass ( if render pass is the right term ). So instead of rendering to a shared context, and doing a blit onto the window's context before presentation, we were just rendering directly to the window's context the whole time. Which would explain why it stopped working when we tried to make sure the context we wanted to be current was the current one on dereference.

Originally posted by @zicklag in #3133

Decouple surfaces from GL contexts: servo/surfman#202

Surfman never really made this split correctly, but there was some suggestions on how to do it:

in modern EGL, a combination of EGLImageKHR and this extension gives you decoupling as well https://www.khronos.org/registry/EGL/extensions/KHR/EGL_KHR_surfaceless_context.txt

Originally posted by @kvark in #450

Proposal to drop surfman and use EGL directly: gfx-rs/gfx#3468

Then, surfman was decided to be dropped in favor of just using EGL, as it's purpose wasn't met and it was flaky.

Therefore, what if we can just directly depend on EGL unconditionally (on non-wasm targets)? No need for optional glutin, surfman, etc, features. Just make it so our GL backend uses EGL for context creation.

• Instance = egl::Display, created by eglGetDisplay. We could choose the default one - I think this is the most tricky part in the proposal so far.

• Surface = egl::Surface, created by eglCreateWindowSurface

• Adapter = egl::Config. When enumerate_adapters is called, we could get all the contexts supported by the display, and then query their attributes to find out which are software renderers, which support presentation, etc

• Device = egl::Context, created by eglCreateContext.

"Is OpenGL Supported Now?" #450

Here there is some discussion about a "raw wayland context". I'm assuming here they mean a context not bound to a window. And how it may work to share resources between these.

The raw wayland context is what you'd want to create (on wayland only) to back the Instance.

Then, when the user creates a surface, you'd want to back it with another context that is not pure and tied to a window, but also shares all resources with the original context of the instance. This is at least the way I could see it working.

Suggestion to use glutin to manage OpenGL contexts: #5643

Then, someone suggested bringing back glutin, but it seems that the idea was rejected somewhat.

wgpu used to use glutin and surfman at times but moved away from it a while ago. The main reasons we moved away from them were that we didn't need most of what the abstractions provided, and we need access to extensions at the EGL/WGL/etc. level anyway (context/surface creation order to match wgpu, resource sharing, etc.).
There's some discussion across wgpu and gfx (the original backend for wgpu) if you're interested:
gfx-rs/gfx#3468
gfx-rs/gfx#3151
#450
Because of the history I think we'd probably want to avoid glutin for now unless there's a pretty strong reason to reintroduce an abstraction at that level. I'm not sure there's too much value in trying to support CGL right now since we already have a couple options that work well there (Metal being the preferred backend for Apple).

Originally posted by @grovesNL in #5643

BadDisplay Issue: #5505

Then we realized that there's a problem with the fact that we have this temporary independent wl_display->EGLDisplay used to query being the initial context. It seems that this temporary context got used for too much, and then got hacked around and leaked in a way that seems to have unsound behavior impossible to fix. I think that it's because when you hand it an external context, it can't share it, so it has to current/uncurrent it in a way that it can't prove is safe. I think this is probably a wrong interpretation...

All in all it looks like wgpu got EGL/WGL context handling wrong. There is a reason raw-window-handle separated out RawWindowHandle from RawDisplayHandle, yet wgpu somehow only passes the latter around when a surface is created? This makes it impossible to correctly initialize a context API that is relevant to the current display connection (set up by winit or otherwise) and requires ugly hacks like fn open_x_display(), fn test_wayland_display() and recreating/overwriting the context.

To whomever is planning to fix these bugs, I recommend starting by deleting the entire gles module and using glutin to implement the context API going forward. A good starting point is its example which clearly highlights important platform differences.
Originally posted by @MarijnS95 in #5505

This was posted as a possible way we could do this. Let WGPU live in its own independent windowless world and use DMA-BUF to share the rendered texture with the other completely independent world - so that the wl_display/context sharing all doesn't matter. But this then means you can't render direct- I'd assume.

Accessing the raw texture for sharing via DMA-BUF would still be possible after rewriting the gles module?

In this poc I export the raw WGPU OpenGL texture via DMA-BUF and use it in Slint or Glutin:

https://github.com/BartBM/wgpu-dma-buf/blob/main/src/texture_export_wgpu.rs
Originally posted by @BartBM in #5505

Then, some good background and maybe some advice for looking into glutin:

I'm not sure if this is helpful, but for context some of the original workarounds were attempting to support surfaceless/headless GL contexts for initial queries, then being able to optionally associate the context with a surface afterwards through whatever context-specific APIs were available for that platform (e.g., context sharing). The implementation may have drifted over time but that was the original intent at least.

glutin and surfman didn't support this use case at the time, so wgpu would have needed a separate API for GL to fit glutin's API. Maybe glutin supports this now though?
Originally posted by @grovesNL in #5505

Then a potential issue was brought up with the general strategy of making a temporary set of displays/contexts, and swapping them out when you get a surface.

Specifically on EGL, this might be tricky if the select EGLConfig is incompatible with a window/surface down the line, requiring you to create a new context. This context could be shared with the dummy context though, allowing them to share resources while their "current-ness" is independent.

(Except that on at least wglShareLists() there are some extra constraints when calling).
Originally posted by @MarijnS95 in #5505

Then some more info on a strategy for sharing contexts.

If all you need is sharing resources between two GL contexts inside the same process (especially Textures) OpenGL context sharing should be exactly what you need. For example GStreamer uses these extensively to share textures and buffers across the pipeline while still being able to (un)current contexts on different threads independently of each other.

This is currently not at all possible with wgpu (and even resulted in some strange internal hacks), and is why @ErichDonGubler opened #6211 to see if we can rectify that. Even though that issue is specific to the current "impossible" soundness requirements on some unsafe functions inside the GLES backend, I don't think we can fix it without properly reimplementing context initialization with support for shared contexts and basing things off of existing RawDisplayHandles for compatibility (etc).

@grovesNL thanks for the link - preliminary reading shows that there's no RawDisplayHandle in there at the moment, so we'd have to find a way to add it.

Originally posted by @MarijnS95 in #5505

Then, somewhat of a go-ahead to start ripping out egl.rs and replacing with glutin:

Thanks @cwfitzgerald. Would be awesome to start tackling those issues, if there's sign-off to delete the current atrocity and rewrite it with glutin 😁

Originally posted by @MarijnS95 in #6289

A bit of a design with trying to get the display handle into the adapter.

One thing I wanted to get into first and foremost, as outlined in the last paragraph of #6211 (comment), is getting a raw_window_handle::DisplayHandle into Adapter so that we can create a Context for the adequate windowing system. Is that something you think is feasible, or otherwise achievable by creating the context in a different place? I'd hate to have to persist hacks that overwrite the context/surface whenever it finally becomes known to wgpu.
Originally posted by @MarijnS95 in #5505

I believe this is talking about a world where we force users to go down a purely linear path to make the APIs happy and all agree on the contexts/displays. But that you currently can set up an adapter without any regard for the surface you'd use it on and share the adapter between surfaces.

One thing I wanted to get into first and foremost, as outlined in the last paragraph of #6211 (comment), is getting a raw_window_handle::DisplayHandle into Adapter so that we can create a Context for the adequate windowing system. Is that something you think is feasible, or otherwise achievable by creating the context in a different place

So we already do this on WebGL. As all contexts come from canvases, we require that you call create_surface before you call enumerate adapters, or else you simply won't get any adapters. We could have something like this requirement for native too, though the big missing piece would be how to deal with headless adapters. The advantage of how the situation is currently set up is that you can make an adapter without regard for the surface you will use it on, or share between surfaces etc. This seems to be plausibly defined, at least in my understanding of WGL and EGL and it would be disappointing to lose that ability.

We do currently have the notion of surface compatibility, so if we could expose one adapter per surface, then one headless adapter, we could have the surface compatibility logic give you the correct surface.

If we do want to take on this limitation, we'd need to think about user education and if there is a better way to expose this causality to get people to fall into the pit of success more easily.

Additionally we need to think about surfaces being unconfigured for a given adapter. This is a not-particularly-well-tested part of the api and probably is horribly broken in a lot of ways anyway, but the api does exist and we should at least put some passing thought to it.

Originally posted by @cwfitzgerald in #5505

Current hack to get AM62 GPU GL working

The re-init hack when creating a surface:

wgpu/wgpu-hal/src/gles/egl.rs

Lines 935 to 979 in 6f16ea4

	#[cfg(not(Emscripten))]
	(Rwh::Wayland(_), raw_window_handle::RawDisplayHandle::Wayland(display_handle)) => {
	if inner
	.wl_display
	.map(|ptr| ptr != display_handle.display.as_ptr())
	.unwrap_or(true)
	{
	/* Wayland displays are not sharable between surfaces so if the
	* surface we receive from this handle is from a different
	* display, we must re-initialize the context.
	*
	* See gfx-rs/gfx#3545
	*/
	log::warn!("Re-initializing Gles context due to Wayland window");
	use std::ops::DerefMut;
	let display_attributes = [khronos_egl::ATTRIB_NONE];
	let display = unsafe {
	inner
	.egl
	.instance
	.upcast::<khronos_egl::EGL1_5>()
	.unwrap()
	.get_platform_display(
	EGL_PLATFORM_WAYLAND_KHR,
	display_handle.display.as_ptr(),
	&display_attributes,
	)
	}
	.unwrap();
	let new_inner = Inner::create(
	self.flags,
	Arc::clone(&inner.egl.instance),
	display,
	inner.force_gles_minor_version,
	)?;
	let old_inner = std::mem::replace(inner.deref_mut(), new_inner);
	inner.wl_display = Some(display_handle.display.as_ptr());
	drop(old_inner);
	}
	}

This hack makes it not usable for me on the AM62 SoC GPU, I haven't tracked down exactly what part of the context is leaked but I can get it running by:

1. Removing the hack linked above
2. Creating a static global ugly raw c pointer variable to hold the wl_display*.
3. In the app/example that uses winit, grab the raw display pointer, and shove it into that global static before WGPU does anything.
4. Use it instead of DEFAULT_DISPLAY here

   wgpu/wgpu-hal/src/gles/egl.rs

   Line 802 in 6f16ea4

   khronos_egl::DEFAULT_DISPLAY,

5. Use it instead of None here:

   wgpu/wgpu-hal/src/gles/egl.rs

   Line 656 in 6f16ea4

   wl_display: None,

Current state

• We have buy-in from @cwfitzgerald to replace egl.rs stuff with glutin.
• It looks like @MarijnS95 isn't currently working on the project.
• glutin may simplify or solve:
  • The temporary context problem: You need to create a bunch of temporary stuff just to enumerate adapters and capabilities, before any surface is created.
  • Sharing contexts (I'm not fully clear on how this lets us work around the problems around wl_display* matching though).
  • Creating a true windowless surface that can be associated with multiple contexts.
• At least on my platforms, the BadDisplay/DEFAULT_DISPLAY is fixed by providing some wl_display when getting eglGetPlatformDisplay, but this is not sufficient to prevent crashing as something is leaked or some context does not match up, causing it to crash when you start using the context after reinitializing from the winit wl_display*.
• It seems that matching wl_display up works with my ugly hack.

Solutions?

1. Separate contexts, share resource to bridge the wl_display gap

• Keep the wl_display* and related contexts independent between winit and wgpu,
• Share some resource, context, or buffer across this, and have the application side find out a way to render what wgpu did onto the window.
• See if glutin does any of this for us - unlikely?
• This doesn't seem like a good approach. Adds an extra render to get to a window, probably relies on very specific buffer sharing APIs.

2. Change API to first require a surface

• See if we can copy what WebGPU requires. Force users to first get a surface, then go from there. Not sure if this is necessary.

3. Fix the current hacks.

• See if we can pass around more context, delay creation of resources until we have a wl_display*, and then use that to reinitialize what is needed.

4. Use compatible surface when enumerating adapters.

[kchibisov]

To make it clear, wl_display is not the same thing as the DISPLAY on X11, wayland is also generally different here. On X11 everything refers resource on the server, while on wayland it's not. Thus on Wayland, you must pass the same wl_display everywhere for things to be able to interact.