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>

BUILD.gn

@@ -980,6 +980,7 @@ test("dawn_end2end_tests") {
     "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",

src/dawn_native/metal/CommandBufferMTL.mm

@@ -408,26 +408,32 @@ namespace dawn_native { namespace metal {
                 case Command::Draw: {
                     DrawCmd* draw = mCommands.NextCommand<DrawCmd>();
 
+                    // 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
+                    // The index and instance count must be non-zero, otherwise no-op
-                        drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology()
+                    if (draw->indexCount != 0 && draw->instanceCount != 0) {
+                        [encoder drawIndexedPrimitives:lastPipeline->GetMTLPrimitiveTopology()
                                             indexCount:draw->indexCount
                                              indexType:lastPipeline->GetMTLIndexType()
                                            indexBuffer:indexBuffer
-                            indexBufferOffset:indexBufferBaseOffset + draw->firstIndex * formatSize
+                                     indexBufferOffset:indexBufferBaseOffset +
+                                                       draw->firstIndex * formatSize
                                          instanceCount:draw->instanceCount
                                             baseVertex:draw->baseVertex
                                           baseInstance:draw->firstInstance];
+                    }
                 } break;
 
                 case Command::SetRenderPipeline: {

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)