ShBuiltInResources::ArrayIndexClampingStrategy of zero doesn't result in the default choice

[GoogleCodeExporter]

While upgrading Firefox's ANGLE, we hit started to see failures where shaders 
were being injected with webgl_int_clamp calls, but not getting the definition 
injected as well.

It turns out we zero `resources` before populating it and sending it to 
TCompiler::Init. Since we didn't manually set 
`resources.ArrayIndexClampingStrategy`, we should be getting the default, but 
TCompiler::Init unconditionally sets the clamping strategy to 0. 0 is not a 
valid clamping strategy, as the choices are:
SH_CLAMP_WITH_CLAMP_INTRINSIC: 1
SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION: 2

Since the code assumes it's one of those two (though it doesn't bother to 
assert this), we see it inject clamping assuming 
SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION, but not supplying the definition 
for the user-defined clamping function, since the strategy isn't 
SH_CLAMP_WITH_USER_DEFINED_INT_CLAMP_FUNCTION.

Really, TCompiler::Init should treat `resources.foo = 0` as a request for 
default for foo. All other cases do this, but ArrayIndexClampingStrategy does 
not.

What steps will reproduce the problem?
1. Call TCompiler::Init with `resources.ArrayIndexClampingStrategy = 0`.
2. Run `1.0.1/conformance/glsl/misc/shader-with-arbitrary-indexing.vert.html`.

What is the expected output? What do you see instead?
Test should pass, but doesn't. (fails to compile: no such symbol 
'webgl_int_clamp')

What version of the product are you using? On what operating system?
r2042.

I don't know if this patch will apply cleanly, as I just fixed up the paths 
from the patch against mozilla-central.

Original issue reported on code.google.com by jda...@gmail.com on 10 Apr 2013 at 12:24

Attachments:

• angle-default-clamp-strat.patch

[GoogleCodeExporter]

If this issue is still relevant, please submit the patch by following the steps 
described in https://code.google.com/p/angleproject/wiki/ContributingCode

Original comment by c...@chromium.org on 21 May 2014 at 7:19

[GoogleCodeExporter]

Was doing a Mozilla-specific bug sweep; I went ahead and updated the patch for 
the new file locations, and stuck it in Gerrit here, since it does look like 
the issue still applies: https://chromium-review.googlesource.com/#/c/244580/1

Original comment by shannonw...@chromium.org on 30 Jan 2015 at 12:58

[GoogleCodeExporter]

Comment on the patch, but hopefully here jdashg might see it:

I'm not an expert on the ShBuiltInResources stuff, but shouldn't you initialize 
this structure with ShInitBuiltInResources? ShInitBuiltInResources memsets to 
zero, as well as providing some sane initial values. I'm not sure if we really 
define zero as default for the values of the resources.

Original comment by jmad...@chromium.org on 30 Jan 2015 at 3:03

[GoogleCodeExporter]

Calling code should absolutely call ShInitBuiltInResources. Not doing so is an 
error in usage. I don't think any workarounds should be placed in ANGLE for 
users that don't follow the contract.

Original comment by kbr@chromium.org on 2 Feb 2015 at 10:01

[GoogleCodeExporter]

Ah, fair enough; I'm less familiar with the shader translator initialization 
API. Abandoning CL, and closing this as wontfix.

Original comment by shannonw...@chromium.org on 2 Feb 2015 at 10:16

• Changed state: WontFix