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;
> +
> + 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
