Sign in to follow this  
Silverlan

GLSL Error C1502 (Nvidia): "index must be constant expression"

Recommended Posts

Silverlan    662

I have a uniform block in my shader, which I'm accessing within a loop:

#version 330 core

const int MAX_LIGHTS = 8; // Maximum amount of lights
uniform int numLights; // Actual amount of lights (Cannot exceed MAX_LIGHTS)
layout (std140) uniform LightSourceBlock
{
    vec3 position;
    [...]
} LightSources[MAX_LIGHTS]; // Light Data

void Test()
{
    for(int i=0;i<numLights;i++)
    {
        vec3 pos = LightSources[i].position; // Causes "index must be constant expression" error on Nvidia cards
        [...]
    }
}

This works fine on my AMD card, however on a Nvidia card it generates the error "index must be constant expression".

I've tried changing the shader to this:

#version 330 core

const int MAX_LIGHTS = 8; // Maximum amount of lights
uniform int numLights; // Actual amount of lights (Cannot exceed MAX_LIGHTS)
layout (std140) uniform LightSourceBlock
{
    vec3 position;
    [...]
} LightSources[MAX_LIGHTS]; // Light Data

void Test()
{
    for(int i=0;i<MAX_LIGHTS;i++)
    {
        if(i >= numLights)
        	break;
        vec3 pos = LightSources[i].position; // Causes "index must be constant expression" error on Nvidia cards
        [...]
    }
}

I figured this way it might consider "i" to be a constant, but the error remains.

 

So how can I access "LightSources" with a non-const index, without having to break up the loop and just pasting the same code below each other a bunch of times?

Share this post


Link to post
Share on other sites
Hodgman    51328

You could try moving to a higher #version number. Version 330 is for cards that are 5 years old, some of which probably can't perform uniform array indexing.
D3D/HLSL will compile that code, but the actual runtime asm might be equivalent to the following (which is shit, so a compiler error may be better tongue.png)

void Test()
{
    for(int i=0;i<numLights;i++)
    {
        vec3 pos;
        if(i==0) pos = LightSources[0].position;
        else if(i==1) pos = LightSources[1].position;
        else if(i==2) pos = LightSources[2].position;
        //and so on...
        [...]
    }
}

To avoid such shenanigans on old GPUs, store your arrays in a texture or buffer (aka "texture buffer" in GL jargon, not a "uniform buffer") and fetch the values from them.

Edited by Hodgman

Share this post


Link to post
Share on other sites
BitMaster    8651
Are you absolutely certain that line is causing the problem and not some other line? I did a quick search and found this post on StackOverflow which seems to suggest your initial usage is okay (which is also what I would have said before checking).
From experience I would be a bit surprised to learn an AMD driver accepts something it should not according to the standard but an Nvidia driver complains. Have you tried running it through the reference compiler?

Share this post


Link to post
Share on other sites
You could try moving to a higher #version number.

This.

 

See here: https://www.opengl.org/wiki/Interface_Block_%28GLSL%29

 

For uniform and shader storage blocks, the array index must be a dynamically-uniform integral expression.

[...]

If an expression is said to be dynamically uniform, and you're not using GLSL 4.00 or above, this should be read as "constant expression". Only 4.00-class hardware and greater is able to make this distinction.

 

So basically, you need #version 400 or higher for this to work.

Edited by samoth

Share this post


Link to post
Share on other sites
Silverlan    662
To avoid such shenanigans on old GPUs, store your arrays in a texture or buffer (aka "texture buffer" in GL jargon, not a "uniform buffer") and fetch the values from them.

Hm... My uniform block contains both integer and float data, I suppose I'd have to add the integers as floats, and then cast them back in the shader? Couldn't that result in imprecision errors? (Although I suppose I could just round the value to be sure)

 

 

Have you tried running it through the reference compiler?

I didn't realize there was such a thing.

Well, I tried running my example through the glslangValidator and no errors were generated.

Edited by Silverlan

Share this post


Link to post
Share on other sites
Ubik    1339

This overlaps with what Matias said, but is from a bit different perspective, and I'm not someone with too much actual in-depth knowledge, but here goes anyway:

 

The array of blocks syntax creates several blocks, with separate binding points for each, and each of them can be backed by a completely different buffer. My intuition says indexing blocks is therefore possibly implemented very differently than indexing within a block, where arrays are actually laid sequentially in memory.

 

On second thought, on cards where the blocks are only 16kb at most or something similarly small, the data could be copied around in the GPU when glBindBufferRange or glBindBufferBase is called so that they end up being laid out in an array for the shader to access, but there doesn't seem to be any guarantee for that to be case.

 

I also just tried a very simple vertex shader with an array block, and the program crashes at shader linking in a way that hints the cause might be outside my code. The code has so far worked well in other (simple test) cases and the reference compiler has no complaints about the shader, but of course there's still a chance it's a bug in my code. (In any case, I'm running on an oldish Radeon card on Linux with the free drivers, so knowledge of this potential bug may not be very relevant to you.)

Share this post


Link to post
Share on other sites
Hodgman    51328

y uniform block contains both integer and float data, I suppose I'd have to add the integers as floats, and then cast them back in the shader?

v330 introduced the intBitsToFloat/floatBitsToInt functions, which allow bitwise/reinterpret casts. You can use a 32bit int buffer and then reinterpret it's elements as floats as required.

Share this post


Link to post
Share on other sites
TheChubu    9454

lol yeah, I remember when I got this issue (same GLSL version, nVidia hardware), apparently "const int" isn't constant enough for nVidia :D As Mathias said, a #define works fine in this case.

Share this post


Link to post
Share on other sites
Silverlan    662

lol yeah, I remember when I got this issue (same GLSL version, nVidia hardware), apparently "const int" isn't constant enough for nVidia biggrin.png As Mathias said, a #define works fine in this case.

 

Thanks, sadly the same error still occurs.

Changing the version to "#version 420 core" also didn't help.

 

 

y uniform block contains both integer and float data, I suppose I'd have to add the integers as floats, and then cast them back in the shader?

v330 introduced the intBitsToFloat/floatBitsToInt functions, which allow bitwise/reinterpret casts. You can use a 32bit int buffer and then reinterpret it's elements as floats as required.

 

Are buffer texture lookups as expensive as regular texture lookups? I have 12 variables(=512 bytes) per light, and since the data types differ (int,float,vec3,mat3,...) I suppose GL_R32I makes the most sense for the texture format? However, that would result in a lot of lookups.

 

Also, I've noticed that [i][URL=https://www.opengl.org/sdk/docs/man/html/texelFetch.xhtml]texelFetch[/URL][/i] returns a 4-component vector. So, if I have a 1-component texture format, would that give me the data of 4 texels?

Share this post


Link to post
Share on other sites
JohnnyCode    1046


Thanks, sadly the same error still occurs.

Changing the version to "#version 420 core" also didn't help.

If you have a cycle whose condition expression is evaluated entirely from uniform variables, you should be ok even in the down deep versions of GLSL, even in pixel function.

 

Quite complicative is your break instruction inside the loop. Though this instruction has in condition an uniform variable again, it still could confuse NVIDIA driver too much, you can try to rewrite the code to this to emit the same instructions:

const int MAX_LIGHTS = 8; // Maximum amount of lights
uniform int numLights; // Actual amount of lights (Cannot exceed MAX_LIGHTS)
.
.
.

void Test()
{
    int shadejobcount= min(MAX_LIGHTS,numLights);
    for(int i=0;i<shadejobcount;i++)
    {
       /* if(i >= numLights)
        	break;  */
        vec3 pos = LightSources[i].position; // Causes "index must be constant expression" error on Nvidia cards
        [...]
    }
}

Might help.

If still would not, you may be facing a "safe gpu code" kind of alert from the NVIDIA driver, refusing to compile a shader in which it does not know the (maximum) amount of instructions at compile time. In that case, I cannot think of better limitting method than a min() with a compile constant and the uniform :)

Share this post


Link to post
Share on other sites
TheChubu    9454


Thanks, sadly the same error still occurs.
Have you tried what Mathias suggested? Redefining the array inside the block, not outside?

 

layout (std140) uniform LightSourceBlock
{
    LightSource lights[MAX_LIGHTS];
} LightSources;

 

This works. Its how I define UBOs. Tried it on nVidia hardware (GT21x, Fermi), and AMD (HD 5xxx, HD 4xxx), GLSL 330.

Share this post


Link to post
Share on other sites
Silverlan    662

 


Thanks, sadly the same error still occurs.
Have you tried what Mathias suggested? Redefining the array inside the block, not outside?
layout (std140) uniform LightSourceBlock
{
    LightSource lights[MAX_LIGHTS];
} LightSources;

This works. Its how I define UBOs. Tried it on nVidia hardware (GT21x, Fermi), and AMD (HD 5xxx, HD 4xxx), GLSL 330.

I wanted to give both approaches a shot, but after trying Mathias' solution it worked immediately. Thanks!

 

However, I've stumbled over another shader error on Nvidia, which doesn't occur on my AMD-card:

error C5208: Sampler needs to be a uniform (global or parameter to main), need to inline function or resolve conditional expression

The line it gives me just points me to my main function which does the processing, which doesn't help me at all. The error doesn't make much sense to me either, all of my samplers are uniforms.

The only possible cause I could think of are my shadow samplers, which are a uniform array:

uniform sampler2DShadow shadowMaps[MAX_LIGHTS];

 

The sampler for each light is then accessed by shadowMaps[LightSources.lights[i].shadowMapID]. (shadowMapID is an int)

Could this be the cause? If so, is there a simple way to get around it?

 

I apologize for the sparse information, I would just do some tests myself, but I can only get access to the Nvidia machine for testing sporadically, and the shaders work fine on my AMD-setup.

Maybe someone has encountered this error before?

Edited by Silverlan

Share this post


Link to post
Share on other sites
Matias Goldberg    9580

The sampler for each light is then accessed by [i]shadowMaps[/i][i][LightSources.lights[i].shadowMapID][/i]. ([i][i]shadowMapID[/i][/i] is an int)
Could this be the cause? If so, is there a simple way to get around it?

Yes. That's the problem.
Indexing a sampler like that needs GL4 hardware, actually pretty modern like version 430.

There are two possible workarounds:

1. Since you have up to 8 lights, sort them in CPU.
So lights[0] corresponds to shadowMaps[0], lights[1] corresponds to shadowMaps[1], and so on. This is what a lot of people do. Even on modern hardware, it also saves you an indirection GPU side so GPU performance should go up (in exchange for a little of CPU power for sorting the lights/samplers).

2. Use texture arrays. Declare your sampler as a single sampler2DArrayShadow shadowMaps and pass the shadowMapID as the array slice. The only problem with this approach is that the shadow maps must all be of the same resolution and format.

Share this post


Link to post
Share on other sites
Silverlan    662
Use this instead:
struct LightSource
{
vec3 position;
[...]
};

layout (std140) uniform LightSourceBlock
{
LightSource lights[MAX_LIGHTS];
} LightSources;

//Index like this: LightSources.lights[i]
This uses one constant buffer and places all lights in that single buffer. That's way more efficient, more compatible, and the method everyone uses.

 

One more thing I forgot to ask:

Previously I had one buffer per light, each containing the respective light data, and then I just bound (Using glBindBufferBase) the buffers of the visible lights to the slots of the LightSources array.

 

How would I do that with this approach? I'd need the opposite of glBindBufferRange (To bind a buffer to a range of an indexed buffer target), but that doesn't seem to exist.

Do I have to use a single buffer for all light data? That would leave me with two alternatives:

- Store ALL lights within a large buffer and impose a limit of max lights allowed in a level (The MAX_LIGHTS only referred to max visible(in the view frustum)/active lights thus far), or

- Use a smaller buffer (sizeof(lightData) *MAX_LIGHTS) and re-upload the light data of all visible lights whenever there's a change.

I don't really like either solution, is there a third option I'm missing?

Share this post


Link to post
Share on other sites
Matias Goldberg    9580

Welcome to game development! Where there's no best choice and your options suck!

You just (re)discovered the problems of traditional Forward rendering (as opposed to Deferred Shading, or Forward+, or Clustered Forward). Best way to learn is the hard way.

What you're doing is called Forward shading, and no, you're not missing anything.

You either:

  1. Create a buffer with a fixed maximum number of lights (e.g. 8), use the same lights for all objects.
  2. Create a megabuffer of N*L where N is the number of objects, and L the max number or lights per object (e.g. 1000 objects, 8 lights per object = 8000)
  3. Use a non-forward solution

Forward+ and clustered forward for example, create a single buffer with all lights in scene; then via a special algorithm, it generates a second buffer with an index of the lights being used by that tile or cluster. e.g. Send all 500 lights in scene; then a special pass determines that a tile of pixels (or a 'cluster') uses lights 0, 12 and 485. Then the whole tile/cluster is shaded by those three lights:

for( int i=0; int i<lightsInTile; ++i )
{
     colour += shade( lights[tileList[i]] );
}
Edited by Matias Goldberg

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this