Ilia: please review patch v4. I have added a separate patch to fix the GLES3 sRGB workaround, which fixes the colorspace issue.
On Tue, Apr 5, 2016 at 5:20 PM, Ilia Mirkin <[email protected]> wrote: > Sounds right. That's glEnable/glDisable(GL_FRAMEBUFFER_SRGB). It > defaults to off for desktop GL, and on for GL ES (why? no clue.) > > On Tue, Apr 5, 2016 at 8:11 PM, Haixia Shi <[email protected]> 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 <[email protected]> > wrote: > >> > >> On Tue, Apr 5, 2016 at 4:57 PM, Haixia Shi <[email protected]> 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 <[email protected]> > >> > wrote: > >> >> > >> >> On Tue, Apr 5, 2016 at 4:33 PM, Haixia Shi <[email protected]> > 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 <[email protected]> > >> >> > Reviewed-by: Stéphane Marchesin <[email protected]> > >> >> > Cc: [email protected] > >> >> > > >> >> > 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 > >> >> > [email protected] > >> >> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > >> > > >> > > > > > >
_______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
