From 2a30e14074c266cff97b8632072af2871683d457 Mon Sep 17 00:00:00 2001
From: Yunchao He <yunchao.he@intel.com>
Date: Tue, 2 Apr 2019 07:42:18 +0000
Subject: [PATCH] Generate an error if scissor rect is empty

We need to change related tests in end2end_tests and unittests.

Bug=dawn:127
TEST=dawn_end2end_tests, dawn_unittests

Change-Id: I523d4eeb930990b5db381544b228d2f11912049b
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/6240
Commit-Queue: Yunchao He <yunchao.he@intel.com>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
---
 src/dawn_native/RenderPassEncoder.cpp         |  4 +++
 src/tests/end2end/ScissorTests.cpp            | 28 +------------------
 .../DynamicStateCommandValidationTests.cpp    | 22 +++++++++++++--
 3 files changed, 25 insertions(+), 29 deletions(-)

diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp
index 6bf0b8eda1..23f923dea7 100644
--- a/src/dawn_native/RenderPassEncoder.cpp
+++ b/src/dawn_native/RenderPassEncoder.cpp
@@ -115,6 +115,10 @@ namespace dawn_native {
         if (mTopLevelEncoder->ConsumedError(ValidateCanRecordCommands())) {
             return;
         }
+        if (width == 0 || height == 0) {
+            mTopLevelEncoder->HandleError("Width and height must be greater than 0.");
+            return;
+        }
 
         SetScissorRectCmd* cmd = mAllocator->Allocate<SetScissorRectCmd>(Command::SetScissorRect);
         new (cmd) SetScissorRectCmd;
diff --git a/src/tests/end2end/ScissorTests.cpp b/src/tests/end2end/ScissorTests.cpp
index c564418c7a..6208d92f99 100644
--- a/src/tests/end2end/ScissorTests.cpp
+++ b/src/tests/end2end/ScissorTests.cpp
@@ -91,32 +91,6 @@ TEST_P(ScissorTest, LargerThanAttachment) {
     EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 255, 0, 255), renderPass.color, 99, 99);
 }
 
-// Test setting an empty scissor rect
-TEST_P(ScissorTest, EmptyRect) {
-    DAWN_SKIP_TEST_IF(IsMetal());
-    DAWN_SKIP_TEST_IF(IsWindows() && IsVulkan() && IsIntel());
-
-    utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 2, 2);
-    dawn::RenderPipeline pipeline = CreateQuadPipeline(renderPass.colorFormat);
-
-    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
-    {
-        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
-        pass.SetPipeline(pipeline);
-        pass.SetScissorRect(0, 0, 0, 0);
-        pass.Draw(6, 1, 0, 0);
-        pass.EndPass();
-    }
-
-    dawn::CommandBuffer commands = encoder.Finish();
-    queue.Submit(1, &commands);
-
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), renderPass.color, 0, 0);
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), renderPass.color, 0, 1);
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), renderPass.color, 1, 0);
-    EXPECT_PIXEL_RGBA8_EQ(RGBA8(0, 0, 0, 0), renderPass.color, 1, 1);
-}
-
 // Test setting a partial scissor (not empty, not full attachment)
 TEST_P(ScissorTest, PartialRect) {
     utils::BasicRenderPass renderPass = utils::CreateBasicRenderPass(device, 100, 100);
@@ -156,7 +130,7 @@ TEST_P(ScissorTest, NoInheritanceBetweenRenderPass) {
     // RenderPass 1 set the scissor
     {
         dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
-        pass.SetScissorRect(0, 0, 0, 0);
+        pass.SetScissorRect(1, 1, 1, 1);
         pass.EndPass();
     }
     // RenderPass 2 draw a full quad, it shouldn't be scissored
diff --git a/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp b/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
index 07770356b7..282a72ae8f 100644
--- a/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
+++ b/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
@@ -30,17 +30,35 @@ TEST_F(SetScissorRectTest, Success) {
     encoder.Finish();
 }
 
-// Test to check that an empty scissor is allowed
+// Test to check that an empty scissor is not allowed
 TEST_F(SetScissorRectTest, EmptyScissor) {
     DummyRenderPass renderPass(device);
 
     dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+
+    // Width of scissor rect is zero.
+    {
+        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetScissorRect(0, 0, 0, 1);
+        pass.EndPass();
+    }
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+
+    // Height of scissor rect is zero.
+    {
+        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.SetScissorRect(0, 0, 1, 0);
+        pass.EndPass();
+    }
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+
+    // Both width and height of scissor rect are zero.
     {
         dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
         pass.SetScissorRect(0, 0, 0, 0);
         pass.EndPass();
     }
-    encoder.Finish();
+    ASSERT_DEVICE_ERROR(encoder.Finish());
 }
 
 // Test to check that a scissor larger than the framebuffer is allowed