[Mesa-dev] [PATCH v2] dri/i965: fix incorrect rgbFormat in intelCreateBuffer().

[Haixia Shi]

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.

On Tue, Apr 5, 2016 at 1:52 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:

> On Tue, Apr 5, 2016 at 4:33 PM, Haixia Shi <hshi at chromium.org> 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 <hshi at chromium.org>
> > Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>
> > Cc: kenneth.w.graunke at intel.com
> >
> > 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
> > mesa-dev at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160405/712ff1fc/attachment.html>