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