Implement debug markers and groups for CommandEncoder

BUG=dawn:22

Change-Id: I1fc6ac3dec936268a043753169ed1d4a405881bd
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/10962
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>

dawn.json

@@ -313,6 +313,22 @@
                     {"name": "destination", "type": "texture copy view", "annotation": "const*"},
                     {"name": "copy size", "type": "extent 3D", "annotation": "const*"}
                 ]
+            },
+            {
+                "name": "insert debug marker",
+                "args": [
+                    {"name": "group label", "type": "char", "annotation": "const*", "length": "strlen"}
+                ]
+            },
+            {
+                "name": "pop debug group",
+                "args": []
+            },
+            {
+                "name": "push debug group",
+                "args": [
+                    {"name": "group label", "type": "char", "annotation": "const*", "length": "strlen"}
+                ]
             }
         ]
     },

src/dawn_native/CommandEncoder.cpp

@@ -667,6 +667,40 @@ namespace dawn_native {
         });
     }
 
+    void CommandEncoderBase::InsertDebugMarker(const char* groupLabel) {
+        mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
+            InsertDebugMarkerCmd* cmd =
+                allocator->Allocate<InsertDebugMarkerCmd>(Command::InsertDebugMarker);
+            cmd->length = strlen(groupLabel);
+
+            char* label = allocator->AllocateData<char>(cmd->length + 1);
+            memcpy(label, groupLabel, cmd->length + 1);
+
+            return {};
+        });
+    }
+
+    void CommandEncoderBase::PopDebugGroup() {
+        mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
+            allocator->Allocate<PopDebugGroupCmd>(Command::PopDebugGroup);
+
+            return {};
+        });
+    }
+
+    void CommandEncoderBase::PushDebugGroup(const char* groupLabel) {
+        mEncodingContext.TryEncode(this, [&](CommandAllocator* allocator) -> MaybeError {
+            PushDebugGroupCmd* cmd =
+                allocator->Allocate<PushDebugGroupCmd>(Command::PushDebugGroup);
+            cmd->length = strlen(groupLabel);
+
+            char* label = allocator->AllocateData<char>(cmd->length + 1);
+            memcpy(label, groupLabel, cmd->length + 1);
+
+            return {};
+        });
+    }
+
     CommandBufferBase* CommandEncoderBase::Finish(const CommandBufferDescriptor* descriptor) {
         TRACE_EVENT0(GetDevice()->GetPlatform(), TRACE_DISABLED_BY_DEFAULT("gpu.dawn"),
                      "CommandEncoderBase::Finish");
@@ -689,6 +723,8 @@ namespace dawn_native {
         // encoding context. Subsequent calls to encode commands will generate errors.
         DAWN_TRY(mEncodingContext.Finish());
 
+        uint64_t debugGroupStackSize = 0;
+
         CommandIterator* commands = mEncodingContext.GetIterator();
         commands->Reset();
 
@@ -820,11 +856,29 @@ namespace dawn_native {
                     mResourceUsages.topLevelTextures.insert(copy->destination.texture.Get());
                 } break;
 
+                case Command::InsertDebugMarker: {
+                    InsertDebugMarkerCmd* cmd = commands->NextCommand<InsertDebugMarkerCmd>();
+                    commands->NextData<char>(cmd->length + 1);
+                } break;
+
+                case Command::PopDebugGroup: {
+                    commands->NextCommand<PopDebugGroupCmd>();
+                    DAWN_TRY(ValidateCanPopDebugGroup(debugGroupStackSize));
+                    debugGroupStackSize--;
+                } break;
+
+                case Command::PushDebugGroup: {
+                    PushDebugGroupCmd* cmd = commands->NextCommand<PushDebugGroupCmd>();
+                    commands->NextData<char>(cmd->length + 1);
+                    debugGroupStackSize++;
+                } break;
                 default:
                     return DAWN_VALIDATION_ERROR("Command disallowed outside of a pass");
             }
         }
 
+        DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize));
+
         return {};
     }
 

src/dawn_native/CommandEncoder.h

@@ -38,6 +38,7 @@ namespace dawn_native {
         // Dawn API
         ComputePassEncoderBase* BeginComputePass(const ComputePassDescriptor* descriptor);
         RenderPassEncoderBase* BeginRenderPass(const RenderPassDescriptor* descriptor);
+
         void CopyBufferToBuffer(BufferBase* source,
                                 uint64_t sourceOffset,
                                 BufferBase* destination,
@@ -52,6 +53,11 @@ namespace dawn_native {
         void CopyTextureToTexture(const TextureCopyView* source,
                                   const TextureCopyView* destination,
                                   const Extent3D* copySize);
+
+        void InsertDebugMarker(const char* groupLabel);
+        void PopDebugGroup();
+        void PushDebugGroup(const char* groupLabel);
+
         CommandBufferBase* Finish(const CommandBufferDescriptor* descriptor);
 
       private:

src/dawn_native/CommandValidation.cpp

@@ -26,29 +26,6 @@ namespace dawn_native {
 
     namespace {
 
-        inline MaybeError PushDebugMarkerStack(unsigned int* counter) {
-            *counter += 1;
-            return {};
-        }
-
-        inline MaybeError PopDebugMarkerStack(unsigned int* counter) {
-            if (*counter == 0) {
-                return DAWN_VALIDATION_ERROR("Pop must be balanced by a corresponding Push.");
-            } else {
-                *counter -= 1;
-            }
-
-            return {};
-        }
-
-        inline MaybeError ValidateDebugGroups(const unsigned int counter) {
-            if (counter != 0) {
-                return DAWN_VALIDATION_ERROR("Each Push must be balanced by a corresponding Pop.");
-            }
-
-            return {};
-        }
-
         void TrackBindGroupResourceUsage(BindGroupBase* group,
                                          PassResourceUsageTracker* usageTracker) {
             const auto& layoutInfo = group->GetLayout()->GetBindingInfo();
@@ -88,7 +65,7 @@ namespace dawn_native {
                                                       PassResourceUsageTracker* usageTracker,
                                                       CommandBufferStateTracker* commandBufferState,
                                                       const AttachmentState* attachmentState,
-                                                      unsigned int* debugGroupStackSize,
+                                                      uint64_t* debugGroupStackSize,
                                                       const char* disallowedMessage) {
             switch (type) {
                 case Command::Draw: {
@@ -122,13 +99,14 @@ namespace dawn_native {
 
                 case Command::PopDebugGroup: {
                     commands->NextCommand<PopDebugGroupCmd>();
-                    DAWN_TRY(PopDebugMarkerStack(debugGroupStackSize));
+                    DAWN_TRY(ValidateCanPopDebugGroup(*debugGroupStackSize));
+                    *debugGroupStackSize -= 1;
                 } break;
 
                 case Command::PushDebugGroup: {
                     PushDebugGroupCmd* cmd = commands->NextCommand<PushDebugGroupCmd>();
                     commands->NextData<char>(cmd->length + 1);
-                    DAWN_TRY(PushDebugMarkerStack(debugGroupStackSize));
+                    *debugGroupStackSize += 1;
                 } break;
 
                 case Command::SetRenderPipeline: {
@@ -178,12 +156,26 @@ namespace dawn_native {
 
     }  // namespace
 
+    MaybeError ValidateCanPopDebugGroup(uint64_t debugGroupStackSize) {
+        if (debugGroupStackSize == 0) {
+            return DAWN_VALIDATION_ERROR("Pop must be balanced by a corresponding Push.");
+        }
+        return {};
+    }
+
+    MaybeError ValidateFinalDebugGroupStackSize(uint64_t debugGroupStackSize) {
+        if (debugGroupStackSize != 0) {
+            return DAWN_VALIDATION_ERROR("Each Push must be balanced by a corresponding Pop.");
+        }
+        return {};
+    }
+
     MaybeError ValidateRenderBundle(CommandIterator* commands,
                                     const AttachmentState* attachmentState,
                                     PassResourceUsage* resourceUsage) {
         PassResourceUsageTracker usageTracker;
         CommandBufferStateTracker commandBufferState;
-        unsigned int debugGroupStackSize = 0;
+        uint64_t debugGroupStackSize = 0;
 
         Command type;
         while (commands->NextCommandId(&type)) {
@@ -192,7 +184,7 @@ namespace dawn_native {
                                                  "Command disallowed inside a render bundle"));
         }
 
-        DAWN_TRY(ValidateDebugGroups(debugGroupStackSize));
+        DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize));
         DAWN_TRY(usageTracker.ValidateRenderPassUsages());
         ASSERT(resourceUsage != nullptr);
         *resourceUsage = usageTracker.AcquireResourceUsage();
@@ -205,7 +197,7 @@ namespace dawn_native {
                                   std::vector<PassResourceUsage>* perPassResourceUsages) {
         PassResourceUsageTracker usageTracker;
         CommandBufferStateTracker commandBufferState;
-        unsigned int debugGroupStackSize = 0;
+        uint64_t debugGroupStackSize = 0;
 
         // Track usage of the render pass attachments
         for (uint32_t i : IterateBitSet(renderPass->attachmentState->GetColorAttachmentsMask())) {
@@ -231,8 +223,7 @@ namespace dawn_native {
                 case Command::EndRenderPass: {
                     commands->NextCommand<EndRenderPassCmd>();
 
-                    DAWN_TRY(ValidateDebugGroups(debugGroupStackSize));
+                    DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize));
-
                     DAWN_TRY(usageTracker.ValidateRenderPassUsages());
                     ASSERT(perPassResourceUsages != nullptr);
                     perPassResourceUsages->push_back(usageTracker.AcquireResourceUsage());
@@ -299,7 +290,7 @@ namespace dawn_native {
                                    std::vector<PassResourceUsage>* perPassResourceUsages) {
         PassResourceUsageTracker usageTracker;
         CommandBufferStateTracker commandBufferState;
-        unsigned int debugGroupStackSize = 0;
+        uint64_t debugGroupStackSize = 0;
 
         Command type;
         while (commands->NextCommandId(&type)) {
@@ -307,8 +298,7 @@ namespace dawn_native {
                 case Command::EndComputePass: {
                     commands->NextCommand<EndComputePassCmd>();
 
-                    DAWN_TRY(ValidateDebugGroups(debugGroupStackSize));
+                    DAWN_TRY(ValidateFinalDebugGroupStackSize(debugGroupStackSize));
-
                     DAWN_TRY(usageTracker.ValidateComputePassUsages());
                     ASSERT(perPassResourceUsages != nullptr);
                     perPassResourceUsages->push_back(usageTracker.AcquireResourceUsage());
@@ -334,13 +324,14 @@ namespace dawn_native {
 
                 case Command::PopDebugGroup: {
                     commands->NextCommand<PopDebugGroupCmd>();
-                    DAWN_TRY(PopDebugMarkerStack(&debugGroupStackSize));
+                    DAWN_TRY(ValidateCanPopDebugGroup(debugGroupStackSize));
+                    debugGroupStackSize--;
                 } break;
 
                 case Command::PushDebugGroup: {
                     PushDebugGroupCmd* cmd = commands->NextCommand<PushDebugGroupCmd>();
                     commands->NextData<char>(cmd->length + 1);
-                    DAWN_TRY(PushDebugMarkerStack(&debugGroupStackSize));
+                    debugGroupStackSize++;
                 } break;
 
                 case Command::SetComputePipeline: {

src/dawn_native/CommandValidation.h

@@ -26,6 +26,9 @@ namespace dawn_native {
     struct BeginRenderPassCmd;
     struct PassResourceUsage;
 
+    MaybeError ValidateCanPopDebugGroup(uint64_t debugGroupStackSize);
+    MaybeError ValidateFinalDebugGroupStackSize(uint64_t debugGroupStackSize);
+
     MaybeError ValidateRenderBundle(CommandIterator* commands,
                                     const AttachmentState* attachmentState,
                                     PassResourceUsage* resourceUsage);

src/tests/unittests/validation/DebugMarkerValidationTests.cpp

@@ -114,4 +114,95 @@ TEST_F(DebugMarkerValidationTest, ComputeUnbalancedPop) {
     }
 
     ASSERT_DEVICE_ERROR(encoder.Finish());
-}
+}
+
+// Correct usage of debug markers should succeed in command encoder.
+TEST_F(DebugMarkerValidationTest, CommandEncoderSuccess) {
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    encoder.PushDebugGroup("Event Start");
+    encoder.InsertDebugMarker("Marker");
+    encoder.PopDebugGroup();
+    encoder.PopDebugGroup();
+    encoder.Finish();
+}
+
+// A PushDebugGroup call without a following PopDebugGroup produces an error in command encoder.
+TEST_F(DebugMarkerValidationTest, CommandEncoderUnbalancedPush) {
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    encoder.PushDebugGroup("Event Start");
+    encoder.InsertDebugMarker("Marker");
+    encoder.PopDebugGroup();
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+}
+
+// A PopDebugGroup call without a preceding PushDebugGroup produces an error in command encoder.
+TEST_F(DebugMarkerValidationTest, CommandEncoderUnbalancedPop) {
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    encoder.InsertDebugMarker("Marker");
+    encoder.PopDebugGroup();
+    encoder.PopDebugGroup();
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+}
+
+// It is possible to nested pushes in a compute pass in a command encoder.
+TEST_F(DebugMarkerValidationTest, NestedComputeInCommandEncoder) {
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    {
+        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
+        pass.PushDebugGroup("Event Start");
+        pass.InsertDebugMarker("Marker");
+        pass.PopDebugGroup();
+        pass.EndPass();
+    }
+    encoder.PopDebugGroup();
+    encoder.Finish();
+}
+
+// Command encoder and compute pass pushes must be balanced independently.
+TEST_F(DebugMarkerValidationTest, NestedComputeInCommandEncoderIndependent) {
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    {
+        dawn::ComputePassEncoder pass = encoder.BeginComputePass();
+        pass.InsertDebugMarker("Marker");
+        pass.PopDebugGroup();
+        pass.EndPass();
+    }
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+}
+
+// It is possible to nested pushes in a render pass in a command encoder.
+TEST_F(DebugMarkerValidationTest, NestedRenderInCommandEncoder) {
+    DummyRenderPass renderPass(device);
+
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    {
+        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.PushDebugGroup("Event Start");
+        pass.InsertDebugMarker("Marker");
+        pass.PopDebugGroup();
+        pass.EndPass();
+    }
+    encoder.PopDebugGroup();
+    encoder.Finish();
+}
+
+// Command encoder and render pass pushes must be balanced independently.
+TEST_F(DebugMarkerValidationTest, NestedRenderInCommandEncoderIndependent) {
+    DummyRenderPass renderPass(device);
+
+    dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+    encoder.PushDebugGroup("Event Start");
+    {
+        dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass);
+        pass.InsertDebugMarker("Marker");
+        pass.PopDebugGroup();
+        pass.EndPass();
+    }
+    ASSERT_DEVICE_ERROR(encoder.Finish());
+}