search

KhronosGroup / glslang

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

Multiview: wrong storage class of variables decorated as ViewIndex #759

Closed ghost closed 7 years ago

ghost commented 7 years ago

glslang generates following SPIR-V for ViewIndex builld-in from multiview EXT:

Decorate 12(gl_ViewIndex) BuiltIn ViewIndex
....
12(gl_ViewIndex):     11(ptr) Variable UniformConstant

However the Vulkan spec says: 

The code:ViewIndexKHX decoration can: be applied to a shader *input*`

My reading of above is the variable storage should be 'Input', not 'UniformConstant'

dgkoch commented 7 years ago

This is being clarified in the VK_KHX_multiview spec that "The variable decorated with code:ViewIndex must: be declared using the code:Input storage class."

johnkslang commented 7 years ago

Is it also correct then that these are

in int gl_ViewIndex;
in int gl_DeviceIndex;

so that they map to the input storage class in SPIR-V? They were added to glslang as uniform.

johnkslang commented 7 years ago

I think the in vs. uniform distinction would now be a spec. modification, if anything, as that shouldn't break GLSL content, but is needed to get correct SPIR-V content. Hence, that's addressed by 7a44a31.

dgkoch commented 7 years ago

They are already defined as "in" in the GLSL spec. See https://www.khronos.org/registry/vulkan/specs/misc/GL_EXT_device_group.txt https://www.khronos.org/registry/vulkan/specs/misc/GL_EXT_multiview.txt

johnkslang commented 7 years ago

Okay, everything should be good now then, closing.

johnkslang commented 7 years ago

Actually, those specs leave the storage qualifier off and don't say where to add them. They say to add them to the built-ins, but not where, or whether they are in in/out blocks, etc. They do refer to them as input variables, but it's not enough resolution to know how to add them.

I think this delta needs to be described against the 4.50 specification.

johnkslang commented 7 years ago

I reverted this until knowing more about it, and also to see what impact it had on the test bot.

johnkslang commented 7 years ago

c08fb8ab9c0d3aeafe11a8eeeed177c882ef76a9 correctly implements what the extension is supposed to say. Updated extensions will be published in the future. Main point is the variables are in (Input SPIR-V), and only device index is available in compute shaders.