commit 25eeaa3d39afcc82fe9d384949d4e9c99e6e7903
author Corentin Wallez <cwallez@chromium.org> Tue Oct 27 11:31:26 2020 +0000
committer Commit Bot service account <commit-bot@chromium.org> Tue Oct 27 11:31:26 2020 +0000
tree 010e11279f12a1103a1a92be3c2dd5b7fb102a14
parent 41c24933da1e6c5ec111f824bfc5977be244d3e4

Make Surface reference its attached SwapChain

This solves an issues where when switching swapchains the previous
one was destroyed before the new one was created, doing so detached
itself from the Surface, which in turn made the new swapchain not do
a graceful transition via vkSwapchainCreateInfoKHR::oldSwapchain.

Keeping the reference on the surface makes sure we always have
knowledge of the previous swapchain when replacing it. It requires
re-working the lifetime model of NewSwapChainBase to not require a
call to DetachFromSurface in the destructor, and having the Device
explicitly tell a swapchain it got attached on creation (otherwise
there are ASSERTs firing when swapchain creation fails).

In addition, backends are changed to use a SwapChain::Create method
and fail with a validation error (for now) when the previous swapchain
didn't use the same API.

vulkan::SwapChain is updated to use the previous swapchain's device's
fenced deleter to destroy it which is important in the device
switching tests.

The SwapChainValidationTests are updated because with the lifetime
changes the texture view can be kept alive after the application has
lost the last reference to the wgpu::SwapChain.

TBRing since it was reviewed in a different CL (but for the wrong
branch).

TBR=enga@chromium.org
TBR=senorblanco@chromium.org

Bug: dawn:269
Change-Id: Ie4374b5685af990d68969ab9cd7767e53c287ace
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/31041
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>

src/dawn_native/Device.cpp

diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index 84210db..e5454f8 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -1017,21 +1017,24 @@
             DAWN_TRY(ValidateSwapChainDescriptor(this, surface, descriptor));
         }
 
+        // TODO(dawn:269): Remove this code path once implementation-based swapchains are removed.
         if (surface == nullptr) {
             DAWN_TRY_ASSIGN(*result, CreateSwapChainImpl(descriptor));
         } else {
             ASSERT(descriptor->implementation == 0);
 
             NewSwapChainBase* previousSwapChain = surface->GetAttachedSwapChain();
-            NewSwapChainBase* newSwapChain;
-            DAWN_TRY_ASSIGN(newSwapChain,
-                            CreateSwapChainImpl(surface, previousSwapChain, descriptor));
+            ResultOrError<NewSwapChainBase*> maybeNewSwapChain =
+                CreateSwapChainImpl(surface, previousSwapChain, descriptor);
 
             if (previousSwapChain != nullptr) {
-                ASSERT(!previousSwapChain->IsAttached());
+                previousSwapChain->DetachFromSurface();
             }
-            ASSERT(newSwapChain->IsAttached());
 
+            NewSwapChainBase* newSwapChain = nullptr;
+            DAWN_TRY_ASSIGN(newSwapChain, std::move(maybeNewSwapChain));
+
+            newSwapChain->SetIsAttached();
             surface->SetAttachedSwapChain(newSwapChain);
             *result = newSwapChain;
         }