Skip to content

wayland and GL backend does not like sRGB formats on integrated intel gpu #1981

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

Closed
m4b opened this issue Sep 20, 2021 · 6 comments · Fixed by #2076
Closed

wayland and GL backend does not like sRGB formats on integrated intel gpu #1981

m4b opened this issue Sep 20, 2021 · 6 comments · Fixed by #2076
Labels
type: bug Something isn't working

Comments

@m4b
Copy link
Contributor

m4b commented Sep 20, 2021

Description
Shadow example (and all others, etc.) surfaces a panic (failure to configure surface basically), which originates from an error coming from this call right here:

egl.create_platform_window_surface(
with a BAD_MATCH error when calling https://www.khronos.org/registry/EGL/sdk/docs/man/html/eglCreatePlatformWindowSurface.xhtml

Repro steps
On archlinux, running gnome wayland:

RUST_LOG=debug WGPU_BACKEND=gl cargo run --example shadow

ends with:

[2021-09-20T05:00:05Z INFO  wgpu_core::device] configuring surface with SurfaceConfiguration { usage: RENDER_ATTACHMENT, format: Bgra8UnormSrgb, width: 800, height: 600, present_mode: Mailbox }
[2021-09-20T05:00:05Z WARN  wgpu_core::device] Surface does not support present mode: Mailbox, falling back to FIFO
[2021-09-20T05:00:05Z ERROR wgpu_hal::gles::egl] EGL 'eglCreateWindowSurface' code 0x3009: Unsupported surfacetype/colorspace configuration
[2021-09-20T05:00:05Z WARN  wgpu_hal::gles::egl] Error in create_platform_window_surface: BadMatch
thread 'main' panicked at 'Error in Surface::configure: invalid surface', wgpu/src/backend/direct.rs:197:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[2021-09-20T05:00:05Z INFO  wgpu_core::hub] Dropping Global

Expected vs observed behavior
According to capabilities, it should run without panicking on a sRGB surface, e.g. logs return:

[2021-09-20T05:18:53Z INFO  wgpu_hal::gles::egl] 	Trying native-render
[2021-09-20T05:18:53Z INFO  wgpu_hal::gles::egl] 	EGL context: +debug
[2021-09-20T05:18:53Z INFO  wgpu_hal::gles::egl] 	EGL context: +robust access
[2021-09-20T05:18:53Z INFO  wgpu_hal::gles::egl] 	EGL context: +surfaceless
[2021-09-20T05:18:53Z INFO  wgpu_hal::gles::egl] 	EGL surface: +srgb

Which mean we hit this line:

log::info!("\tEGL surface: +srgb");

which is what sets srgb_kind, which is what enables the enable_srgb flag, which is what returns the sRGB surfaces as supported in caps.

Extra materials
Changing to the following fixes (though it clearly renders in linear colorspace):

diff --git a/wgpu-hal/src/gles/egl.rs b/wgpu-hal/src/gles/egl.rs
index c91c1a2d..66b9f68f 100644
--- a/wgpu-hal/src/gles/egl.rs
+++ b/wgpu-hal/src/gles/egl.rs
@@ -883,7 +883,7 @@ impl crate::Surface<super::Api> for Surface {
                         },
                         // Always enable sRGB in EGL 1.5
                         egl::GL_COLORSPACE as usize,
-                        egl::GL_COLORSPACE_SRGB as usize,
+                        egl::GL_COLORSPACE_LINEAR as usize,
                         egl::ATTRIB_NONE,
                     ];

Platform
Archlinux x86_64, running gnome + wayland, targetting gl backend.

@kvark
Copy link
Member

kvark commented Sep 20, 2021

Thank you for investigating!
So the driver says "Unsupported surfacetype/colorspace configuration". We know that the driver can do SRGB, but apparently not for this particular surface type (which is a "window"?). I wonder if there is a way to detect this compatibility, or we should just process the error and try out a different thing.

@cwfitzgerald cwfitzgerald added the type: bug Something isn't working label Sep 20, 2021
@valpackett
Copy link
Contributor

Looks like Mesa's EGL-Wayland support just does not handle sRGB at all like the Vulkan WSI does. I'll look into adding the missing support. But in any case you probably shouldn't rely on this always working, this sRGB format stuff seems relatively new and obscure-ish (?).

@valpackett
Copy link
Contributor

hm, tracing the driver a bit, it's not that the sRGB configs weren't added, it's the incoming EGLConfig pointing to a DRI config that doesn't support sRGB.

@kvark
Copy link
Member

kvark commented Sep 21, 2021

So we are picking a wrong config?

@valpackett
Copy link
Contributor

valpackett commented Sep 21, 2021

Oh! Yes. I know the issue.

Hint: this is the same issue that used to cause GTK4 applications to have non-smooth shadows (!)

… get it yet?

You're picking a 10-bit color format like ARGB2101010 by just using choose_first_config and assuming that the first config is the one you want. These formats don't have sRGB support. If you request [egl::ALPHA_SIZE, 8], you'll get an 8-bit format that does. (But it's still a good idea not to bail on that error. And/or examine matching formats more closely…)

@kvark
Copy link
Member

kvark commented Sep 21, 2021

Ok, interesting. This is a bit problematic. We are currently choosing FBConfig at instance creation time. But we don't know what surface format is going to be requested by configure() at this point. So I could think of the following ways to proceed:

  1. Try to pick a good FBConfig initially, e.g. by requesting ALPHA_SIZE == 8. Then filter the available formats, like we do now for SRGB.
  2. Have each FBConfig (or a group of them) represented by a separate Adapter. The adapter would therefore know precisely what formats it can present, and what formats it can't.
  3. (for completeness) similar to Wayland path, recreate FBConfig and everything at configure() time. This would be very sad.
  4. (for completeness) see that window surface creation failed, and fall back to using non-SRGB.

I think (1) would work best. Open to other proposals!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants