Bug 102354

Just note that it's a bug in STK, because we request srgb-capable framebuffer and don't handle the case when it fails. So you can just close it, or do whatever you want...

(In reply to Deve from comment #1)
> Just note that it's a bug in STK, because we request srgb-capable
> framebuffer and don't handle the case when it fails. So you can just close
> it, or do whatever you want...

Although the crash is an STK bug, the fact that Mesa 17.1.x can give a SRGB-capable framebuffer but Mesa 17.2 does not is a Mesa regression.

Will update title accordingly.

Is it using GLX or EGL?  I diffed the output of glxinfo between these versions:

- Mesa 17.1.0 (git-15a38605fc3)
+ Mesa 17.2.0-rc3 (git-29df4deef2)

and there were no changes in visuals whatsoever.

(In reply to Kenneth Graunke from comment #3)
> Is it using GLX or EGL?  I diffed the output of glxinfo between these
> versions:
> 
> - Mesa 17.1.0 (git-15a38605fc3)
> + Mesa 17.2.0-rc3 (git-29df4deef2)
> 
> and there were no changes in visuals whatsoever.

You may want to see https://bugs.freedesktop.org/show_bug.cgi?id=92759#c7

It uses GLX, but I can test EGL if you want (it's already done for Wayland, but disabled by default).

glxinfo doesn't report ANY config with srgb-capable framebuffer, even though B8G8R8A8 IS srgb-capable (we use GLX_ALPHA_SIZE=1 for intel since mesa 10.6 to force srgb-capable fb).

Most likely I will eventually get rid of all "glEnable(GL_FRAMEBUFFER_SRGB);" calls in STK because it causes too many issues and we need it for OpenGL ES renderer on Android anyway.

And the reason of this bug may be that old mesa was ignoring the GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB, and now it recognizes it and thus fails because there is no config with srgb-capable framebuffer?

And btw. current git STK doesn't crash anymore.

Please check if following patch makes difference:
https://lists.freedesktop.org/archives/mesa-dev/2017-September/168453.html

I just checked the patch and nothing changed for me (with GLX, I didn't test EGL). I patched mesa version 4d807d7fe272db97fb9e20800872d5970fa2696d.

If you run current git STK, there are two warnings in console:

[warn   ] irr_driver: Created visual is not sRGB-capable. Re-creating device to workaround the issue.
[warn   ] [IrrDriver Temp Logger]: Level 2: No sRGB framebuffer available.

The first one happens for intel when
glGetFramebufferAttachmentParameteriv(GL_DRAW_FRAMEBUFFER, GL_BACK_LEFT,
                              GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING, &param);

returns GL_LINEAR, so this is true for all mesa versions >= 10.6 (precisely since MESA_FORMAT_R8G8B8X8_UNORM has been added). In this case we make an ugly hack and re-create device with alpha channel, that forces to use MESA_FORMAT_R8G8B8A8_SRGB.

The second warning happens when request a config with GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB fails. So then we try to get a config without this flag.

And glxinfo still doesn't show any srgb configs for me. Are there some particular requirements, such as current libdrm, DRI2/DRI3 etc...?

(In reply to Deve from comment #7)
> I just checked the patch and nothing changed for me (with GLX, I didn't test
> EGL). I patched mesa version 4d807d7fe272db97fb9e20800872d5970fa2696d.
> 
> If you run current git STK, there are two warnings in console:
> 
> [warn   ] irr_driver: Created visual is not sRGB-capable. Re-creating device
> to workaround the issue.
> [warn   ] [IrrDriver Temp Logger]: Level 2: No sRGB framebuffer available.
> 
> The first one happens for intel when
> glGetFramebufferAttachmentParameteriv(GL_DRAW_FRAMEBUFFER, GL_BACK_LEFT,
>                               GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING,
> &param);
> 
> returns GL_LINEAR, so this is true for all mesa versions >= 10.6 (precisely
> since MESA_FORMAT_R8G8B8X8_UNORM has been added). In this case we make an
> ugly hack and re-create device with alpha channel, that forces to use
> MESA_FORMAT_R8G8B8A8_SRGB.
> 
> The second warning happens when request a config with
> GLX_FRAMEBUFFER_SRGB_CAPABLE_ARB fails. So then we try to get a config
> without this flag.
> 
> And glxinfo still doesn't show any srgb configs for me. Are there some
> particular requirements, such as current libdrm, DRI2/DRI3 etc...?

No additional requirements, I get additional visuals/fbconfigs to glxinfo on my HSW, I'll attach the diff from output.

Created attachment 134151 [details]
changes in glxinfo output

additional visuals/fbconfigs in glxinfo with the patch mentioned

My GLX skills are poor, but I've attempted to build a small test app.

In the list returned by glXGetFBConfigs I do see configs that have GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT set. However, when calling glXChooseVisual with 'GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT' I get no visual at all. So something is missing? I think glXCreateContext should be called with XVisualInfo capable to sRGB in order for this to work. Is this the same error that is seen with STK?

(In reply to Tapani Pälli from comment #10)
> My GLX skills are poor, but I've attempted to build a small test app.
> 
> In the list returned by glXGetFBConfigs I do see configs that have
> GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT set. However, when calling glXChooseVisual
> with 'GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT' I get no visual at all. So something
> is missing? I think glXCreateContext should be called with XVisualInfo
> capable to sRGB in order for this to work. Is this the same error that is
> seen with STK?

ok it seems I was not using API properly, I do get sRGB visual fine with following attribs defined:

--- 8< ---
            int srgb_attr[] = {
               GLX_RENDER_TYPE, GLX_RGBA_BIT,
               GLX_RGBA,
/* these are not really necessary */
#if 0          
               GLX_RED_SIZE,   4,
               GLX_GREEN_SIZE, 4,
               GLX_BLUE_SIZE,  4,
               GLX_DEPTH_SIZE, 4,
               GLX_DOUBLEBUFFER, True,
               GLX_STENCIL_SIZE, 1,
#endif
               GLX_ALPHA_SIZE, 0,
               GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT,
               None
            };
--- 8< ---

and later get 0x8c40 as GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING

I also noticed that STK sets a value of 0/1 to attribute GLX_FRAMEBUFFER_SRGB_CAPABLE_EXT, IMO this is wrong as this attribute is a boolean attribute, not int or enum. Booleans have default value of false and are turned true by including attrib on the list (there should be no value after attrib). Please correct me if this is wrong interpretation of the spec.

I tried to make a clean build with current git mesa and your patch to make sure that I did everything properly, but unfortunately with no luck. You can see glxinfo log in attachment.

STK uses glXChooseFBConfig by default and from what I see in the specs after a quick look: "All attributes in attrib_list, including boolean attributes, are immediately followed by the corresponding desired value."

It has a fallback with glXChooseVisual for glx <= 1.2, but tbh. I don't think that it's used somewhere...

Created attachment 134169 [details]
glxinfo log from patched mesa

(In reply to Deve from comment #13)
> I tried to make a clean build with current git mesa and your patch to make
> sure that I did everything properly, but unfortunately with no luck. You can
> see glxinfo log in attachment.
> 
> STK uses glXChooseFBConfig by default and from what I see in the specs after
> a quick look: "All attributes in attrib_list, including boolean attributes,
> are immediately followed by the corresponding desired value."
> 
> It has a fallback with glXChooseVisual for glx <= 1.2, but tbh. I don't
> think that it's used somewhere...

ok, I'll do some testing on other machines to see if there is some dependency after all, I don't really understand why your glxinfo does not have any srgb configs listed :/

One thing that I noticed is that when you added new formats in intel_screen.c, now it should be probably:

   if (intel_loader_get_cap(dri_screen, DRI_LOADER_CAP_RGBA_ORDERING))
      num_formats = ARRAY_SIZE(formats);
   else
      num_formats = 5;

rather than num_formats = 3. At least for me intel_loader_get_cap(dri_screen, DRI_LOADER_CAP_RGBA_ORDERING) returns false.

In this case I see quite a lot configs with modes->sRGBCapable = true in driCreateConfigs in utils.c, but still no srgb-capable format in glxinfo :(

(In reply to Deve from comment #16)
> One thing that I noticed is that when you added new formats in
> intel_screen.c, now it should be probably:
> 
>    if (intel_loader_get_cap(dri_screen, DRI_LOADER_CAP_RGBA_ORDERING))
>       num_formats = ARRAY_SIZE(formats);
>    else
>       num_formats = 5;
> 
> rather than num_formats = 3. At least for me
> intel_loader_get_cap(dri_screen, DRI_LOADER_CAP_RGBA_ORDERING) returns false.
> 
> In this case I see quite a lot configs with modes->sRGBCapable = true in
> driCreateConfigs in utils.c, but still no srgb-capable format in glxinfo :(

Yep, that is a clear bug in the patch. With the change I get even more configs/visuals that have srgb set to 's'. Will debug some more.

Created attachment 134247 [details]
glx test application

Here's a test application. Deve, could you try running this and see what comes up? For me output is (tested on HSW and KBL machines):

--- 8< ---
created window with visual id 36
using visual 0x1ee (srgb 1) (rgba 8880)
GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING 0x8c40
--- 8< ---

Program has 3 different ways of setting up GLX (see init_glx() function), for 2 of those I'm getting sRGB visual with my patch. Remember to set LD_LIBRARY_PATH and LIBGL_DRIVERS_PATH when running against custom Mesa tree.

Created attachment 134248 [details] [review]
hopeful fix

v2 of the patch

I think I also understood why this works for you and not for me. It works nicely if you run on Fedora (x-wayland) but on Ubuntu you will need to replace the DRI driver used by X server. When this is done, sRGB visuals are exposed correctly.

(In reply to Tapani Pälli from comment #20)
> I think I also understood why this works for you and not for me. It works

*sigh* .. meant to say "why this works for me and not for you" :)

I understood :)

Regarding to LD_LIBRARY_PATH and LIBGL_DRIVERS_PATH, this is what I'm doing.

I didn't install everything to avoid a mess in the system, but I tried to set LD_LIBRARY_PATH to directory that contains patched libgl/i965_dri and current git libdrm and started second X server, but it doesn't make any difference.

In your test app, the first method prints:
created window with visual id 33
found a matching visual id (33)
using visual 0x21 (srgb 0) (rgba 8888)
GL_FRAMEBUFFER_ATTACHMENT_RED_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_GREEN_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_BLUE_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_ALPHA_SIZE 0x8
GL_FRAMEBUFFER_ATTACHMENT_COLOR_ENCODING 0x8c40

And next two methods:
created window with visual id 33
error initializing glx

But... I started gnome wayland session on second tty and it works there! Both your example and glxinfo.

As I said before driCreateConfigs in utils.c creates srgb-capable configs, so something must filter and remove/ignore it later. Maybe some unsupported formats or extensions, no idea tbh...

Ok, I replaced the system i965_dri.so and it works now in X session too! I'm not sure why xserver ignored this library in path from LD_LIBRARY_PATH. But now I can at least confirm that it works fine for me :)

On my system Xorg is a wrapper script that starts the xserver as root, and does not pass LD_LIBRARY_PATH through.

(In reply to Deve from comment #23)
> Ok, I replaced the system i965_dri.so and it works now in X session too! I'm
> not sure why xserver ignored this library in path from LD_LIBRARY_PATH. But
> now I can at least confirm that it works fine for me :)

Good to know, now I think we still need some KDE/Plasma testing. I'll send my patch to list later and CC KDE people.

I just tested and EGL_GL_COLORSPACE_SRGB works too. You can check it in current git STK with "cmake .. -DENABLE_WAYLAND_DEVICE=1".

Btw. is there a way to let xserver know that is should use self-compiled dri rather than the one in /usr/lib?

(In reply to Deve from comment #26)
> I just tested and EGL_GL_COLORSPACE_SRGB works too. You can check it in
> current git STK with "cmake .. -DENABLE_WAYLAND_DEVICE=1".
> 
> Btw. is there a way to let xserver know that is should use self-compiled dri
> rather than the one in /usr/lib?

I'm not aware of any, I was told that LIBGL_DRIVERS_PATH (and other env) is ignored for security reasons because xserver used to run as root.

Following commit is now available in Mesa master. It exposes BGRA8888 sRGB visuals and should fix your problem. Please let me know if this is not the case and I can try to help further. I will try to promote this patch to Mesa stable releases if we find no issues.

--- 8< ----
commit c591b1e59454db2e8854e36852e0d413ce38b2f2
Author: Tapani Pälli <tapani.palli@intel.com>
Date:   Tue Oct 31 10:56:28 2017 +0200

    i965: expose SRGB visuals and turn on EGL_KHR_gl_colorspace
    
    Patch exposes sRGB visuals and adds DRI integer query support for
    __DRI2_RENDERER_HAS_FRAMEBUFFER_SRGB. Further changes make sure that
    we mark if the app explicitly wanted sRGB and for these framebuffers
    we don't turn sRGB off in intel_gles3_srgb_workaround. This way we
    keep compatibility for existing applications relying on default sRGB
    and ony add more visual support.
    
    With this change, following dEQP tests start to pass:
    
       dEQP-EGL.functional.wide_color.window_8888_colorspace_srgb
       dEQP-EGL.functional.wide_color.pbuffer_8888_colorspace_srgb
    
    v2: some code cleanup (Emil Velikov)
        update num_formats correctly (reported by deveee@gmail.com)
    
    v3: cleanup, remove redundant is_srgb
        rename explicit_srgb as 'need_srgb' to follow style better
    
    Signed-off-by: Tapani Pälli <tapani.palli@intel.com>
    Reviewed-by: Emil Velikov <emil.velikov@collabora.com> (v2)
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102264
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102354
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102503