aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTim Van Patten <timvp@google.com>2019-11-25 16:31:43 -0700
committerCommit Bot <commit-bot@chromium.org>2020-04-18 03:08:37 +0000
commit33b58ebb7ea83ef98c19d746aa44a846ac2f88fd (patch)
treec1feb1a86e4ba928e876235ff5684a47b994aef6
parent2d5da0298b9b97f36cc9bc6bd2a0dc3fc835049d (diff)
downloadangle-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.cpp10
-rw-r--r--src/compiler/translator/blocklayout.cpp3
-rw-r--r--src/tests/deqp_support/deqp_gles31_test_expectations.txt6
-rw-r--r--src/tests/gl_tests/ShaderStorageBufferTest.cpp100
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