diff options
| author | Tim Foley <tfoleyNV@users.noreply.github.com> | 2019-04-29 08:32:24 -0700 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2019-04-29 08:32:24 -0700 |
| commit | b7c60910367f2af2c359d76783975de0a4659c68 (patch) | |
| tree | b5a4fd9a6435737d85b8711e854bfb01ba862933 /tests/bugs | |
| parent | d3709c0a58eae582d3750310a9a19b5593876e3b (diff) | |
Enable appropriate GLSL extension for unbounded-size resource arrays (#957)
Fixes #941
The GLSL we were emitting for unbounded-size arrays was the obvious:
```hlsl
// This HLSL:
Texture2D t[];
```
```glsl
// ... becomes this GLSL:
texture2D t[];
```
Unfortunately, the legacy GLSL behavior for an array without a declared size is what is called an "implicitly-sized" array, which means that it is assumed to actually have a fixed size, which is determined by the maximum integer constant value used to index into it (and only integer constants are allowed to be used when indexing into it).
Users hadn't noticed the issue for a while, because most of our users who rely on unbounded-size arrays were also using the HLSL `NonUniformResourceIndex` function:
```hlsl
float4 v = t[NonUniformResourceIndex(idx)].Sample(...);
```
When mapping such code to GLSL we use the `nonuniformEXT` qualifier added by the `GL_EXT_nonuniform_qualifier` extension, and it turns out that a secondary feature of that extension is that it changes the GLSL language semantics for arrays (of resources) with an unspecified size, so that they instead behave like we want. So users were happy and we were blissfully ignorant of the lurking issue.
The problem is that as soon as a user neglects to use `NonUniformResourceIndex` (perhaps because an index really is uniform):
```hlsl
cbuffer C { uint definitelyUniform; }
...
float4 v = t[definitelyUniform].Sample(...);
```
Now the code we emit doesn't need `nonuniformEXT` so it doesn't enable `GL_EXT_nonuniform_qualifier` and the declaration of `t` now falls under the "implicitly-sized" array rules, and thus the code fails because `definitelyUniform` is being used as an index but is *not* an integer constant.
The fix is pretty simple: when emitting a declaration of a global shader parameter to GLSL, we check if it is an unbounded-size array of resources and, if so, enable the `GL_EXT_nonuniform_qualifier` extension.
We don't need any clever handling to deal with resource parameters nested in `struct` types or in entry-point parameter lists, etc., because previous IR passes will have split up complex types and moved everything to the global scope already.
Diffstat (limited to 'tests/bugs')
| -rw-r--r-- | tests/bugs/gh-941.slang | 17 | ||||
| -rw-r--r-- | tests/bugs/gh-941.slang.glsl | 38 |
2 files changed, 55 insertions, 0 deletions
diff --git a/tests/bugs/gh-941.slang b/tests/bugs/gh-941.slang new file mode 100644 index 000000000..f66c25e97 --- /dev/null +++ b/tests/bugs/gh-941.slang @@ -0,0 +1,17 @@ +//TEST:CROSS_COMPILE: -profile ps_5_0 -entry main -target spirv-assembly + +// Ensure that we add the `GL_EXT_nonuniform_qualifier` extension for any code that uses unbounded-size arrays of resources. + +Texture2D t[]; +SamplerState s; + +cbuffer C +{ + float2 uv; + uint index; +} + +float4 main() : SV_Target +{ + return t[index].Sample(s, uv); +} diff --git a/tests/bugs/gh-941.slang.glsl b/tests/bugs/gh-941.slang.glsl new file mode 100644 index 000000000..d3c29820d --- /dev/null +++ b/tests/bugs/gh-941.slang.glsl @@ -0,0 +1,38 @@ +//TEST_IGNORE_FILE: + +#version 450 + +#extension GL_EXT_nonuniform_qualifier : require + +struct SLANG_ParameterGroup_C_0 +{ + vec2 uv_0; + uint index_0; +}; + +layout(binding = 2) +layout(std140) +uniform _S1 +{ + SLANG_ParameterGroup_C_0 _data; +} C_0; + +layout(binding = 0) +uniform texture2D t_0[]; + +layout(binding = 1) +uniform sampler s_0; + +layout(location = 0) +out vec4 _S2; + +void main() +{ + vec4 _S3 = texture( + sampler2D( + t_0[C_0._data.index_0], + s_0), + C_0._data.uv_0); + _S2 = _S3; + return; +}
\ No newline at end of file |
