Vulkan: Allow gaps in vertex attributes by packing them.
Work around restrictions with setting null vertex buffers on command buffers
by binding ranges of non-null buffers.
Enable VertexAttributeTest on ES2/Vulkan
BUG=angleproject:2792
BUG=angleproject:2797
Change-Id: Ief9db1e60c8c9045a4101abe859302d529decbcb
Reviewed-on: https://chromium-review.googlesource.com/1194282
Commit-Queue: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index e1bd3e9..355fa73 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -51,6 +51,46 @@
ANGLE_TRY(dynamicBuffer->flush(contextVk));
return angle::Result::Continue();
}
+
+void BindNonNullVertexBufferRanges(vk::CommandBuffer *commandBuffer,
+ const gl::AttributesMask &nonNullAttribMask,
+ uint32_t maxAttrib,
+ const gl::AttribArray<VkBuffer> &arrayBufferHandles,
+ const gl::AttribArray<VkDeviceSize> &arrayBufferOffsets)
+{
+ // Vulkan does not allow binding a null vertex buffer but the default state of null buffers is
+ // valid.
+
+ // We can detect if there are no gaps in active attributes by using the mask of the program
+ // attribs and the max enabled attrib.
+ ASSERT(maxAttrib > 0);
+ if (nonNullAttribMask.to_ulong() == (maxAttrib - 1))
+ {
+ commandBuffer->bindVertexBuffers(0, maxAttrib, arrayBufferHandles.data(),
+ arrayBufferOffsets.data());
+ return;
+ }
+
+ // Find ranges of non-null buffers and bind them all together.
+ for (uint32_t attribIdx = 0; attribIdx < maxAttrib; attribIdx++)
+ {
+ if (arrayBufferHandles[attribIdx] != VK_NULL_HANDLE)
+ {
+ // Find the end of this range of non-null handles
+ uint32_t rangeCount = 1;
+ while (attribIdx + rangeCount < maxAttrib &&
+ arrayBufferHandles[attribIdx + rangeCount] != VK_NULL_HANDLE)
+ {
+ rangeCount++;
+ }
+
+ commandBuffer->bindVertexBuffers(attribIdx, rangeCount, &arrayBufferHandles[attribIdx],
+ &arrayBufferOffsets[attribIdx]);
+ attribIdx += rangeCount;
+ }
+ }
+}
+
} // anonymous namespace
#define INIT \
@@ -541,6 +581,7 @@
ContextVk *contextVk = vk::GetImpl(context);
const gl::State &state = context->getGLState();
const gl::Program *programGL = state.getProgram();
+ const gl::AttributesMask &programAttribsMask = programGL->getActiveAttribLocationsMask();
const gl::AttributesMask &clientAttribs = context->getStateCache().getActiveClientAttribsMask();
uint32_t maxAttrib = programGL->getState().getMaxActiveAttribLocation();
@@ -583,15 +624,15 @@
&mCurrentArrayBufferOffsets[attribIndex]));
}
- commandBuffer->bindVertexBuffers(0, maxAttrib, mCurrentArrayBufferHandles.data(),
- mCurrentArrayBufferOffsets.data());
+ BindNonNullVertexBufferRanges(commandBuffer, programAttribsMask, maxAttrib,
+ mCurrentArrayBufferHandles, mCurrentArrayBufferOffsets);
}
else if (mVertexBuffersDirty || newCommandBuffer)
{
if (maxAttrib > 0)
{
- commandBuffer->bindVertexBuffers(0, maxAttrib, mCurrentArrayBufferHandles.data(),
- mCurrentArrayBufferOffsets.data());
+ BindNonNullVertexBufferRanges(commandBuffer, programAttribsMask, maxAttrib,
+ mCurrentArrayBufferHandles, mCurrentArrayBufferOffsets);
const gl::AttributesMask &bufferedAttribs =
context->getStateCache().getActiveBufferedAttribsMask();
diff --git a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
index cd25dfb..7e7432e 100644
--- a/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
+++ b/src/libANGLE/renderer/vulkan/vk_cache_utils.cpp
@@ -481,14 +481,11 @@
{
const auto attribIndex = static_cast<uint32_t>(attribIndexSizeT);
- VkVertexInputBindingDescription &bindingDesc = bindingDescs[attribIndex];
- VkVertexInputAttributeDescription &attribDesc = attributeDescs[attribIndex];
+ VkVertexInputBindingDescription &bindingDesc = bindingDescs[vertexAttribCount];
+ VkVertexInputAttributeDescription &attribDesc = attributeDescs[vertexAttribCount];
const PackedVertexInputBindingDesc &packedBinding = mVertexInputBindings[attribIndex];
const PackedVertexInputAttributeDesc &packedAttrib = mVertexInputAttribs[attribIndex];
- // TODO(jmadill): Support for gaps in vertex attribute specification.
- vertexAttribCount = attribIndex + 1;
-
bindingDesc.binding = attribIndex;
bindingDesc.inputRate = static_cast<VkVertexInputRate>(packedBinding.inputRate);
bindingDesc.stride = static_cast<uint32_t>(packedBinding.stride);
@@ -497,6 +494,8 @@
attribDesc.format = static_cast<VkFormat>(packedAttrib.format);
attribDesc.location = static_cast<uint32_t>(packedAttrib.location);
attribDesc.offset = packedAttrib.offset;
+
+ vertexAttribCount++;
}
// The binding descriptions are filled in at draw time.
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 5f1c21c..8b9c2be 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp
@@ -304,6 +304,9 @@
TEST_P(VertexAttributeTest, UnsignedByteUnnormalized)
{
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
+
std::array<GLubyte, kVertexCount> inputData = {
{0, 1, 2, 3, 4, 5, 6, 7, 125, 126, 127, 128, 129, 250, 251, 252, 253, 254, 255}};
std::array<GLfloat, kVertexCount> expectedData;
@@ -319,6 +322,10 @@
TEST_P(VertexAttributeTest, UnsignedByteNormalized)
{
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
+ ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
+
std::array<GLubyte, kVertexCount> inputData = {
{0, 1, 2, 3, 4, 5, 6, 7, 125, 126, 127, 128, 129, 250, 251, 252, 253, 254, 255}};
std::array<GLfloat, kVertexCount> expectedData;
@@ -334,6 +341,9 @@
TEST_P(VertexAttributeTest, ByteUnnormalized)
{
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
+
std::array<GLbyte, kVertexCount> inputData = {
{0, 1, 2, 3, 4, -1, -2, -3, -4, 125, 126, 127, -128, -127, -126}};
std::array<GLfloat, kVertexCount> expectedData;
@@ -348,6 +358,10 @@
TEST_P(VertexAttributeTest, ByteNormalized)
{
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
+ ANGLE_SKIP_TEST_IF(IsAndroid() && IsVulkan());
+
std::array<GLbyte, kVertexCount> inputData = {
{0, 1, 2, 3, 4, -1, -2, -3, -4, 125, 126, 127, -128, -127, -126}};
std::array<GLfloat, kVertexCount> expectedData;
@@ -698,6 +712,9 @@
// TODO(jmadill): Figure out why we get this error on AMD/OpenGL.
ANGLE_SKIP_TEST_IF(IsAMD() && IsOpenGL());
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsLinux() && IsVulkan() && IsIntel());
+
GLint maxAttribs;
glGetIntegerv(GL_MAX_VERTEX_ATTRIBS, &maxAttribs);
ASSERT_GL_NO_ERROR();
@@ -941,6 +958,13 @@
// Known failure on Retina MBP: http://crbug.com/635081
ANGLE_SKIP_TEST_IF(IsOSX() && IsNVIDIA());
+ // TODO: Support this test on Vulkan. http://anglebug.com/2797
+ ANGLE_SKIP_TEST_IF(IsLinux() && IsVulkan() && IsIntel());
+
+ // TODO: Fix syncing of default attributes when there is no call to enable/disable.
+ // http://anglebug.com/2800
+ ANGLE_SKIP_TEST_IF(IsVulkan());
+
constexpr char vsSource[] =
"attribute vec4 a_position;\n"
"attribute vec4 a_color;\n"
@@ -1568,6 +1592,61 @@
}
}
+// Test that if there are gaps in the attribute indices, the attributes have their correct values.
+TEST_P(VertexAttributeTest, UnusedVertexAttribWorks)
+{
+ constexpr char kVertexShader[] = R"(attribute vec2 position;
+attribute float actualValue;
+uniform float expectedValue;
+varying float result;
+void main()
+{
+ result = (actualValue == expectedValue) ? 1.0 : 0.0;
+ gl_Position = vec4(position, 0, 1);
+})";
+
+ constexpr char kFragmentShader[] = R"(varying mediump float result;
+void main()
+{
+ gl_FragColor = result > 0.0 ? vec4(0, 1, 0, 1) : vec4(1, 0, 0, 1);
+})";
+
+ ANGLE_GL_PROGRAM(program, kVertexShader, kFragmentShader);
+
+ // Force a gap in attributes by using location 0 and 3
+ GLint positionLocation = 0;
+ glBindAttribLocation(program, positionLocation, "position");
+
+ GLint attribLoc = 3;
+ glBindAttribLocation(program, attribLoc, "actualValue");
+
+ // Re-link the program to update the attribute locations
+ glLinkProgram(program);
+ ASSERT_TRUE(CheckLinkStatusAndReturnProgram(program, true));
+
+ glUseProgram(program);
+
+ GLint uniLoc = glGetUniformLocation(program, "expectedValue");
+ ASSERT_NE(-1, uniLoc);
+
+ glVertexAttribPointer(attribLoc, 1, GL_FLOAT, GL_FALSE, 0, nullptr);
+
+ ASSERT_NE(-1, positionLocation);
+ setupQuadVertexBuffer(0.5f, 1.0f);
+ glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, 0);
+ glEnableVertexAttribArray(positionLocation);
+
+ std::array<GLfloat, 4> testValues = {{1, 2, 3, 4}};
+ for (GLfloat testValue : testValues)
+ {
+ glUniform1f(uniLoc, testValue);
+ glVertexAttrib1f(attribLoc, testValue);
+ glDrawArrays(GL_TRIANGLES, 0, 6);
+ ASSERT_GL_NO_ERROR();
+ EXPECT_PIXEL_COLOR_EQ(0, 0, GLColor::green);
+ }
+}
+
// Tests that repeatedly updating a disabled vertex attribute works as expected.
// This covers an ANGLE bug where dirty bits for current values were ignoring repeated updates.
TEST_P(VertexAttributeTest, DisabledAttribUpdates)
@@ -1777,7 +1856,8 @@
ES2_OPENGL(),
ES3_OPENGL(),
ES2_OPENGLES(),
- ES3_OPENGLES());
+ ES3_OPENGLES(),
+ ES2_VULKAN());
ANGLE_INSTANTIATE_TEST(VertexAttributeTestES3, ES3_D3D11(), ES3_OPENGL(), ES3_OPENGLES());