[Mesa-dev] [PATCH] llvmpipe: fix stencil issues

[sroland at vmware.com]

From: Roland Scheidegger <sroland at vmware.com>

Two (somewhat related) issues:
1) We did mask checks between depth/stencil testing and depth/stencil write.
This meant that if the depth/stencil test killed off all fragments we never
actually wrote the new stencil value. This issue affected all early/late
test/write combinations.
2) We actually did early depth/stencil test and late depth/stencil write even
when the shader could kill the fragment (alpha test or discard). Since it
matters for the new stencil value if the fragment is killed by depth/stencil
test or by the shader (in which case it will not reach the depth/stencil test)
this simply cannot work.
So fix these issues by moving the mask check after depth/stencil write (only
for early write it would work for late write too but probably not worth the
mask check there) and disable early depth test when it can't work correctly.
This addresses https://bugs.freedesktop.org/show_bug.cgi?id=41787 though
replaying the trace it still looks somewhat wrong to me, so maybe more bugs...
Verified this fixes affected piglit tests (glean stencil2 and some from hiz
group) if the simple_shader optimization in generate_fs_loop() is forced to
false (otherwise we skip mask checks hence don't hit issue 1 - I don't think
there's anything in piglit which would exhibit issue 2).
---
 src/gallium/drivers/llvmpipe/lp_bld_depth.c |   31 ++++++++++-----------------
 src/gallium/drivers/llvmpipe/lp_state_fs.c  |   20 ++++++++++++-----
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_bld_depth.c b/src/gallium/drivers/llvmpipe/lp_bld_depth.c
index 2376ca7..afc2d9d 100644
--- a/src/gallium/drivers/llvmpipe/lp_bld_depth.c
+++ b/src/gallium/drivers/llvmpipe/lp_bld_depth.c
@@ -1096,23 +1096,18 @@ lp_build_depth_stencil_test(struct gallivm_state *gallivm,
                                   stencil_shift, "");
 
    /* Finally, merge the z/stencil values */
-   if ((depth->enabled && depth->writemask) ||
-       (stencil[0].enabled && (stencil[0].writemask ||
-                               (stencil[1].enabled && stencil[1].writemask)))) {
-
-      if (format_desc->block.bits <= 32) {
-         if (have_z && have_s)
-            *z_value = LLVMBuildOr(builder, z_dst, stencil_vals, "");
-         else if (have_z)
-            *z_value = z_dst;
-         else
-            *z_value = stencil_vals;
-         *s_value = *z_value;
-      }
-      else {
+   if (format_desc->block.bits <= 32) {
+      if (have_z && have_s)
+         *z_value = LLVMBuildOr(builder, z_dst, stencil_vals, "");
+      else if (have_z)
          *z_value = z_dst;
-         *s_value = stencil_vals;
-      }
+      else
+         *z_value = stencil_vals;
+      *s_value = *z_value;
+   }
+   else {
+      *z_value = z_dst;
+      *s_value = stencil_vals;
    }
 
    if (s_pass_mask)
@@ -1120,9 +1115,5 @@ lp_build_depth_stencil_test(struct gallivm_state *gallivm,
 
    if (depth->enabled && stencil[0].enabled)
       lp_build_mask_update(mask, z_pass);
-
-   if (do_branch)
-      lp_build_mask_check(mask);
-
 }
 
diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c b/src/gallium/drivers/llvmpipe/lp_state_fs.c
index 1dfc75a..ae63615 100644
--- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
+++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
@@ -266,13 +266,20 @@ generate_fs_loop(struct gallivm_state *gallivm,
       assert(zs_format_desc);
 
       if (!shader->info.base.writes_z) {
-         if (key->alpha.enabled || shader->info.base.uses_kill)
+         if (key->alpha.enabled || shader->info.base.uses_kill) {
             /* With alpha test and kill, can do the depth test early
              * and hopefully eliminate some quads.  But need to do a
              * special deferred depth write once the final mask value
-             * is known.
+             * is known. This only works though if there's either no
+             * stencil test or the stencil value isn't written.
              */
-            depth_mode = EARLY_DEPTH_TEST | LATE_DEPTH_WRITE;
+            if (key->stencil[0].enabled && (key->stencil[0].writemask ||
+                                            (key->stencil[1].enabled &&
+                                             key->stencil[1].writemask)))
+               depth_mode = LATE_DEPTH_TEST | LATE_DEPTH_WRITE;
+            else
+               depth_mode = EARLY_DEPTH_TEST | LATE_DEPTH_WRITE;
+         }
          else
             depth_mode = EARLY_DEPTH_TEST | EARLY_DEPTH_WRITE;
       }
@@ -281,9 +288,9 @@ generate_fs_loop(struct gallivm_state *gallivm,
       }
 
       if (!(key->depth.enabled && key->depth.writemask) &&
-          !((key->stencil[0].enabled && (key->stencil[0].writemask ||
+          !(key->stencil[0].enabled && (key->stencil[0].writemask ||
                                         (key->stencil[1].enabled &&
-                                         key->stencil[1].writemask)))))
+                                         key->stencil[1].writemask))))
          depth_mode &= ~(LATE_DEPTH_WRITE | EARLY_DEPTH_WRITE);
    }
    else {
@@ -360,6 +367,9 @@ generate_fs_loop(struct gallivm_state *gallivm,
                                                NULL, NULL, NULL, loop_state.counter,
                                                depth_ptr, depth_stride,
                                                z_value, s_value);
+
+         if (!simple_shader && key->stencil[0].enabled)
+            lp_build_mask_check(&mask);
       }
    }
 
-- 
1.7.9.5