KhronosGroup / glslang

Khronos-reference front end for GLSL/ESSL, partial front end for HLSL, and a SPIR-V generator.
Other
2.91k stars 817 forks source link

Access layoutLocation via public headers #3491

Closed paroj closed 2 months ago

paroj commented 5 months ago

I used to do

program->getUniform(i).getType()->getQualifier().layoutLocation

to get the uniform location declared as:

layout(location = 2) uniform ...

now that the TType definition is gone from the public headers, is there another way to get to it? Or is this something that needs to be added back to the public API?

arcady-lunarg commented 5 months ago

Let me look into this, if there isn't a way to get this only using public headers then we'll have to add some stuff back to the set of installed headers.

paroj commented 2 months ago

just a quick reminder, wondering whether you found something?

PaulZFox commented 2 months ago

Wondering the same. Trying to compile GLSLangPlugin - have checked 1.3.279 and 1.3.280.1 (latest at the moment) versions of Vulkan SDK - there is no definition of TType, just declaration. Compilation failed due that.

Is there any resolution please?

Thanks, Sergei.

arcady-lunarg commented 2 months ago

Would using the spirv-reflect tool help here? Otherwise, does the ShGetUniformLocation() entry point help at all? If not, I think the thing to do would be to expose a new entry point for this.

PaulZFox commented 2 months ago

Arcady, sorry, probably I'm missing smth, but I don't think any tool/utility can help here. The point is: in Vulkan SDK header files ($VULKAN_SDK/x86_64/include/glslang/Public/ShaderLang.h:146 particularly), there is only declaration of class TType and no definition. However, in OGRE sources there are references to TType class members, like:

TType utype; if( utype->isOpaque()) ...

Compiler fails bc TType is not defined anywhere - no class TType member definitions.

Formally, what we have in $VULKAN_SDK/x86_64/include/glslang/Public/ShaderLang.h:146 is the only thing compiler knows 'bout class TType.

Thanks, Sergei.

arcady-lunarg commented 2 months ago

Yes, this is intentional: TType is part of the internal implementation of the compiler and was never really meant as a public interface. Please see issue #3320 for more justification about why we're doing these changes. In particular, I don't envision ever reverting TType to being public. You can vendor glslang in your project if you want and then the publicly installed headers don't matter, or we can work together to define another interface that is more suitable as a public API without as much compatibility maintenance burden.

PaulZFox commented 2 months ago

Thanks, Arcady, for clarifications provided.

or we can work together to define another interface that is more suitable

Frankly, I'm new to Ogre and Vulkan and would prefer @Paroj to comment on that.

Thanks, Sergei.

paroj commented 2 months ago

Would using the spirv-reflect tool help here? Otherwise, does the ShGetUniformLocation() entry point help at all? If not, I think the thing to do would be to expose a new entry point for this

spirv-reflect would probably work, however I would rather rely on the information generated at compile time then extracting the reflection a-posteriori from SPIRV.

The API is mostly there and only layoutLocation is missing. This is probably an oversight as per uniform layoutLocation is specific to the OpenGL target and does not exist in Vulkan.

I will need to check ShGetUniformLocation and report back whether it is a valid replacement.

@PaulZFox in any way the Ogre plugin will need to be updated to work with glslang v14 and require a new Ogre release.

@arcady-lunarg my goal is to get the API I can upgrade Ogre to. However from a distro perspective, there is currently no way to package ogre with GLSLang support and glslang v14+.

bubbleguuum commented 2 months ago

A distro can package the private includes. This is what openSUSE Tumbleweed did, especially for Ogre.

EDIT: they are packaged separately as glslang-nonstd-devel package.

paroj commented 2 months ago

Otherwise, does the ShGetUniformLocation() entry point help at all? If not, I think the thing to do would be to expose a new entry point for this.

@arcady-lunarg I took a look at this and the function is part of the old C interface, while I am using the C++ interface (as recommended in the readme). This means I cannot easily check whether the output of the method is plausible and the C++ interface would need to be extended anyway.