On 02/25/2014 01:39 PM, Anuj Phogat wrote:
On Tue, Feb 25, 2014 at 7:43 AM, Ian Romanick <i...@freedesktop.org> wrote:
On 02/24/2014 05:34 PM, Anuj Phogat wrote:
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;
Signed-off-by: Anuj Phogat <anuj.pho...@gmail.com>
Cc: <mesa-sta...@lists.freedesktop.org>
---
This series is also available at:
https://github.com/aphogat/mesa.git, branch='review'
src/glsl/ast_to_hir.cpp | 60
+++++++++++++++++++++++++++++++++++++++++
src/glsl/glsl_parser_extras.cpp | 5 ++++
src/glsl/glsl_parser_extras.h | 12 +++++++++
3 files changed, 77 insertions(+)
diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
index f06baeb..9fe3095 100644
--- a/src/glsl/ast_to_hir.cpp
+++ b/src/glsl/ast_to_hir.cpp
@@ -2295,6 +2295,36 @@ 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)
state can also be const.
+{
+ bool conflicting_origin_upper_left =
+ (state->fs_origin_upper_left && !qual->flags.q.origin_upper_left) ||
+ (!state->fs_origin_upper_left && qual->flags.q.origin_upper_left &&
+ state->fs_redeclares_gl_fragcoord);
+
+ bool conflicting_pixel_center_integer =
+ (state->fs_pixel_center_integer && !qual->flags.q.pixel_center_integer)
||
+ (!state->fs_pixel_center_integer && qual->flags.q.pixel_center_integer &&
+ state->fs_redeclares_gl_fragcoord);
Does this condition also work? It seems easier to understand.
/* If gl_FragCoord was previously declared, and the qualifiers were
* different in any way, return false.
*/
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 true;
I think you meant 'return false' here? It works after making this change in last
statement. Using '!=' made the function look simpler. Thanks for suggestion.
Yes. When I wrote that line I was thinking the function returned true
for valid.
+
+ return (conflicting_origin_upper_left || conflicting_pixel_center_integer);
+}
+
static void
apply_type_qualifier_to_variable(const struct ast_type_qualifier *qual,
ir_variable *var,
@@ -2459,6 +2489,36 @@ apply_type_qualifier_to_variable(const struct
ast_type_qualifier *qual,
qual_string);
}
+ if (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 d7f5202..fcbbf44 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.
*
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
