KhronosGroup / OpenGL-API

OpenGL and OpenGL ES API Issue Tracker
34 stars 5 forks source link

Explicit uniform location and cross-stage uniforms. #49

Closed NicolBolas closed 5 years ago

NicolBolas commented 5 years ago

The GLSL standard says this:

Uniforms in shaders all share a single global name space when linked into a program or separable program. Hence, the types, initializers, and any location specifiers of all statically used uniform variables with the same name must match across all shaders that are linked into a single program.

This is (part of) the rule that allows us to merge uniform variables across shader stages within a program. So if you declare a uniform with matching name and type in two stages, then both stages are accessing the same uniform.

Then ARB_explicit_uniform_location came along and added this statement:

No two default-block uniform variables in the program can have the same location, even if they are unused, otherwise a compile-time or link-time error will be generated.

The meaning of this line appears to be that, if you use explicit locations, you cannot use the same location in another shader stage, even for a variable with the same name and type. That is, if you want to use explicit locations, you have to give up uniform merging.

Now, one thing that makes this interpretation problematic is that NVIDIA's implementation just ignores it. But there's also the statement I quoted at the top. It explicitly mentions "any location specifiers," which strongly suggests that the intent is to allow them to be merged. After all, if you can't set the location to the same value in different shaders in the same program, why bother saying you can?

The intent seems to be that "two default-block uniform variables" are only different if they have different names, types, and explicit location values.

Here's the really weird part: that phrase is new. It was added to GLSL in 4.60 (possibly one of the revisions of it, but I'm only looking at the current version). It doesn't appear to have come from an extension specification, and I don't believe it was mentioned in the lengthy set of changes. It just showed up.

So... what's meant to be going on here? Is the current intent to allow explicit uniform locations to work across stages, to name the same variable with the same type and explicit location and have them be merged?

pdaniell-nv commented 5 years ago

Thanks for reporting this issue. I don't think the second quoted spec language is new. It appears in the original extension GL_ARB_explicit_uniform_location and also in GLSL 4.30.

I think what it's trying to say is that you can't have two non-matching variables using the same location. For example "layout(location=0) uniform vec4 foo;" in a vertex shader and "layout(location=0) uniform vec2 bar;" in a fragment shader linked together in the same non-separable program.

I think that sentence can be improved by saying instead something like:

Default-block uniform variable declarations sharing the same location linked in the program have to match by name, type and qualifiers.

NicolBolas commented 5 years ago

I don't think the second quoted spec language is new.

It's the first one that's new. It's the "any location specifiers" bit that was a recent addition. I should have made that more clear in my initial post.

But that's the main issue. If the first quote doesn't exist, then the meaning of the second quote is clear and unequivocal: you cannot have two uniforms in the same program with the same location, period. As you pointed out, that statement has been there since explicit uniform locations were first added. So since GLSL 4.30, the specification stated pretty strongly that uniform locations cannot be used on two variables, regardless of whether they have the same name/type or not.

It is only the presence of the first quote, which is new, that throws this interpretation into doubt.

It seems that the goal always was to allow explicit uniform locations to merge uniform definitions if they match, and the language didn't make that clear.

pdaniell-nv commented 5 years ago

Oh sorry. It seems like that "any location specifiers" part comes as a result of the merging of the GLSL and ESSL sources when we converted over to asciidoc. We went through an exercise of making the source common between GLSL and ESSL where possible. In this case (internally https://gitlab.khronos.org/opengl/GLSL/merge_requests/8) it appears the "any location specifiers" was added to GLSL from ESSL in the merged source. I think it was intentional and is valid. I think the GLSL spec has always allowed multiple declarations of the same default-block uniform within a program.

I think the source of this issue is perhaps your interpretation of the 2nd quote? It supposed to mean that variable declarations given the same location have to match. In other words, two different variables given the same location result in compile-time or link-time error.

Given that, perhaps my suggestion https://github.com/KhronosGroup/OpenGL-API/issues/49#issuecomment-490598572 is still valid?

NicolBolas commented 5 years ago

@pdaniell-nv

Given that, perhaps my suggestion #49 (comment) is still valid?

If that's the desired behavior, then the quote in your comment should make that quite clear.

pdaniell-nv commented 5 years ago

A refinement of my proposed change in https://github.com/KhronosGroup/OpenGL-API/issues/49#issuecomment-490598572 to include arrayness and structs:

Default-block uniform variable declarations sharing the same location linked in the program have to match by name, type, qualifiers and arrayness. For arrays their array dimensionality and array sizes must match. For structs this rule applies recursively to all members.

johnkslang commented 5 years ago

This is now published in Rev. 7 of GLSL 4.60, and Rev. 6 of ESSL 3.20.