Revert "Destroy backend implementation for Buffers"
This reverts commit b6a80b321e8fa0b79d9a947656ea0ad649ed5a3c.
Reason for revert: dawn_end2end_tests are failing on the Chromium GPU FYI bots. Example here: https://ci.chromium.org/p/chromium/builders/ci/Win10%20FYI%20Release%20%28NVIDIA%29/4226
Original change's description:
> 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>
TBR=cwallez@chromium.org,kainino@chromium.org,enga@chromium.org,rafael.cintron@microsoft.com,natlee@microsoft.com
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: dawn:46
Change-Id: Iadf37a8a6675c744207ec7daaa3fd2fde7da3714
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/5480
Reviewed-by: Austin Eng <enga@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
diff --git a/BUILD.gn b/BUILD.gn
index ac84461..024119e 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -645,7 +645,6 @@
"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 4a68302..d619fa4 100644
--- a/src/dawn_native/Buffer.cpp
+++ b/src/dawn_native/Buffer.cpp
@@ -47,9 +47,6 @@
void UnmapImpl() override {
UNREACHABLE();
}
- void DestroyImpl() override {
- UNREACHABLE();
- }
};
} // anonymous namespace
@@ -235,7 +232,6 @@
if (mState == BufferState::Mapped) {
Unmap();
}
- DestroyImpl();
mState = BufferState::Destroyed;
}
diff --git a/src/dawn_native/Buffer.h b/src/dawn_native/Buffer.h
index 28653dc..22bd170 100644
--- a/src/dawn_native/Buffer.h
+++ b/src/dawn_native/Buffer.h
@@ -76,7 +76,6 @@
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 307118d..c24ce83 100644
--- a/src/dawn_native/d3d12/BufferD3D12.cpp
+++ b/src/dawn_native/d3d12/BufferD3D12.cpp
@@ -105,7 +105,7 @@
}
Buffer::~Buffer() {
- DestroyImpl();
+ ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
}
uint32_t Buffer::GetD3D12Size() const {
@@ -189,11 +189,6 @@
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 9667085..9398fb5 100644
--- a/src/dawn_native/d3d12/BufferD3D12.h
+++ b/src/dawn_native/d3d12/BufferD3D12.h
@@ -42,7 +42,6 @@
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 4cdbb5a..1d9a011 100644
--- a/src/dawn_native/metal/BufferMTL.h
+++ b/src/dawn_native/metal/BufferMTL.h
@@ -37,7 +37,6 @@
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 3e91b43..6d0aa9c 100644
--- a/src/dawn_native/metal/BufferMTL.mm
+++ b/src/dawn_native/metal/BufferMTL.mm
@@ -31,7 +31,8 @@
}
Buffer::~Buffer() {
- DestroyImpl();
+ [mMtlBuffer release];
+ mMtlBuffer = nil;
}
id<MTLBuffer> Buffer::GetMTLBuffer() {
@@ -61,11 +62,6 @@
// 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 9cf3824..f2832d0 100644
--- a/src/dawn_native/null/DeviceNull.cpp
+++ b/src/dawn_native/null/DeviceNull.cpp
@@ -226,9 +226,6 @@
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 e38f88e..0b5d40f 100644
--- a/src/dawn_native/null/DeviceNull.h
+++ b/src/dawn_native/null/DeviceNull.h
@@ -146,7 +146,6 @@
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 50e2d0b..348126e 100644
--- a/src/dawn_native/opengl/BufferGL.cpp
+++ b/src/dawn_native/opengl/BufferGL.cpp
@@ -27,10 +27,6 @@
glBufferData(GL_ARRAY_BUFFER, GetSize(), nullptr, GL_STATIC_DRAW);
}
- Buffer::~Buffer() {
- DestroyImpl();
- }
-
GLuint Buffer::GetHandle() const {
return mBuffer;
}
@@ -62,9 +58,4 @@
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 5a2b4a5..ba84267 100644
--- a/src/dawn_native/opengl/BufferGL.h
+++ b/src/dawn_native/opengl/BufferGL.h
@@ -26,7 +26,6 @@
class Buffer : public BufferBase {
public:
Buffer(Device* device, const BufferDescriptor* descriptor);
- ~Buffer();
GLuint GetHandle() const;
@@ -35,7 +34,6 @@
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 e38a716..3b7ef78 100644
--- a/src/dawn_native/vulkan/BufferVk.cpp
+++ b/src/dawn_native/vulkan/BufferVk.cpp
@@ -137,7 +137,14 @@
}
Buffer::~Buffer() {
- DestroyImpl();
+ Device* device = ToBackend(GetDevice());
+
+ device->GetMemoryAllocator()->Free(&mMemoryAllocation);
+
+ if (mHandle != VK_NULL_HANDLE) {
+ device->GetFencedDeleter()->DeleteWhenUnused(mHandle);
+ mHandle = VK_NULL_HANDLE;
+ }
}
void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) {
@@ -218,15 +225,6 @@
// 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 348f9be..ee703b4 100644
--- a/src/dawn_native/vulkan/BufferVk.h
+++ b/src/dawn_native/vulkan/BufferVk.h
@@ -44,7 +44,6 @@
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
deleted file mode 100644
index 37722fd..0000000
--- a/src/tests/end2end/DestroyBufferTests.cpp
+++ /dev/null
@@ -1,133 +0,0 @@
-// 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
