From ef7447334713d9f22a8129f78d3da9f6ae26cf8b Mon Sep 17 00:00:00 2001 From: Jiawei Shao Date: Sun, 23 Aug 2020 06:08:05 +0000 Subject: [PATCH] Fix issues in end2end tests for enabling buffer lazy clear by default This patch cleans up some issues in the end2end tests that will cause test failures when we enable buffer lazy initialization by default. This patch also skips a test that always fails with Vulkan validation layer. BUG=dawn:414 TEST=dawn_end2end_tests Change-Id: I40f643615b3fec4e52c90d576285534a99950915 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26960 Reviewed-by: Austin Eng Reviewed-by: Corentin Wallez Commit-Queue: Jiawei Shao --- src/dawn_native/opengl/BufferGL.cpp | 19 ++++++++++++++++ src/dawn_native/opengl/BufferGL.h | 5 +++++ src/dawn_native/opengl/TextureGL.cpp | 6 +++-- src/tests/DawnTest.cpp | 12 +++++----- src/tests/end2end/TextureSubresourceTests.cpp | 4 ++++ src/tests/end2end/TextureZeroInitTests.cpp | 22 ++++++++++++++----- 6 files changed, 56 insertions(+), 12 deletions(-) diff --git a/src/dawn_native/opengl/BufferGL.cpp b/src/dawn_native/opengl/BufferGL.cpp index a582f72aef..a41b1d41b3 100644 --- a/src/dawn_native/opengl/BufferGL.cpp +++ b/src/dawn_native/opengl/BufferGL.cpp @@ -21,6 +21,18 @@ namespace dawn_native { namespace opengl { // Buffer + // static + ResultOrError> Buffer::CreateInternalBuffer(Device* device, + const BufferDescriptor* descriptor, + bool shouldLazyClear) { + Ref buffer = AcquireRef(new Buffer(device, descriptor, shouldLazyClear)); + if (descriptor->mappedAtCreation) { + DAWN_TRY(buffer->MapAtCreation()); + } + + return std::move(buffer); + } + Buffer::Buffer(Device* device, const BufferDescriptor* descriptor) : BufferBase(device, descriptor) { // TODO(cwallez@chromium.org): Have a global "zero" buffer instead of creating a new 4-byte @@ -38,6 +50,13 @@ namespace dawn_native { namespace opengl { } } + Buffer::Buffer(Device* device, const BufferDescriptor* descriptor, bool shouldLazyClear) + : Buffer(device, descriptor) { + if (!shouldLazyClear) { + SetIsDataInitialized(); + } + } + Buffer::~Buffer() { DestroyInternal(); } diff --git a/src/dawn_native/opengl/BufferGL.h b/src/dawn_native/opengl/BufferGL.h index 54d6a954b8..2f6f6d9466 100644 --- a/src/dawn_native/opengl/BufferGL.h +++ b/src/dawn_native/opengl/BufferGL.h @@ -25,6 +25,10 @@ namespace dawn_native { namespace opengl { class Buffer final : public BufferBase { public: + static ResultOrError> CreateInternalBuffer(Device* device, + const BufferDescriptor* descriptor, + bool shouldLazyClear); + Buffer(Device* device, const BufferDescriptor* descriptor); GLuint GetHandle() const; @@ -34,6 +38,7 @@ namespace dawn_native { namespace opengl { void EnsureDataInitializedAsDestination(const CopyTextureToBufferCmd* copy); private: + Buffer(Device* device, const BufferDescriptor* descriptor, bool shouldLazyClear); ~Buffer() override; MaybeError MapAsyncImpl(wgpu::MapMode mode, size_t offset, size_t size) override; void UnmapImpl() override; diff --git a/src/dawn_native/opengl/TextureGL.cpp b/src/dawn_native/opengl/TextureGL.cpp index e0930d7c02..3bc247b907 100644 --- a/src/dawn_native/opengl/TextureGL.cpp +++ b/src/dawn_native/opengl/TextureGL.cpp @@ -334,8 +334,10 @@ namespace dawn_native { namespace opengl { return DAWN_OUT_OF_MEMORY_ERROR("Unable to allocate buffer."); } - // TODO(natlee@microsoft.com): use Dynamic Uplaoder here for temp buffer - Ref srcBuffer = AcquireRef(ToBackend(device->CreateBuffer(&descriptor))); + // We don't count the lazy clear of srcBuffer because it is an internal buffer. + // TODO(natlee@microsoft.com): use Dynamic Uploader here for temp buffer + Ref srcBuffer; + DAWN_TRY_ASSIGN(srcBuffer, Buffer::CreateInternalBuffer(device, &descriptor, false)); // Fill the buffer with clear color memset(srcBuffer->GetMappedRange(0, descriptor.size), clearColor, descriptor.size); diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp index 33b3701afb..77585ba8c1 100644 --- a/src/tests/DawnTest.cpp +++ b/src/tests/DawnTest.cpp @@ -1011,13 +1011,15 @@ void DawnTestBase::FlushWire() { DawnTestBase::ReadbackReservation DawnTestBase::ReserveReadback(uint64_t readbackSize) { // For now create a new MapRead buffer for each readback // TODO(cwallez@chromium.org): eventually make bigger buffers and allocate linearly? - wgpu::BufferDescriptor descriptor; - descriptor.size = readbackSize; - descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst; - ReadbackSlot slot; slot.bufferSize = readbackSize; - slot.buffer = device.CreateBuffer(&descriptor); + + // Create and initialize the slot buffer so that it won't unexpectedly affect the count of + // resource lazy clear in the tests. + const std::vector initialBufferData(readbackSize, 0u); + slot.buffer = + utils::CreateBufferFromData(device, initialBufferData.data(), readbackSize, + wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst); ReadbackReservation reservation; reservation.buffer = slot.buffer; diff --git a/src/tests/end2end/TextureSubresourceTests.cpp b/src/tests/end2end/TextureSubresourceTests.cpp index d0390dcab4..801f61ce80 100644 --- a/src/tests/end2end/TextureSubresourceTests.cpp +++ b/src/tests/end2end/TextureSubresourceTests.cpp @@ -167,6 +167,10 @@ TEST_P(TextureSubresourceTest, MipmapLevelsTest) { // Test different array layers TEST_P(TextureSubresourceTest, ArrayLayersTest) { + // TODO(crbug.com/dawn/517): The Vulkan backend hits this validation rule. + // https://www.khronos.org/registry/vulkan/specs/1.2-extensions/html/vkspec.html#VUID-vkCmdDraw-None-02687 + DAWN_SKIP_TEST_IF(IsBackendValidationEnabled() && IsVulkan()); + // Create a texture with 1 mipmap level and 2 layers wgpu::Texture texture = CreateTexture(1, 2, diff --git a/src/tests/end2end/TextureZeroInitTests.cpp b/src/tests/end2end/TextureZeroInitTests.cpp index 3b0414bca3..07967b7a18 100644 --- a/src/tests/end2end/TextureZeroInitTests.cpp +++ b/src/tests/end2end/TextureZeroInitTests.cpp @@ -1365,10 +1365,12 @@ TEST_P(TextureZeroInitTest, CopyTextureToBufferNonRenderableUnaligned) { { uint32_t bytesPerRow = Align(kUnalignedSize, kTextureBytesPerRowAlignment); - wgpu::BufferDescriptor bufferDesc; - bufferDesc.size = kUnalignedSize * bytesPerRow; - bufferDesc.usage = wgpu::BufferUsage::CopyDst; - wgpu::Buffer buffer = device.CreateBuffer(&bufferDesc); + // Create and initialize the destination buffer to ensure we only count the times of + // texture lazy initialization in this test. + const uint64_t bufferSize = kUnalignedSize * bytesPerRow; + const std::vector initialBufferData(bufferSize, 0u); + wgpu::Buffer buffer = utils::CreateBufferFromData(device, initialBufferData.data(), + bufferSize, wgpu::BufferUsage::CopyDst); wgpu::TextureCopyView textureCopyView = utils::CreateTextureCopyView(texture, 0, {0, 0, 0}); wgpu::BufferCopyView bufferCopyView = @@ -1635,10 +1637,20 @@ TEST_P(TextureZeroInitTest, WriteTextureHalfAtMipLevel) { kMipLevel, 0); } +// TODO(jiawei.shao@intel.com): remove "lazy_clear_buffer_on_first_use" when we complete the +// support of buffer lazy initialization. DAWN_INSTANTIATE_TEST(TextureZeroInitTest, D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}), D3D12Backend({"nonzero_clear_resources_on_creation_for_testing"}, {"use_d3d12_render_pass"}), + D3D12Backend({"nonzero_clear_resources_on_creation_for_testing", + "lazy_clear_buffer_on_first_use"}), OpenGLBackend({"nonzero_clear_resources_on_creation_for_testing"}), + OpenGLBackend({"nonzero_clear_resources_on_creation_for_testing", + "lazy_clear_buffer_on_first_use"}), MetalBackend({"nonzero_clear_resources_on_creation_for_testing"}), - VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"})); + MetalBackend({"nonzero_clear_resources_on_creation_for_testing", + "lazy_clear_buffer_on_first_use"}), + VulkanBackend({"nonzero_clear_resources_on_creation_for_testing"}), + VulkanBackend({"nonzero_clear_resources_on_creation_for_testing", + "lazy_clear_buffer_on_first_use"}));