glGetUniformIndices behaviour changed since Nvidia 197.13/16

Started by Kai, April 01, 2010, 20:41:40

Previous topic - Next topic

Kai

This is just an information for everyone that upgraded their drivers, use Uniform Buffer Objects and found, that their OpenGL applications suddenly don't work anymore.
It took me some time to figure that out, as I went crazy in front of my OpenGL application when I only saw a black screen...

NVidia actually changed the behaviour of glGetUniformIndices, which is used to query the index of a uniform member in a shader program.
I used uniform buffer objects a lot and queried the indices of the members to be able to update their values via a buffer object.
Before this new driver, glGetUniformIndices took the unqualified member names only!

An example:
#version 150

layout(shared) uniform CameraSettings {
  mat4 viewMatrix;
  mat4 projectionMatrix;
} camera;

in vec4 position;

void main(void) {
  gl_Position = camera.projectionMatrix * camera.projectionMatrix * position;
}


When trying to retrieve the index of the "viewMatrix" inside the uniform block with interface name "CameraSettings", before 197.xx, you had to query via glGetUniformIndices with simply "viewMatrix". Now, this does not work any longer and you need to query with the fully qualified name that consists of the interface name, a dot (.), and the member name: "CameraSettings.viewMatrix"..

Sadly, the spec (http://www.opengl.org/registry/specs/ARB/uniform_buffer_object.txt) merely says:
Quote
    Each uniform variable, declared in a shader, is broken down into one or
    more strings using the "." (dot) and "[]" operators, if necessary, to the
    point that it is legal to pass each string back into GetUniformLocation,
    for default uniform block uniform names, or GetUniformIndices, for
    named uniform block uniform names.

So, it does not even specify that for a named uniform block, the fullly-qualified name must be used. The name should merely be "legal" for that OpenGL functions...

elFarto

Hi

I've started noticing a strangely similar issue.

         int numUniforms = GL31.glGetActiveUniformBlock(program, nBlock, GL31.GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS);

         IntBuffer uniformIndicies = BufferUtils.createIntBuffer(Math.max(16, numUniforms));
         GL31.glGetActiveUniformBlock(program, nBlock, GL31.GL_UNIFORM_BLOCK_ACTIVE_UNIFORM_INDICES, uniformIndicies);

         IntBuffer uniformTypes = BufferUtils.createIntBuffer(Math.max(16, numUniforms));
         IntBuffer uniformOffsets = BufferUtils.createIntBuffer(Math.max(16, numUniforms));

         uniformIndicies.limit(numUniforms);

         GL31.glGetActiveUniforms(program, uniformIndicies, GL31.GL_UNIFORM_TYPE, uniformTypes);
         GL31.glGetActiveUniforms(program, uniformIndicies, GL31.GL_UNIFORM_OFFSET, uniformOffsets);


When a uniform in a uniform block isn't referenced, the glGetActiveUniforms calls fail with a GL_INVALID_VALUE and for the life of me I can't figure out why. Here's the block in question:

layout(shared) uniform PerMaterial
{
   vec3 ambientColour;
   vec3 diffuseColour;
   vec3 specularColour;
   float specPower;
};


numUniforms is 4, even when ambientColour isn't in use (which is correct I think). I have driver version 197.15.

Regards
elFarto

spasi

Quote from: Kai on April 01, 2010, 20:41:40When trying to retrieve the index of the "viewMatrix" inside the uniform block with interface name "CameraSettings", before 197.xx, you had to query via glGetUniformIndices with simply "viewMatrix". Now, this does not work any longer and you need to query with the fully qualified name that consists of the interface name, a dot (.), and the member name: "CameraSettings.viewMatrix"..

Sadly, the spec (http://www.opengl.org/registry/specs/ARB/uniform_buffer_object.txt) merely says:
Quote
    Each uniform variable, declared in a shader, is broken down into one or
    more strings using the "." (dot) and "[]" operators, if necessary, to the
    point that it is legal to pass each string back into GetUniformLocation,
    for default uniform block uniform names, or GetUniformIndices, for
    named uniform block uniform names.

So, it does not even specify that for a named uniform block, the fullly-qualified name must be used. The name should merely be "legal" for that OpenGL functions...

According to the GLSL spec, the behavior you're seeing is correct and it's what happens with my ATI driver too. Named uniform blocks must be fully qualified. Uniform blocks were added initially in GLSL version 1.40. Version 1.50 added the ability to name the uniform blocks and actually renamed them to interface blocks (since they're also used for in/out blocks). Quoting spec 1.50, section 4.3.7:

QuoteIf an instance name (instance-name) is not used, the names declared inside the block are scoped at the
global level and accessed as if they were declared outside the block. If an instance name (instance-name)
is used, then it puts all the members inside a scope within its own name space, accessed with the field
selector ( . ) operator (analogously to structures).

...

Outside the shading language (i.e., in the API), members are similarly identified except the block name is
always used in place of the instance name (API accesses are to interfaces, not to shaders). If there is no
instance name, then the API does not use the block name to access a member, just the member name.

spasi

Quote from: elFarto on April 02, 2010, 15:31:32When a uniform in a uniform block isn't referenced, the glGetActiveUniforms calls fail with a GL_INVALID_VALUE and for the life of me I can't figure out why. Here's the block in question:

layout(shared) uniform PerMaterial
{
   vec3 ambientColour;
   vec3 diffuseColour;
   vec3 specularColour;
   float specPower;
};


numUniforms is 4, even when ambientColour isn't in use (which is correct I think). I have driver version 197.15.

I've tried your code on ATI and it works without errors. The GL_UNIFORM_BLOCK_ACTIVE_UNIFORMS number should indeed be 4 for std140 and shared, it would be 3 with packed layout. Have you tried printing the uniformIndicies contents? Is the error triggered by using glGetActiveUniforms with GL_UNIFORM_TYPE or GL_UNIFORM_OFFSET (or both)?

elFarto

The error is with both GL_UNIFORM_TYPE and GL_UNIFORM_OFFSET. The indices are correct as I can use glGetActiveUniformName to retrieve its name. I'm starting to suspect a driver bug, as removing uniforms is a optimisation, and the user shouldn't be aware of it.

*edit*, noticed something odd:

         for (int i = 0; i < numUniforms; i++)
         {            
            String n = GL31.glGetActiveUniformName(program, uniformIndicies.get(i), 256);
            int type = GL31.glGetActiveUniforms(program, uniformIndicies.get(i), GL31.GL_UNIFORM_TYPE);
            int offset = GL31.glGetActiveUniforms(program, uniformIndicies.get(i), GL31.GL_UNIFORM_OFFSET);
            System.out.println("uniform index: " + uniformIndicies.get(i) + ", " + n +", " + type + ", " + offset);
         }


Prints this:

uniform index: 0, ambientColour, 4, 4
uniform index: 1, diffuseColour, 35665, 16
uniform index: 6, specPower, 16, 16
uniform index: 7, specularColour, 16, 16


Seems since your new methods are reusing a Buffer, the values are leaking across calls. While I don't think it's causing the issue, that could trip a lot of people up. Any chance of getting it to reset the buffer to 0 before making the call?

Regards
elFarto

spasi

Quote from: elFarto on April 02, 2010, 17:50:04as removing uniforms is a optimisation, and the user shouldn't be aware of it.

That's true in most cases, but with uniform blocks it isn't, since the correct behavior is described in the spec. With std140 or shared layouts, the driver should not remove any uniforms. With packed layout, inactive uniforms will be removed.

Quote from: elFarto on April 02, 2010, 17:50:04Seems since your new methods are reusing a Buffer, the values are leaking across calls. While I don't think it's causing the issue, that could trip a lot of people up. Any chance of getting it to reset the buffer to 0 before making the call?

Ah, you're using an old build then. This has been fixed a few commits ago. The current implementation does this internally:

IntBuffer params = APIUtils.getBufferInt();
nglGetActiveUniformsiv(program, 1, params.put(1, uniformIndex), 1, pname, params, params.position(), function_pointer);
return params.get(0);


Also, here's the output of your code on my ATI:

Quoteuniform index: 0, ambientColour, 35665, 0
uniform index: 1, diffuseColour, 35665, 16
uniform index: 8, specPower, 5126, 48
uniform index: 9, specularColour, 35665, 32

elFarto

Quote from: spasi on April 02, 2010, 18:41:26Ah, you're using an old build then. This has been fixed a few commits ago. The current implementation does this internally:
....
Ah, good.

Thanks for your help.

Regards
elFarto

Kai

QuoteAccording to the GLSL spec, the behavior you're seeing is correct and it's what happens with my ATI driver too. Named uniform blocks must be fully qualified. Uniform blocks were added initially in GLSL version 1.40. Version 1.50 added the ability to name the uniform blocks and actually renamed them to interface blocks (since they're also used for in/out blocks). Quoting spec 1.50, section 4.3.7:

Thanks spasi for pointing that out. I should've looked into the GLSL spec as well. Then, the driver before 197.13/16 did handle that incorrectly.