[Mesa-dev] [PATCH] i965: Fix clear code for ignoring colormask for XRGB formats on Gen9+.

[Iago Toral]

On Wed, 2016-04-20 at 18:38 -0700, Kenneth Graunke wrote:
> In commit cda886a4851ab767fba40e8474d6fa8190347e4f, Neil made us stop
> advertising RGBX formats on Gen9+, as the hardware apparently no longer
> has working fast clear support for those formats.  Instead, we just
> fall back to RGBA formats, and use SCS to override alpha to 1.0.
> 
> This is fine, but had one unintended side effect: it made us fall back
> to slow clears when the color mask disables alpha.  Normally, we ignore
> the color mask for non-existent channels.  This includes alpha for XRGB
> formats as writing garbage to the X channel is harmless.  But, now that
> we use RGBA, we think there's a real alpha channel, and can't do the
> optimization.
> 
> To hack around this, check if _BaseFormat is GL_RGB and ignore alpha.
> 
> Improves WebGL Aquarium performance on Skylake GT3e by about 50%
> by letting it use repclears instead of slow clears.
> 
> Cc: Ben Widawsky <ben at bwidawsk.net>
> Cc: Topi Pohjolainen <topi.pohjolainen at intel.com>
> Cc: mesa-stable at lists.freedesktop.org
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  src/mesa/drivers/dri/i965/brw_meta_fast_clear.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> index 1fb5dc8..5d89294 100644
> --- a/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> +++ b/src/mesa/drivers/dri/i965/brw_meta_fast_clear.c
> @@ -645,6 +645,7 @@ brw_meta_fast_clear(struct brw_context *brw, struct gl_framebuffer *fb,
>        GLubyte *color_mask = ctx->Color.ColorMask[buf];
>        for (int i = 0; i < 4; i++) {
>           if (_mesa_format_has_color_component(irb->mt->format, i) &&
> +             !(i == 3 && irb->Base.Base._BaseFormat == GL_RGB) &&

I was wondering if we could generalize the fix so we just ignore any
channels that are not present in the base format. Something like this:

i <= _mesa_base_format_component_count(irb->Base.Base._BaseFormat)

Basically, we do the same check we do against the mesa format, but with
the base format.

What do you think?

Either way,
Reviewed-by: Iago Toral Quiroga <itoral at igalia.com>

>               !color_mask[i]) {
>              perf_debug("Falling back to plain clear on %dx%d buffer because of color mask\n",
>                         irb->mt->logical_width0, irb->mt->logical_height0);