BADHACK: gles/egl: Require surface before creating instance / query adapter on Wayland #7969
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connections
Fixes #5505
Could address #3176 if we make
Inner
anOption
Fixes #7965
Description
In
egl
,Inner
has been madeClone
in #2350, and prior to that some EGL fields were copied intoAdapterContext
since #1729.At the same time
wgpu
to this day doesn't seem to accept aRawDisplayHandle
when creating anInstance
, meaning that on Wayland at least we end up creating a bogusEGLDisplay
(and respective GL context) that isn't compatible with the window/surface that the user later wantswgpu
to present to (not to mention, the user/caller has zero control over the Wayland or other connection to use, this is all hardcoded based on library detection and connecting to the default Wayland socket...).Each time
wl_display_connect()
is called, which is likely also what happens internally whenEGL_DEFAULT_DISPLAY
is passed forEGL_PLATFORM_WAYLAND_KHR
, a different and incompatible display connection is used. Even ifwgpu
'stest_wayland_display()
would have connected to the same Wayland server aseglGetPlatformDisplay(WAYLAND, DEFAULT_DISPLAY)
and the one the caller uses for its surface, these cannot be used interchangeably.To patch up this bogus display, gfx-rs/gfx#3545 inserted some logic to replace the internal
EGLDisplay
and GL context whenever a surface is created which does receive aRawDisplayHandle
with the correctwl_display
handle to use to be compatible with the givenwl_surface
:wgpu/wgpu-hal/src/gles/egl.rs
Lines 991 to 1034 in 96ef5ec
Now all the way back to the first sentence, about cloning a bunch of EGL handles. The problem mainly occurs when the user either enumerated
Adapter
s before this operation, and worse if they started creating resources on it. Because of this clone:wgpu/wgpu-hal/src/gles/egl.rs
Lines 1100 to 1103 in 96ef5ec
Those adapters have the old
EGLDisplay
and old GL context. Which we just closed when replacingInner
with the "newwl_display
-based"EGLDisplay
and context:wgpu/wgpu-hal/src/gles/egl.rs
Lines 707 to 721 in 96ef5ec
Whenever the user now uses the adapter to make the context current, a
BadDisplay
/BadContext
error is generated because the context and display handles are both destroyed.To solve that, one of the proposed solutions was to follow what Web does: require a surface when enumerating
Adapter
s so that the rightEGLDisplay
is copied over. This is however overly restrictive:RawDisplayHandle
could be passed.For now, see if this unblocks folks while we work on a more likable solution. Stay tuned.
Testing
amdgpu
with Wayland compositor, with e.g.;Squash or Rebase?
rebase
Checklist
cargo fmt
.taplo format
.cargo clippy --tests
. If applicable, add:--target wasm32-unknown-unknown
cargo xtask test
to run tests.CHANGELOG.md
entry.