GLSL array parameter passing strangeness? (with workaround)

[pix]

This is kind of a followup of my post here:

http://www.ogre3d.org/phpBB2/viewtopic. ... 127#155127

.. in which I port crashy's HLSL instancing code to GLSL.

It includes code if you want to look in to any of these issues, but it's not the most minimal demonstration of the problem.

I guess I'll just list a few of the problems I ran in to. I'm not submitting a patch because I still think it's likely that I just went about this the wrong way.

The main thing the instancing program does is to set up a lot of custom parameters on the batchInstance renderable that get written into the elements of an array in the vertex shader.

The constants aren't referenced in the material file because there are a lot of them, so they are added in code. in the original HLSL version from crashy, this is done simply with:

Code: Select all

 for (int i =0;i<objectCount;i++)
   {
           params->setAutoConstant(i, GpuProgramParameters::ACT_CUSTOM,i);
   }

after a lot of poking around in the ogre source I realised that ogre GLSL implementation doesn't use indexed parameters at all, so this obviously wouldn't work.

i needed to use named constants, but I had no idea what names to use to refer to an array. in the shader it is defined as

Code: Select all

uniform vec4 Instance_Posititons[200]

to find out, i put some tracewrites into the GLSLGpuLinkProgram code, and found out that the elements appear as separate vec4 elements with names like

Code: Select all

Instance_Positions[0]
Instance_Positions[1]
...
Instance_Positions[199]

so i tried using setNamedAutoConstant, like this:

Code: Select all

for (int i=0;i<objectCount;++i)
   {
       String name = "Instance_Position["+StringConverter::toString(i)+"]";
       params->setNamedAutoConstant(name,GpuProgramParameters::ACT_CUSTOM,i);
       
   }

this had the problem that all of the named auto constants were getting assigned the same position in the mRealConstants array. this is because the

Code: Select all

setNamed*Constant

functions all just make a call to

Code: Select all

getParamIndex(name)

which returns the current highest index in either mRealConstants or mIntConstants. however, these arrays ar only actally resized on write. so unless you make a dummy write to each constant before creating the next one, everything gets allocated to the last element of the array. so:

Code: Select all

for (int i=0;i<objectCount;++i)
   {
       String name = "Instance_Position["+StringConverter::toString(i)+"]";

       params->setNamedConstant(name,Vector4::ZERO);
       params->setNamedAutoConstant(name,GpuProgramParameters::ACT_CUSTOM,i);

   }

still not working. finally putting some tracewrites into the GLSLLinkProgram::updateUniforms function I see that it actually sees these individual elements as arrays, that is,

Instance_Position[0] is an array of 200 elements
Instance_Position[1] is an array of 199 elements
etc

so each step it goes through and assigns all 200 RealConstants to Instance_Position[0]. then the next constants 1-199 to Instance_Position[1], 2-199 to Instance_Position[2] etc.. it seems that you need to write to the whole array at once, because it seems the final call writing the last value into Instance_Position[199] is the only value remaining in the array when the shader gets run. this is why in my earlier screenshot in the linked thread, shows each batch of ninjas only containing the last ninja.

i tried commenting out the array handling code in updateUniforms but it writing each element of the array individually still had the same effect, so i needed a way to get it to write the whole array. basically i came up with this solution, naming the parameters thusly:

Code: Select all

String name = i==0?"Instance_Position[0]":"Instance_Position-"+StringConverter::toString(i);

so the first parameter is called Instance_Position[0] and it is followed by 199 parameters called Instance_Position-1 through Instance_Position-199. although the Instance_Position-* parameters don't actually match any of the uniforms in the shader, they are used because the array handling part of updateUniforms gets the rest of the array values by stepping through the mRealConstants list.

anyhow, i hope this makes some sense to someone who has enough knowledge of this part of the code to fix it.

in a sense this all would have been easier had GLSL supported indexed parameters, but this probably also would have played havoc with its very-late-binding of parameter names.

pix.

[pix]

i just tried this same code on a linux box with an nvidia card and i just get behaviour as if all of the parameters were set to 0,0,0. so perhaps this has to do with the specific way the card lists it's uniforms params? i don't have all of the same debug stuff in the ogre on that machine so i can't say too much more. but anyhow, since it's maybe card specific i should add that my original code works on a linux machine running the fglrx (ati) driver, with this card/version:

OpenGL renderer string: MOBILITY RADEON 9700 Generic
OpenGL version string: 2.0.5755 (8.24.

pix.

[klauss]

Ehm... Is it just me, or all those nitpickyness should be handled by the GLSL gpu program class?

A patch supporting indexed variables for GLSL, even if faked indices, would be necessary... right?

[pix]

Ehm... Is it just me, or all those nitpickyness should be handled by the GLSL gpu program class?

definitely!

A patch supporting indexed variables for GLSL, even if faked indices, would be necessary... right?

they shouldn't need to be faked, it's just that the intermediate data structures that are being used don't seem to support doing both. i guess maybe the other languages do, but i haven't looked into their implementations.

i can think of plenty of hacky ways of solving this (like magic parameter names which specify the index to write to) but i'm sure someone else has some better ideas.

pix.

[pix]

btw, on the nvidia card, the Instance_Position array only appears as a single uniform called "Instance_Position" (ie, no subscripted uniforms for each element of the array). so if you change the "Instance_Position[0]" in my code to "Instance_Position" it works.

ugly, huh?

also, in 1.2.1, you need to comment out this line:

Code: Select all

 
mRoot->addFrameListener(new LightRotator(l)

pix.

[sinbad]

after a lot of poking around in the ogre source I realised that ogre GLSL implementation doesn't use indexed parameters at all, so this obviously wouldn't work.

The problem is that IIRC GLSL doesn't provide any way to retrieve the index for a named parameter, which is why we don't support indexed parameters on GLSL. In other shader languages a name is an alias to a constant index, but GLSL, if it uses that approach, hides it from you - I'm assuming that gives them more flexibility for param linking for future shader approaches whilst other languages are piggy-backing the technique used all the way back to SM1.

Yes, there are some gotchas still with GLSL array parameters; I fixed one aspect of this before Dagon was released but it clearly only resolved it for manual parameters (I needed to resolve it for the HDR shader which uses an array of gaussian coefficients). I'll log this as an issue.

[klauss]

BTW - a nice way to fake indexed access support would be to convert indices to names for passing to GLSL, by enumerating all variables and then sorting them. Number the sorted list, and then pick the right one. That will be invariant, given the same list of names, and should be enough to make it well-behaved in most situations, assuming arrays are properly detected and numbered along the way.

Seems obvious, but I though I'd post it just in case it was not.

[pix]

The problem is that IIRC GLSL doesn't provide any way to retrieve the index for a named parameter, which is why we don't support indexed parameters on GLSL.

actually it looks like you can, but only at link time, as OgreGLSLLinkProgram makes this call:

Code: Select all

glGetUniformLocationARB(mGLHandle, uniformName);

could some of these problems be solved by linking the program much earlier to get more information about the parameters?

pix.

[nfz]

I have been working on this problem since it was me that put this bug in when I put together the GLSL plugin back in August 2004. The problem is that Ogre gpu parameters are created when the script is parsed ie at load time but in GLSL you can't find out about available parameters until after compiling the GLSL and then linking all objects together. Linking will not occur until the material is used ie at the start of a rendering. Thats a problem.

Currently, space is allocated for Ogre gpu parameters (doesn't matter if its GLSL, HLSL, Cg) during script loading and is normally based on the parameters found by querying the all ready compiled shader (HLSL, Cg, asm) in the script except for GLSL. This can't happen for GLSL until the material is loaded, which doesn't occur until it is used. GLSL shaders are compiled when loaded but you can not get usefull parameter info at that time. Linking can't occur until you know which vertex and fragment shaders go together and there can be several. Currently its assumed that the user will have all the uniforms defined as gpu parameters in the material script and for most situations this works ok. Dummy gpu parameters are created based on what is in the script.

At link time the dummy space that was previously allocated (which may or may not be correct since its based on the users material script which could be wrong) is verified with the parameter info from the link. For parameters that are a fixed size ie float, vec2, vec3, mat3, mat4 this is not a problem. For arrays this is a problem since its not easy to change the spacing of the Ogre gpu parameters if the array size is wrong. So the current code basically ignores arrays (for almost 18 months no one has complained ). This is the reason why you currently cannot do skeletal animation with GLSL in Ogre. There are ways to work around it but it does not work on all cards.

So, to make a long story short, I came up with the solution of using dynamically allocated gpu parameters that can change in size and type on the fly but it did cause some interface breaking changes which I am trying to correct so that the code can go into Dagon.

[charlietmcheng]

i just tried this same code on a linux box with an nvidia card and i just get behaviour as if all of the parameters were set to 0,0,0.

Same here.

I just compiled your 800-ninja demo on my Linux box (with an nVidia 6600 PCI-E) and run. It looks like there is only one ninja standing on the origin, about which a light source rotates, but the number of triangles is 723410.

OpenGL renderer: GeForce 6600/PCI/SSE2
OpenGL version: 2.1.0 NVIDIA 96.31

[pix]

i haven't been paying too much attention but last time i looked, crashy had moved from HLSL to Cg for the instancing. Cg has a limited number of parameters, so it means smaller batches, but also means that we get to sidestep this linux incompatability. I haven't tried to get the latest Cg versions running on linux though.

[sinbad]

GLSL had some gotchas in previous versions because it does a number of things differently than the other shader languages (like parameter packing, vertex / fragment programs not being queryable until they're linked) and getting it to work with the framework established for Cg and HLSL was a little tricky. It came along a bit late compared to other shader languages so it was a design retrofit.

I've given the parameters system a complete facelift in Eihort to take the differences into account properly in the core design and so GLSL array parameter passing should be fine there.