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

[Marek Olšák]

I prefer not to have any negative caps.

Marek

On Tue, Nov 6, 2018, 11:14 AM Ilia Mirkin <imirkin at alum.mit.edu wrote:

> On Tue, Nov 6, 2018 at 10:27 AM Gert Wollny <gert.wollny at collabora.com>
> wrote:
> >
> > Am Dienstag, den 06.11.2018, 09:29 -0500 schrieb Ilia Mirkin:
> > > On Tue, Nov 6, 2018 at 5:59 AM Gert Wollny <gert.wollny at collabora.com
> > > > wrote:
> > > >
> > > >
> > > > 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?
> > Ther is no fallback, when the guest uses
> > glEnable/Disable(GL_FRAMEBUFFER_EXT) and the host doesn't support it,
> > and there it also fails. This is to be expected and only shows that the
> > guest shouldn't expose EXT_framebuffer_sRGB solely based on available
> > formats.
>
> I mean in the code, there's a "goto fallback" in various situations,
> which does the operation on the CPU. If you're 100% in that path, then
> you won't have issues.
>
> >
> > > 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.
> > That was my guess too. I think if that code would do something wrong
> > without the guest explicitely en/disabling GL_FRAMEBUFFER_SRGB we would
> > have seen it.
>
> sRGB issues can be subtle. I doubt there's strong test coverage of all
> the various cases, and visually scenes just appear "dark", not
> "totally broken". I'd encourage you to trace the code and try to
> construct a test case that hits the path I pointed out as problematic.
> (iirc the blit in st_TexSubImage, which uses a normalized dst format,
> but I don't have the code in front of me)
>
> >
> > [...]
> > > > 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.
> > Hmm, currently mesa exposes GLES 3.0 based on EXT_framebuffer_sRGB,
> > which is more then is needed, and EXT_texture_sRGB is not enough, so I
> > need one more flag that reflect the GLES requirement of format support
> > without the need to support the format switching, probably in gl_const,
> > because this is what is passed to mesa/compute_version_e2(...).
>
> I'm not as intimately familiar with GLES extensions / features, but
> would EXT_sRGB be the extension that covers that support? Mesa does
> not presently implement this extension, but if it's the right one,
> perhaps adding an implementation of that, and then doing something
> like
>
> EXT_sRGB = is_format_supported(SRGB)
> EXT_framebuffer_sRGB / ARB_framebuffer_sRGB / EXT_sRGB_write_control =
> is_format_supported(SRGB) &&
> !PIPE_CAP_REQUIRE_SURFACE_RESOURCE_FORMAT_MATCH
> GLES3 = EXT_sRGB (& others, obviously)
>
> And that PIPE_CAP would then in turn only ever be "enabled" by virgl
> when running on a host without EXT_sRGB_write_control (or equivalent)
> support.
>
> Cheers,
>
>   -ilia
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20181106/53005753/attachment.html>