[Mesa-dev] [PATCH] glsl: Allow any sort of sampler array indexing with GLSL ES < 3.00

[Francisco Jerez]

Tapani Pälli <tapani.palli at intel.com> writes:

> On 04/07/2015 01:22 PM, Francisco Jerez wrote:
>> Tapani Pälli <tapani.palli at intel.com> writes:
>>
>>> From: Kalyan Kondapally <kalyan.kondapally at intel.com>
>>>
>>> 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)
>>>
>>> Signed-off-by: Kalyan Kondapally <kalyan.kondapally at intel.com>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84225
>>
>> Looks good, but did you check what happens now if the shader uses actual
>> variable indexing (i.e. which lowering cannot turn into a constant) on
>> an implementation that doesn't support it?  Hopefully no crashes or
>> hangs?
>
> I could test something like this, can you throw example of a good victim 
> platform and some ugly corner case? I have a shader_test that has 
> expression with a uniform in it as index as a starter.
>
I guess SNB would be "bad" enough.  The hardware actually supports
dynamically uniform indexing of surfaces but we don't implement it and
it would likely violate some assumptions in the back-end if it gets that
far.

> As a plan B, I think loop analysis could store some information which 
> can be then used for additional validation of array index in a later 
> step (skip it in AST and check only later for ES 1.00).
>

Yeah, well.  It seems rather annoying to get right at the GLSL IR level
too, because you'd have to traverse variable defs and built-in function
calls, except maybe after optimization (after loop unrolling and
constant folding at least).  At that point a valid ESSL 1.0 program
should only have sampler arrays indexed by constants, what will probably
make your job easier.

>>> ---
>>>   src/glsl/ast_array_index.cpp | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/src/glsl/ast_array_index.cpp b/src/glsl/ast_array_index.cpp
>>> index ecef651..b2609b6 100644
>>> --- a/src/glsl/ast_array_index.cpp
>>> +++ b/src/glsl/ast_array_index.cpp
>>> @@ -226,7 +226,7 @@ _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->is_version(130, 300)) {
>>>   	    if (state->es_shader) {
>>>   	       _mesa_glsl_warning(&loc, state,
>>>   				  "sampler arrays indexed with non-constant "
>>> --
>>> 2.1.0
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150407/c6bed586/attachment-0001.sig>