[Mesa-dev] [PATCH] i965/gen6: Fix multisample resolve blits for luminance/intensity 32F formats.

[Kenneth Graunke]

On 11/27/2013 08:37 AM, Paul Berry wrote:
> On gen6, multisamble resolve blits use the SAMPLE message to blend
> together the 4 samples for each texel.  For some reason, SAMPLE
> doesn't blend together the proper samples when the source format is
> L32_FLOAT or I32_FLOAT, resulting in blocky artifacts.
> 
> To work around this problem, sample from the source surface using
> R32_FLOAT.  This shouldn't affect rendering correctness, because when
> doing these resolve blits, the destination format is R32_FLOAT, so the
> channel replication done by L32_FLOAT and I32_FLOAT is unnecessary.
> 
> Fixes piglit tests on Sandy Bridge:
> - spec/ARB_texture_float/multisample-formats 2 GL_ARB_texture_float
> - spec/ARB_texture_float/multisample-formats 4 GL_ARB_texture_float
> 
> No piglit regressions on Sandy Bridge.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70601
> 
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>  src/mesa/drivers/dri/i965/brw_blorp_blit.cpp | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> index 2d2edc1..9c48eac 100644
> --- a/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_blorp_blit.cpp
> @@ -2099,6 +2099,21 @@ brw_blorp_blit_params::brw_blorp_blit_params(struct brw_context *brw,
>        src.brw_surfaceformat = dst.brw_surfaceformat;
>     }
>  
> +   /* When doing a multisample resolve of a GL_LUMINANCE32F or GL_INTENSITY32F
> +    * texture, the above code configures the source format for L32_FLOAT or
> +    * I32_FLOAT, and the destination format for R32_FLOAT.  On Sandy Bridge,
> +    * the SAMPLE message appears to handle multisampled L32_FLOAT and
> +    * I32_FLOAT textures incorrectly, resulting in blocky artifacts.  So work
> +    * around the problem by using a source format of R32_FLOAT.  This
> +    * shouldn't affect rendering correctness, since the destination format is
> +    * R32_FLOAT, so only the contents of the red channel matters.
> +    */
> +   if (brw->gen == 6 && src.num_samples > 1 && dst.num_samples <= 1 &&
> +       src_mt->format == dst_mt->format &&
> +       dst.brw_surfaceformat == BRW_SURFACEFORMAT_R32_FLOAT) {
> +      src.brw_surfaceformat = dst.brw_surfaceformat;
> +   }
> +
>     use_wm_prog = true;
>     memset(&wm_prog_key, 0, sizeof(wm_prog_key));

I wasn't sure whether we should apply this more broadly, since any L or
I -> R blit could sensibly be overridden.  But I kind of like how you've
isolated the exact case that's broken and needs special treatment.

Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>