[Mesa-dev] [PATCH 2/3] i915: Blit RGBX<->RGBA drawpixels

[Kenneth Graunke]

On Friday, June 05, 2015 03:14:30 PM Chris Wilson wrote:
> The blitter already has code to accommodate filling in the alpha channel
> for BGRX destination formats, so expand this to also allow filling the
> alpha channgel in RGBX formats.
> 
> More importantly for the next patch is moving the test into its own
> function for the purpose of exporting the check to the callers.
> 
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Jason Ekstrand <jason at jlekstrand.net>
> Cc: Alexander Monakov <amonakov at gmail.com>
> Cc: Kristian Høgsberg <krh at bitplanet.net>
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/intel_blit.c | 38 +++++++++++++++++++++++++++-------
>  1 file changed, 30 insertions(+), 8 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/intel_blit.c b/src/mesa/drivers/dri/i965/intel_blit.c
> index 360bfe8..02666dc 100644
> --- a/src/mesa/drivers/dri/i965/intel_blit.c
> +++ b/src/mesa/drivers/dri/i965/intel_blit.c
> @@ -138,6 +138,31 @@ static int blt_pitch(struct intel_mipmap_tree *mt)
>     return pitch;
>  }
>  
> +static bool blt_compatible_formats(mesa_format src, mesa_format dst)

static bool
blt_compatible_formats(mesa_format src, mesa_format dst)

> +{
> +   /* The BLT doesn't handle sRGB conversion */
> +   assert(src == _mesa_get_srgb_format_linear(src));
> +   assert(dst == _mesa_get_srgb_format_linear(dst));
> +
> +   /* No swizzle or format conversions possible, except... */
> +   if (src == dst)
> +      return true;
> +
> +   /*
> +    * ... we can either discard the alpha channel when going from A->X,

/* ...we can either discard the alpha channel when going from A->X,

> +    * or we can fill the alpha channel with 0xff when going from X->A
> +    */
> +   if (src == MESA_FORMAT_B8G8R8A8_UNORM || src == MESA_FORMAT_B8G8R8X8_UNORM)
> +      return (dst == MESA_FORMAT_B8G8R8A8_UNORM ||
> +              dst == MESA_FORMAT_B8G8R8X8_UNORM);
> +
> +   if (src == MESA_FORMAT_R8G8B8A8_UNORM || src == MESA_FORMAT_R8G8B8X8_UNORM)
> +      return (dst == MESA_FORMAT_R8G8B8A8_UNORM ||
> +              dst == MESA_FORMAT_R8G8B8X8_UNORM);
> +
> +   return false;
> +}
> +
>  /**
>   * Implements a rectangular block transfer (blit) of pixels between two
>   * miptrees.
> @@ -180,11 +205,7 @@ intel_miptree_blit(struct brw_context *brw,
>      * the X channel don't matter), and XRGB8888 to ARGB8888 by setting the A
>      * channel to 1.0 at the end.
>      */
> -   if (src_format != dst_format &&
> -      ((src_format != MESA_FORMAT_B8G8R8A8_UNORM &&
> -        src_format != MESA_FORMAT_B8G8R8X8_UNORM) ||
> -       (dst_format != MESA_FORMAT_B8G8R8A8_UNORM &&
> -        dst_format != MESA_FORMAT_B8G8R8X8_UNORM))) {
> +   if (!blt_compatible_formats(src_format, dst_format)) {
>        perf_debug("%s: Can't use hardware blitter from %s to %s, "
>                   "falling back.\n", __func__,
>                   _mesa_get_format_name(src_format),
> @@ -269,12 +290,13 @@ intel_miptree_blit(struct brw_context *brw,
>        return false;
>     }
>  
> -   if (src_mt->format == MESA_FORMAT_B8G8R8X8_UNORM &&
> -       dst_mt->format == MESA_FORMAT_B8G8R8A8_UNORM) {
> +   /* XXX This could be done in a single pass using XY_FULL_MONO_PATTERN_BLT */
> +   if (src_format != dst_format &&
> +       (dst_format == MESA_FORMAT_B8G8R8X8_UNORM ||
> +        dst_format == MESA_FORMAT_R8G8B8X8_UNORM))
>        intel_miptree_set_alpha_to_one(brw, dst_mt,
>                                       dst_x, dst_y,
>                                       width, height);
> -   }
>  

As Alexander noted, this is backwards.  How about this as a solution:

   if (_mesa_get_format_bits(src_format, GL_ALPHA_BITS) == 0 &&
       _mesa_get_format_bits(dst_format, GL_ALPHA_BITS) > 0) {
      intel_miptree_set_alpha_to_one(brw, dst_mt,
                                     dst_x, dst_y,
                                     width, height);
   }

That reduces the number of places that need to enumerate formats, and
expresses the condition pretty clearly - if we don't have alpha data,
but need it, fill it in.

With that change, the series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

Thanks a lot for fixing this, Chris!

> -   }
>     return true;
>  }
> 
-------------- 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: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150607/44ef98f7/attachment.sig>