[SH_TIMING_RESTRICTIONS] Restrict sampler dependent values from the tex coord parameter of all GLSL…

Texture cache access time might leak texture information, if a sampler dependent value is used as the texture coordinate or LOD bias in a sampling operation.

For example:
vec4 c1 = texture2D(u_texture, v_texCoord);
vec4 c2 = texture2D(u_texture, vec2(c1.r)); // c1 can't be tex coord!

This patch makes SH_TIMING_RESTRICTIONS restrict sampler dependent expressions from appearing as the coord or bias arguments in *all* WebGL sampling operations (texture2D, textureCube, etc.). Before, we just checked texture2D.

This patch also adds a "-x=r" flag to translator.cpp to enable the ARB_texture_rectangle extension. This is so that we can easily test   texture2DRect and texture2DRectProj.

[mvujovic, 2012-06-19 21:48:58]

Hi guys,

Could you review this when you get a chance?

Thanks,
Max

[kbr1, 2012-06-19 23:31:30]

Nicolas or Daniel, could one of you take care of this review? Appreciated.

[nicolas, 2012-06-20 19:14:47]

Can a shader have a user-defined texture2DRect when ARB_texture_rectangle is not
enabled? The parser prevents it from compiling in the other case, right?

Looks good to me if that behavior is as intended.

[mvujovic, 2012-06-20 23:28:30]

On 2012/06/20 19:14:47, nicolas wrote:
> Can a shader have a user-defined texture2DRect when ARB_texture_rectangle is
not
> enabled?

Yes, that's fine.

> The parser prevents it from compiling in the other case, right?

No, it seems the parser is ok with the other case as well, but maybe it
shouldn't be. Redefining a built-in function compiles without error. For
example, redefining texture2D works:

"""
precision mediump float;

uniform sampler2D u_texture;

vec4 texture2D(sampler2D sampler, vec2 coord)
{
    return vec4(0.0);
}

void main()
{
    texture2D(u_texture, vec2(0.0));
}
"""

According to the GLSL ES 1.0 spec, "User code can overload the built-in
functions but cannot redefine them."
(See p.63 and p.90 in
http://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf)

Sounds like the parser should be failing for the above code, yes?

> Looks good to me if that behavior is as intended.

Thanks for the review, Nicolas.

[mvujovic, 2012-06-25 16:51:29]

On 2012/06/20 23:28:30, mvujovic wrote:
> On 2012/06/20 19:14:47, nicolas wrote:
> > Can a shader have a user-defined texture2DRect when ARB_texture_rectangle is
> not
> > enabled?
> 
> Yes, that's fine.
> 
> > The parser prevents it from compiling in the other case, right?
> 
> No, it seems the parser is ok with the other case as well, but maybe it
> shouldn't be. Redefining a built-in function compiles without error. For
> example, redefining texture2D works:
> 
> """
> precision mediump float;
> 
> uniform sampler2D u_texture;
> 
> vec4 texture2D(sampler2D sampler, vec2 coord)
> {
>     return vec4(0.0);
> }
> 
> void main()
> {
>     texture2D(u_texture, vec2(0.0));
> }
> """
> 
> According to the GLSL ES 1.0 spec, "User code can overload the built-in
> functions but cannot redefine them."
> (See p.63 and p.90 in
>
http://www.khronos.org/registry/gles/specs/2.0/GLSL_ES_Specification_1.0.17.pdf)
> 
> Sounds like the parser should be failing for the above code, yes?
> 
> > Looks good to me if that behavior is as intended.
> 
> Thanks for the review, Nicolas.

Hi Nicolas,

I've created a new issue for the existing bug: "Parser allows user code to
redefine built-in functions"
(http://code.google.com/p/angleproject/issues/detail?id=343).

Is it ok to commit this patch and address that issue separately?

Thanks,
Max

[nicolas, 2012-06-25 18:07:56]

Yes, as far as I'm concerned your patch is fine and the parser issue will have
to be fixed separately. Thanks for creating the issue report.

[mvujovic, 2012-06-25 18:26:34]

On 2012/06/25 18:07:56, nicolas wrote:
> Yes, as far as I'm concerned your patch is fine and the parser issue will have
> to be fixed separately. Thanks for creating the issue report.

Sounds good. Thanks Nicolas!

[mvujovic, 2012-06-25 20:42:27]

On 2012/06/25 18:26:34, mvujovic wrote:
> On 2012/06/25 18:07:56, nicolas wrote:
> > Yes, as far as I'm concerned your patch is fine and the parser issue will
have
> > to be fixed separately. Thanks for creating the issue report.
> 
> Sounds good. Thanks Nicolas!

Committed in r1167.