diff options
author | Tim Van Patten <timvp@google.com> | 2019-11-25 16:31:43 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-04-18 03:08:37 +0000 |
commit | 33b58ebb7ea83ef98c19d746aa44a846ac2f88fd (patch) | |
tree | c1feb1a86e4ba928e876235ff5684a47b994aef6 | |
parent | 2d5da0298b9b97f36cc9bc6bd2a0dc3fc835049d (diff) | |
download | angle-33b58ebb7ea83ef98c19d746aa44a846ac2f88fd.tar.gz |
Vulkan: Give unsized arrays at least 1 entry
The function ShaderVariable::getNestedArraySize needs to return at least
1 entry for unsized arrays to ensure the shader buffer sizes are
reported correctly.
This also allows ANGLE to treat unsized arrays of structs and basic
types the same way, allowing for the special treatment of unsized arrays
in TraverseStructArrayVariable() to be removed.
Bug: angleproject:3596
Test: dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.*
Change-Id: I3b2a3a68c25e0913b79e989d7c719b34ce9b75fd
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/1934952
Commit-Queue: Tim Van Patten <timvp@google.com>
Reviewed-by: Cody Northrop <cnorthrop@google.com>
-rw-r--r-- | src/compiler/translator/ShaderVars.cpp | 10 | ||||
-rw-r--r-- | src/compiler/translator/blocklayout.cpp | 3 | ||||
-rw-r--r-- | src/tests/deqp_support/deqp_gles31_test_expectations.txt | 6 | ||||
-rw-r--r-- | src/tests/gl_tests/ShaderStorageBufferTest.cpp | 100 |
4 files changed, 114 insertions, 5 deletions
diff --git a/src/compiler/translator/ShaderVars.cpp b/src/compiler/translator/ShaderVars.cpp index f5e4f21ef2..44780948b7 100644 --- a/src/compiler/translator/ShaderVars.cpp +++ b/src/compiler/translator/ShaderVars.cpp @@ -163,7 +163,15 @@ void ShaderVariable::indexIntoArray(unsigned int arrayIndex) unsigned int ShaderVariable::getNestedArraySize(unsigned int arrayNestingIndex) const { ASSERT(arraySizes.size() > arrayNestingIndex); - return arraySizes[arraySizes.size() - 1u - arrayNestingIndex]; + unsigned int arraySize = arraySizes[arraySizes.size() - 1u - arrayNestingIndex]; + + if (arraySize == 0) + { + // Unsized array, so give it at least 1 entry + arraySize = 1; + } + + return arraySize; } unsigned int ShaderVariable::getBasicTypeElementCount() const diff --git a/src/compiler/translator/blocklayout.cpp b/src/compiler/translator/blocklayout.cpp index 9cbc96b365..d681e4a1d1 100644 --- a/src/compiler/translator/blocklayout.cpp +++ b/src/compiler/translator/blocklayout.cpp @@ -82,8 +82,7 @@ void TraverseStructArrayVariable(const ShaderVariable &variable, // Nested arrays are processed starting from outermost (arrayNestingIndex 0u) and ending at the // innermost. We make a special case for unsized arrays. const unsigned int currentArraySize = variable.getNestedArraySize(0); - unsigned int count = std::max(currentArraySize, 1u); - for (unsigned int arrayElement = 0u; arrayElement < count; ++arrayElement) + for (unsigned int arrayElement = 0u; arrayElement < currentArraySize; ++arrayElement) { visitor->enterArrayElement(variable, arrayElement); ShaderVariable elementVar = variable; diff --git a/src/tests/deqp_support/deqp_gles31_test_expectations.txt b/src/tests/deqp_support/deqp_gles31_test_expectations.txt index a48e874448..f165f87393 100644 --- a/src/tests/deqp_support/deqp_gles31_test_expectations.txt +++ b/src/tests/deqp_support/deqp_gles31_test_expectations.txt @@ -180,8 +180,10 @@ // Debug (test bug): 3590 SWIFTSHADER : dEQP-GLES31.functional.debug.negative_coverage.get_error.buffer.framebuffer_texture2d = FAIL -// Front-end query bugs: -3596 VULKAN : dEQP-GLES31.functional.program_interface_query.shader_storage_block.buffer_data_size.* = FAIL +// GLSL length() method returning number of bytes in an array instead of number of items +4098 SWIFTSHADER : dEQP-GLES31.functional.compute.basic.write_multiple_unsized_arr_multiple_groups = FAIL +4098 SWIFTSHADER : dEQP-GLES31.functional.compute.basic.write_multiple_unsized_arr_single_invocation = FAIL + // Not failing in last official run, but failed recently: 4110 SWIFTSHADER : dEQP-GLES31.functional.shaders.helper_invocation.* = FAIL diff --git a/src/tests/gl_tests/ShaderStorageBufferTest.cpp b/src/tests/gl_tests/ShaderStorageBufferTest.cpp index d73a1b1503..b2472adea8 100644 --- a/src/tests/gl_tests/ShaderStorageBufferTest.cpp +++ b/src/tests/gl_tests/ShaderStorageBufferTest.cpp @@ -2258,6 +2258,106 @@ void main() EXPECT_GL_NO_ERROR(); } +// Verify the size of the buffer with unsized struct array is calculated correctly +TEST_P(ShaderStorageBufferTest31, BigStructUnsizedStructArraySize) +{ + // TODO(http://anglebug.com/3596) + ANGLE_SKIP_TEST_IF(IsAMD() && IsWindows() && IsOpenGL()); + + constexpr char kComputeShaderSource[] = + R"(#version 310 es +layout (local_size_x=1) in; + +struct S +{ + mat4 m; // 4 vec4 = 16 floats + vec4 a[10]; // 10 vec4 = 40 floats +}; + +layout(binding=0) buffer B +{ + vec4 precedingMember; // 4 floats + S precedingMemberUnsizedArray[]; // 56 floats +} b; + +void main() +{ + if (false) + { + b.precedingMember = vec4(1.0, 1.0, 1.0, 1.0); + } +} +)"; + + ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource); + EXPECT_GL_NO_ERROR(); + + glUseProgram(program); + glDispatchCompute(1, 1, 1); + EXPECT_GL_NO_ERROR(); + + GLuint resourceIndex = glGetProgramResourceIndex(program, GL_SHADER_STORAGE_BLOCK, "B"); + EXPECT_GL_NO_ERROR(); + EXPECT_NE(resourceIndex, 0xFFFFFFFF); + + GLenum property = GL_BUFFER_DATA_SIZE; + GLint queryData = -1; + glGetProgramResourceiv(program, GL_SHADER_STORAGE_BLOCK, resourceIndex, 1, &property, 1, + nullptr, &queryData); + EXPECT_GL_NO_ERROR(); + + // 60 * sizeof(float) = 240 + // Vulkan rounds up to the required buffer alignment, so >= 240 + EXPECT_GE(queryData, 240); +} + +// Verify the size of the buffer with unsized float array is calculated correctly +TEST_P(ShaderStorageBufferTest31, BigStructUnsizedFloatArraySize) +{ + // TODO(http://anglebug.com/3596) + ANGLE_SKIP_TEST_IF(IsAMD() && IsWindows() && IsOpenGL()); + + constexpr char kComputeShaderSource[] = + R"(#version 310 es +layout (local_size_x=1) in; + +layout(binding=0) buffer B +{ + vec4 precedingMember; // 4 floats + float precedingMemberUnsizedArray[]; // "1" float +} b; + +void main() +{ + if (false) + { + b.precedingMember = vec4(1.0, 1.0, 1.0, 1.0); + } +} +)"; + + ANGLE_GL_COMPUTE_PROGRAM(program, kComputeShaderSource); + EXPECT_GL_NO_ERROR(); + + glUseProgram(program); + glDispatchCompute(1, 1, 1); + EXPECT_GL_NO_ERROR(); + + GLuint resourceIndex = glGetProgramResourceIndex(program, GL_SHADER_STORAGE_BLOCK, "B"); + EXPECT_GL_NO_ERROR(); + EXPECT_NE(resourceIndex, 0xFFFFFFFF); + + GLenum property = GL_BUFFER_DATA_SIZE; + GLint queryData = -1; + glGetProgramResourceiv(program, GL_SHADER_STORAGE_BLOCK, resourceIndex, 1, &property, 1, + nullptr, &queryData); + EXPECT_GL_NO_ERROR(); + + // 5 * sizeof(float) = 20 + // Vulkan rounds up to the required buffer alignment, so >= 20 + EXPECT_GE(queryData, 20); +} + ANGLE_INSTANTIATE_TEST_ES31(ShaderStorageBufferTest31); } // namespace |