[Mesa-dev] [PATCH V4 1/4] glsl: Compile error if fs defines conflicting qualifiers for gl_FragCoord

[Anuj Phogat]

GLSL 1.50 spec says:
   "If gl_FragCoord is redeclared in any fragment shader in a program,
    it must be redeclared in all the fragment shaders in that
    program that have a static use gl_FragCoord. All redeclarations of
    gl_FragCoord in all fragment shaders in a single program must
    have the same set of qualifiers."

This patch makes the glsl compiler to generate an error if we have a
fragment shader defined with conflicting layout qualifier declarations
for gl_FragCoord. For example:

layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;
layout(pixel_center_integer) in vec4 gl_FragCoord;

void main()
{
}

V2: Some code refactoring for better readability.
    Add compiler error conditions for redeclarations like:

layout(origin_upper_left) in vec4 gl_FragCoord;
layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;

and

in vec4 gl_FragCoord;
layout(origin_upper_left, pixel_center_integer) in vec4 gl_FragCoord;

V3: Simplify function is_conflicting_fragcoord_redeclaration()
V4: Check for null pointer before doing strcmp(var->name, "gl_FragCoord").

Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
Cc: <mesa-stable at lists.freedesktop.org>
---
 src/glsl/ast_to_hir.cpp         | 58 +++++++++++++++++++++++++++++++++++++++++
 src/glsl/glsl_parser_extras.cpp |  5 ++++
 src/glsl/glsl_parser_extras.h   | 12 +++++++++
 3 files changed, 75 insertions(+)

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index 8f6e901..b241ccf 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2298,6 +2298,34 @@ apply_image_qualifier_to_variable(const struct ast_type_qualifier *qual,
    }
 }
 
+static inline const char*
+get_layout_qualifier_string(bool origin_upper_left, bool pixel_center_integer)
+{
+   if (origin_upper_left && pixel_center_integer)
+      return "origin_upper_left, pixel_center_integer";
+   else if (origin_upper_left)
+      return "origin_upper_left";
+   else if (pixel_center_integer)
+      return "pixel_center_integer";
+   else
+      return " ";
+}
+
+static inline bool
+is_conflicting_fragcoord_redeclaration(struct _mesa_glsl_parse_state *state,
+                                       const struct ast_type_qualifier *qual)
+{
+   /* If gl_FragCoord was previously declared, and the qualifiers were
+    * different in any way, return true.
+    */
+   if (state->fs_redeclares_gl_fragcoord) {
+      return (state->fs_pixel_center_integer != qual->flags.q.pixel_center_integer
+         || state->fs_origin_upper_left != qual->flags.q.origin_upper_left);
+   }
+
+   return false;
+}
+
 static void
 apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
 				 ir_variable *var,
@@ -2462,6 +2490,36 @@ apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
 		       qual_string);
    }
 
+   if (var->name != NULL && strcmp(var->name, "gl_FragCoord") == 0) {
+
+      /* Make sure all gl_FragCoord redeclarations specify the same layout
+       * qualifiers.
+       */
+      if (is_conflicting_fragcoord_redeclaration(state, qual)) {
+         const char *const qual_string =
+            get_layout_qualifier_string(qual->flags.q.origin_upper_left,
+                                        qual->flags.q.pixel_center_integer);
+
+         const char *const state_string =
+            get_layout_qualifier_string(state->fs_origin_upper_left,
+                                        state->fs_pixel_center_integer);
+
+         _mesa_glsl_error(loc, state,
+                          "gl_FragCoord redeclared with different layout "
+                          "qualifiers (%s) and (%s) ",
+                          state_string,
+                          qual_string);
+      }
+      state->fs_origin_upper_left = qual->flags.q.origin_upper_left;
+      state->fs_pixel_center_integer = qual->flags.q.pixel_center_integer;
+      state->fs_redeclares_gl_fragcoord_with_no_layout_qualifiers =
+         !qual->flags.q.origin_upper_left && !qual->flags.q.pixel_center_integer;
+      state->fs_redeclares_gl_fragcoord =
+         state->fs_origin_upper_left ||
+         state->fs_pixel_center_integer ||
+         state->fs_redeclares_gl_fragcoord_with_no_layout_qualifiers;
+   }
+
    if (qual->flags.q.explicit_location) {
       validate_explicit_location(qual, var, state, loc);
    } else if (qual->flags.q.explicit_index) {
diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 61ae621..340b128 100644
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -201,6 +201,11 @@ _mesa_glsl_parse_state::_mesa_glsl_parse_state(struct gl_context *_ctx,
    this->default_uniform_qualifier->flags.q.shared = 1;
    this->default_uniform_qualifier->flags.q.column_major = 1;
 
+   this->fs_redeclares_gl_fragcoord = false;
+   this->fs_origin_upper_left = false;
+   this->fs_pixel_center_integer = false;
+   this->fs_redeclares_gl_fragcoord_with_no_layout_qualifiers = false;
+
    this->gs_input_prim_type_specified = false;
    this->gs_input_size = 0;
    this->in_qualifier = new(this) ast_type_qualifier();
diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index 300d900..2017913 100644
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -204,6 +204,18 @@ struct _mesa_glsl_parse_state {
    struct ast_type_qualifier *default_uniform_qualifier;
 
    /**
+    * Variables to track different cases if a fragment shader redeclares
+    * built-in variable gl_FragCoord.
+    *
+    * Note: These values are computed at ast_to_hir time rather than at parse
+    * time.
+    */
+   bool fs_redeclares_gl_fragcoord;
+   bool fs_origin_upper_left;
+   bool fs_pixel_center_integer;
+   bool fs_redeclares_gl_fragcoord_with_no_layout_qualifiers;
+
+   /**
     * True if a geometry shader input primitive type was specified using a
     * layout directive.
     *
-- 
1.8.3.1