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 <[email protected]> > > 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 <[email protected]> > --- > 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 [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
