<div dir="ltr">Ilia: please review patch v4. I have added a separate patch to fix the GLES3 sRGB workaround, which fixes the colorspace issue.</div><div class="gmail_extra"> <div class="gmail_quote">On Tue, Apr 5, 2016 at 5:20 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu" target="_blank">imirkin@alum.mit.edu</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Sounds right. That's glEnable/glDisable(GL_FRAMEBUFFER_SRGB). It defaults to off for desktop GL, and on for GL ES (why? no clue.) <div class="HOEnZb"><div class="h5"> On Tue, Apr 5, 2016 at 8:11 PM, Haixia Shi <<a href="mailto:hshi@chromium.org">hshi@chromium.org</a>> wrote: > Are you referring to the following check > > mesa_format > _mesa_get_render_format(const struct gl_context *ctx, mesa_format format) > { > if (ctx->Color.sRGBEnabled) > return format; > else > return _mesa_get_srgb_format_linear(format); > } > > So if the platform sets ctx->Color.sRGBEnabled = false then I should not > have this problem? Thanks > > On Tue, Apr 5, 2016 at 2:04 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> wrote: >> >> On Tue, Apr 5, 2016 at 4:57 PM, Haixia Shi <<a href="mailto:hshi@chromium.org">hshi@chromium.org</a>> wrote: >> > CC Stephane >> > >> > In my opinion we should not assume sRGB-capable when alpha is requested, >> > because UNORM is the more common use case. I actually have a use case >> > where >> > alpha is requested but not sRGBCapable, and if we assume SRGB by default >> > then we will get really white-washed faded color. >> >> That most likely means that your application is buggy. Even though the >> SRGB format is returned internally, that doesn't mean that SRGB >> encoding/decoding is actually enabled. Just provides the option for it >> to be used. >> >> > >> > On Tue, Apr 5, 2016 at 1:52 PM, Ilia Mirkin <<a href="mailto:imirkin@alum.mit.edu">imirkin@alum.mit.edu</a>> >> > wrote: >> >> >> >> On Tue, Apr 5, 2016 at 4:33 PM, Haixia Shi <<a href="mailto:hshi@chromium.org">hshi@chromium.org</a>> wrote: >> >> > It is incorrect to assume that pixel format is always in BGR byte >> >> > order. >> >> > We need to check bitmask parameters (such as |redMask|) to determine >> >> > whether >> >> > the RGB or BGR byte order is requested. >> >> > >> >> > Furthermore when parameter |sRGBCapable| is set to false, we should >> >> > be >> >> > using >> >> > UNORM format by default. >> >> >> >> Unfortunately none of the visuals are exported with that flag, so it >> >> will always be false in practice. The current logic is also very >> >> fragile -- you only get a sRGB-capable fb if you happen to request >> >> alpha. >> >> >> >> > >> >> > v2: reformat code to stay within 80 character per line limit. >> >> > >> >> > Signed-off-by: Haixia Shi <<a href="mailto:hshi@chromium.org">hshi@chromium.org</a>> >> >> > Reviewed-by: Stéphane Marchesin <<a href="mailto:marcheu@chromium.org">marcheu@chromium.org</a>> >> >> > Cc: <a href="mailto:kenneth.w.graunke@intel.com">kenneth.w.graunke@intel.com</a> >> >> > >> >> > Change-Id: Ib75087aef1fbfb51baa72517207fed410dcd7b1e >> >> > --- >> >> > src/mesa/drivers/dri/i965/intel_screen.c | 20 ++++++++++++-------- >> >> > 1 file changed, 12 insertions(+), 8 deletions(-) >> >> > >> >> > diff --git a/src/mesa/drivers/dri/i965/intel_screen.c >> >> > b/src/mesa/drivers/dri/i965/intel_screen.c >> >> > index c6eb50a..a27d7ee 100644 >> >> > --- a/src/mesa/drivers/dri/i965/intel_screen.c >> >> > +++ b/src/mesa/drivers/dri/i965/intel_screen.c >> >> > @@ -1000,15 +1000,19 @@ intelCreateBuffer(__DRIscreen * driScrnPriv, >> >> > fb->Visual.samples = num_samples; >> >> > } >> >> > >> >> > - if (mesaVis->redBits == 5) >> >> > - rgbFormat = MESA_FORMAT_B5G6R5_UNORM; >> >> > - else if (mesaVis->sRGBCapable) >> >> > - rgbFormat = MESA_FORMAT_B8G8R8A8_SRGB; >> >> > - else if (mesaVis->alphaBits == 0) >> >> > - rgbFormat = MESA_FORMAT_B8G8R8X8_UNORM; >> >> > - else { >> >> > - rgbFormat = MESA_FORMAT_B8G8R8A8_SRGB; >> >> > + if (mesaVis->redBits == 5) { >> >> > + rgbFormat = mesaVis->redMask == 0x1f ? >> >> > MESA_FORMAT_R5G6B5_UNORM >> >> > + : >> >> > MESA_FORMAT_B5G6R5_UNORM; >> >> > + } else if (mesaVis->alphaBits == 0) { >> >> > + rgbFormat = mesaVis->redMask == 0xff ? >> >> > MESA_FORMAT_R8G8B8X8_UNORM >> >> > + : >> >> > MESA_FORMAT_B8G8R8X8_UNORM; >> >> > + } else if (mesaVis->sRGBCapable) { >> >> > + rgbFormat = mesaVis->redMask == 0xff ? >> >> > MESA_FORMAT_R8G8B8A8_SRGB >> >> > + : >> >> > MESA_FORMAT_B8G8R8A8_SRGB; >> >> > fb->Visual.sRGBCapable = true; >> >> > + } else { >> >> > + rgbFormat = mesaVis->redMask == 0xff ? >> >> > MESA_FORMAT_R8G8B8A8_UNORM >> >> > + : >> >> > MESA_FORMAT_B8G8R8A8_UNORM; >> >> > } >> >> > >> >> > /* setup the hardware-based renderbuffers */ >> >> > -- >> >> > 2.8.0.rc3.226.g39d4020 >> >> > >> >> > _______________________________________________ >> >> > mesa-dev mailing list >> >> > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> >> >> > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> >> > >> > > > </div></div></blockquote></div> </div>