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

[Ilia Mirkin]

On Tue, Nov 6, 2018 at 5:59 AM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> 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).

Well, have a look at the code, and see if you can get it to trigger
with a SRGB format. I suspect there's just no test that hits that
particular scenario. Or perhaps you're 100% fallback-path driven on
virgl? In that case, that particular code is fine, but I didn't do a
full audit of the codebase. These tests would presumably be failing
already, since the issue is that you *don't* have
EXT_sRGB_write_control on the host and are trying to implement SRGB
formats.

>
> > 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.).

I'd go the other way -- have a cap that shows that you CANT support
surface formats differing from the resource format. The current
implication is that this is always supported. There's a handful of
such anti-CAP's, and given that this is a violation of the existing
gallium interface (where the surface format != resource format is
generally supported), I think that's the way to go.

Cheers,

  -ilia