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>
This commit is contained in:
Austin Eng 2019-03-09 00:06:38 +00:00 committed by Commit Bot service account
parent 8c1a90199a
commit 9bf529ec94
14 changed files with 11 additions and 179 deletions

View File

@ -645,7 +645,6 @@ test("dawn_end2end_tests") {
"src/tests/end2end/CopyTests.cpp", "src/tests/end2end/CopyTests.cpp",
"src/tests/end2end/DebugMarkerTests.cpp", "src/tests/end2end/DebugMarkerTests.cpp",
"src/tests/end2end/DepthStencilStateTests.cpp", "src/tests/end2end/DepthStencilStateTests.cpp",
"src/tests/end2end/DestroyBufferTests.cpp",
"src/tests/end2end/DrawIndexedTests.cpp", "src/tests/end2end/DrawIndexedTests.cpp",
"src/tests/end2end/DrawTests.cpp", "src/tests/end2end/DrawTests.cpp",
"src/tests/end2end/FenceTests.cpp", "src/tests/end2end/FenceTests.cpp",

View File

@ -47,9 +47,6 @@ namespace dawn_native {
void UnmapImpl() override { void UnmapImpl() override {
UNREACHABLE(); UNREACHABLE();
} }
void DestroyImpl() override {
UNREACHABLE();
}
}; };
} // anonymous namespace } // anonymous namespace
@ -235,7 +232,6 @@ namespace dawn_native {
if (mState == BufferState::Mapped) { if (mState == BufferState::Mapped) {
Unmap(); Unmap();
} }
DestroyImpl();
mState = BufferState::Destroyed; mState = BufferState::Destroyed;
} }

View File

@ -76,7 +76,6 @@ namespace dawn_native {
virtual void MapReadAsyncImpl(uint32_t serial) = 0; virtual void MapReadAsyncImpl(uint32_t serial) = 0;
virtual void MapWriteAsyncImpl(uint32_t serial) = 0; virtual void MapWriteAsyncImpl(uint32_t serial) = 0;
virtual void UnmapImpl() = 0; virtual void UnmapImpl() = 0;
virtual void DestroyImpl() = 0;
MaybeError ValidateSetSubData(uint32_t start, uint32_t count) const; MaybeError ValidateSetSubData(uint32_t start, uint32_t count) const;
MaybeError ValidateMap(dawn::BufferUsageBit requiredUsage) const; MaybeError ValidateMap(dawn::BufferUsageBit requiredUsage) const;

View File

@ -105,7 +105,7 @@ namespace dawn_native { namespace d3d12 {
} }
Buffer::~Buffer() { Buffer::~Buffer() {
DestroyImpl(); ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
} }
uint32_t Buffer::GetD3D12Size() const { uint32_t Buffer::GetD3D12Size() const {
@ -189,11 +189,6 @@ namespace dawn_native { namespace d3d12 {
mWrittenMappedRange = {}; mWrittenMappedRange = {};
} }
void Buffer::DestroyImpl() {
ToBackend(GetDevice())->GetResourceAllocator()->Release(mResource);
mResource = nullptr;
}
MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) { MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {
} }

View File

@ -42,7 +42,6 @@ namespace dawn_native { namespace d3d12 {
void MapReadAsyncImpl(uint32_t serial) override; void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override;
ComPtr<ID3D12Resource> mResource; ComPtr<ID3D12Resource> mResource;
bool mFixedResourceState = false; bool mFixedResourceState = false;

View File

@ -37,7 +37,6 @@ namespace dawn_native { namespace metal {
void MapReadAsyncImpl(uint32_t serial) override; void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override;
id<MTLBuffer> mMtlBuffer = nil; id<MTLBuffer> mMtlBuffer = nil;
}; };

View File

@ -31,7 +31,8 @@ namespace dawn_native { namespace metal {
} }
Buffer::~Buffer() { Buffer::~Buffer() {
DestroyImpl(); [mMtlBuffer release];
mMtlBuffer = nil;
} }
id<MTLBuffer> Buffer::GetMTLBuffer() { id<MTLBuffer> Buffer::GetMTLBuffer() {
@ -61,11 +62,6 @@ namespace dawn_native { namespace metal {
// Nothing to do, Metal StorageModeShared buffers are always mapped. // Nothing to do, Metal StorageModeShared buffers are always mapped.
} }
void Buffer::DestroyImpl() {
[mMtlBuffer release];
mMtlBuffer = nil;
}
MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) { MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {
} }

View File

@ -226,9 +226,6 @@ namespace dawn_native { namespace null {
void Buffer::UnmapImpl() { void Buffer::UnmapImpl() {
} }
void Buffer::DestroyImpl() {
}
// CommandBuffer // CommandBuffer
CommandBuffer::CommandBuffer(Device* device, CommandEncoderBase* encoder) CommandBuffer::CommandBuffer(Device* device, CommandEncoderBase* encoder)

View File

@ -146,7 +146,6 @@ namespace dawn_native { namespace null {
void MapReadAsyncImpl(uint32_t serial) override; void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override;
void MapAsyncImplCommon(uint32_t serial, bool isWrite); void MapAsyncImplCommon(uint32_t serial, bool isWrite);

View File

@ -27,10 +27,6 @@ namespace dawn_native { namespace opengl {
glBufferData(GL_ARRAY_BUFFER, GetSize(), nullptr, GL_STATIC_DRAW); glBufferData(GL_ARRAY_BUFFER, GetSize(), nullptr, GL_STATIC_DRAW);
} }
Buffer::~Buffer() {
DestroyImpl();
}
GLuint Buffer::GetHandle() const { GLuint Buffer::GetHandle() const {
return mBuffer; return mBuffer;
} }
@ -62,9 +58,4 @@ namespace dawn_native { namespace opengl {
glUnmapBuffer(GL_ARRAY_BUFFER); glUnmapBuffer(GL_ARRAY_BUFFER);
} }
void Buffer::DestroyImpl() {
glDeleteBuffers(1, &mBuffer);
mBuffer = 0;
}
}} // namespace dawn_native::opengl }} // namespace dawn_native::opengl

View File

@ -26,7 +26,6 @@ namespace dawn_native { namespace opengl {
class Buffer : public BufferBase { class Buffer : public BufferBase {
public: public:
Buffer(Device* device, const BufferDescriptor* descriptor); Buffer(Device* device, const BufferDescriptor* descriptor);
~Buffer();
GLuint GetHandle() const; GLuint GetHandle() const;
@ -35,7 +34,6 @@ namespace dawn_native { namespace opengl {
void MapReadAsyncImpl(uint32_t serial) override; void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override;
GLuint mBuffer = 0; GLuint mBuffer = 0;
}; };

View File

@ -137,7 +137,14 @@ namespace dawn_native { namespace vulkan {
} }
Buffer::~Buffer() { 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) { void Buffer::OnMapReadCommandSerialFinished(uint32_t mapSerial, const void* data) {
@ -218,15 +225,6 @@ namespace dawn_native { namespace vulkan {
// No need to do anything, we keep CPU-visible memory mapped at all time. // 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::MapRequestTracker(Device* device) : mDevice(device) { MapRequestTracker::MapRequestTracker(Device* device) : mDevice(device) {

View File

@ -44,7 +44,6 @@ namespace dawn_native { namespace vulkan {
void MapReadAsyncImpl(uint32_t serial) override; void MapReadAsyncImpl(uint32_t serial) override;
void MapWriteAsyncImpl(uint32_t serial) override; void MapWriteAsyncImpl(uint32_t serial) override;
void UnmapImpl() override; void UnmapImpl() override;
void DestroyImpl() override;
VkBuffer mHandle = VK_NULL_HANDLE; VkBuffer mHandle = VK_NULL_HANDLE;
DeviceMemoryAllocation mMemoryAllocation; DeviceMemoryAllocation mMemoryAllocation;

View File

@ -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);