Re: [Mesa-dev] sampler arrays indexed with non-constant expressions
On 11/19/2013 01:16 AM, Erik Faye-Lund wrote: > On Mon, Nov 18, 2013 at 9:04 PM, Paul Berry wrote: >> On 17 November 2013 00:24, Victor Luchitz wrote: >>> If compiler actually attempted to unroll the loop above, it would notice >>> that the does compile correctly. Instead it just emits and error and in my >>> opinion, contradicts the comment above but not allowing the "useful thing" >>> in question. >>> >>> Can the compiler be changed to _first_ attempt to unroll the loop and then >>> check for sampler array indices being constants? >> >> >> Thanks for the feedback, Victor. We are always interested in hearing >> suggestions about how to improve Mesa. >> >> Unfortunately, in this case, your suggestion would make Mesa non-conformant >> with the GLSL specs because it would allow shaders that are prohibited by >> the spec. Generally we try to avoid this sort of non-spec-conformance >> because it leads to portability problems for developers like you (e.g. your >> shader works with Mesa, but it fails with other GL implementations). A >> further problem is that Mesa has heuristics to decide whether to unroll >> loops; if we followed your suggestion, then your shader might work today, >> but fail tomorrow if you make a small change to the shader that makes Mesa >> decide not to unroll the loop (or we make a change to the heuristics in a >> future release of Mesa). > > Just as minor nit, I'd like to point out that the OpenGL ES 2.0 > shading language specification defines a concept of > "constant-index-expressions" which includes loop indices, and allows > sampler arrays to be indexed by these expressions. This only works > because it at the same time does not allow arbitrary loops. See > appendix A, section 4 and 5 of the spec for the details. Right. Basically the ES 2.0 limitation is that any loop can be unrolled... because quite a bit of the original ES 2.0 target hardware didn't have *any* branching. This was hardware like i915 or r300. The case that Paul describes isn't hypothetical, either. We actually encountered this a couple years ago. There was something in the loop that prevented the compile from unrolling it, and the texture accesses in the loop caused assertion failures. We eventually made the loop unroll, but I seem to recall that had other bad side effects. I *thin* the problem was the loop, for some reason or another, had a huge number of instructions. I'm fairly sure there was an associated bug report, but it's probably too much effort to make it worth finding. > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] sampler arrays indexed with non-constant expressions
On Mon, Nov 18, 2013 at 9:04 PM, Paul Berry wrote: > On 17 November 2013 00:24, Victor Luchitz wrote: >> If compiler actually attempted to unroll the loop above, it would notice >> that the does compile correctly. Instead it just emits and error and in my >> opinion, contradicts the comment above but not allowing the "useful thing" >> in question. >> >> Can the compiler be changed to _first_ attempt to unroll the loop and then >> check for sampler array indices being constants? > > > Thanks for the feedback, Victor. We are always interested in hearing > suggestions about how to improve Mesa. > > Unfortunately, in this case, your suggestion would make Mesa non-conformant > with the GLSL specs because it would allow shaders that are prohibited by > the spec. Generally we try to avoid this sort of non-spec-conformance > because it leads to portability problems for developers like you (e.g. your > shader works with Mesa, but it fails with other GL implementations). A > further problem is that Mesa has heuristics to decide whether to unroll > loops; if we followed your suggestion, then your shader might work today, > but fail tomorrow if you make a small change to the shader that makes Mesa > decide not to unroll the loop (or we make a change to the heuristics in a > future release of Mesa). Just as minor nit, I'd like to point out that the OpenGL ES 2.0 shading language specification defines a concept of "constant-index-expressions" which includes loop indices, and allows sampler arrays to be indexed by these expressions. This only works because it at the same time does not allow arbitrary loops. See appendix A, section 4 and 5 of the spec for the details. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] sampler arrays indexed with non-constant expressions
Hello Paul, thank you for the explanation. I thought that relaxing the spec conformance in this particular harmless case would be beneficial both to programmers and users. I understand and accept your reasoning even though deep in my heart I feel that by-the-book conformance is overly dogmatic. Guess the bug 71723 may be closed now. Thanks again! -- Best regards, Victor Luchitz ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] sampler arrays indexed with non-constant expressions
On 17 November 2013 00:24, Victor Luchitz wrote:
> Hello,
>
> in my opinion GLSL compiler in mesa is too restrictive when it comes to
> sampler arrays. The following code can not be compiled due to the "sampler
> arrays indexed with non-constant expressions is forbidden in GLSL 1.30 and
> later":
>
> #define MAX_SHADOWS 2
>
> varying vec4 v_ShadowProjVector[MAX_SHADOWS];
>
> uniform sampler2DShadow u_ShadowmapTexture[MAX_SHADOWS];
> # define qfShadow2D(t,v) float(shadow2D(t,v))
>
> uniform float u_ShadowAlpha;
> uniform float u_ShadowProjDistance[MAX_SHADOWS];
> uniform vec4 u_ShadowmapTextureParams[MAX_SHADOWS];
>
> void main(void)
> {
> float finalcolor = 1.0;
>
> for (int i = 0; i < MAX_SHADOWS; i++)
> {
> vec3 shadowmaptc = vec3(v_ShadowProjVector[i].xyz /
> v_ShadowProjVector[i].w);
>
> // this keeps shadows from appearing on surfaces behind frustum's
> nearplane
> float d = step(v_ShadowProjVector[i].w, 0.0);
>
> //shadowmaptc = (shadowmaptc + vec3 (1.0)) * vec3 (0.5);
> shadowmaptc.xy = shadowmaptc.xy * u_ShadowmapTextureParams[i].xy;
> // .x - texture width
> shadowmaptc.z = clamp(shadowmaptc.z, 0.0, 1.0);
> shadowmaptc.xy = vec2(clamp(shadowmaptc.x, 0.0,
> u_ShadowmapTextureParams[i].x), clamp(shadowmaptc.y, 0.0,
> u_ShadowmapTextureParams[i].y));
>
> vec2 ShadowMap_TextureScale = u_ShadowmapTextureParams[i].zw;
>
> float f;
>
> f = qfShadow2D(u_ShadowmapTexture[i], vec3(shadowmaptc.xy *
> ShadowMap_TextureScale, shadowmaptc.z));
>
> finalcolor *= clamp(max(max(f, d), u_ShadowAlpha), 0.0, 1.0);
> }
>
> gl_FragColor = vec4(vec3(finalcolor),1.0);
> }
>
>
> Lines 159-136 of src/glsl/ast_array_index.cpp say:
>
> * This restriction was added in GLSL 1.30. Shaders using earlier
> version
> * of the language should not be rejected by the compiler front-end for
> * using this construct. This allows useful things such as using a loop
> * counter as the index to an array of samplers. If the loop in
> unrolled,
> * the code should compile correctly. Instead, emit a warning.
>
> If compiler actually attempted to unroll the loop above, it would notice
> that the does compile correctly. Instead it just emits and error and in my
> opinion, contradicts the comment above but not allowing the "useful thing"
> in question.
>
> Can the compiler be changed to _first_ attempt to unroll the loop and then
> check for sampler array indices being constants?
>
Thanks for the feedback, Victor. We are always interested in hearing
suggestions about how to improve Mesa.
Unfortunately, in this case, your suggestion would make Mesa non-conformant
with the GLSL specs because it would allow shaders that are prohibited by
the spec. Generally we try to avoid this sort of non-spec-conformance
because it leads to portability problems for developers like you (e.g. your
shader works with Mesa, but it fails with other GL implementations). A
further problem is that Mesa has heuristics to decide whether to unroll
loops; if we followed your suggestion, then your shader might work today,
but fail tomorrow if you make a small change to the shader that makes Mesa
decide not to unroll the loop (or we make a change to the heuristics in a
future release of Mesa).
Note that once we finish implementing ARB_gpu_shader5 (which we hope to
finish soon), you will be able to use that--ARB_gpu_shader5 lifts the
restriction that sampler array indices must be constant.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] sampler arrays indexed with non-constant expressions
Hello,
in my opinion GLSL compiler in mesa is too restrictive when it comes to
sampler arrays. The following code can not be compiled due to the "sampler
arrays indexed with non-constant expressions is forbidden in GLSL 1.30 and
later":
#define MAX_SHADOWS 2
varying vec4 v_ShadowProjVector[MAX_SHADOWS];
uniform sampler2DShadow u_ShadowmapTexture[MAX_SHADOWS];
# define qfShadow2D(t,v) float(shadow2D(t,v))
uniform float u_ShadowAlpha;
uniform float u_ShadowProjDistance[MAX_SHADOWS];
uniform vec4 u_ShadowmapTextureParams[MAX_SHADOWS];
void main(void)
{
float finalcolor = 1.0;
for (int i = 0; i < MAX_SHADOWS; i++)
{
vec3 shadowmaptc = vec3(v_ShadowProjVector[i].xyz /
v_ShadowProjVector[i].w);
// this keeps shadows from appearing on surfaces behind frustum's
nearplane
float d = step(v_ShadowProjVector[i].w, 0.0);
//shadowmaptc = (shadowmaptc + vec3 (1.0)) * vec3 (0.5);
shadowmaptc.xy = shadowmaptc.xy * u_ShadowmapTextureParams[i].xy;
// .x - texture width
shadowmaptc.z = clamp(shadowmaptc.z, 0.0, 1.0);
shadowmaptc.xy = vec2(clamp(shadowmaptc.x, 0.0,
u_ShadowmapTextureParams[i].x), clamp(shadowmaptc.y, 0.0,
u_ShadowmapTextureParams[i].y));
vec2 ShadowMap_TextureScale = u_ShadowmapTextureParams[i].zw;
float f;
f = qfShadow2D(u_ShadowmapTexture[i], vec3(shadowmaptc.xy *
ShadowMap_TextureScale, shadowmaptc.z));
finalcolor *= clamp(max(max(f, d), u_ShadowAlpha), 0.0, 1.0);
}
gl_FragColor = vec4(vec3(finalcolor),1.0);
}
Lines 159-136 of src/glsl/ast_array_index.cpp say:
* This restriction was added in GLSL 1.30. Shaders using earlier
version
* of the language should not be rejected by the compiler front-end for
* using this construct. This allows useful things such as using a loop
* counter as the index to an array of samplers. If the loop in
unrolled,
* the code should compile correctly. Instead, emit a warning.
If compiler actually attempted to unroll the loop above, it would notice
that the does compile correctly. Instead it just emits and error and in my
opinion, contradicts the comment above but not allowing the "useful thing"
in question.
Can the compiler be changed to _first_ attempt to unroll the loop and then
check for sampler array indices being constants?
--
Best regards,
Victor Luchitz
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
