[Mesa-dev] [PATCH v3 2/3] mesa/st: enable EXT_sRGB_write_control for drivers that support it
Ilia Mirkin
imirkin at alum.mit.edu
Fri Oct 19 12:25:49 UTC 2018
I don't think a PIPE_CAP is the answer here... I think you're on the
right track with format checks and whatnot.
On Fri, Oct 19, 2018 at 2:54 AM Gert Wollny <gert.wollny at collabora.com> wrote:
>
> Considering how virgl has to deal with host capabilities the extension
> should be enabled via a CAP, so I'll rework this patch another time.
>
> Sorry for being so noisy,
> Gert
>
>
> Am Donnerstag, den 18.10.2018, 18:39 +0200 schrieb Gert Wollny:
> > 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.
> >
> > Signed-off-by: Gert Wollny <gert.wollny at collabora.com>
> > ---
> > src/mesa/state_tracker/st_extensions.c | 6 +++++
> > src/mesa/state_tracker/st_manager.c | 37 ++++++++++++++++------
> > ----
> > 2 files changed, 29 insertions(+), 14 deletions(-)
> >
> > diff --git a/src/mesa/state_tracker/st_extensions.c
> > b/src/mesa/state_tracker/st_extensions.c
> > index 798ee60875..401609d728 100644
> > --- a/src/mesa/state_tracker/st_extensions.c
> > +++ b/src/mesa/state_tracker/st_extensions.c
> > @@ -1169,6 +1169,12 @@ void st_init_extensions(struct pipe_screen
> > *screen,
> > void_formats, 32,
> > PIPE_BIND_RENDER_TARGET);
> >
> > + extensions->EXT_sRGB_write_control =
> > + screen->is_format_supported(screen,
> > PIPE_FORMAT_R8G8B8A8_SRGB,
> > + PIPE_TEXTURE_2D, 0, 0,
> > + (PIPE_BIND_DISPLAY_TARGET |
> > + PIPE_BIND_RENDER_TARGET));
> > +
> > if (extensions->AMD_framebuffer_multisample_advanced) {
> > /* AMD_framebuffer_multisample_advanced */
> > /* This can be greater than storage samples. */
> > 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
> > enabled by
> > - * default. To avoid unexpected results, we should not advertise
> > the
> > - * capability. This could change when we add support for
> > - * EGL_KHR_gl_colorspace.
> > + * For GLES, however, sRGB framebuffer write is initially only
> > controlled
> > + * by the capability of the framebuffer, with
> > GL_EXT_sRGB_write_control
> > + * control is given back to the applications, but
> > GL_FRAMEBUFFER_SRGB is
> > + * still enabled by default since this is the behaviour when
> > + * EXT_sRGB_write_control is not available.
> > */
> > - if (_mesa_is_desktop_gl(st->ctx)) {
> > + if (_mesa_is_desktop_gl(st->ctx) ||
> > + st->ctx->Extensions.EXT_sRGB_write_control)
> > + {
> > struct pipe_screen *screen = st->pipe->screen;
> > const enum pipe_format srgb_format =
> > util_format_srgb(stfbi->visual->color_format);
> > @@ -475,8 +477,14 @@ st_framebuffer_create(struct st_context *st,
> > PIPE_TEXTURE_2D, stfbi-
> > >visual->samples,
> > stfbi->visual->samples,
> > (PIPE_BIND_DISPLAY_TARGET |
> > - PIPE_BIND_RENDER_TARGET)))
> > + PIPE_BIND_RENDER_TARGET))) {
> > mode.sRGBCapable = GL_TRUE;
> > + /* Since GL_FRAMEBUFFER_SRGB is enabled by default on GLES
> > we must not
> > + * create renderbuffers with an sRGB format derived from
> > the
> > + * visual->color_format, but we still want this for desktop
> > GL.
> > + */
> > + prefer_srgb = _mesa_is_desktop_gl(st->ctx);
> > + }
> > }
> >
> > _mesa_initialize_window_framebuffer(&stfb->Base, &mode);
> > @@ -487,13 +495,13 @@ st_framebuffer_create(struct st_context *st,
> >
> > /* add the color buffer */
> > idx = stfb->Base._ColorDrawBufferIndexes[0];
> > - if (!st_framebuffer_add_renderbuffer(stfb, idx)) {
> > + if (!st_framebuffer_add_renderbuffer(stfb, idx, prefer_srgb)) {
> > free(stfb);
> > return NULL;
> > }
> >
> > - st_framebuffer_add_renderbuffer(stfb, BUFFER_DEPTH);
> > - st_framebuffer_add_renderbuffer(stfb, BUFFER_ACCUM);
> > + st_framebuffer_add_renderbuffer(stfb, BUFFER_DEPTH, false);
> > + st_framebuffer_add_renderbuffer(stfb, BUFFER_ACCUM, false);
> >
> > stfb->stamp = 0;
> > st_framebuffer_update_attachments(stfb);
> > @@ -1189,7 +1197,8 @@ st_manager_add_color_renderbuffer(struct
> > st_context *st,
> > return FALSE;
> > }
> >
> > - if (!st_framebuffer_add_renderbuffer(stfb, idx))
> > + if (!st_framebuffer_add_renderbuffer(stfb, idx,
> > + stfb-
> > >Base.Visual.sRGBCapable))
> > return FALSE;
> >
> > st_framebuffer_update_attachments(stfb);
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list