Destroy backend implementation for Buffers
Destroy can be used to free the GPU memory associated with resources
without waiting for javascript garbage collection to occur.
The buffer is validated at submission to the queue.
So any buffer that has been destroyed before submission, will then
invalidate the submit and result in an error.
Bug: dawn:46
Change-Id: I40df56ce97baef01deea7552d7a6d40b558fc985
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/5320
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Natasha Lee <natlee@microsoft.com>
diff --git a/BUILD.gn b/BUILD.gn
index eb5664f..c05f01e 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -660,6 +660,7 @@
"src/tests/end2end/CopyTests.cpp",
"src/tests/end2end/DebugMarkerTests.cpp",
"src/tests/end2end/DepthStencilStateTests.cpp",
+ "src/tests/end2end/DestroyBufferTests.cpp",
"src/tests/end2end/DrawIndexedTests.cpp",
"src/tests/end2end/DrawTests.cpp",
"src/tests/end2end/FenceTests.cpp",
diff --git a/src/dawn_native/Buffer.cpp b/src/dawn_native/Buffer.cpp
index d619fa4..4a68302 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -47,6 +47,9 @@
void UnmapImpl() override {
UNREACHABLE();
}
+ void DestroyImpl() override {
+ UNREACHABLE();
+ }
};
} // anonymous namespace
@@ -232,6 +235,7 @@
if (mState == BufferState::Mapped) {
Unmap();
}
+ DestroyImpl();
mState = BufferState::Destroyed;
}
diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h
index 22bd170..28653dc 100644
--- a/src/dawn_native/Buffer.h
+++ b/src/dawn_native/Buffer.h
@@ -76,6 +76,7 @@
virtual void MapReadAsyncImpl(uint32_t serial) = 0;
virtual void MapWriteAsyncImpl(uint32_t serial) = 0;
virtual void UnmapImpl() = 0;
+ virtual void DestroyImpl() = 0;
MaybeError ValidateSetSubData(uint32_t start, uint32_t count) const;
MaybeError ValidateMap(dawn::BufferUsageBit requiredUsage) const;
diff --git a/src/dawn_native/d3d12/BufferD3D12.cpp b/src/dawn_native/d3d12/BufferD3D12.cpp
index c24ce83..307118d 100644
--- a/src/dawn_native/d3d12/BufferD3D12.cpp
+++ b/src/dawn_native/d3d12/BufferD3D12.cpp
@@ -105,7 +105,7 @@
}
Buffer::~Buffer() {
- ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
+ DestroyImpl();
}
uint32_t Buffer::GetD3D12Size() const {
@@ -189,6 +189,11 @@
mWrittenMappedRange = {};
}
+ void Buffer::DestroyImpl() {
+ ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
+ mResource = nullptr;
+ }
+
MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {
}
diff --git a/src/dawn_native/d3d12/BufferD3D12.h b/src/dawn_native/d3d12/BufferD3D12.h
index 9398fb5..9667085 100644
--- a/src/dawn_native/d3d12/BufferD3D12.h
+++ b/src/dawn_native/d3d12/BufferD3D12.h
@@ -42,6 +42,7 @@
void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override;
+ void DestroyImpl() override;
ComPtr<ID3D12Resource> mResource;
bool mFixedResourceState = false;
diff --git a/src/dawn_native/metal/BufferMTL.h b/src/dawn_native/metal/BufferMTL.h
index 1d9a011..4cdbb5a 100644
--- a/src/dawn_native/metal/BufferMTL.h
+++ b/src/dawn_native/metal/BufferMTL.h
@@ -37,6 +37,7 @@
void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override;
+ void DestroyImpl() override;
id<MTLBuffer> mMtlBuffer = nil;
};
diff --git a/src/dawn_native/metal/BufferMTL.mm b/src/dawn_native/metal/BufferMTL.mm
index 6d0aa9c..3e91b43 100644
--- a/src/dawn_native/metal/BufferMTL.mm
+++ b/src/dawn_native/metal/BufferMTL.mm
@@ -31,8 +31,7 @@
}
Buffer::~Buffer() {
- [mMtlBuffer release];
- mMtlBuffer = nil;
+ DestroyImpl();
}
id<MTLBuffer> Buffer::GetMTLBuffer() {
@@ -62,6 +61,11 @@
// Nothing to do, Metal StorageModeShared buffers are always mapped.
}
+ void Buffer::DestroyImpl() {
+ [mMtlBuffer release];
+ mMtlBuffer = nil;
+ }
+
MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {
}
diff --git a/src/dawn_native/null/DeviceNull.cpp b/src/dawn_native/null/DeviceNull.cpp
index f2832d0..9cf3824 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -226,6 +226,9 @@
void Buffer::UnmapImpl() {
}
+ void Buffer::DestroyImpl() {
+ }
+
// CommandBuffer
CommandBuffer::CommandBuffer(Device* device, CommandEncoderBase* encoder)
diff --git a/src/dawn_native/null/DeviceNull.h b/src/dawn_native/null/DeviceNull.h
index 0b5d40f..e38f88e 100644
--- a/src/dawn_native/null/DeviceNull.h
+++ b/src/dawn_native/null/DeviceNull.h
@@ -146,6 +146,7 @@
void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override;
+ void DestroyImpl() override;
void MapAsyncImplCommon(uint32_t serial, bool isWrite);
diff --git a/src/dawn_native/opengl/BufferGL.cpp b/src/dawn_native/opengl/BufferGL.cpp
index 348126e..50e2d0b 100644
--- a/src/dawn_native/opengl/BufferGL.cpp
+++ b/src/dawn_native/opengl/BufferGL.cpp
@@ -27,6 +27,10 @@
glBufferData(GL_ARRAY_BUFFER, GetSize(), nullptr, GL_STATIC_DRAW);
}
+ Buffer::~Buffer() {
+ DestroyImpl();
+ }
+
GLuint Buffer::GetHandle() const {
return mBuffer;
}
@@ -58,4 +62,9 @@
glUnmapBuffer(GL_ARRAY_BUFFER);
}
+ void Buffer::DestroyImpl() {
+ glDeleteBuffers(1, &mBuffer);
+ mBuffer = 0;
+ }
+
}} // namespace dawn_native::opengl
diff --git a/src/dawn_native/opengl/BufferGL.h b/src/dawn_native/opengl/BufferGL.h
index ba84267..5a2b4a5 100644
--- a/src/dawn_native/opengl/BufferGL.h
+++ b/src/dawn_native/opengl/BufferGL.h
@@ -26,6 +26,7 @@
class Buffer : public BufferBase {
public:
Buffer(Device* device, const BufferDescriptor* descriptor);
+ ~Buffer();
GLuint GetHandle() const;
@@ -34,6 +35,7 @@
void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override;
+ void DestroyImpl() override;
GLuint mBuffer = 0;
};
diff --git a/src/dawn_native/vulkan/BufferVk.cpp b/src/dawn_native/vulkan/BufferVk.cpp
index 3b7ef78..e38a716 100644
--- a/src/dawn_native/vulkan/BufferVk.cpp
+++ b/src/dawn_native/vulkan/BufferVk.cpp
@@ -137,14 +137,7 @@
}
Buffer::~Buffer() {
- Device* device = ToBackend(GetDevice());
-
- device->GetMemoryAllocator()->Free(&mMemoryAllocation);
-
- if (mHandle != VK_NULL_HANDLE) {
- device->GetFencedDeleter()->DeleteWhenUnused(mHandle);
- mHandle = VK_NULL_HANDLE;
- }
+ DestroyImpl();
}
void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) {
@@ -225,6 +218,15 @@
// No need to do anything, we keep CPU-visible memory mapped at all time.
}
+ void Buffer::DestroyImpl() {
+ ToBackend(GetDevice())->GetMemoryAllocator()->Free(&mMemoryAllocation);
+
+ if (mHandle != VK_NULL_HANDLE) {
+ ToBackend(GetDevice())->GetFencedDeleter()->DeleteWhenUnused(mHandle);
+ mHandle = VK_NULL_HANDLE;
+ }
+ }
+
// MapRequestTracker
MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {
diff --git a/src/dawn_native/vulkan/BufferVk.h b/src/dawn_native/vulkan/BufferVk.h
index ee703b4..348f9be 100644
--- a/src/dawn_native/vulkan/BufferVk.h
+++ b/src/dawn_native/vulkan/BufferVk.h
@@ -44,6 +44,7 @@
void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override;
+ void DestroyImpl() override;
VkBuffer mHandle = VK_NULL_HANDLE;
DeviceMemoryAllocation mMemoryAllocation;
diff --git a/src/tests/end2end/DestroyBufferTests.cpp b/src/tests/end2end/DestroyBufferTests.cpp
new file mode 100644
index 0000000..37722fd
--- /dev/null
+++ b/src/tests/end2end/DestroyBufferTests.cpp
@@ -0,0 +1,133 @@
+// Copyright 2019 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/DawnTest.h"
+
+#include "utils/ComboRenderPipelineDescriptor.h"
+#include "utils/DawnHelpers.h"
+
+constexpr uint32_t kRTSize = 4;
+
+class DestroyBufferTest : public DawnTest {
+ protected:
+ void SetUp() override {
+ DawnTest::SetUp();
+
+ renderPass = utils::CreateBasicRenderPass(device, kRTSize, kRTSize);
+ dawn::VertexInputDescriptor input;
+ input.inputSlot = 0;
+ input.stride = 4 * sizeof(float);
+ input.stepMode = dawn::InputStepMode::Vertex;
+
+ dawn::VertexAttributeDescriptor attribute;
+ attribute.shaderLocation = 0;
+ attribute.inputSlot = 0;
+ attribute.offset = 0;
+ attribute.format = dawn::VertexFormat::FloatR32G32B32A32;
+
+ dawn::InputState inputState =
+ device.CreateInputStateBuilder().SetInput(&input).SetAttribute(&attribute).GetResult();
+
+ dawn::ShaderModule vsModule =
+ utils::CreateShaderModule(device, dawn::ShaderStage::Vertex, R"(
+ #version 450
+ layout(location = 0) in vec4 pos;
+ void main() {
+ gl_Position = pos;
+ })");
+
+ dawn::ShaderModule fsModule =
+ utils::CreateShaderModule(device, dawn::ShaderStage::Fragment, R"(
+ #version 450
+ layout(location = 0) out vec4 fragColor;
+ void main() {
+ fragColor = vec4(0.0, 1.0, 0.0, 1.0);
+ })");
+
+ utils::ComboRenderPipelineDescriptor descriptor(device);
+ descriptor.cVertexStage.module = vsModule;
+ descriptor.cFragmentStage.module = fsModule;
+ descriptor.primitiveTopology = dawn::PrimitiveTopology::TriangleStrip;
+ descriptor.indexFormat = dawn::IndexFormat::Uint32;
+ descriptor.inputState = inputState;
+ descriptor.cColorStates[0]->format = renderPass.colorFormat;
+
+ pipeline = device.CreateRenderPipeline(&descriptor);
+
+ vertexBuffer = utils::CreateBufferFromData<float>(
+ device, dawn::BufferUsageBit::Vertex,
+ {// The bottom left triangle
+ -1.0f, -1.0f, 0.0f, 1.0f, 1.0f, 1.0f, 0.0f, 1.0f, -1.0f, 1.0f, 0.0f, 1.0f});
+ }
+
+ utils::BasicRenderPass renderPass;
+ dawn::RenderPipeline pipeline;
+ dawn::Buffer vertexBuffer;
+
+ dawn::CommandBuffer CreateTriangleCommandBuffer() {
+ uint32_t zeroOffset = 0;
+ dawn::CommandEncoder encoder = device.CreateCommandEncoder();
+ {
+ dawn::RenderPassEncoder pass = encoder.BeginRenderPass(&renderPass.renderPassInfo);
+ pass.SetPipeline(pipeline);
+ pass.SetVertexBuffers(0, 1, &vertexBuffer, &zeroOffset);
+ pass.Draw(3, 1, 0, 0);
+ pass.EndPass();
+ }
+ dawn::CommandBuffer commands = encoder.Finish();
+ return commands;
+ }
+};
+
+// Destroy before submit will result in error, and nothing drawn
+TEST_P(DestroyBufferTest, DestroyBeforeSubmit) {
+ RGBA8 notFilled(0, 0, 0, 0);
+
+ dawn::CommandBuffer commands = CreateTriangleCommandBuffer();
+ vertexBuffer.Destroy();
+ ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+
+ EXPECT_PIXEL_RGBA8_EQ(notFilled, renderPass.color, 1, 3);
+}
+
+// Destroy after submit will draw successfully
+TEST_P(DestroyBufferTest, DestroyAfterSubmit) {
+ RGBA8 filled(0, 255, 0, 255);
+
+ dawn::CommandBuffer commands = CreateTriangleCommandBuffer();
+ queue.Submit(1, &commands);
+
+ EXPECT_PIXEL_RGBA8_EQ(filled, renderPass.color, 1, 3);
+ vertexBuffer.Destroy();
+}
+
+// First submit succeeds, draws triangle, second submit fails
+// after destroy is called on the buffer, pixel does not change
+TEST_P(DestroyBufferTest, SubmitDestroySubmit) {
+ RGBA8 filled(0, 255, 0, 255);
+
+ dawn::CommandBuffer commands = CreateTriangleCommandBuffer();
+ queue.Submit(1, &commands);
+ EXPECT_PIXEL_RGBA8_EQ(filled, renderPass.color, 1, 3);
+
+ vertexBuffer.Destroy();
+
+ // Submit fails because vertex buffer was destroyed
+ ASSERT_DEVICE_ERROR(queue.Submit(1, &commands));
+
+ // Pixel stays the same
+ EXPECT_PIXEL_RGBA8_EQ(filled, renderPass.color, 1, 3);
+}
+
+DAWN_INSTANTIATE_TEST(DestroyBufferTest, D3D12Backend, MetalBackend, OpenGLBackend, VulkanBackend);
\ No newline at end of file
