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

[Kenneth Graunke]

On Thursday, April 7, 2016 11:05:09 AM PDT Haixia Shi 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.
> 
> v2: reformat code to stay within 80 character per line limit.
> v3: just fix the byte order problem first and investigate SRGB later.
> v4: rebased on top of the GLES3 sRGB workaround fix.
> v5: rebased on top of the GLES3 sRGB workaround fix v2.
> 
> 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..bc8c5d3 100644
> --- a/src/mesa/drivers/dri/i965/intel_screen.c
> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
> @@ -1000,14 +1000,18 @@ 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->sRGBCapable) {
> +      rgbFormat = mesaVis->redMask == 0xff ? MESA_FORMAT_R8G8B8A8_SRGB
> +                                           : MESA_FORMAT_B8G8R8A8_SRGB;
> +   } else if (mesaVis->alphaBits == 0) {
> +      rgbFormat = mesaVis->redMask == 0xff ? MESA_FORMAT_R8G8B8X8_UNORM
> +                                           : MESA_FORMAT_B8G8R8X8_UNORM;
> +   } else {
> +      rgbFormat = mesaVis->redMask == 0xff ? MESA_FORMAT_R8G8B8A8_SRGB
> +                                           : MESA_FORMAT_B8G8R8A8_SRGB;
>        fb->Visual.sRGBCapable = true;
>     }
>  
> 

As Kristian mentioned, this shouldn't ever happen with upstream code
today - we never expose any visuals with the RGBA byte order instead of
BGRA.  But Kristian seemed to think Android would hit this.

Regardless, it looks correct and seems reasonable, so this patch is.
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Kristian, are you OK with pushing this patch?  Should I add an R-b from
you as well?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160412/a2126612/attachment.sig>