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

[Kenneth Graunke]

On Friday, April 22, 2016 11:49:03 AM PDT Topi Pohjolainen wrote:
> This is equivalent of 73b01e2711ff45a1f313d5372d6c8fa4fe55d4d2
> for blorp.
> 
> CC: Iago Toral Quiroga <itoral at igalia.com>
> CC: Kenneth Graunke <kenneth at whitecape.org>
> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_blorp_clear.cpp | 36 ++++++++++++++++++++
+------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp b/src/mesa/
drivers/dri/i965/brw_blorp_clear.cpp
> index 41ff2a5..f4513be 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_clear.cpp
> @@ -25,6 +25,7 @@
>  #include "main/blend.h"
>  #include "main/fbobject.h"
>  #include "main/renderbuffer.h"
> +#include "main/glformats.h"
>  
>  #include "util/ralloc.h"
>  
> @@ -135,6 +136,32 @@ 
brw_blorp_const_color_program::~brw_blorp_const_color_program()
>     ralloc_free(mem_ctx);
>  }
>  
> +static bool
> +set_write_disables(const struct intel_renderbuffer *irb, 
> +                   const GLubyte *color_mask, bool *color_write_disable)
> +{
> +   /* Format information in the renderbuffer represents the requirements
> +    * given by the client. There are cases where the backing miptree uses,
> +    * for example, RGBA to represent RGBX. Since the client is only 
expecting
> +    * RGB we can treat alpha as not used and write whatever we like into 
it.
> +    */
> +   const GLenum base_format = irb->Base.Base._BaseFormat;
> +   const mesa_format format = irb->mt->format;
> +   const int components = _mesa_base_format_component_count(base_format);
> +   bool disables = false;
> +
> +   assert(components > 0);
> +
> +   for (int i = 0; i < components; i++) {
> +     if (_mesa_format_has_color_component(format, i) && !color_mask[i]) {

I think the _mesa_format_has_color_component() stuff shouldn't be
necessary now that we're doing the equivalent thing with _BaseFormat.

So you could probably simplify this to:

      if (!color_mask[i]) {

Either way, series is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

It might also be nice to set the remaining color_write_disable[i] flags
to false explicitly here - they're set in the constructor, but this
would consolidate that logic.  *shrug*

> +         color_write_disable[i] = true;
> +         disables = true;
> +      }
> +   }
> +
> +   return disables;
> +}
> +
>  brw_blorp_clear_params::brw_blorp_clear_params(struct brw_context *brw,
>                                                 struct gl_framebuffer *fb,
>                                                 struct gl_renderbuffer *rb,
> @@ -185,13 +212,8 @@ brw_blorp_clear_params::brw_blorp_clear_params(struct 
brw_context *brw,
>     /* Constant color writes ignore everyting in blend and color calculator
>      * state.  This is not documented.
>      */
> -   for (int i = 0; i < 4; i++) {
> -      if (_mesa_format_has_color_component(irb->mt->format, i) &&
> -          !color_mask[i]) {
> -         color_write_disable[i] = true;
> -         wm_prog_key.use_simd16_replicated_data = false;
> -      }
> -   }
> +   if (set_write_disables(irb, color_mask, color_write_disable))
> +      wm_prog_key.use_simd16_replicated_data = false;
>  
>     if (irb->mt->fast_clear_state != INTEL_FAST_CLEAR_STATE_NO_MCS &&
>         !partial_clear && wm_prog_key.use_simd16_replicated_data &&
> 

-------------- 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/20160422/d89ad45d/attachment.sig>