[Mesa-dev] [PATCH v4 2/3] mesa/st: enable EXT_sRGB_write_control for Gallium drivers that support it

[Ilia Mirkin]

On Tue, Oct 30, 2018 at 4:45 PM Marek Olšák <maraeo at gmail.com> wrote:
>
> On Tue, Oct 23, 2018 at 1:30 PM Gert Wollny <gw.fossdev at gmail.com> wrote:
>>
>> From: Gert Wollny <gert.wollny at collabora.com>
>>
>> With this patch the extension EXT_sRGB_write_control is enabled for
>> gallium drivers that support sRGB formats as render targets.
>>
>> Tested (and pass) on r600 (evergreen) and softpipe:
>>
>>   dEQP-GLES31.functional.fbo.srgb_write_control.framebuffer_srgb_enabled*
>>
>> with "MESA_GLES_VERSION_OVERRIDE=3.2" (the tests needlessly check for this), and
>>
>>   dEQP-GLES31.functional.fbo.srgb_write_control.framebuffer_srgb_unsupported_enum
>>
>> when extension is manually disabled via MESA_EXTENSION_OVERRIDE
>>
>> v2: - always enable the extension when sRGB is supported (Ilia Mirkin).
>>     - Correct handling by moving extension initialization to the
>>       place where gallium/st actually takes care of this. This also
>>       fixes properly disabling the extension via MESA_EXTENSION_OVERRIDE
>>     - reinstate check for desktop GL and add check for the extension
>>       when creating the framebuffer
>>
>> v3: - Only create sRGB renderbuffers based on Visual.srgbCapable when
>>       on desktop GL.
>>
>> v4: - Use PIPE_FORMAT_B8G8R8A8_SRGB to check for the capability, since this
>>       is also the format that is used top check for EGL_KHR_gl_colorspace
>>       support.  virgl on a GLES host usually doesn't provide this format but
>>       one can make it available to signal that the host supports this
>>       extension.
>>
>> Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
>> ---
>>  src/gallium/docs/source/screen.rst     |  3 +++
>>  src/mesa/state_tracker/st_extensions.c |  8 +++++-
>>  src/mesa/state_tracker/st_manager.c    | 37 ++++++++++++++++----------
>>  3 files changed, 33 insertions(+), 15 deletions(-)
>>
>> diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst
>> index 0abd164494..da677eb04b 100644
>> --- a/src/gallium/docs/source/screen.rst
>> +++ b/src/gallium/docs/source/screen.rst
>> @@ -477,6 +477,9 @@ subpixel precision bias in bits during conservative rasterization.
>>    0 means no limit.
>>  * ``PIPE_CAP_MAX_VERTEX_ELEMENT_SRC_OFFSET``: The maximum supported value for
>>    of pipe_vertex_element::src_offset.
>> +* ``PIPE_CAP_SRGB_WRITE_CONTROL``: Indicates whether the drivers on GLES supports
>> +  enabling/disabling the conversion from linear space to sRGB at framebuffer or
>> +  blend time.
>>
>>  .. _pipe_capf:
>>
>> diff --git a/src/mesa/state_tracker/st_extensions.c b/src/mesa/state_tracker/st_extensions.c
>> index 798ee60875..38d6a3ed1d 100644
>> --- a/src/mesa/state_tracker/st_extensions.c
>> +++ b/src/mesa/state_tracker/st_extensions.c
>> @@ -1167,7 +1167,7 @@ void st_init_extensions(struct pipe_screen *screen,
>>        consts->MaxFramebufferSamples =
>>           get_max_samples_for_formats(screen, ARRAY_SIZE(void_formats),
>>                                       void_formats, 32,
>> -                                     PIPE_BIND_RENDER_TARGET);
>> +                                     PIPE_BIND_RENDER_TARGET);
>>
>>        if (extensions->AMD_framebuffer_multisample_advanced) {
>>           /* AMD_framebuffer_multisample_advanced */
>> @@ -1393,6 +1393,12 @@ void st_init_extensions(struct pipe_screen *screen,
>>     }
>>  #endif
>>
>> +   extensions->EXT_sRGB_write_control =
>> +         screen->is_format_supported(screen, PIPE_FORMAT_B8G8R8A8_SRGB,
>> +                                     PIPE_TEXTURE_2D, 0, 0,
>> +                                     (PIPE_BIND_DISPLAY_TARGET |
>> +                                      PIPE_BIND_RENDER_TARGET));
>
>
> This is kinda a hack because DISPLAY_TARGET has nothing to do with sRGB. sRGB is a 3D/framebuffer feature, unrelated to the window system. The sRGB property of the format is private within the application and is not visible outside of it. The window system doesn't care what is and isn't sRGB. I think you are just hiding a bug in your driver.

Yeah, it's unclear to me what GL_EXT_sRGB_write_control does beyond
using a sRGB framebuffer format for a FBO. Either you can handle that
format or you can't...

  -ilia