Validate that mapped buffers aren't used in submits.

Likewise "presented" textures won't be available for use in submits so
this sets up state-tracking and validation for textures too.

Command buffer resource usage is now stored in the frontend instead of
done per-backend because it is used to validate resources are allowed in
the submits.

Also adds a test.

BUG=dawn:9

Change-Id: I0537c5113bb33a089509b4f2af4ddf4eff8051ea
Reviewed-on: https://dawn-review.googlesource.com/c/2142
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

BUILD.gn

@@ -778,6 +778,7 @@ test("dawn_unittests") {
     "src/tests/unittests/validation/DynamicStateCommandValidationTests.cpp",
     "src/tests/unittests/validation/InputStateValidationTests.cpp",
     "src/tests/unittests/validation/PushConstantsValidationTests.cpp",
+    "src/tests/unittests/validation/QueueSubmitValidationTests.cpp",
     "src/tests/unittests/validation/RenderPassDescriptorValidationTests.cpp",
     "src/tests/unittests/validation/RenderPipelineValidationTests.cpp",
     "src/tests/unittests/validation/ShaderModuleValidationTests.cpp",

src/dawn_native/Buffer.cpp

@@ -72,6 +72,13 @@ namespace dawn_native {
         return mUsage;
     }
 
+    MaybeError BufferBase::ValidateCanUseInSubmitNow() const {
+        if (mIsMapped) {
+            return DAWN_VALIDATION_ERROR("Buffer used in a submit while mapped");
+        }
+        return {};
+    }
+
     void BufferBase::CallMapReadCallback(uint32_t serial,
                                          dawnBufferMapAsyncStatus status,
                                          const void* pointer) {

src/dawn_native/Buffer.h

@@ -42,6 +42,8 @@ namespace dawn_native {
         uint32_t GetSize() const;
         dawn::BufferUsageBit GetUsage() const;
 
+        MaybeError ValidateCanUseInSubmitNow() const;
+
         // Dawn API
         BufferViewBuilder* CreateBufferViewBuilder();
         void SetSubData(uint32_t start, uint32_t count, const uint8_t* data);

src/dawn_native/CommandBuffer.cpp

@@ -286,7 +286,11 @@ namespace dawn_native {
     // CommandBuffer
 
     CommandBufferBase::CommandBufferBase(CommandBufferBuilder* builder)
-        : ObjectBase(builder->GetDevice()) {
+        : ObjectBase(builder->GetDevice()), mResourceUsages(builder->AcquireResourceUsages()) {
+    }
+
+    const CommandBufferResourceUsage& CommandBufferBase::GetResourceUsages() const {
+        return mResourceUsages;
     }
 
     // CommandBufferBuilder
@@ -308,10 +312,10 @@ namespace dawn_native {
         return std::move(mIterator);
     }
 
-    std::vector<PassResourceUsage> CommandBufferBuilder::AcquirePassResourceUsage() {
-        ASSERT(!mWerePassUsagesAcquired);
-        mWerePassUsagesAcquired = true;
-        return std::move(mPassResourceUsages);
+    CommandBufferResourceUsage CommandBufferBuilder::AcquireResourceUsages() {
+        ASSERT(!mWereResourceUsagesAcquired);
+        mWereResourceUsagesAcquired = true;
+        return std::move(mResourceUsages);
     }
 
     CommandBufferBase* CommandBufferBuilder::GetResultImpl() {
@@ -372,6 +376,9 @@ namespace dawn_native {
                                               dawn::BufferUsageBit::TransferSrc));
                     DAWN_TRY(ValidateCanUseAs(copy->destination.buffer.Get(),
                                               dawn::BufferUsageBit::TransferDst));
+
+                    mResourceUsages.topLevelBuffers.insert(copy->source.buffer.Get());
+                    mResourceUsages.topLevelBuffers.insert(copy->destination.buffer.Get());
                 } break;
 
                 case Command::CopyBufferToTexture: {
@@ -391,6 +398,9 @@ namespace dawn_native {
                                               dawn::BufferUsageBit::TransferSrc));
                     DAWN_TRY(ValidateCanUseAs(copy->destination.texture.Get(),
                                               dawn::TextureUsageBit::TransferDst));
+
+                    mResourceUsages.topLevelBuffers.insert(copy->source.buffer.Get());
+                    mResourceUsages.topLevelTextures.insert(copy->destination.texture.Get());
                 } break;
 
                 case Command::CopyTextureToBuffer: {
@@ -410,6 +420,9 @@ namespace dawn_native {
                                               dawn::TextureUsageBit::TransferSrc));
                     DAWN_TRY(ValidateCanUseAs(copy->destination.buffer.Get(),
                                               dawn::BufferUsageBit::TransferDst));
+
+                    mResourceUsages.topLevelTextures.insert(copy->source.texture.Get());
+                    mResourceUsages.topLevelBuffers.insert(copy->destination.buffer.Get());
                 } break;
 
                 default:
@@ -431,7 +444,7 @@ namespace dawn_native {
                     mIterator.NextCommand<EndComputePassCmd>();
 
                     DAWN_TRY(usageTracker.ValidateUsages(PassType::Compute));
-                    mPassResourceUsages.push_back(usageTracker.AcquireResourceUsage());
+                    mResourceUsages.perPass.push_back(usageTracker.AcquireResourceUsage());
                     return {};
                 } break;
 
@@ -495,7 +508,7 @@ namespace dawn_native {
                     mIterator.NextCommand<EndRenderPassCmd>();
 
                     DAWN_TRY(usageTracker.ValidateUsages(PassType::Render));
-                    mPassResourceUsages.push_back(usageTracker.AcquireResourceUsage());
+                    mResourceUsages.perPass.push_back(usageTracker.AcquireResourceUsage());
                     return {};
                 } break;
 

src/dawn_native/CommandBuffer.h

@@ -42,6 +42,11 @@ namespace dawn_native {
     class CommandBufferBase : public ObjectBase {
       public:
         CommandBufferBase(CommandBufferBuilder* builder);
+
+        const CommandBufferResourceUsage& GetResourceUsages() const;
+
+      private:
+        CommandBufferResourceUsage mResourceUsages;
     };
 
     class CommandBufferBuilder : public Builder<CommandBufferBase> {
@@ -52,7 +57,7 @@ namespace dawn_native {
         MaybeError ValidateGetResult();
 
         CommandIterator AcquireCommands();
-        std::vector<PassResourceUsage> AcquirePassResourceUsage();
+        CommandBufferResourceUsage AcquireResourceUsages();
 
         // Dawn API
         ComputePassEncoderBase* BeginComputePass();
@@ -116,9 +121,9 @@ namespace dawn_native {
         CommandIterator mIterator;
         bool mWasMovedToIterator = false;
         bool mWereCommandsAcquired = false;
-        bool mWerePassUsagesAcquired = false;
 
-        std::vector<PassResourceUsage> mPassResourceUsages;
+        bool mWereResourceUsagesAcquired = false;
+        CommandBufferResourceUsage mResourceUsages;
     };
 
 }  // namespace dawn_native

src/dawn_native/PassResourceUsage.h

@@ -17,6 +17,7 @@
 
 #include "dawn_native/dawn_platform.h"
 
+#include <set>
 #include <vector>
 
 namespace dawn_native {
@@ -35,6 +36,12 @@ namespace dawn_native {
         std::vector<dawn::TextureUsageBit> textureUsages;
     };
 
+    struct CommandBufferResourceUsage {
+        std::vector<PassResourceUsage> perPass;
+        std::set<BufferBase*> topLevelBuffers;
+        std::set<TextureBase*> topLevelTextures;
+    };
+
 }  // namespace dawn_native
 
 #endif  // DAWNNATIVE_PASSRESOURCEUSAGE_H

src/dawn_native/Queue.cpp

@@ -14,8 +14,10 @@
 
 #include "dawn_native/Queue.h"
 
+#include "dawn_native/Buffer.h"
 #include "dawn_native/CommandBuffer.h"
 #include "dawn_native/Device.h"
+#include "dawn_native/Texture.h"
 
 namespace dawn_native {
 
@@ -32,7 +34,27 @@ namespace dawn_native {
         SubmitImpl(numCommands, commands);
     }
 
-    MaybeError QueueBase::ValidateSubmit(uint32_t, CommandBufferBase* const*) {
+    MaybeError QueueBase::ValidateSubmit(uint32_t numCommands, CommandBufferBase* const* commands) {
+        for (uint32_t i = 0; i < numCommands; ++i) {
+            const CommandBufferResourceUsage& usages = commands[i]->GetResourceUsages();
+
+            for (const PassResourceUsage& passUsages : usages.perPass) {
+                for (const BufferBase* buffer : passUsages.buffers) {
+                    DAWN_TRY(buffer->ValidateCanUseInSubmitNow());
+                }
+                for (const TextureBase* texture : passUsages.textures) {
+                    DAWN_TRY(texture->ValidateCanUseInSubmitNow());
+                }
+            }
+
+            for (const BufferBase* buffer : usages.topLevelBuffers) {
+                DAWN_TRY(buffer->ValidateCanUseInSubmitNow());
+            }
+            for (const TextureBase* texture : usages.topLevelTextures) {
+                DAWN_TRY(texture->ValidateCanUseInSubmitNow());
+            }
+        }
+
         return {};
     }
 

src/dawn_native/Texture.cpp

@@ -237,6 +237,10 @@ namespace dawn_native {
         return mUsage;
     }
 
+    MaybeError TextureBase::ValidateCanUseInSubmitNow() const {
+        return {};
+    }
+
     TextureViewBase* TextureBase::CreateDefaultTextureView() {
         TextureViewDescriptor descriptor = MakeDefaultTextureViewDescriptor(this);
         return GetDevice()->CreateTextureView(this, &descriptor);

src/dawn_native/Texture.h

@@ -52,6 +52,8 @@ namespace dawn_native {
         uint32_t GetNumMipLevels() const;
         dawn::TextureUsageBit GetUsage() const;
 
+        MaybeError ValidateCanUseInSubmitNow() const;
+
         // Dawn API
         TextureViewBase* CreateDefaultTextureView();
         TextureViewBase* CreateTextureView(const TextureViewDescriptor* descriptor);

src/dawn_native/d3d12/CommandBufferD3D12.cpp

@@ -234,9 +234,7 @@ namespace dawn_native { namespace d3d12 {
     }  // anonymous namespace
 
     CommandBuffer::CommandBuffer(CommandBufferBuilder* builder)
-        : CommandBufferBase(builder),
-          mCommands(builder->AcquireCommands()),
-          mPassResourceUsages(builder->AcquirePassResourceUsage()) {
+        : CommandBufferBase(builder), mCommands(builder->AcquireCommands()) {
     }
 
     CommandBuffer::~CommandBuffer() {
@@ -281,6 +279,7 @@ namespace dawn_native { namespace d3d12 {
             }
         };
 
+        const std::vector<PassResourceUsage>& passResourceUsages = GetResourceUsages().perPass;
         uint32_t nextPassNumber = 0;
 
         Command type;
@@ -289,7 +288,7 @@ namespace dawn_native { namespace d3d12 {
                 case Command::BeginComputePass: {
                     mCommands.NextCommand<BeginComputePassCmd>();
 
-                    TransitionForPass(commandList, mPassResourceUsages[nextPassNumber]);
+                    TransitionForPass(commandList, passResourceUsages[nextPassNumber]);
                     bindingTracker.SetInComputePass(true);
                     RecordComputePass(commandList, &bindingTracker);
 
@@ -300,7 +299,7 @@ namespace dawn_native { namespace d3d12 {
                     BeginRenderPassCmd* beginRenderPassCmd =
                         mCommands.NextCommand<BeginRenderPassCmd>();
 
-                    TransitionForPass(commandList, mPassResourceUsages[nextPassNumber]);
+                    TransitionForPass(commandList, passResourceUsages[nextPassNumber]);
                     bindingTracker.SetInComputePass(false);
                     RecordRenderPass(commandList, &bindingTracker,
                                      ToBackend(beginRenderPassCmd->info.Get()));

src/dawn_native/d3d12/CommandBufferD3D12.h

@@ -42,7 +42,6 @@ namespace dawn_native { namespace d3d12 {
                               RenderPassDescriptor* renderPass);
 
         CommandIterator mCommands;
-        std::vector<PassResourceUsage> mPassResourceUsages;
     };
 
 }}  // namespace dawn_native::d3d12

src/dawn_native/vulkan/CommandBufferVk.cpp

@@ -111,9 +111,7 @@ namespace dawn_native { namespace vulkan {
     }  // anonymous namespace
 
     CommandBuffer::CommandBuffer(CommandBufferBuilder* builder)
-        : CommandBufferBase(builder),
-          mCommands(builder->AcquireCommands()),
-          mPassResourceUsages(builder->AcquirePassResourceUsage()) {
+        : CommandBufferBase(builder), mCommands(builder->AcquireCommands()) {
     }
 
     CommandBuffer::~CommandBuffer() {
@@ -135,6 +133,7 @@ namespace dawn_native { namespace vulkan {
             }
         };
 
+        const std::vector<PassResourceUsage>& passResourceUsages = GetResourceUsages().perPass;
         size_t nextPassNumber = 0;
 
         Command type;
@@ -207,7 +206,7 @@ namespace dawn_native { namespace vulkan {
                 case Command::BeginRenderPass: {
                     BeginRenderPassCmd* cmd = mCommands.NextCommand<BeginRenderPassCmd>();
 
-                    TransitionForPass(commands, mPassResourceUsages[nextPassNumber]);
+                    TransitionForPass(commands, passResourceUsages[nextPassNumber]);
                     RecordRenderPass(commands, ToBackend(cmd->info.Get()));
 
                     nextPassNumber++;
@@ -216,7 +215,7 @@ namespace dawn_native { namespace vulkan {
                 case Command::BeginComputePass: {
                     mCommands.NextCommand<BeginComputePassCmd>();
 
-                    TransitionForPass(commands, mPassResourceUsages[nextPassNumber]);
+                    TransitionForPass(commands, passResourceUsages[nextPassNumber]);
                     RecordComputePass(commands);
 
                     nextPassNumber++;

src/dawn_native/vulkan/CommandBufferVk.h

@@ -35,7 +35,6 @@ namespace dawn_native { namespace vulkan {
         void RecordRenderPass(VkCommandBuffer commands, RenderPassDescriptor* renderPass);
 
         CommandIterator mCommands;
-        std::vector<PassResourceUsage> mPassResourceUsages;
     };
 
 }}  // namespace dawn_native::vulkan

src/tests/unittests/validation/QueueSubmitValidationTests.cpp

@@ -0,0 +1,69 @@
+// Copyright 2018 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/unittests/validation/ValidationTest.h"
+
+#include "utils/DawnHelpers.h"
+
+namespace {
+
+class QueueSubmitValidationTest : public ValidationTest {
+};
+
+static void StoreTrueMapWriteCallback(dawnBufferMapAsyncStatus status, void*, dawnCallbackUserdata userdata) {
+    bool* userdataPtr = reinterpret_cast<bool*>(static_cast<intptr_t>(userdata));
+    *userdataPtr = true;
+}
+
+// Test submitting with a mapped buffer is disallowed
+TEST_F(QueueSubmitValidationTest, SubmitWithMappedBuffer) {
+    // Create a map-write buffer.
+    dawn::BufferDescriptor descriptor;
+    descriptor.usage = dawn::BufferUsageBit::MapWrite | dawn::BufferUsageBit::TransferSrc;
+    descriptor.size = 4;
+    dawn::Buffer buffer = device.CreateBuffer(&descriptor);
+
+    // Create a fake copy destination buffer
+    descriptor.usage = dawn::BufferUsageBit::TransferDst;
+    dawn::Buffer targetBuffer = device.CreateBuffer(&descriptor);
+
+    // Create a command buffer that reads from the mappable buffer.
+    dawn::CommandBuffer commands;
+    {
+        dawn::RenderPassDescriptor renderpass = CreateSimpleRenderPass();
+        dawn::CommandBufferBuilder builder = device.CreateCommandBufferBuilder();
+        builder.CopyBufferToBuffer(buffer, 0, targetBuffer, 0, 4);
+        commands = builder.GetResult();
+    }
+
+    dawn::Queue queue = device.CreateQueue();
+
+    // Submitting when the buffer has never been mapped should succeed
+    queue.Submit(1, &commands);
+
+    // Map the buffer, submitting when the buffer is mapped should fail
+    bool mapWriteFinished = false;
+    dawnCallbackUserdata userdata = static_cast<dawnCallbackUserdata>(reinterpret_cast<intptr_t>(&mapWriteFinished));
+    buffer.MapWriteAsync(0, 4, StoreTrueMapWriteCallback, userdata);
+    queue.Submit(0, nullptr);
+    ASSERT_TRUE(mapWriteFinished);
+
+    ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+
+    // Unmap the buffer, queue submit should succeed
+    buffer.Unmap();
+    queue.Submit(1, &commands);
+}
+
+}  // anonymous namespace