glsl: Link error if fs defines conflicting qualifiers for gl_FragCoord

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 causes the shader link to fail if we have multiple fragment
shaders with conflicting layout qualifiers for gl_FragCoord.

V2: Restructure the code and add conditions to correctly handle the
    following case:

fragment shader 1:
layout(origin_upper_left) in vec4 gl_FragCoord;
void main()
{
    foo();
    gl_FragColor = gl_FragData;
}

fragment shader 2:
layout(pixel_center_integer) in vec4 gl_FragCoord;
void foo()
{
}

V3:
Allow linking in the following case:
fragment shader 1:
void main()
{
    foo();
    gl_FragColor = gl_FragCoord;
}

fragment shader 2:
in vec4 gl_FragCoord;
void foo()
{
   ...
}

Signed-off-by: Anuj Phogat <anuj.phogat@gmail.com>
Cc: <mesa-stable@lists.freedesktop.org>
Reviewed-by: Ian Romanick <ian.d.romanick@intel.com>

diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index f257a24868047aa0e1960c077c0c24b5f6d546e7..7516c33e1dfd70867f799d3ba04081dbb3355a97 100644 (file)
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -123,6 +123,11 @@ _mesa_ast_to_hir(exec_list *instructions, struct _mesa_glsl_parse_state *state)
       instructions->push_head(var);
    }
 
+   /* Figure out if gl_FragCoord is actually used in fragment shader */
+   ir_variable *const var = state->symbols->get_variable("gl_FragCoord");
+   if (var != NULL)
+      state->fs_uses_gl_fragcoord = var->data.used;
+
    /* From section 7.1 (Built-In Language Variables) of the GLSL 4.10 spec:
     *
     *     If multiple shaders using members of a built-in block belonging to

diff --git a/src/glsl/glsl_parser_extras.cpp b/src/glsl/glsl_parser_extras.cpp
index 03c2a972a6e92ded304958e8551a3cea961a3c31..600658d9788572f2d714fac2f7d8b52b067ec766 100644 (file)
--- a/src/glsl/glsl_parser_extras.cpp
+++ b/src/glsl/glsl_parser_extras.cpp
@@ -197,6 +197,12 @@ _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_uses_gl_fragcoord = false;
+   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();
@@ -1356,6 +1362,14 @@ set_shader_inout_layout(struct gl_shader *shader,
       assert(!state->cs_input_local_size_specified);
    }
 
+   if (shader->Stage != MESA_SHADER_FRAGMENT) {
+      /* Should have been prevented by the parser. */
+      assert(!state->fs_uses_gl_fragcoord);
+      assert(!state->fs_redeclares_gl_fragcoord);
+      assert(!state->fs_pixel_center_integer);
+      assert(!state->fs_origin_upper_left);
+   }
+
    switch (shader->Stage) {
    case MESA_SHADER_GEOMETRY:
       shader->Geom.VerticesOut = 0;
@@ -1389,6 +1403,13 @@ set_shader_inout_layout(struct gl_shader *shader,
       }
       break;
 
+   case MESA_SHADER_FRAGMENT:
+      shader->redeclares_gl_fragcoord = state->fs_redeclares_gl_fragcoord;
+      shader->uses_gl_fragcoord = state->fs_uses_gl_fragcoord;
+      shader->pixel_center_integer = state->fs_pixel_center_integer;
+      shader->origin_upper_left = state->fs_origin_upper_left;
+      break;
+
    default:
       /* Nothing to do. */
       break;

diff --git a/src/glsl/glsl_parser_extras.h b/src/glsl/glsl_parser_extras.h
index e3db877be49af2c0da6e514773ece812f0ae6e48..2e10574034674432120639bfc05aad33d6b34d02 100644 (file)
--- a/src/glsl/glsl_parser_extras.h
+++ b/src/glsl/glsl_parser_extras.h
@@ -423,6 +423,7 @@ struct _mesa_glsl_parse_state {
    const struct gl_extensions *extensions;
 
    bool uses_builtin_functions;
+   bool fs_uses_gl_fragcoord;
 
    /**
     * For geometry shaders, size of the most recently seen input declaration

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index a2e43073e53a265879126e184a0661b36242dd44..9a018774f80f086be95f768afdbf8319fc835bd2 100644 (file)
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1194,6 +1194,82 @@ private:
    hash_table *unnamed_interfaces;
 };
 
+/**
+ * Performs the cross-validation of layout qualifiers specified in
+ * redeclaration of gl_FragCoord for the attached fragment shaders,
+ * and propagates them to the linked FS and linked shader program.
+ */
+static void
+link_fs_input_layout_qualifiers(struct gl_shader_program *prog,
+                               struct gl_shader *linked_shader,
+                               struct gl_shader **shader_list,
+                               unsigned num_shaders)
+{
+   linked_shader->redeclares_gl_fragcoord = false;
+   linked_shader->uses_gl_fragcoord = false;
+   linked_shader->origin_upper_left = false;
+   linked_shader->pixel_center_integer = false;
+
+   if (linked_shader->Stage != MESA_SHADER_FRAGMENT || prog->Version < 150)
+      return;
+
+   for (unsigned i = 0; i < num_shaders; i++) {
+      struct gl_shader *shader = shader_list[i];
+      /* From the GLSL 1.50 spec, page 39:
+       *
+       *   "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."
+       *
+       * Exclude the case when one of the 'linked_shader' or 'shader' redeclares
+       * gl_FragCoord with no layout qualifiers but the other one doesn't
+       * redeclare it. If we strictly follow GLSL 1.50 spec's language, it
+       * should be a link error. But, generating link error for this case will
+       * be a wrong behaviour which spec didn't intend to do and it could also
+       * break some applications.
+       */
+      if ((linked_shader->redeclares_gl_fragcoord
+           && !shader->redeclares_gl_fragcoord
+           && shader->uses_gl_fragcoord
+           && (linked_shader->origin_upper_left
+               || linked_shader->pixel_center_integer))
+          || (shader->redeclares_gl_fragcoord
+              && !linked_shader->redeclares_gl_fragcoord
+              && linked_shader->uses_gl_fragcoord
+              && (shader->origin_upper_left
+                  || shader->pixel_center_integer))) {
+             linker_error(prog, "fragment shader defined with conflicting "
+                         "layout qualifiers for gl_FragCoord\n");
+      }
+
+      /* From the GLSL 1.50 spec, page 39:
+       *
+       *   "All redeclarations of gl_FragCoord in all fragment shaders in a
+       *    single program must have the same set of qualifiers."
+       */
+      if (linked_shader->redeclares_gl_fragcoord && shader->redeclares_gl_fragcoord
+          && (shader->origin_upper_left != linked_shader->origin_upper_left
+          || shader->pixel_center_integer != linked_shader->pixel_center_integer)) {
+         linker_error(prog, "fragment shader defined with conflicting "
+                      "layout qualifiers for gl_FragCoord\n");
+      }
+
+      /* Update the linked shader state.  Note that uses_gl_fragcoord should
+       * accumulate the results.  The other values should replace.  If there
+       * are multiple redeclarations, all the fields except uses_gl_fragcoord
+       * are already known to be the same.
+       */
+      if (shader->redeclares_gl_fragcoord || shader->uses_gl_fragcoord) {
+         linked_shader->redeclares_gl_fragcoord =
+            shader->redeclares_gl_fragcoord;
+         linked_shader->uses_gl_fragcoord = linked_shader->uses_gl_fragcoord
+            || shader->uses_gl_fragcoord;
+         linked_shader->origin_upper_left = shader->origin_upper_left;
+         linked_shader->pixel_center_integer = shader->pixel_center_integer;
+      }
+   }
+}
+
 /**
  * Performs the cross-validation of geometry shader max_vertices and
  * primitive type layout qualifiers for the attached geometry shaders,
@@ -1471,6 +1547,7 @@ link_intrastage_shaders(void *mem_ctx,
    linked->NumUniformBlocks = num_uniform_blocks;
    ralloc_steal(linked, linked->UniformBlocks);
 
+   link_fs_input_layout_qualifiers(prog, linked, shader_list, num_shaders);
    link_gs_inout_layout_qualifiers(prog, linked, shader_list, num_shaders);
    link_cs_input_layout_qualifiers(prog, linked, shader_list, num_shaders);
 

diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 5fbfffe98e0fe6abc7efab925970d4fa4a89b09a..d9fb297a96691c1ad009d31b6ad4cd85aa6f2b63 100644 (file)
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -2432,6 +2432,14 @@ struct gl_shader
    struct glsl_symbol_table *symbols;
 
    bool uses_builtin_functions;
+   bool uses_gl_fragcoord;
+   bool redeclares_gl_fragcoord;
+
+   /**
+    * Fragment shader state from GLSL 1.50 layout qualifiers.
+    */
+   bool origin_upper_left;
+   bool pixel_center_integer;
 
    /**
     * Geometry shader state from GLSL 1.50 layout qualifiers.