[Mesa-dev] [PATCH] anv: fix alphaToCoverage when there is no color attachment

[Samuel Iglesias Gonsálvez]

Still unreviewed.

Sam

On Thu, 2019-02-21 at 12:08 +0100, Samuel Iglesias Gonsálvez wrote:
> CL#3532 added a test for alpha to coverage without a color
> attachment.
> First the test draws a primitive with alpha 0 and a subpass with only
> a depth buffer. No writes to a depth buffer are expected. Then a
> second draw with a color buffer and the same depth buffer is done to
> verify the depth buffer still has the original clear values.
> 
> This behavior is not explicitly forbidden by the Vulkan spec, so
> it seems it is allowed.
> 
> When there is no color attachment for a given output, we discard it
> so at the end we have an FS assembly like:
> 
> Native code for unnamed fragment shader (null)
> SIMD16 shader: 1 instructions. 0 loops. 4 cycles. 0:0 spills:fills.
> Promoted 0 constants. Compacted 16 to 16 bytes (0%)
>    START B0 (4 cycles)
> sendc(16)       null<1>UW       g120<0,1,0>F    0x90031000
> 
> render MsgDesc: RT write SIMD16 LastRT Surface = 0 mlen 8 rlen 0 {
> align1 1H EOT };
> 
> As g120 is not initialized, we see random writes to the depth buffer
> due to the alphaToCoverage enablement. This commit fixes that by
> keeping the output and creating a null render target for it.
> 
> Fixes tests:
> 
> dEQP-VK.pipeline.multisample.alpha_to_coverage_no_color_attachment.*
> 
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>  src/intel/vulkan/anv_pipeline.c | 35 +++++++++++++++++++++++++----
> ----
>  1 file changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/src/intel/vulkan/anv_pipeline.c
> b/src/intel/vulkan/anv_pipeline.c
> index e2024212bd9..70a958bf3a8 100644
> --- a/src/intel/vulkan/anv_pipeline.c
> +++ b/src/intel/vulkan/anv_pipeline.c
> @@ -792,7 +792,9 @@ anv_pipeline_compile_gs(const struct brw_compiler
> *compiler,
>  
>  static void
>  anv_pipeline_link_fs(const struct brw_compiler *compiler,
> -                     struct anv_pipeline_stage *stage)
> +                     struct anv_pipeline_stage *stage,
> +                     const struct anv_subpass *subpass,
> +                     const VkPipelineMultisampleStateCreateInfo
> *ms_info)
>  {
>     unsigned num_rts = 0;
>     const int max_rt = FRAG_RESULT_DATA7 - FRAG_RESULT_DATA0 + 1;
> @@ -843,12 +845,28 @@ anv_pipeline_link_fs(const struct brw_compiler
> *compiler,
>        const unsigned rt = var->data.location - FRAG_RESULT_DATA0;
>        if (rt >= MAX_RTS ||
>            !(stage->key.wm.color_outputs_valid & (1 << rt))) {
> -         /* Unused or out-of-bounds, throw it away */
> -         deleted_output = true;
> -         var->data.mode = nir_var_function_temp;
> -         exec_node_remove(&var->node);
> -         exec_list_push_tail(&impl->locals, &var->node);
> -         continue;
> +         if (rt == 0 && ms_info && ms_info->alphaToCoverageEnable &&
> +             subpass->depth_stencil_attachment && rt_to_bindings[rt]
> == -1) {
> +            /* Don't throw away the unused output, because we needed
> it for
> +             * calculate correctly the alpha to coverage samples
> when there
> +             * is no color buffer attached at location 0.
> +             */
> +            rt_to_bindings[rt] = num_rts;
> +            /* We need a null render target */
> +            rt_bindings[rt_to_bindings[rt]] = (struct
> anv_pipeline_binding) {
> +               .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS,
> +               .binding = 0,
> +               .index = UINT32_MAX,
> +            };
> +            num_rts++;
> +         } else {
> +            /* Unused or out-of-bounds, throw it away */
> +            deleted_output = true;
> +            var->data.mode = nir_var_function_temp;
> +            exec_node_remove(&var->node);
> +            exec_list_push_tail(&impl->locals, &var->node);
> +            continue;
> +         }
>        }
>  
>        /* Give it the new location */
> @@ -1075,7 +1093,8 @@ anv_pipeline_compile_graphics(struct
> anv_pipeline *pipeline,
>           anv_pipeline_link_gs(compiler, &stages[s], next_stage);
>           break;
>        case MESA_SHADER_FRAGMENT:
> -         anv_pipeline_link_fs(compiler, &stages[s]);
> +         anv_pipeline_link_fs(compiler, &stages[s],
> +                              pipeline->subpass, info-
> >pMultisampleState);
>           break;
>        default:
>           unreachable("Invalid graphics shader stage");
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20190304/9799904a/attachment.sig>