commit 2a30e14074c266cff97b8632072af2871683d457
author Yunchao He <yunchao.he@intel.com> Tue Apr 02 07:42:18 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Apr 02 07:42:18 2019 +0000
tree 90fd0317e059a27c8009407ed0f06841d9a12fe9
parent 8e50ba19ba2eba83cbe89a945388ec40393001e1

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

diff --git a/src/dawn_native/RenderPassEncoder.cpp b/src/dawn_native/RenderPassEncoder.cpp
index 6bf0b8e..23f923d 100644
--- a/src/dawn_native/RenderPassEncoder.cpp
+++ b/src/dawn_native/RenderPassEncoder.cpp
@@ -115,6 +115,10 @@
         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;

src/tests/end2end/ScissorTests.cpp

diff --git a/src/tests/end2end/ScissorTests.cpp b/src/tests/end2end/ScissorTests.cpp
index c564418..6208d92 100644
--- a/src/tests/end2end/ScissorTests.cpp
+++ b/src/tests/end2end/ScissorTests.cpp
@@ -91,32 +91,6 @@
     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 @@
     // 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

src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp

diff --git a/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp b/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
index 0777035..282a72a 100644
--- a/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
+++ b/src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp
@@ -30,17 +30,35 @@
     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