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

[Kenneth Graunke]

On Thursday, April 21, 2016 10:00:57 AM PDT Iago Toral wrote:
> 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>

That seems like a good idea - I figured there had to be a function like
_mesa_base_format_component_count(), but I didn't find it when writing
the patch.

We'd need a little more code - _mesa_base_format_component_count() can
return -1 in some cases, so we'd need to override it to 4.  It's doable.

As far as I know, the only cases where this will be useful is XRGB and
maybe luminance/intensity.  For luminance/intensity, I think the right
solution is to use RED formats.  So, for all practical purposes, this
should be good enough.

Also, Topi's just about on the verge of replacing the meta fast clear
code with BLORP clears.  So this is all going away - I mostly wanted
a quick fix that could be backported to stable and the ChromeOS tree.

So I think I'll go with the patch as is, as it's tested, and we can
try and do this more generally in the new BLORP framework.

Thanks!
-------------- 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/20160421/cb519d14/attachment.sig>