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 <sigles...@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");
signature.asc
Description: This is a digitally signed message part
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
