[Mesa-dev] [PATCH 1/3] glsl/linker: fix varying packing for non-flat integer varyings.

[Paul Berry]

Commit dfb57e7 (glsl: Fix error checking on "flat" keyword to match
GLSL ES 3.00, GLSL 1.50) relaxed the rules for integral varyings: they
only need to be declared as "flat" if they are a fragment shader
inputs.  This allowed for the possibility of a vertex shader output
being a non-flat integer, provided that it was not matched to a
fragment shader input.  A non-contrived situation where this might
arise is if a vertex shader generates some integral outputs which are
consumed by tranform feedback, but not by the fragment shader.

Unfortunately, lower_packed_varyings assumes that *all* integral
varyings are flat, regardless of whether they are consumed by the
fragment shader.  As a result, attempting to create a non-flat
integral vertex output of a size that required packing (i.e. a size
other than ivec4 or uvec4) would cause an assertion failure in
lower_packed_varyings.

This patch prevents the assertion failure by forcing vertex shader
outputs to be "flat" whenever they are not consumed by the fragment
shader.  This should have no effect on rendering since the "flat"
keyword only affects the behaviour of fragment shader inputs.

Fixes piglit test "spec/EXT_transform_feedback/nonflat-integral".

NOTE: This is a candidate for the 9.1 release branch.
---
 src/glsl/link_varyings.cpp | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/src/glsl/link_varyings.cpp b/src/glsl/link_varyings.cpp
index 04c9fdd..431d8fd 100644
--- a/src/glsl/link_varyings.cpp
+++ b/src/glsl/link_varyings.cpp
@@ -656,6 +656,10 @@ varying_matches::~varying_matches()
  * If \c producer_var has already been paired up with a consumer_var, or
  * producer_var is part of fixed pipeline functionality (and hence already has
  * a location assigned), this function has no effect.
+ *
+ * Note: as a side effect this function may change the interpolation type of
+ * \c producer_var, but only when the change couldn't possibly affect
+ * rendering.
  */
 void
 varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
@@ -668,6 +672,21 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
       return;
    }
 
+   if (consumer_var == NULL) {
+      /* Since there is no consumer_var, the interpolation type of this
+       * varying cannot possibly affect rendering.  Also, since the GL spec
+       * only requires integer varyings to be "flat" when they are fragment
+       * shader inputs, it is possible that this variable is non-flat and is
+       * (or contains) an integer.
+       *
+       * lower_packed_varyings requires all integer varyings to flat,
+       * regardless of where they appear.  We can trivially satisfy that
+       * requirement by changing the interpolation type to flat here.
+       */
+      producer_var->centroid = false;
+      producer_var->interpolation = INTERP_QUALIFIER_FLAT;
+   }
+
    if (this->num_matches == this->matches_capacity) {
       this->matches_capacity *= 2;
       this->matches = (match *)
-- 
1.8.2