[Mesa-dev] [PATCH 17/17] glsl: Don't monkey about with the interpolation modes

Ian Romanick idr at freedesktop.org
Wed Aug 24 22:12:43 UTC 2016


From: Ian Romanick <ian.d.romanick at intel.com>

Previously we'd munge the interpolation mode so that later checks in the
GLSL linker would pass.  The caused problems for similar checks in SSO
IO validation.  Instead, make the check smarter, use the same check in
both places, and don't modify the interpolation mode.

Fixes piglit tests:

    oes_geometry_shader/sso_validation/user-defined-gs-input-in-block
    oes_geometry_shader/sso_validation/user-defined-gs-input-not-in-block

v2: Don't apply none-smooth or smooth-none matching in desktop GL at
link time.  Suggested by Timothy Arceri.

v3: Simplify the checks.  Only allow SMOOTH/NONE matching when the
consumer stage is fragment.

Signed-off-by: Ian Romanick <ian.d.romanick at intel.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=96358
Cc: "12.0" <mesa-stable at lists.freedesktop.org>
Cc: Gregory Hainaut <gregory.hainaut at gmail.com>
Cc: Ilia Mirkin <imirkin at alum.mit.edu>
---
 src/compiler/glsl/ast_to_hir.cpp    | 11 -----------
 src/compiler/glsl/link_varyings.cpp | 39 +++++++++++++++++++++++++++++++++----
 src/compiler/glsl/link_varyings.h   |  6 ++++++
 src/mesa/main/shader_query.cpp      |  5 ++++-
 4 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index 581367b..19282ef 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3023,17 +3023,6 @@ interpret_interpolation_qualifier(const struct ast_type_qualifier *qual,
       interpolation = INTERP_MODE_NOPERSPECTIVE;
    else if (qual->flags.q.smooth)
       interpolation = INTERP_MODE_SMOOTH;
-   else if (state->es_shader &&
-            ((mode == ir_var_shader_in &&
-              state->stage != MESA_SHADER_VERTEX) ||
-             (mode == ir_var_shader_out &&
-              state->stage != MESA_SHADER_FRAGMENT)))
-      /* Section 4.3.9 (Interpolation) of the GLSL ES 3.00 spec says:
-       *
-       *    "When no interpolation qualifier is present, smooth interpolation
-       *    is used."
-       */
-      interpolation = INTERP_MODE_SMOOTH;
    else
       interpolation = INTERP_MODE_NONE;
 
diff --git a/src/compiler/glsl/link_varyings.cpp b/src/compiler/glsl/link_varyings.cpp
index 1bce3e0..0831492 100644
--- a/src/compiler/glsl/link_varyings.cpp
+++ b/src/compiler/glsl/link_varyings.cpp
@@ -201,6 +201,34 @@ anonymous_struct_type_matches(const glsl_type *output_type,
            to_match->record_compare(output_type);
 }
 
+bool
+interpolation_compatible(gl_shader_stage producer_stage,
+                         gl_shader_stage consumer_stage,
+                         enum glsl_interp_mode producer_interp,
+                         enum glsl_interp_mode consumer_interp)
+{
+   /* Section 4.5 (Interpolation Qualifiers) of the GLSL ES 3.20 spec says:
+    *
+    *    When no interpolation qualifier is present, smooth interpolation is
+    *    used.
+    *
+    * For the purpose of this function, use the ES rule even on desktop.  Link
+    * validation for desktop will use a stronger check, but SSO IO validation
+    * will use this check.  This allows using an undecorated output with a
+    * "smooth" decorated input, for example.  There is no specific
+    * justification for this in the OpenGL spec, but applications are known to
+    * depend on this.
+    */
+   if (producer_interp == consumer_interp)
+      return true;
+
+   return consumer_stage == MESA_SHADER_FRAGMENT &&
+          ((producer_interp == INTERP_MODE_NONE &&
+            consumer_interp == INTERP_MODE_SMOOTH) ||
+           (producer_interp == INTERP_MODE_SMOOTH &&
+            consumer_interp == INTERP_MODE_NONE));
+}
+
 /**
  * Validate the types and qualifiers of an output from one stage against the
  * matching input to another stage.
@@ -347,8 +375,12 @@ cross_validate_types_and_qualifiers(struct gl_shader_program *prog,
     *     qualifiers of variables of the same name do not match.
     *
     */
-   if (input->data.interpolation != output->data.interpolation &&
-       prog->Version < 440) {
+   if ((prog->IsES &&
+        !interpolation_compatible(producer_stage, consumer_stage,
+                                  glsl_interp_mode(output->data.interpolation),
+                                  glsl_interp_mode(input->data.interpolation))) ||
+       (!prog->IsES && prog->Version < 440 &&
+        input->data.interpolation != output->data.interpolation)) {
       linker_error(prog,
                    "%s shader output `%s' specifies %s "
                    "interpolation qualifier, "
@@ -1390,8 +1422,7 @@ varying_matches::record(ir_variable *producer_var, ir_variable *consumer_var)
       (producer_var->type->contains_integer() ||
        producer_var->type->contains_double());
 
-   if (needs_flat_qualifier ||
-       (consumer_stage != -1 && consumer_stage != MESA_SHADER_FRAGMENT)) {
+   if (needs_flat_qualifier) {
       /* Since this varying is not being consumed by the fragment shader, its
        * interpolation type varying cannot possibly affect rendering.
        * Also, this variable is non-flat and is (or contains) an integer
diff --git a/src/compiler/glsl/link_varyings.h b/src/compiler/glsl/link_varyings.h
index afce56e..075bb72 100644
--- a/src/compiler/glsl/link_varyings.h
+++ b/src/compiler/glsl/link_varyings.h
@@ -341,4 +341,10 @@ check_against_input_limit(struct gl_context *ctx,
                           gl_linked_shader *consumer,
                           unsigned num_explicit_locations);
 
+bool
+interpolation_compatible(gl_shader_stage producer_stage,
+                         gl_shader_stage consumer_stage,
+                         enum glsl_interp_mode producer_interp,
+                         enum glsl_interp_mode consumer_interp);
+
 #endif /* GLSL_LINK_VARYINGS_H */
diff --git a/src/mesa/main/shader_query.cpp b/src/mesa/main/shader_query.cpp
index 0eae39a..a0c764a 100644
--- a/src/mesa/main/shader_query.cpp
+++ b/src/mesa/main/shader_query.cpp
@@ -37,6 +37,7 @@
 #include "compiler/glsl/glsl_symbol_table.h"
 #include "compiler/glsl/ir.h"
 #include "compiler/glsl/program.h"
+#include "compiler/glsl/link_varyings.h"
 #include "program/hash_table.h"
 #include "util/strndup.h"
 
@@ -1611,7 +1612,9 @@ validate_io(struct gl_shader_program *producer,
          }
       }
 
-      if (producer_var->interpolation != consumer_var->interpolation) {
+      if (!interpolation_compatible(producer_stage, consumer_stage,
+                                    glsl_interp_mode(producer_var->interpolation),
+                                    glsl_interp_mode(consumer_var->interpolation))) {
          valid = false;
          goto out;
       }
-- 
2.5.5



More information about the mesa-dev mailing list