From e690a6654a733b3e3ebf8edaff16b9a21de66ec0 Mon Sep 17 00:00:00 2001 From: Corentin Wallez Date: Tue, 29 Mar 2022 09:35:53 +0000 Subject: [PATCH] Make SwapChain use a CreateView() instead of APICreateView() This simplifies reference management and make the swapchain's reentrant code avoid APIEntryPoints. Bug: dawn:723 Change-Id: I6c456b9ec349c73d783dbb12a284f322d0be4e1a Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/84763 Reviewed-by: Loko Kung Commit-Queue: Corentin Wallez --- src/dawn/native/SwapChain.cpp | 42 +++++++++++++----------- src/dawn/native/SwapChain.h | 3 +- src/dawn/native/Texture.cpp | 9 +++-- src/dawn/native/Texture.h | 3 ++ src/dawn/native/d3d12/SwapChainD3D12.cpp | 6 ++-- src/dawn/native/d3d12/SwapChainD3D12.h | 2 +- src/dawn/native/metal/SwapChainMTL.h | 2 +- src/dawn/native/metal/SwapChainMTL.mm | 5 ++- src/dawn/native/null/DeviceNull.cpp | 5 ++- src/dawn/native/null/DeviceNull.h | 2 +- src/dawn/native/vulkan/SwapChainVk.cpp | 10 +++--- src/dawn/native/vulkan/SwapChainVk.h | 4 +-- 12 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/dawn/native/SwapChain.cpp b/src/dawn/native/SwapChain.cpp index 5fffc09db1..1bd5fd707f 100644 --- a/src/dawn/native/SwapChain.cpp +++ b/src/dawn/native/SwapChain.cpp @@ -316,34 +316,38 @@ namespace dawn::native { } TextureViewBase* NewSwapChainBase::APIGetCurrentTextureView() { - if (GetDevice()->ConsumedError(ValidateGetCurrentTextureView())) { + Ref result; + if (GetDevice()->ConsumedError(GetCurrentTextureView(), &result, + "calling %s.GetCurrentTextureView()", this)) { return TextureViewBase::MakeError(GetDevice()); } + return result.Detach(); + } + + ResultOrError> NewSwapChainBase::GetCurrentTextureView() { + DAWN_TRY(ValidateGetCurrentTextureView()); if (mCurrentTextureView != nullptr) { - // Calling GetCurrentTextureView always returns a new reference so add it even when - // reusing the existing texture view. - mCurrentTextureView->Reference(); - return mCurrentTextureView.Get(); + // Calling GetCurrentTextureView always returns a new reference. + return mCurrentTextureView; } - TextureViewBase* view = nullptr; - if (GetDevice()->ConsumedError(GetCurrentTextureViewImpl(), &view)) { - return TextureViewBase::MakeError(GetDevice()); - } + DAWN_TRY_ASSIGN(mCurrentTextureView, GetCurrentTextureViewImpl()); // Check that the return texture view matches exactly what was given for this descriptor. - ASSERT(view->GetTexture()->GetFormat().format == mFormat); - ASSERT(IsSubset(mUsage, view->GetTexture()->GetUsage())); - ASSERT(view->GetLevelCount() == 1); - ASSERT(view->GetLayerCount() == 1); - ASSERT(view->GetDimension() == wgpu::TextureViewDimension::e2D); - ASSERT(view->GetTexture()->GetMipLevelVirtualSize(view->GetBaseMipLevel()).width == mWidth); - ASSERT(view->GetTexture()->GetMipLevelVirtualSize(view->GetBaseMipLevel()).height == - mHeight); + ASSERT(mCurrentTextureView->GetTexture()->GetFormat().format == mFormat); + ASSERT(IsSubset(mUsage, mCurrentTextureView->GetTexture()->GetUsage())); + ASSERT(mCurrentTextureView->GetLevelCount() == 1); + ASSERT(mCurrentTextureView->GetLayerCount() == 1); + ASSERT(mCurrentTextureView->GetDimension() == wgpu::TextureViewDimension::e2D); + ASSERT(mCurrentTextureView->GetTexture() + ->GetMipLevelVirtualSize(mCurrentTextureView->GetBaseMipLevel()) + .width == mWidth); + ASSERT(mCurrentTextureView->GetTexture() + ->GetMipLevelVirtualSize(mCurrentTextureView->GetBaseMipLevel()) + .height == mHeight); - mCurrentTextureView = view; - return view; + return mCurrentTextureView; } void NewSwapChainBase::APIPresent() { diff --git a/src/dawn/native/SwapChain.h b/src/dawn/native/SwapChain.h index b19c0b6bcc..48b827032d 100644 --- a/src/dawn/native/SwapChain.h +++ b/src/dawn/native/SwapChain.h @@ -154,7 +154,8 @@ namespace dawn::native { // manner, starting with GetCurrentTextureViewImpl. // The returned texture view must match the swapchain descriptor exactly. - virtual ResultOrError GetCurrentTextureViewImpl() = 0; + ResultOrError> GetCurrentTextureView(); + virtual ResultOrError> GetCurrentTextureViewImpl() = 0; // The call to present must destroy the current view's texture so further access to it are // invalid. virtual MaybeError PresentImpl() = 0; diff --git a/src/dawn/native/Texture.cpp b/src/dawn/native/Texture.cpp index 5f81a7f39a..c7664363d8 100644 --- a/src/dawn/native/Texture.cpp +++ b/src/dawn/native/Texture.cpp @@ -668,12 +668,17 @@ namespace dawn::native { return {clampedCopyExtentWidth, clampedCopyExtentHeight, extent.depthOrArrayLayers}; } + ResultOrError> TextureBase::CreateView( + const TextureViewDescriptor* descriptor) { + return GetDevice()->CreateTextureView(this, descriptor); + } + TextureViewBase* TextureBase::APICreateView(const TextureViewDescriptor* descriptor) { DeviceBase* device = GetDevice(); Ref result; - if (device->ConsumedError(device->CreateTextureView(this, descriptor), &result, - "calling %s.CreateView(%s).", this, descriptor)) { + if (device->ConsumedError(CreateView(descriptor), &result, "calling %s.CreateView(%s).", + this, descriptor)) { return TextureViewBase::MakeError(device); } return result.Detach(); diff --git a/src/dawn/native/Texture.h b/src/dawn/native/Texture.h index 54ed018a58..de0a91e33a 100644 --- a/src/dawn/native/Texture.h +++ b/src/dawn/native/Texture.h @@ -91,6 +91,9 @@ namespace dawn::native { const Origin3D& origin, const Extent3D& extent) const; + ResultOrError> CreateView( + const TextureViewDescriptor* descriptor = nullptr); + // Dawn API TextureViewBase* APICreateView(const TextureViewDescriptor* descriptor = nullptr); void APIDestroy(); diff --git a/src/dawn/native/d3d12/SwapChainD3D12.cpp b/src/dawn/native/d3d12/SwapChainD3D12.cpp index f37f2f7ed8..0c23a01a25 100644 --- a/src/dawn/native/d3d12/SwapChainD3D12.cpp +++ b/src/dawn/native/d3d12/SwapChainD3D12.cpp @@ -332,7 +332,7 @@ namespace dawn::native::d3d12 { return {}; } - ResultOrError SwapChain::GetCurrentTextureViewImpl() { + ResultOrError> SwapChain::GetCurrentTextureViewImpl() { Device* device = ToBackend(GetDevice()); // Synchronously wait until previous operations on the next swapchain buffer are finished. @@ -346,9 +346,7 @@ namespace dawn::native::d3d12 { TextureDescriptor descriptor = GetSwapChainBaseTextureDescriptor(this); DAWN_TRY_ASSIGN(mApiTexture, Texture::Create(ToBackend(GetDevice()), &descriptor, mBuffers[mCurrentBuffer])); - - // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. - return mApiTexture->APICreateView(); + return mApiTexture->CreateView(); } MaybeError SwapChain::DetachAndWaitForDeallocation() { diff --git a/src/dawn/native/d3d12/SwapChainD3D12.h b/src/dawn/native/d3d12/SwapChainD3D12.h index eb99f1c88c..1e8a7d98a1 100644 --- a/src/dawn/native/d3d12/SwapChainD3D12.h +++ b/src/dawn/native/d3d12/SwapChainD3D12.h @@ -63,7 +63,7 @@ namespace dawn::native::d3d12 { // NewSwapChainBase implementation MaybeError PresentImpl() override; - ResultOrError GetCurrentTextureViewImpl() override; + ResultOrError> GetCurrentTextureViewImpl() override; void DetachFromSurfaceImpl() override; // Does the swapchain initialization steps assuming there is nothing we can reuse. diff --git a/src/dawn/native/metal/SwapChainMTL.h b/src/dawn/native/metal/SwapChainMTL.h index a0f1ccc1a2..9cae564a87 100644 --- a/src/dawn/native/metal/SwapChainMTL.h +++ b/src/dawn/native/metal/SwapChainMTL.h @@ -58,7 +58,7 @@ namespace dawn::native::metal { Ref mTexture; MaybeError PresentImpl() override; - ResultOrError GetCurrentTextureViewImpl() override; + ResultOrError> GetCurrentTextureViewImpl() override; void DetachFromSurfaceImpl() override; }; diff --git a/src/dawn/native/metal/SwapChainMTL.mm b/src/dawn/native/metal/SwapChainMTL.mm index 0291a41aee..04e66fb6ec 100644 --- a/src/dawn/native/metal/SwapChainMTL.mm +++ b/src/dawn/native/metal/SwapChainMTL.mm @@ -129,7 +129,7 @@ namespace dawn::native::metal { return {}; } - ResultOrError SwapChain::GetCurrentTextureViewImpl() { + ResultOrError> SwapChain::GetCurrentTextureViewImpl() { ASSERT(mCurrentDrawable == nullptr); mCurrentDrawable = [*mLayer nextDrawable]; @@ -137,8 +137,7 @@ namespace dawn::native::metal { mTexture = Texture::CreateWrapping(ToBackend(GetDevice()), &textureDesc, [*mCurrentDrawable texture]); - // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. - return mTexture->APICreateView(); + return mTexture->CreateView(); } void SwapChain::DetachFromSurfaceImpl() { diff --git a/src/dawn/native/null/DeviceNull.cpp b/src/dawn/native/null/DeviceNull.cpp index 17942933b5..db0924125a 100644 --- a/src/dawn/native/null/DeviceNull.cpp +++ b/src/dawn/native/null/DeviceNull.cpp @@ -421,13 +421,12 @@ namespace dawn::native::null { return {}; } - ResultOrError SwapChain::GetCurrentTextureViewImpl() { + ResultOrError> SwapChain::GetCurrentTextureViewImpl() { TextureDescriptor textureDesc = GetSwapChainBaseTextureDescriptor(this); // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. mTexture = AcquireRef( new Texture(GetDevice(), &textureDesc, TextureBase::TextureState::OwnedInternal)); - // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. - return mTexture->APICreateView(); + return mTexture->CreateView(); } void SwapChain::DetachFromSurfaceImpl() { diff --git a/src/dawn/native/null/DeviceNull.h b/src/dawn/native/null/DeviceNull.h index 102d55c4d0..d810c066ea 100644 --- a/src/dawn/native/null/DeviceNull.h +++ b/src/dawn/native/null/DeviceNull.h @@ -297,7 +297,7 @@ namespace dawn::native::null { Ref mTexture; MaybeError PresentImpl() override; - ResultOrError GetCurrentTextureViewImpl() override; + ResultOrError> GetCurrentTextureViewImpl() override; void DetachFromSurfaceImpl() override; }; diff --git a/src/dawn/native/vulkan/SwapChainVk.cpp b/src/dawn/native/vulkan/SwapChainVk.cpp index 2c80d06f0a..90884f9e56 100644 --- a/src/dawn/native/vulkan/SwapChainVk.cpp +++ b/src/dawn/native/vulkan/SwapChainVk.cpp @@ -593,11 +593,11 @@ namespace dawn::native::vulkan { } } - ResultOrError SwapChain::GetCurrentTextureViewImpl() { + ResultOrError> SwapChain::GetCurrentTextureViewImpl() { return GetCurrentTextureViewInternal(); } - ResultOrError SwapChain::GetCurrentTextureViewInternal(bool isReentrant) { + ResultOrError> SwapChain::GetCurrentTextureViewInternal(bool isReentrant) { Device* device = ToBackend(GetDevice()); // Transiently create a semaphore that will be signaled when the presentation engine is done @@ -664,8 +664,7 @@ namespace dawn::native::vulkan { // In the happy path we can use the swapchain image directly. if (!mConfig.needsBlit) { - // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. - return mTexture->APICreateView(); + return mTexture->CreateView(); } // The blit texture always perfectly matches what the user requested for the swapchain. @@ -673,8 +672,7 @@ namespace dawn::native::vulkan { TextureDescriptor desc = GetSwapChainBaseTextureDescriptor(this); DAWN_TRY_ASSIGN(mBlitTexture, Texture::Create(device, &desc, VK_IMAGE_USAGE_TRANSFER_SRC_BIT)); - // TODO(dawn:723): change to not use AcquireRef for reentrant object creation. - return mBlitTexture->APICreateView(); + return mBlitTexture->CreateView(); } void SwapChain::DetachFromSurfaceImpl() { diff --git a/src/dawn/native/vulkan/SwapChainVk.h b/src/dawn/native/vulkan/SwapChainVk.h index 4ec2ad8fed..bfab4b735d 100644 --- a/src/dawn/native/vulkan/SwapChainVk.h +++ b/src/dawn/native/vulkan/SwapChainVk.h @@ -75,11 +75,11 @@ namespace dawn::native::vulkan { bool needsBlit = false; }; ResultOrError ChooseConfig(const VulkanSurfaceInfo& surfaceInfo) const; - ResultOrError GetCurrentTextureViewInternal(bool isReentrant = false); + ResultOrError> GetCurrentTextureViewInternal(bool isReentrant = false); // NewSwapChainBase implementation MaybeError PresentImpl() override; - ResultOrError GetCurrentTextureViewImpl() override; + ResultOrError> GetCurrentTextureViewImpl() override; void DetachFromSurfaceImpl() override; Config mConfig;