[Mesa-dev,12/17] glsl/ast: Allow redeclaration of gl_LastFragData with different precision qualifier.

Submitted by Francisco Jerez on July 21, 2016, 4:49 a.m.

Details

Message ID 20160721044947.14076-12-currojerez@riseup.net
State New
Headers show
Series "Framebuffer fetch." ( rev: 4 ) in Mesa

Not browsing as part of any series.

Commit Message

Francisco Jerez July 21, 2016, 4:49 a.m.
---
 src/compiler/glsl/ast_to_hir.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Patch hide | download patch | download mbox

diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
index c050a3f..ac651a9 100644
--- a/src/compiler/glsl/ast_to_hir.cpp
+++ b/src/compiler/glsl/ast_to_hir.cpp
@@ -3948,6 +3948,19 @@  get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
 
       earlier->data.depth_layout = var->data.depth_layout;
 
+   } else if (state->has_framebuffer_fetch() &&
+              !state->is_version(420, 300) &&
+              strcmp(var->name, "gl_LastFragData") == 0 &&
+              var->type == earlier->type &&
+              var->data.mode == ir_var_auto) {
+      /* According to the EXT_shader_framebuffer_fetch spec:
+       *
+       *   "By default, gl_LastFragData is declared with the mediump precision
+       *    qualifier. This can be changed by redeclaring the corresponding
+       *    variables with the desired precision qualifier."
+       */
+      earlier->data.precision = var->data.precision;
+
    } else if (allow_all_redeclarations) {
       if (earlier->data.mode != var->data.mode) {
          _mesa_glsl_error(&loc, state,

Comments

On Wednesday, July 20, 2016 9:49:42 PM PDT Francisco Jerez wrote:
> ---
>  src/compiler/glsl/ast_to_hir.cpp | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
> index c050a3f..ac651a9 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3948,6 +3948,19 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
>  
>        earlier->data.depth_layout = var->data.depth_layout;
>  
> +   } else if (state->has_framebuffer_fetch() &&
> +              !state->is_version(420, 300) &&

I think you can drop the is_version check - there won't be an
gl_LastFragData to redeclare in the other case, so earlier == NULL
and we would have bailed out earlier.

With the caveat that I haven't seen the specs, and am just assuming
they work like I think they should, the series looks good to me.

For the series:
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

> +              strcmp(var->name, "gl_LastFragData") == 0 &&
> +              var->type == earlier->type &&
> +              var->data.mode == ir_var_auto) {
> +      /* According to the EXT_shader_framebuffer_fetch spec:
> +       *
> +       *   "By default, gl_LastFragData is declared with the mediump precision
> +       *    qualifier. This can be changed by redeclaring the corresponding
> +       *    variables with the desired precision qualifier."
> +       */
> +      earlier->data.precision = var->data.precision;
> +
>     } else if (allow_all_redeclarations) {
>        if (earlier->data.mode != var->data.mode) {
>           _mesa_glsl_error(&loc, state,
>
Kenneth Graunke <kenneth@whitecape.org> writes:

> On Wednesday, July 20, 2016 9:49:42 PM PDT Francisco Jerez wrote:
>> ---
>>  src/compiler/glsl/ast_to_hir.cpp | 13 +++++++++++++
>>  1 file changed, 13 insertions(+)
>> 
>> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_hir.cpp
>> index c050a3f..ac651a9 100644
>> --- a/src/compiler/glsl/ast_to_hir.cpp
>> +++ b/src/compiler/glsl/ast_to_hir.cpp
>> @@ -3948,6 +3948,19 @@ get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
>>  
>>        earlier->data.depth_layout = var->data.depth_layout;
>>  
>> +   } else if (state->has_framebuffer_fetch() &&
>> +              !state->is_version(420, 300) &&
>
> I think you can drop the is_version check - there won't be an
> gl_LastFragData to redeclare in the other case, so earlier == NULL
> and we would have bailed out earlier.
>
Good point, dropped locally.

> With the caveat that I haven't seen the specs, and am just assuming
> they work like I think they should, the series looks good to me.
>
> For the series:
> Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
>
Thanks.

>> +              strcmp(var->name, "gl_LastFragData") == 0 &&
>> +              var->type == earlier->type &&
>> +              var->data.mode == ir_var_auto) {
>> +      /* According to the EXT_shader_framebuffer_fetch spec:
>> +       *
>> +       *   "By default, gl_LastFragData is declared with the mediump precision
>> +       *    qualifier. This can be changed by redeclaring the corresponding
>> +       *    variables with the desired precision qualifier."
>> +       */
>> +      earlier->data.precision = var->data.precision;
>> +
>>     } else if (allow_all_redeclarations) {
>>        if (earlier->data.mode != var->data.mode) {
>>           _mesa_glsl_error(&loc, state,
>>