[Mesa-dev] [PATCH 6/6] i965/msaa: Fix centroid interpolation of unlit pixels.

[Paul Berry]

i965 hardware doesn't do centroid interpolation correctly on unlit
pixels, causing incorrect values for derivatives near triangle edges.
To work around this problem, after doing centroid interpolation, we
replace the centroid-interpolated values for unlit pixels with
non-centroid-interpolated values (which are interpolated at pixel
centers).  This produces correct rendering at the expense of a slight
increase in shader execution time.

I've conditioned the workaround with a runtime flag
(brw->needs_unlit_centroid_workaround) in the hopes that we won't need
it in future chip generations.

Fixes piglit tests "EXT_framebuffer_multisample/interpolation {2,4}
{centroid-deriv,centroid-deriv-disabled}".  All MSAA interpolation
tests pass now.
---
 src/mesa/drivers/dri/i965/brw_context.c |    4 ++++
 src/mesa/drivers/dri/i965/brw_context.h |    9 +++++++++
 src/mesa/drivers/dri/i965/brw_fs.cpp    |   12 ++++++++++++
 src/mesa/drivers/dri/i965/brw_wm.c      |   18 ++++++++++++++----
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
index f7073cd..72a7f7e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -296,6 +296,10 @@ brwCreateContext(int api,
       brw->has_negative_rhw_bug = true;
    }
 
+   if (intel->gen <= 7) {
+      brw->needs_unlit_centroid_workaround = true;
+   }
+
    brw->prim_restart.in_progress = false;
    brw->prim_restart.enable_cut_index = false;
 
diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
index 6e0e1ad..e59252d 100644
--- a/src/mesa/drivers/dri/i965/brw_context.h
+++ b/src/mesa/drivers/dri/i965/brw_context.h
@@ -724,6 +724,15 @@ struct brw_context
    bool has_pln;
    bool precompile;
 
+   /**
+    * Some versions of Gen hardware don't do centroid interpolation correctly
+    * on unlit pixels, causing incorrect values for derivatives near triangle
+    * edges.  Enabling this flag causes the fragment shader to use
+    * non-centroid interpolation for unlit pixels, at the expense of two extra
+    * fragment shader instructions.
+    */
+   bool needs_unlit_centroid_workaround;
+
    struct {
       struct brw_state_flags dirty;
    } state;
diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
index 6cef08a..a9ef96c 100644
--- a/src/mesa/drivers/dri/i965/brw_fs.cpp
+++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
@@ -506,6 +506,18 @@ fs_visitor::emit_general_interpolation(ir_variable *ir)
 		  struct brw_reg interp = interp_reg(location, k);
                   emit_linterp(attr, fs_reg(interp), interpolation_mode,
                                ir->centroid);
+                  if (brw->needs_unlit_centroid_workaround && ir->centroid) {
+                     /* Get the pixel/sample mask into f0 so that we know
+                      * which pixels are lit.  Then, for each channel that is
+                      * unlit, replace the centroid data with non-centroid
+                      * data.
+                      */
+                     emit(FS_OPCODE_GET_MASK, attr);
+                     fs_inst *inst = emit_linterp(attr, fs_reg(interp),
+                                                  interpolation_mode, false);
+                     inst->predicated = true;
+                     inst->predicate_inverse = true;
+                  }
 		  if (intel->gen < 6) {
 		     emit(BRW_OPCODE_MUL, attr, attr, this->pixel_w);
 		  }
diff --git a/src/mesa/drivers/dri/i965/brw_wm.c b/src/mesa/drivers/dri/i965/brw_wm.c
index 4a7225c..ae6c6bf 100644
--- a/src/mesa/drivers/dri/i965/brw_wm.c
+++ b/src/mesa/drivers/dri/i965/brw_wm.c
@@ -130,7 +130,8 @@ brw_wm_non_glsl_emit(struct brw_context *brw, struct brw_wm_compile *c)
  * (see enum brw_wm_barycentric_interp_mode) is needed by the fragment shader.
  */
 static unsigned
-brw_compute_barycentric_interp_modes(bool shade_model_flat,
+brw_compute_barycentric_interp_modes(struct brw_context *brw,
+                                     bool shade_model_flat,
                                      const struct gl_fragment_program *fprog)
 {
    unsigned barycentric_interp_modes = 0;
@@ -154,11 +155,18 @@ brw_compute_barycentric_interp_modes(bool shade_model_flat,
       if (attr == FRAG_ATTRIB_WPOS || attr == FRAG_ATTRIB_FACE)
          continue;
 
+      /* Determine the set (or sets) of barycentric coordinates needed to
+       * interpolate this variable.  Note that when
+       * brw->needs_unlit_centroid_workaround is set, centroid interpolation
+       * uses PIXEL interpolation for unlit pixels and CENTROID interpolation
+       * for lit pixels, so we need both sets of barycentric coordinates.
+       */
       if (interp_qualifier == INTERP_QUALIFIER_NOPERSPECTIVE) {
          if (is_centroid) {
             barycentric_interp_modes |=
                1 << BRW_WM_NONPERSPECTIVE_CENTROID_BARYCENTRIC;
-         } else {
+         }
+         if (!is_centroid || brw->needs_unlit_centroid_workaround) {
             barycentric_interp_modes |=
                1 << BRW_WM_NONPERSPECTIVE_PIXEL_BARYCENTRIC;
          }
@@ -168,7 +176,8 @@ brw_compute_barycentric_interp_modes(bool shade_model_flat,
          if (is_centroid) {
             barycentric_interp_modes |=
                1 << BRW_WM_PERSPECTIVE_CENTROID_BARYCENTRIC;
-         } else {
+         }
+         if (!is_centroid || brw->needs_unlit_centroid_workaround) {
             barycentric_interp_modes |=
                1 << BRW_WM_PERSPECTIVE_PIXEL_BARYCENTRIC;
          }
@@ -289,7 +298,8 @@ bool do_wm_prog(struct brw_context *brw,
    brw_init_compile(brw, &c->func, c);
 
    c->prog_data.barycentric_interp_modes =
-      brw_compute_barycentric_interp_modes(c->key.flat_shade, &fp->program);
+      brw_compute_barycentric_interp_modes(brw, c->key.flat_shade,
+                                           &fp->program);
 
    if (prog && prog->_LinkedShaders[MESA_SHADER_FRAGMENT]) {
       if (!brw_wm_fs_emit(brw, c, prog))
-- 
1.7.7.6