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

2018-10-31 Thread Gert Wollny 
Am Dienstag, den 30.10.2018, 16:52 -0400 schrieb Ilia Mirkin:

[snip]
> > > }
> > >  #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...
> 
This is actually just some copy-paste problem, i.e. I took what is used
in st_manager.c:st_framebuffer_create to check for sRGB support of the
sepcific format used there, adjusted the pipe format to a defined
format and cleaned the sample parameters. If this is not needed for
some other reason, then I'll just drop it.

Best, 
Gert
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-10-30 Thread Ilia Mirkin 
On Tue, Oct 30, 2018 at 4:45 PM Marek Olšák  wrote:
>
> On Tue, Oct 23, 2018 at 1:30 PM Gert Wollny  wrote:
>>
>> From: Gert Wollny 
>>
>> 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 
>> ---
>>  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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-10-30 Thread Marek Olšák 
On Tue, Oct 23, 2018 at 1:30 PM Gert Wollny  wrote:

> From: Gert Wollny 
>
> 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 
> ---
>  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.

Marek
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-10-23 Thread Gert Wollny 
From: Gert Wollny 

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 
---
 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));
+
if (screen->get_param(screen, PIPE_CAP_DOUBLES)) {
   extensions->ARB_gpu_shader_fp64 = GL_TRUE;
   extensions->ARB_vertex_attrib_64bit = GL_TRUE;
diff --git a/src/mesa/state_tracker/st_manager.c 
b/src/mesa/state_tracker/st_manager.c
index ceb48dd490..df898beb23 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -295,7 +295,7 @@ st_framebuffer_update_attachments(struct st_framebuffer 
*stfb)
  */
 static boolean
 st_framebuffer_add_renderbuffer(struct st_framebuffer *stfb,
-gl_buffer_index idx)
+gl_buffer_index idx, bool prefer_srgb)
 {
struct gl_renderbuffer *rb;
enum pipe_format format;
@@ -318,7 +318,7 @@ st_framebuffer_add_renderbuffer(struct st_framebuffer *stfb,
   break;
default:
   format = stfb->iface->visual->color_format;
-  if (stfb->Base.Visual.sRGBCapable)
+  if (prefer_srgb)
  format = util_format_srgb(format);
   sw = FALSE;
   break;
@@ -436,6 +436,7 @@ st_framebuffer_create(struct st_context *st,
struct st_framebuffer *stfb;
struct gl_config mode;
gl_buffer_index idx;
+   bool prefer_srgb = false;
 
if (!stfbi)
   return NULL;
@@ -457,14 +458,15 @@ st_framebuffer_create(struct st_context *st,
 * format such that util_format_srgb(visual->color_format) can be supported
 * by the pipe driver.  We still need to advertise the capability here.
 *
-* For GLES, however, sRGB framebuffer write is controlled only by the
-* capability of the framebuffer.  There is GL_EXT_sRGB_write_control to
-* give applications the control back, but sRGB write is still