Am Freitag, den 19.10.2018, 08:25 -0400 schrieb Ilia Mirkin: > I don't think a PIPE_CAP is the answer here... I think you're on the > right track with format checks and whatnot.
The problem is that with an GLES host, the checked sRGB format might be supported by virgl, but not EXT_sRGB_write_control, so the guest will still expose the extension, but then one of the dEQP tests fails. If I use host caps in this case to disable the the extension in the guest I can avoid exposing the not properly working extension, and for this I need the caps. I will dig a bit to figure out whether this is actually problem in virglrenderer, or how Gallium handles enabling and disabling FRAMEBUFFER_SRGB, but I'm not that optimistic. Best, Gert > On Fri, Oct 19, 2018 at 2:54 AM Gert Wollny > <gert.wol...@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.wol...@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_unsuppo > > > rted > > > _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.wol...@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@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev