[Mesa-dev] [PATCH v5 0/3] Add and enable extension EXT_sRGB_write_control

[Gert Wollny]

Am Montag, den 05.11.2018, 11:17 -0500 schrieb Ilia Mirkin:
> On Mon, Nov 5, 2018 at 11:02 AM Gert Wollny <gert.wollny at collabora.co
> m> wrote:
> > 
> > Am Montag, den 05.11.2018, 08:56 -0500 schrieb Ilia Mirkin:
> > > At the gallium level, it's a promise that some PIPE_FORMAT_*_SRGB
> > > is
> > > supported for rendering and that you can use a non-srgb format in
> > > the
> > > surface even if the resource was originally allocated with an
> > > srgb
> > > format (and vice-versa). The latter bit is always a requirement
> > > for
> > > gallium drivers, of course.
> > 
> > How so? What if the hardware does not support using a non-srgb
> > format if the resource was originally allocated with an srgb
> > format?
> > You might not like it when I bring this example, but if a
> > virglrenderer
> > GLES host driver doesn't support EXT_sRGB_write_control, then we
> > have
> > exactly this example: the "hardware" is missing the option to flip
> > a
> > bit that makes it possible to use a non-srgb format with a resource
> > that was allocated with an srgb format, but it still supports some
> > SRGB
> > formats, and since Gallium enables EXT_framebuffer_sRGB based on
> > these
> > I can not simply use the EXT_framebuffer_sRGB flag to expose
> > EXT_sRGB_write_control.
> 
> The scenario you describe is not representable using current internal
> API's/settings. We use bind flags (in Gallium) to determine if a
> format is renderable or not. There is no "oh, it's renderable if
> this, but not if that". Have a look at e.g. st_cb_texture.c, it will
> happily blit as RGBA8_UNORM instead of RGBA8_SRGB, which will break
> your scheme.

I'm not so sure that things break, at least we have all the dEQP master
tests that are enabled by the available extension (level GLES 3.2+)
passing on virgl, which would indicate that what is done in mesa and
virglrender is sufficiently correct for running a gles 3.2 environment
on top of a gles 3.2 host also without EXT_sRGB_write_control
(obviously also not using this functionbality in the guest).

> This use-case is just not supported right now. If you'd like to add
> support for it, that's fine too -- however it's a lot more than a
> single enable flag. You have to pipe it all the way through, and
> adjust the (internal) users to respect it.
I think you underestimate how robust the code is written with respect
to that. Pipeing through whether turning the sRGB - linear conversion
on or off is  already in place, because that's what OpenGL supports,
and if virgl is run on an OpenGL host this also works fine because one
can call glEnable/glDisable (GL_FRAMEBUFFER_SRGB). 
When a GLES guest that runs on a GLES host that doesn't support 
EXT_sRGB_write_control is not trying to use this extension, then things
are also fine. 

The only thing that is needed is to decide whether the user can
actually call glEnable/glDisable (GL_FRAMEBUFFER_SRGB), and at a few
points the code needed adjustment to really allow this on GLES if
sRGB_write_control is available, but with the series I proposed here
this worked out fine (at least with the relevant dEQP tests).

> I'd say the expedient thing for now is if a GLES host doesn't support
> EXT_sRGB_write_control, don't expose any SRGB formats at all.
This would be a regression with respect to  running GLES 3.x on top of
a GLES 3.x host that doesn't support EXT_sRGB_write_control. 

I can now also understand your point of view that
EXT_sRGB_write_control and EXT_framebuffer_sRGB constitute the same
functionality, but then there is still some flag missing that indicates
for GLES that sRGB formats are available, but write control isn't, and
using only a texture format test to identify whether
EXT_framebuffer_sRGB is supported is not sufficint for the scenario
above (which brings me back to actually using a CAP to indicate whether
the "hardware" supports sRGB write control.).

Best, 
Gert