commit bde734c4ab8d553cb6411b18636af07f5750f3c5
author Corentin Wallez <cwallez@chromium.org> Mon Jun 10 15:23:01 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Mon Jun 10 15:23:01 2024 +0000
tree 233d977247003ee942e1864e9af58cdec2b59b5d
parent 85b0b2f115c86096992dc56cb2a28657db6d209e

GetCurrentTexture: Return ...Status::Error when a validation error happens

See https://github.com/webgpu-native/webgpu-headers/issues/291

Bug: 42241264
Change-Id: Ia8669fe9838c98f86942af9adaf835fddaf36fa6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/192185
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>

src/dawn/dawn.json

diff --git a/src/dawn/dawn.json b/src/dawn/dawn.json
index 9dc6374..1158712 100644
--- a/src/dawn/dawn.json
+++ b/src/dawn/dawn.json
@@ -4074,7 +4074,8 @@
             {"value": 2, "name": "outdated"},
             {"value": 3, "name": "lost"},
             {"value": 4, "name": "out of memory"},
-            {"value": 5, "name": "device lost"}
+            {"value": 5, "name": "device lost"},
+            {"value": 6, "name": "error"}
         ]
     },
     "texture aspect": {

src/dawn/native/Surface.cpp

diff --git a/src/dawn/native/Surface.cpp b/src/dawn/native/Surface.cpp
index c9f9ae0..4c7d39b 100644
--- a/src/dawn/native/Surface.cpp
+++ b/src/dawn/native/Surface.cpp
@@ -542,9 +542,20 @@
 }
 
 MaybeError Surface::GetCurrentTexture(SurfaceTexture* surfaceTexture) const {
+    surfaceTexture->texture = nullptr;
+    surfaceTexture->suboptimal = false;
+
+    surfaceTexture->status = wgpu::SurfaceGetCurrentTextureStatus::Error;
     DAWN_INVALID_IF(IsError(), "%s is invalid.", this);
     DAWN_INVALID_IF(!mSwapChain.Get(), "%s is not configured.", this);
 
+    surfaceTexture->status = wgpu::SurfaceGetCurrentTextureStatus::DeviceLost;
+    DAWN_TRY(GetCurrentDevice()->ValidateIsAlive());
+
+    // Set an error status that will be overwritten if there is a success or some other more
+    // specific error.
+    surfaceTexture->status = wgpu::SurfaceGetCurrentTextureStatus::Error;
+
     auto deviceLock(GetCurrentDevice()->GetScopedLock());
     DAWN_TRY_ASSIGN(*surfaceTexture, mSwapChain->GetCurrentTexture());
 
@@ -610,13 +621,7 @@
 void Surface::APIGetCurrentTexture(SurfaceTexture* surfaceTexture) const {
     MaybeError maybeError = GetCurrentTexture(surfaceTexture);
     if (!GetCurrentDevice()) {
-        if (mInstance->ConsumedError(std::move(maybeError))) {
-            // TODO(dawn:2320): This is the closest status to "surface was not configured so there
-            // is no associated device" but SurfaceTexture may change soon upstream.
-            surfaceTexture->status = wgpu::SurfaceGetCurrentTextureStatus::DeviceLost;
-            surfaceTexture->suboptimal = true;
-            surfaceTexture->texture = nullptr;
-        }
+        [[maybe_unused]] bool error = mInstance->ConsumedError(std::move(maybeError));
     } else {
         [[maybe_unused]] bool error = GetCurrentDevice()->ConsumedError(std::move(maybeError));
     }

src/dawn/tests/end2end/SurfaceTests.cpp

diff --git a/src/dawn/tests/end2end/SurfaceTests.cpp b/src/dawn/tests/end2end/SurfaceTests.cpp
index 9235ee6..3072b76 100644
--- a/src/dawn/tests/end2end/SurfaceTests.cpp
+++ b/src/dawn/tests/end2end/SurfaceTests.cpp
@@ -266,7 +266,7 @@
     surface.Present();
 }
 
-// Test destroying the swapchain after GetCurrentTexture
+// Test unconfiguring after GetCurrentTexture but before the Present
 TEST_P(SurfaceTests, UnconfigureAfterGet) {
     wgpu::Surface surface = CreateTestSurface();
     wgpu::SurfaceConfiguration config = GetPreferredConfiguration(surface);
@@ -426,22 +426,36 @@
 // It cannot raise a device error at this stage since it has never been configured with a device
 TEST_P(SurfaceTests, GetWithoutConfigure) {
     wgpu::Surface surface = CreateTestSurface();
+
     wgpu::SurfaceTexture surfaceTexture;
     surface.GetCurrentTexture(&surfaceTexture);
-    EXPECT_NE(surfaceTexture.status, wgpu::SurfaceGetCurrentTextureStatus::Success);
+    EXPECT_EQ(surfaceTexture.status, wgpu::SurfaceGetCurrentTextureStatus::Error);
 }
 
 // Getting current texture after unconfiguring fails
 TEST_P(SurfaceTests, GetAfterUnconfigure) {
     wgpu::Surface surface = CreateTestSurface();
     wgpu::SurfaceConfiguration config = GetPreferredConfiguration(surface);
-    wgpu::SurfaceTexture surfaceTexture;
-
     surface.Configure(&config);
 
     surface.Unconfigure();
 
+    wgpu::SurfaceTexture surfaceTexture;
     ASSERT_DEVICE_ERROR(surface.GetCurrentTexture(&surfaceTexture));
+    EXPECT_EQ(surfaceTexture.status, wgpu::SurfaceGetCurrentTextureStatus::Error);
+}
+
+// Getting current texture after losing the device
+TEST_P(SurfaceTests, GetAfterDeviceLoss) {
+    wgpu::Surface surface = CreateTestSurface();
+    wgpu::SurfaceConfiguration config = GetPreferredConfiguration(surface);
+    surface.Configure(&config);
+
+    LoseDeviceForTesting();
+
+    wgpu::SurfaceTexture surfaceTexture;
+    surface.GetCurrentTexture(&surfaceTexture);
+    EXPECT_EQ(surfaceTexture.status, wgpu::SurfaceGetCurrentTextureStatus::DeviceLost);
 }
 
 // Presenting without configuring fails