[Mesa-dev] [PATCH 1/2] glsl: Allow dynamic sampler array indexing with GLSL ES < 3.00

[Tapani Pälli]

On 05/19/2015 04:20 PM, Francisco Jerez wrote:
> Tapani Pälli <tapani.palli at intel.com> writes:
>
>> Dynamic indexing of sampler arrays is prohibited by GLSL ES 3.00.
>> Earlier versions allow 'constant-index-expression' indexing, where
>> index can contain a loop induction variable.
>>
>> Patch allows dynamic indexing for sampler arrays when GLSL ES < 3.00.
>> This change makes 'sampler-array-index.frag' parser test in Piglit
>> pass + fishgl.com works when running Chrome on OpenGL ES 2.0 backend
>>
>> v2: small change and some more commit message (Tapani)
>> v3: refactor checks to make it more readable (Ian Romanick)
>>
>> Signed-off-by: Tapani Pälli <tapani.palli at intel.com>
>> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
>> Cc: "10.5" and "10.6" <mesa-stable at lists.freedesktop.org>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84225
>> ---
>>   src/glsl/ast_array_index.cpp | 37 +++++++++++++++++++------------------
>>   1 file changed, 19 insertions(+), 18 deletions(-)
>>
>> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
>> index ecef651..ac99a7c 100644
>> --- a/src/glsl/ast_array_index.cpp
>> +++ b/src/glsl/ast_array_index.cpp
>> @@ -226,24 +226,25 @@ _mesa_ast_array_index_to_hir(void *mem_ctx,
>>          * dynamically uniform expression is undefined.
>>          */
>>         if (array->type->element_type()->is_sampler()) {
>> -	 if (!state->is_version(130, 100)) {
>> -	    if (state->es_shader) {
>> -	       _mesa_glsl_warning(&loc, state,
>> -				  "sampler arrays indexed with non-constant "
>> -				  "expressions is optional in %s",
>> -				  state->get_version_string());
>> -	    } else {
>> -	       _mesa_glsl_warning(&loc, state,
>> -				  "sampler arrays indexed with non-constant "
>> -				  "expressions will be forbidden in GLSL 1.30 "
>> -				  "and later");
>> -	    }
>> -	 } else if (!state->is_version(400, 0) && !state->ARB_gpu_shader5_enable) {
>> -	    _mesa_glsl_error(&loc, state,
>> -			     "sampler arrays indexed with non-constant "
>> -			     "expressions is forbidden in GLSL 1.30 and "
>> -			     "later");
>> -	 }
>> +         if (!state->is_version(400, 0) && !state->ARB_gpu_shader5_enable) {
>> +            if (state->is_version(130, 300))
>> +               _mesa_glsl_error(&loc, state,
>> +                                "sampler arrays indexed with non-constant "
>> +                                "expressions are forbidden in GLSL %s "
>> +                                "and later",
>> +                                state->es_shader ? "ES 3.00" : "1.30");
>> +            else if (state->es_shader)
>> +               _mesa_glsl_warning(&loc, state,
>> +                                 "sampler arrays indexed with non-constant "
>> +                                 "expressions are optional in %s and will "
>> +                                 "be forbidden in GLSL ES 3.00 and later",
>> +                                 state->get_version_string());
>
> This warning message (which I see you have copied almost literally from
> what was there before) seems rather misleading, because GLSL ES 1.0
> *did* require support for certain kind of non-constant expression (what
> the spec calls a constant-index-expression).  How about we print the
> same warning as in the non-ES case below? (referring to GLSL ES 3.0
> instead of GLSL 1.3)

That is OK for me, I'll fix this.

>> +            else
>> +               _mesa_glsl_warning(&loc, state,
>> +                                  "sampler arrays indexed with non-constant "
>> +                                  "expressions will be forbidden in GLSL "
>> +                                  "1.30 and later");
>> +         }
>>         }
>>      }
>>
>> --
>> 2.1.0