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

Submitted by Iago Toral Quiroga on May 3, 2019, 11:52 a.m.

Details

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

Not browsing as part of any series.

Commit Message

Iago Toral Quiroga May 3, 2019, 11:52 a.m.
From: Samuel Iglesias Gonsálvez <siglesias@igalia.com>

There are tests in CTS for alpha to coverage without a color attachment
that are failing. This happens because when we remove the shader color
outputs when we don't have a valid color attachment for them, but when
alpha to coverage is enabled we still want to preserve the the output
at location 0 since we need its alpha component for alpha to coverage.
In that case we will also need to create a null render target for RT 0.

v2:
  - We already create a null rt when we don't have any, so reuse that
    for this case (Jason)
  - Simplify the code a bit (Iago)

v3:
  - Take alpha to coverage from the key and don't tie this to depth-only
    rendering only, we want the same behavior if we have multiple render
    targets but the one at location 0 is not used. (Jason).
  - Rewrite commit message (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 | 48 +++++++++++++++++++++++++--------
 1 file changed, 37 insertions(+), 11 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/intel/vulkan/anv_pipeline.c b/src/intel/vulkan/anv_pipeline.c
index 20eab548fb2..f379dd2752e 100644
--- a/src/intel/vulkan/anv_pipeline.c
+++ b/src/intel/vulkan/anv_pipeline.c
@@ -818,15 +818,28 @@  anv_pipeline_link_fs(const struct brw_compiler *compiler,
    memset(rt_used, 0, sizeof(rt_used));
 
    /* Flag used render targets */
+   bool needs_null_rt_for_alpha_to_coverage = false;
    nir_foreach_variable_safe(var, &stage->nir->outputs) {
       if (var->data.location < FRAG_RESULT_DATA0)
          continue;
 
       const unsigned rt = var->data.location - FRAG_RESULT_DATA0;
-      /* Unused or out-of-bounds */
-      if (rt >= MAX_RTS || !(stage->key.wm.color_outputs_valid & (1 << rt)))
+      /* Out-of-bounds */
+      if (rt >= MAX_RTS)
          continue;
 
+      /* Unused */
+      if (!(stage->key.wm.color_outputs_valid & (1 << rt))) {
+         /* If this is the RT at location 0 and we have alpha to coverage
+          * enabled, we'll have to create a null render target and it must
+          * be at index 0.
+          */
+         if (rt == 0 && stage->key.wm.alpha_to_coverage)
+            needs_null_rt_for_alpha_to_coverage = true;
+
+         continue;
+      }
+
       const unsigned array_len =
          glsl_type_is_array(var->type) ? glsl_get_length(var->type) : 1;
       assert(rt + array_len <= max_rt);
@@ -835,7 +848,12 @@  anv_pipeline_link_fs(const struct brw_compiler *compiler,
          rt_used[rt + i] = true;
    }
 
-   /* Set new, compacted, location */
+   /* Make sure we leave the first RT slot available for alpha to coverage
+    * if we don't have a valid RT 0.
+    */
+   if (needs_null_rt_for_alpha_to_coverage)
+      num_rts = 1;
+
    for (unsigned i = 0; i < max_rt; i++) {
       if (!rt_used[i])
          continue;
@@ -857,11 +875,15 @@  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, unless it is the first
+          * RT and we have alpha to coverage.
+          */
+         if (rt != 0 || !stage->key.wm.alpha_to_coverage) {
+            deleted_output = true;
+            var->data.mode = nir_var_function_temp;
+            exec_node_remove(&var->node);
+            exec_list_push_tail(&impl->locals, &var->node);
+         }
          continue;
       }
 
@@ -873,14 +895,18 @@  anv_pipeline_link_fs(const struct brw_compiler *compiler,
    if (deleted_output)
       nir_fixup_deref_modes(stage->nir);
 
-   if (num_rts == 0) {
-      /* If we have no render targets, we need a null render target */
+   /* If we have no render targets or we need to create one for alpha to
+    * coverage, we need a null render target.
+    */
+   if (num_rts == 0 || needs_null_rt_for_alpha_to_coverage) {
       rt_bindings[0] = (struct anv_pipeline_binding) {
          .set = ANV_DESCRIPTOR_SET_COLOR_ATTACHMENTS,
          .binding = 0,
          .index = UINT32_MAX,
       };
-      num_rts = 1;
+
+      if (num_rts == 0)
+         num_rts = 1;
    }
 
    /* Now that we've determined the actual number of render targets, adjust

Comments