This is not required in ESSL 1.00.  Section 10.24 (Samplers) of the ESSL 1.00 spec says:

    Should samplers be allowed as l-values? The specification already
    allows an equivalent behavior:

    Current specification:

        uniform sampler2D sampler[8];
        int index = f(...);
        vec4 tex = texture2D(sampler[index], xy); // allowed

    Using assignment of sampler types:

        uniform sampler2D s;
        s = g(...);
        vec4 tex = texture2D(s, xy); // not allowed

    RESOLUTION: Keep current specification. Support for dynamic
    indexing of arrays of samplers is not mandated for ES2.0
    (specified in the limitations section).

Appendix A (Limitations for ES 2.0), subsecton 5 (Indexing of Arrays, Vectors and Matrices), paragraph "Samplers" (oh good grief!) says:

    GLSL ES 1.00 supports both arrays of samplers and arrays of
    structures which contain samplers. In both these cases, for ES 2.0,
    support for indexing with a constant-index-expression is mandated
    but support for indexing with other values is not mandated.

Mesa supports OpenGL ES 2.0 on some hardware (e.g., i915) that cannot support dynamic indexing of sampler arrays.  I believe we are conformant as-is.

Thx for pointing this out.

What is missing seems to be the "constant-index-expression" part (meaning indexing with an expression that includes loop induction variable). GLSL ES spec states (table on page 110) that "constant-index-expression" must be supported for sampler arrays. I will try to build a test for this behaviour.

I've sent a glslparser test here:
http://lists.freedesktop.org/archives/piglit/2015-March/015360.html

Bug can be also reproduced with following demo when running Chrome with GLES backend:

https://www.modern.ie/en-us/demos/fishgl

I will try to tackle this.

Created attachment 114800 [details] [review]
attempt to fix this issue

Attached is an WIP attempt to fix the issue. It fixes the Piglit test I've sent, unfortunately does not fix the fishgl which is more complex as it uses clamp function inside the index (thus introducing a temporary inside index).

Some more braindump; it looks like it would make sense to move array index validation further down, now we do it in AST->HIR before running optimizations and lowering passes that can potentially change the index expression in to a legal form of indexing. IMO this happens with the fishgl demo.

We've had some discussion about this issue with Curro. It would make sense to make a small patch like Kalyan's to just enabled dynamic indexing and then (if absolutely necessary) some additional validation added later during compilation/linking.

The minimal change would be to just change error to warning with:

--- 8< --------------------------------
if (!state->is_version(130, 300)) {
}
--- 8< --------------------------------

This way we don't need to write potentially a lot of complicate code just because of one silly app.

FYI, I sent a v2 of the 'Fix' to mesa-dev mailing list.

update: I'm still working on this (as a background task), this has taken long time because of unexpected problems.  When adding a check that array index is a constant after compilation I noticed that loops (sometimes or maybe always) get unrolled only during linking which makes me think maybe there are some issues with loop analysis pass, we should be able to unroll a loop during compilation. With my Piglit test unrolling gets called 3 times during compilation but only 4th time (happens during linking) will unroll loop which in order makes the array index constant ... will continue investigation.

according to Kalyan there are 2 more cases hitting this issue:

http://hexgl.bkcore.com/play
http://users.tpg.com.au/rubixcom/index.html

I've sent another Piglit test and 2 patches to Mesa mailing list that fix this.

(In reply to Tapani Pälli from comment #13)
> I've sent another Piglit test and 2 patches to Mesa mailing list that fix
> this.

Sorry, this did not go 'like in Strömsö'. I'm working on another proposal with changes the loop analysis so that we unroll sooner and can make the required checks at compile time, this should go according to language spec.

I've also noticed current WebGL 2.0 conformance tests require this feature (although it is not in GLSL ES 3.0), I've filed a Khronos bug to discuss this issue.

here's a branch with new patches to enable this:

http://cgit.freedesktop.org/~tpalli/mesa/log/?h=unroll_loops

What's the status on this issue?

(In reply to Gavin Hindman from comment #17)
> What's the status on this issue?

I have received feedback on the patches and rebased + corrected issues found. Updated patches are on top of tree here:

http://cgit.freedesktop.org/~tpalli/mesa/log/?h=unroll_loops

I did not receive r-b for one of the patches so I will try to resolve that, It seems I will need to add support for the added option also for i915 and gallium drivers which will be couple of additional patches.

fix pushed to master + planned for 10.5 and 10.6 branches