M120: Metal: only convert vertex if offset is not multiple of 4.
Previously we always convert vertex attribute if its binding offset
is not multiple of the attribute's size. This requirement seems to be
unnecessary.
This CL removes that requirement so the only requirement left for offset
is that it must be multiple of 4.
Bug: chromium:1496807
Change-Id: I35c421951c7817b77bd0c006ed4b72cd04b5a8d7
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5006359
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Quyen Le <lehoangquyen@chromium.org>
(cherry picked from commit 383df961157e868c2ae8c477848f1316a456b4db)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5034617
Auto-Submit: Quyen Le <lehoangquyen@chromium.org>
diff --git a/src/libANGLE/renderer/metal/VertexArrayMtl.mm b/src/libANGLE/renderer/metal/VertexArrayMtl.mm
index afb9b76..03ac18a 100644
--- a/src/libANGLE/renderer/metal/VertexArrayMtl.mm
+++ b/src/libANGLE/renderer/metal/VertexArrayMtl.mm
@@ -466,13 +466,11 @@
uint32_t bufferIdx = mtl::kVboBindingIndexStart + v;
uint32_t bufferOffset = static_cast<uint32_t>(mCurrentArrayBufferOffsets[v]);
- const angle::Format &angleFormat =
- mCurrentArrayBufferFormats[v]->actualAngleFormat();
desc.attributes[v].format = mCurrentArrayBufferFormats[v]->metalFormat;
desc.attributes[v].bufferIndex = bufferIdx;
desc.attributes[v].offset = 0;
- ASSERT((bufferOffset % angleFormat.pixelBytes) == 0);
+ ASSERT((bufferOffset % mtl::kVertexAttribBufferStrideAlignment) == 0);
ASSERT(bufferIdx < mtl::kMaxVertexAttribs);
if (binding.getDivisor() == 0)
@@ -677,7 +675,6 @@
mContentsObservers->enableForBuffer(bufferGL, static_cast<uint32_t>(attribIndex));
bool needConversion =
format.actualFormatId != format.intendedFormatId ||
- (binding.getOffset() % format.actualAngleFormat().pixelBytes) != 0 ||
(binding.getOffset() % mtl::kVertexAttribBufferStrideAlignment) != 0 ||
(binding.getStride() < format.actualAngleFormat().pixelBytes) ||
(binding.getStride() % mtl::kVertexAttribBufferStrideAlignment) != 0;
diff --git a/src/tests/gl_tests/VertexAttributeTest.cpp b/src/tests/gl_tests/VertexAttributeTest.cpp
index 96a5c3f..010662a 100644
--- a/src/tests/gl_tests/VertexAttributeTest.cpp
+++ b/src/tests/gl_tests/VertexAttributeTest.cpp
@@ -1394,6 +1394,111 @@
EXPECT_GL_NO_ERROR();
}
+// Verify that using a GL_FLOATx2 attribute with offset not divisible by 8 works.
+TEST_P(VertexAttributeTest, DrawArraysWith2FloatAtOffsetNotDivisbleBy8)
+{
+ initBasicProgram();
+ glUseProgram(mProgram);
+
+ // input data is GL_FLOATx2 (8 bytes) and stride=36
+ std::array<GLubyte, 36 * kVertexCount> inputData;
+ std::array<GLfloat, 2 * kVertexCount> expectedData;
+ for (size_t i = 0; i < kVertexCount; ++i)
+ {
+ expectedData[2 * i] = 2 * i;
+ expectedData[2 * i + 1] = 2 * i + 1;
+
+ GLubyte *input = inputData.data() + 36 * i;
+ memcpy(input, &expectedData[2 * i], sizeof(float));
+ memcpy(input + sizeof(float), &expectedData[2 * i + 1], sizeof(float));
+ }
+
+ GLBuffer quadBuffer;
+ InitQuadPlusOneVertexBuffer(&quadBuffer);
+
+ GLint positionLocation = glGetAttribLocation(mProgram, "position");
+ ASSERT_NE(-1, positionLocation);
+ glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
+ glEnableVertexAttribArray(positionLocation);
+
+ // offset is not float2 aligned (28)
+ GLsizei dataSize = 36 * kVertexCount + 28;
+ glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
+ glBufferData(GL_ARRAY_BUFFER, dataSize, nullptr, GL_STATIC_DRAW);
+ glBufferSubData(GL_ARRAY_BUFFER, 28, dataSize - 28, inputData.data());
+ glVertexAttribPointer(mTestAttrib, 2, GL_FLOAT, GL_FALSE, /* stride */ 36,
+ reinterpret_cast<void *>(28));
+ glEnableVertexAttribArray(mTestAttrib);
+
+ glBindBuffer(GL_ARRAY_BUFFER, 0);
+ glVertexAttribPointer(mExpectedAttrib, 2, GL_FLOAT, GL_FALSE, 0, expectedData.data());
+ glEnableVertexAttribArray(mExpectedAttrib);
+
+ // Vertex draw with no start vertex offset (second argument is zero).
+ glDrawArrays(GL_TRIANGLES, 0, 6);
+ checkPixels();
+
+ // Draw offset by one vertex.
+ glDrawArrays(GL_TRIANGLES, 1, 6);
+ checkPixels();
+
+ EXPECT_GL_NO_ERROR();
+}
+
+// Verify that using offset=1 for GL_BYTE vertex attribute doesn't mess up the draw.
+// Depending on backend, offset=1 for GL_BYTE could be natively supported or not.
+// In the latter case, a vertex data conversion will have to be performed.
+TEST_P(VertexAttributeTest, DrawArraysWithByteAtOffset1)
+{
+ initBasicProgram();
+ glUseProgram(mProgram);
+
+ // input data is GL_BYTEx3 (3 bytes) but stride=4
+ std::array<GLbyte, 4 * kVertexCount> inputData;
+ std::array<GLfloat, 3 * kVertexCount> expectedData;
+ for (size_t i = 0; i < kVertexCount; ++i)
+ {
+ inputData[4 * i] = 3 * i;
+ inputData[4 * i + 1] = 3 * i + 1;
+ inputData[4 * i + 2] = 3 * i + 2;
+
+ expectedData[3 * i] = 3 * i;
+ expectedData[3 * i + 1] = 3 * i + 1;
+ expectedData[3 * i + 2] = 3 * i + 2;
+ }
+
+ GLBuffer quadBuffer;
+ InitQuadPlusOneVertexBuffer(&quadBuffer);
+
+ GLint positionLocation = glGetAttribLocation(mProgram, "position");
+ ASSERT_NE(-1, positionLocation);
+ glVertexAttribPointer(positionLocation, 3, GL_FLOAT, GL_FALSE, 0, nullptr);
+ glEnableVertexAttribArray(positionLocation);
+
+ // Buffer offset (1)
+ GLsizei dataSize = 4 * kVertexCount + 1;
+ glBindBuffer(GL_ARRAY_BUFFER, mBuffer);
+ glBufferData(GL_ARRAY_BUFFER, dataSize, nullptr, GL_STATIC_DRAW);
+ glBufferSubData(GL_ARRAY_BUFFER, 1, dataSize - 1, inputData.data());
+ glVertexAttribPointer(mTestAttrib, 3, GL_BYTE, GL_FALSE, /* stride */ 4,
+ reinterpret_cast<void *>(1));
+ glEnableVertexAttribArray(mTestAttrib);
+
+ glBindBuffer(GL_ARRAY_BUFFER, 0);
+ glVertexAttribPointer(mExpectedAttrib, 3, GL_FLOAT, GL_FALSE, 0, expectedData.data());
+ glEnableVertexAttribArray(mExpectedAttrib);
+
+ // Vertex draw with no start vertex offset (second argument is zero).
+ glDrawArrays(GL_TRIANGLES, 0, 6);
+ checkPixels();
+
+ // Draw offset by one vertex.
+ glDrawArrays(GL_TRIANGLES, 1, 6);
+ checkPixels();
+
+ EXPECT_GL_NO_ERROR();
+}
+
// Verify that using an aligned but non-multiples of 4 offset vertex attribute doesn't mess up the
// draw.
TEST_P(VertexAttributeTest, DrawArraysWithShortBufferOffsetNotMultipleOf4)
