Fix non-zero validation in Draw and DrawIndexed
Metal validation layer does not allow indexCount or instanceCount to be 0 in
drawIndexedPrimitives and drawPrimitives, otherwise we should draw nothing
with these operations.
BUG=dawn:76
TEST=dawn_end2end_tests
Change-Id: Ic22be73ac992289d4bc8d7b3d4d30d20c4488776
Reviewed-on: https://dawn-review.googlesource.com/c/3900
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index 9475c87..4604f8a 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -980,6 +980,7 @@
"src/tests/end2end/CopyTests.cpp",
"src/tests/end2end/DepthStencilStateTests.cpp",
"src/tests/end2end/DrawIndexedTests.cpp",
+ "src/tests/end2end/DrawTests.cpp",
"src/tests/end2end/FenceTests.cpp",
"src/tests/end2end/IndexFormatTests.cpp",
"src/tests/end2end/InputStateTests.cpp",
diff --git a/src/dawn_native/metal/CommandBufferMTL.mm b/src/dawn_native/metal/CommandBufferMTL.mm
index 22f8241..7fd8ad4 100644
--- a/src/dawn_native/metal/CommandBufferMTL.mm
+++ b/src/dawn_native/metal/CommandBufferMTL.mm
@@ -408,26 +408,32 @@
case Command::Draw: {
DrawCmd* draw = mCommands.NextCommand<DrawCmd>();
- [encoder drawPrimitives:lastPipeline->GetMTLPrimitiveTopology()
- vertexStart:draw->firstVertex
- vertexCount:draw->vertexCount
- instanceCount:draw->instanceCount
- baseInstance:draw->firstInstance];
+ // The instance count must be non-zero, otherwise no-op
+ if (draw->instanceCount != 0) {
+ [encoder drawPrimitives:lastPipeline->GetMTLPrimitiveTopology()
+ vertexStart:draw->firstVertex
+ vertexCount:draw->vertexCount
+ instanceCount:draw->instanceCount
+ baseInstance:draw->firstInstance];
+ }
} break;
case Command::DrawIndexed: {
DrawIndexedCmd* draw = mCommands.NextCommand<DrawIndexedCmd>();
size_t formatSize = IndexFormatSize(lastPipeline->GetIndexFormat());
- [encoder
- drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology()
- indexCount:draw->indexCount
- indexType:lastPipeline->GetMTLIndexType()
- indexBuffer:indexBuffer
- indexBufferOffset:indexBufferBaseOffset + draw->firstIndex * formatSize
- instanceCount:draw->instanceCount
- baseVertex:draw->baseVertex
- baseInstance:draw->firstInstance];
+ // The index and instance count must be non-zero, otherwise no-op
+ if (draw->indexCount != 0 && draw->instanceCount != 0) {
+ [encoder drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology()
+ indexCount:draw->indexCount
+ indexType:lastPipeline->GetMTLIndexType()
+ indexBuffer:indexBuffer
+ indexBufferOffset:indexBufferBaseOffset +
+ draw->firstIndex * formatSize
+ instanceCount:draw->instanceCount
+ baseVertex:draw->baseVertex
+ baseInstance:draw->firstInstance];
+ }
} break;
case Command::SetRenderPipeline: {
diff --git a/src/tests/end2end/DrawTests.cpp b/src/tests/end2end/DrawTests.cpp
new file mode 100644
index 0000000..0df0d90
--- /dev/null
+++ b/src/tests/end2end/DrawTests.cpp
@@ -0,0 +1,113 @@
+// Copyright 2019 The Dawn Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include "tests/DawnTest.h"
+
+#include "utils/ComboRenderPipelineDescriptor.h"
+#include "utils/DawnHelpers.h"
+
+constexpr uint32_t kRTSize = 4;
+
+class DrawTest : public DawnTest {
+ protected:
+ void SetUp() override {
+ DawnTest::SetUp();
+
+ renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
+
+ dawn::InputState inputState =
+ device.CreateInputStateBuilder()
+ .SetInput(0, 4 * sizeof(float), dawn::InputStepMode::Vertex)
+ .SetAttribute(0, 0, dawn::VertexFormat::FloatR32G32B32A32, 0)
+ .GetResult();
+
+ dawn::ShaderModule vsModule =
+ utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, R"(
+ #version 450
+ layout(location = 0) in vec4 pos;
+ void main() {
+ gl_Position = pos;
+ })");
+
+ dawn::ShaderModule fsModule =
+ utils::CreateShaderModule(device, dawn::ShaderStage::Fragment, R"(
+ #version 450
+ layout(location = 0) out vec4 fragColor;
+ void main() {
+ fragColor = vec4(0.0, 1.0, 0.0, 1.0);
+ })");
+
+ utils::ComboRenderPipelineDescriptor descriptor(device);
+ descriptor.cVertexStage.module = vsModule;
+ descriptor.cFragmentStage.module = fsModule;
+ descriptor.primitiveTopology = dawn::PrimitiveTopology::TriangleStrip;
+ descriptor.indexFormat = dawn::IndexFormat::Uint32;
+ descriptor.inputState = inputState;
+ descriptor.cColorAttachments[0]->format = renderPass.colorFormat;
+
+ pipeline = device.CreateRenderPipeline(&descriptor);
+
+ vertexBuffer = utils::CreateBufferFromData<float>(
+ device, dawn::BufferUsageBit::Vertex,
+ {// The bottom left triangle
+ -1.0f, -1.0f, 0.0f, 1.0f, 1.0f, 1.0f, 0.0f, 1.0f, -1.0f, 1.0f, 0.0f, 1.0f,
+
+ // The top right triangle
+ -1.0f, -1.0f, 0.0f, 1.0f, 1.0f, 1.0f, 0.0f, 1.0f, 1.0f, -1.0f, 0.0f, 1.0f});
+ }
+
+ utils::BasicRenderPass renderPass;
+ dawn::RenderPipeline pipeline;
+ dawn::Buffer vertexBuffer;
+
+ void Test(uint32_t vertexCount,
+ uint32_t instanceCount,
+ uint32_t firstIndex,
+ uint32_t firstInstance,
+ RGBA8 bottomLeftExpected,
+ RGBA8 topRightExpected) {
+ uint32_t zeroOffset = 0;
+ dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+ {
+ dawn::RenderPassEncoder pass = builder.BeginRenderPass(renderPass.renderPassInfo);
+ pass.SetPipeline(pipeline);
+ pass.SetVertexBuffers(0, 1, &vertexBuffer, &zeroOffset);
+ pass.Draw(vertexCount, instanceCount, firstIndex, firstInstance);
+ pass.EndPass();
+ }
+
+ dawn::CommandBuffer commands = builder.GetResult();
+ queue.Submit(1, &commands);
+
+ EXPECT_PIXEL_RGBA8_EQ(bottomLeftExpected, renderPass.color, 1, 3);
+ EXPECT_PIXEL_RGBA8_EQ(topRightExpected, renderPass.color, 3, 1);
+ }
+};
+
+// The basic triangle draw.
+TEST_P(DrawTest, Uint32) {
+ RGBA8 filled(0, 255, 0, 255);
+ RGBA8 notFilled(0, 0, 0, 0);
+
+ // Test a draw with no indices.
+ Test(0, 0, 0, 0, notFilled, notFilled);
+ // Test a draw with only the first 3 indices (bottom left triangle)
+ Test(3, 1, 0, 0, filled, notFilled);
+ // Test a draw with only the last 3 indices (top right triangle)
+ Test(3, 1, 3, 0, notFilled, filled);
+ // Test a draw with all 6 indices (both triangles).
+ Test(6, 1, 0, 0, filled, filled);
+}
+
+DAWN_INSTANTIATE_TEST(DrawTest, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend)
