oizma / angleproject

Automatically exported from code.google.com/p/angleproject
Other
0 stars 0 forks source link

Translated HLSL shader fails to compile if variable names are reused #42

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
For the following fragment shader:

FRAGMENT SHADER:
precision highp float;
varying vec2 v_texCoord;
uniform sampler2D diffuse;
void main() {
  vec4 diffuse = texture2D(diffuse, v_texCoord);
  gl_FragColor = diffuse;
}

TRANSLATED HLSL SHADER:
static float2 _v_texCoord = {0, 0};
static float4 gl_Color[1] = {float4(0, 0, 0, 0)};
uniform sampler2D _diffuse;
float4 gl_texture2D(sampler2D s, float2 t)
{
    return tex2D(s, t);
}
void gl_main()
{
float4 _diffuse = gl_texture2D(_diffuse, _v_texCoord);
(gl_Color[0] = _diffuse);
}

The translated code fails to compile with this error message:
error X3017: 'gl_texture2D': cannot implicitly convert from 'float4' to 
'sampler2D'

What is the expected output? What do you see instead?
I am not sure if the input shader is valid. It seems valid under the assumption 
that assignment happens from right to left. So when rhs is evaluated _diffuse 
has not be redefined yet - it is still a sampler2D. All the OpenGL drivers I 
have tested accept the input shader as valid. Apparently HLSL has different 
semantics.

If the input shader is valid, the translated HLSL shader should compile.

Original issue reported on code.google.com by alokp@chromium.org on 20 Sep 2010 at 6:56

GoogleCodeExporter commented 9 years ago
Section 10.22 'Visibility of Declarations' of the GLSL ES spec details the 
expected behavior for this. HLSL has a different semantic and this is not 
currently taken into account by ANGLE. (GLSL is the odd one here; HLSL uses the 
same semantic as C/C++)

There was a similar issue with declaring structures with the same name, within 
different scopes of a function. But that's handled correctly by appending a 
scope suffix to the names. We can probably borrow much of these mechanics to 
fix this issue as well.

Another approach would be to first assign the result to a temporary variable 
with a unique name, and then assign this to the newly declared variable. By 
doing this only when encountering a name that has already been used we can 
avoid making the code unnecessarily harder to read.

I think I prefer the second solution. Please keep me updated on the urgency of 
fixing this issue, and whether you can see any reason to prefer the first 
solution.

Original comment by nicolas....@gmail.com on 23 Sep 2010 at 1:35

GoogleCodeExporter commented 9 years ago
Fixed in r478 and even better in r479.

Original comment by dan...@transgaming.com on 15 Nov 2010 at 4:43