commit d8cdcea0eeb1f0467bdd0c37c7be638b0427b169
author Austin Eng <enga@chromium.org> Mon Aug 19 22:48:48 2019 +0000
committer Commit Bot service account <commit-bot@chromium.org> Mon Aug 19 22:48:48 2019 +0000
tree a2a2bf41698768fae8e0d16dc5f8fc26ccdce523
parent ee3de1e1f3da2386197a2c586304deb263890a35

Validate the RenderBundle debug groups must be well nested

This patch also adds unittests to check the behavior.

Bug: dawn:154
Change-Id: I49f4c0ab98cd823231a701192bdb725937960833
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10260
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>

src/dawn_native/CommandValidation.cpp

diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp
index 78f8949..75101f0 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -192,6 +192,7 @@
                                                  "Command disallowed inside a render bundle"));
         }
 
+        DAWN_TRY(ValidateDebugGroups(debugGroupStackSize));
         DAWN_TRY(usageTracker.ValidateRenderPassUsages());
         ASSERT(resourceUsage != nullptr);
         *resourceUsage = usageTracker.AcquireResourceUsage();

src/tests/unittests/validation/RenderBundleValidationTests.cpp

diff --git a/src/tests/unittests/validation/RenderBundleValidationTests.cpp b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
index daac454..6c38506 100644
--- a/src/tests/unittests/validation/RenderBundleValidationTests.cpp
+++ b/src/tests/unittests/validation/RenderBundleValidationTests.cpp
@@ -173,6 +173,65 @@
     commandEncoder.Finish();
 }
 
+// Test that render bundle debug groups must be well nested.
+TEST_F(RenderBundleValidationTest, DebugGroups) {
+    DummyRenderPass renderPass(device);
+
+    utils::ComboRenderBundleEncoderDescriptor desc = {};
+    desc.colorFormatsCount = 1;
+    desc.cColorFormats[0] = renderPass.attachmentFormat;
+
+    // Test a single debug group works.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PushDebugGroup("group");
+        renderBundleEncoder.PopDebugGroup();
+        renderBundleEncoder.Finish();
+    }
+
+    // Test nested debug groups work.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PushDebugGroup("group");
+        renderBundleEncoder.PushDebugGroup("group2");
+        renderBundleEncoder.PopDebugGroup();
+        renderBundleEncoder.PopDebugGroup();
+        renderBundleEncoder.Finish();
+    }
+
+    // Test popping when no group is pushed is invalid.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PopDebugGroup();
+        ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+    }
+
+    // Test popping too many times is invalid.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PushDebugGroup("group");
+        renderBundleEncoder.PopDebugGroup();
+        renderBundleEncoder.PopDebugGroup();
+        ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+    }
+
+    // Test that a single debug group must be popped.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PushDebugGroup("group");
+        ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+    }
+
+    // Test that all debug groups must be popped.
+    {
+        dawn::RenderBundleEncoder renderBundleEncoder = device.CreateRenderBundleEncoder(&desc);
+        renderBundleEncoder.PushDebugGroup("group");
+        renderBundleEncoder.PushDebugGroup("group2");
+        renderBundleEncoder.PopDebugGroup();
+        ASSERT_DEVICE_ERROR(renderBundleEncoder.Finish());
+    }
+}
+
 // Test render bundles do not inherit command buffer state
 TEST_F(RenderBundleValidationTest, StateInheritance) {
     DummyRenderPass renderPass(device);