[v2] anv: fix alphaToCoverage when there is no color attachment

Submitted by Iago Toral Quiroga on May 2, 2019, 10:46 a.m.

Details

Message ID 20190502104652.10013-1-itoral@igalia.com
State New
Headers show
Series "anv: fix alphaToCoverage when there is no color attachment" ( rev: 2 ) in Mesa

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga May 2, 2019, 10:46 a.m.
From: Samuel Iglesias Gonsálvez <siglesias@igalia.com>

There tests in CTS for 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 patch fixes that by
keeping the output in that case.

v2:
 - No need to create a null render target, the driver is already
   doing that (Jason)
 - Simplified code a bit (Iago)

Fixes the following CTS tests:
dEQP-VK.pipeline.multisample.alpha_to_coverage_no_color_attachment.*

Signed-off-by: Samuel Iglesias Gonsálvez <siglesias@igalia.com>
Signed-off-by: Iago Toral Quiroga <itoral@igalia.com>
---
 src/intel/vulkan/anv_pipeline.c | 25 ++++++++++++++++++-------
 1 file changed, 18 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
index b9c9bfd7598..07f1a939e43 100644
--- a/src/intel/vulkan/anv_pipeline.c
+++ b/src/intel/vulkan/anv_pipeline.c
@@ -808,7 +808,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,
+                     bool has_depth_stencil_att,
+                     bool has_alpha_to_coverage)
 {
    unsigned num_rts = 0;
    const int max_rt = FRAG_RESULT_DATA7 - FRAG_RESULT_DATA0 + 1;
@@ -859,11 +861,17 @@  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);
+         /* Unused or out-of-bounds, throw it away. The exception is depth-only
+          * rendering with alphaToCoverage, as in this case we need to keep the
+          * fragment output in location 0, which we will bind later to a null
+          * render target.
+          */
+         if (rt != 0 || !has_alpha_to_coverage || !has_depth_stencil_att) {
+            deleted_output = true;
+            var->data.mode = nir_var_function_temp;
+            exec_node_remove(&var->node);
+            exec_list_push_tail(&impl->locals, &var->node);
+         }
          continue;
       }
 
@@ -1120,7 +1128,10 @@  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->depth_stencil_attachment,
+                              info->pMultisampleState &&
+                              info->pMultisampleState->alphaToCoverageEnable);
          break;
       default:
          unreachable("Invalid graphics shader stage");

Comments