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>
diff --git a/BUILD.gn b/BUILD.gn
index e235a3b..cc743e1 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -778,6 +778,7 @@
"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",
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp
index 145ed6e..e073491 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -72,6 +72,13 @@
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) {
diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h
index a80032f..ce10ea5 100644
--- a/src/dawn_native/Buffer.h
+++ b/src/dawn_native/Buffer.h
@@ -42,6 +42,8 @@
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);
diff --git a/src/dawn_native/CommandBuffer.cpp b/src/dawn_native/CommandBuffer.cpp
index 919f72c..2a9ffe5 100644
--- a/src/dawn_native/CommandBuffer.cpp
+++ b/src/dawn_native/CommandBuffer.cpp
@@ -286,7 +286,11 @@
// 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 @@
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 @@
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 @@
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 @@
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 @@
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 @@
mIterator.NextCommand<EndRenderPassCmd>();
DAWN_TRY(usageTracker.ValidateUsages(PassType::Render));
- mPassResourceUsages.push_back(usageTracker.AcquireResourceUsage());
+ mResourceUsages.perPass.push_back(usageTracker.AcquireResourceUsage());
return {};
} break;
diff --git a/src/dawn_native/CommandBuffer.h b/src/dawn_native/CommandBuffer.h
index 553fe6e..3565085 100644
--- a/src/dawn_native/CommandBuffer.h
+++ b/src/dawn_native/CommandBuffer.h
@@ -42,6 +42,11 @@
class CommandBufferBase : public ObjectBase {
public:
CommandBufferBase(CommandBufferBuilder* builder);
+
+ const CommandBufferResourceUsage& GetResourceUsages() const;
+
+ private:
+ CommandBufferResourceUsage mResourceUsages;
};
class CommandBufferBuilder : public Builder<CommandBufferBase> {
@@ -52,7 +57,7 @@
MaybeError ValidateGetResult();
CommandIterator AcquireCommands();
- std::vector<PassResourceUsage> AcquirePassResourceUsage();
+ CommandBufferResourceUsage AcquireResourceUsages();
// Dawn API
ComputePassEncoderBase* BeginComputePass();
@@ -116,9 +121,9 @@
CommandIterator mIterator;
bool mWasMovedToIterator = false;
bool mWereCommandsAcquired = false;
- bool mWerePassUsagesAcquired = false;
- std::vector<PassResourceUsage> mPassResourceUsages;
+ bool mWereResourceUsagesAcquired = false;
+ CommandBufferResourceUsage mResourceUsages;
};
} // namespace dawn_native
diff --git a/src/dawn_native/PassResourceUsage.h b/src/dawn_native/PassResourceUsage.h
index 5bae2e4..9a8c2b0 100644
--- a/src/dawn_native/PassResourceUsage.h
+++ b/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 @@
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
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index 46facf5..021a095 100644
--- a/src/dawn_native/Queue.cpp
+++ b/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 @@
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 {};
}
diff --git a/src/dawn_native/Texture.cpp b/src/dawn_native/Texture.cpp
index ffc5da2..53e8d10 100644
--- a/src/dawn_native/Texture.cpp
+++ b/src/dawn_native/Texture.cpp
@@ -237,6 +237,10 @@
return mUsage;
}
+ MaybeError TextureBase::ValidateCanUseInSubmitNow() const {
+ return {};
+ }
+
TextureViewBase* TextureBase::CreateDefaultTextureView() {
TextureViewDescriptor descriptor = MakeDefaultTextureViewDescriptor(this);
return GetDevice()->CreateTextureView(this, &descriptor);
diff --git a/src/dawn_native/Texture.h b/src/dawn_native/Texture.h
index 1817bc1..ab34e96 100644
--- a/src/dawn_native/Texture.h
+++ b/src/dawn_native/Texture.h
@@ -52,6 +52,8 @@
uint32_t GetNumMipLevels() const;
dawn::TextureUsageBit GetUsage() const;
+ MaybeError ValidateCanUseInSubmitNow() const;
+
// Dawn API
TextureViewBase* CreateDefaultTextureView();
TextureViewBase* CreateTextureView(const TextureViewDescriptor* descriptor);
diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.cpp b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
index 27bd589..65e7d6c 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.cpp
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.cpp
@@ -234,9 +234,7 @@
} // 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 @@
}
};
+ const std::vector<PassResourceUsage>& passResourceUsages = GetResourceUsages().perPass;
uint32_t nextPassNumber = 0;
Command type;
@@ -289,7 +288,7 @@
case Command::BeginComputePass: {
mCommands.NextCommand<BeginComputePassCmd>();
- TransitionForPass(commandList, mPassResourceUsages[nextPassNumber]);
+ TransitionForPass(commandList, passResourceUsages[nextPassNumber]);
bindingTracker.SetInComputePass(true);
RecordComputePass(commandList, &bindingTracker);
@@ -300,7 +299,7 @@
BeginRenderPassCmd* beginRenderPassCmd =
mCommands.NextCommand<BeginRenderPassCmd>();
- TransitionForPass(commandList, mPassResourceUsages[nextPassNumber]);
+ TransitionForPass(commandList, passResourceUsages[nextPassNumber]);
bindingTracker.SetInComputePass(false);
RecordRenderPass(commandList, &bindingTracker,
ToBackend(beginRenderPassCmd->info.Get()));
diff --git a/src/dawn_native/d3d12/CommandBufferD3D12.h b/src/dawn_native/d3d12/CommandBufferD3D12.h
index e9e7bfe..95ac8bd 100644
--- a/src/dawn_native/d3d12/CommandBufferD3D12.h
+++ b/src/dawn_native/d3d12/CommandBufferD3D12.h
@@ -42,7 +42,6 @@
RenderPassDescriptor* renderPass);
CommandIterator mCommands;
- std::vector<PassResourceUsage> mPassResourceUsages;
};
}} // namespace dawn_native::d3d12
diff --git a/src/dawn_native/vulkan/CommandBufferVk.cpp b/src/dawn_native/vulkan/CommandBufferVk.cpp
index fa07a9c5..8f5c462 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.cpp
+++ b/src/dawn_native/vulkan/CommandBufferVk.cpp
@@ -111,9 +111,7 @@
} // 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 @@
}
};
+ const std::vector<PassResourceUsage>& passResourceUsages = GetResourceUsages().perPass;
size_t nextPassNumber = 0;
Command type;
@@ -207,7 +206,7 @@
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 @@
case Command::BeginComputePass: {
mCommands.NextCommand<BeginComputePassCmd>();
- TransitionForPass(commands, mPassResourceUsages[nextPassNumber]);
+ TransitionForPass(commands, passResourceUsages[nextPassNumber]);
RecordComputePass(commands);
nextPassNumber++;
diff --git a/src/dawn_native/vulkan/CommandBufferVk.h b/src/dawn_native/vulkan/CommandBufferVk.h
index b6499df..2ca5a62 100644
--- a/src/dawn_native/vulkan/CommandBufferVk.h
+++ b/src/dawn_native/vulkan/CommandBufferVk.h
@@ -35,7 +35,6 @@
void RecordRenderPass(VkCommandBuffer commands, RenderPassDescriptor* renderPass);
CommandIterator mCommands;
- std::vector<PassResourceUsage> mPassResourceUsages;
};
}} // namespace dawn_native::vulkan
diff --git a/src/tests/unittests/validation/QueueSubmitValidationTests.cpp b/src/tests/unittests/validation/QueueSubmitValidationTests.cpp
new file mode 100644
index 0000000..37c1d40
--- /dev/null
+++ b/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
