[wire] Separate the client Device alive state into lost and destroyed. - We separate the alive state on the client Device into two separate states to have a clear distinction between client-side versus server-side states. The destroyed state is always set as soon as the client-side calls device.Destroy(), whereas the lost state is only set once the lost callback has been called. This distinction will allow us to handle synchronous (buffer mapping) and asynchronous (pipeline creation) APIs differently when applicable. - This change also removes the check for Surfaces because the call is actually supposed to generate a validation error which can only happen by sending the command to the server right now. Bug: 521662708 Change-Id: I044f4ee1d667b9a4b7065af1e552e4d9ee84c864 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/322595 Reviewed-by: Kai Ninomiya <kainino@chromium.org> Commit-Queue: Kai Ninomiya <kainino@chromium.org> Commit-Queue: Loko Kung <lokokung@google.com> Auto-Submit: Loko Kung <lokokung@google.com>
diff --git a/src/dawn/tests/unittests/wire/WireInjectSurfaceTests.cpp b/src/dawn/tests/unittests/wire/WireInjectSurfaceTests.cpp index 1192fb2..754ff09 100644 --- a/src/dawn/tests/unittests/wire/WireInjectSurfaceTests.cpp +++ b/src/dawn/tests/unittests/wire/WireInjectSurfaceTests.cpp
@@ -330,33 +330,5 @@ FlushClient(); } -// Test calling GetCurrentTexture after the device is destroyed. -// Note that this tests the same code path as device loss. -TEST_F(WireInjectSurfaceTests, GetCurrentTextureDeviceDestroyed) { - auto [reservation, surface] = ReserveSurface(&mCapabilities); - - WGPUSurface apiSurface = api.GetNewSurface(); - EXPECT_CALL(api, SurfaceAddRef(apiSurface)); - ASSERT_TRUE( - GetWireServer()->InjectSurface(apiSurface, reservation.handle, reservation.instanceHandle)); - - surface.Configure(&mConfiguration); - EXPECT_CALL(api, SurfaceConfigure(apiSurface, _)); - - device.Destroy(); - EXPECT_CALL(api, DeviceDestroy(apiDevice)); - - wgpu::SurfaceTexture texture; - surface.GetCurrentTexture(&texture); - - WGPUTexture apiTexture = api.GetNewTexture(); - EXPECT_CALL(api, DeviceCreateErrorTexture(apiDevice, _)).WillOnce(Return(apiTexture)); - - EXPECT_EQ(texture.status, wgpu::SurfaceGetCurrentTextureStatus::SuccessOptimal); - EXPECT_NE(texture.texture.Get(), nullptr); - - FlushClient(); -} - } // anonymous namespace } // namespace dawn::wire
diff --git a/src/dawn/wire/client/Buffer.cpp b/src/dawn/wire/client/Buffer.cpp index df97863..93c3db4 100644 --- a/src/dawn/wire/client/Buffer.cpp +++ b/src/dawn/wire/client/Buffer.cpp
@@ -153,7 +153,7 @@ } // Device destruction/loss implicitly makes the map requests aborted. - if (!mBuffer->mDevice->IsAlive()) { + if (mBuffer->mDevice->IsDestroyed()) { mStatus = WGPUMapAsyncStatus_Aborted; mMessage = "The Device was lost before mapping was resolved."; }
diff --git a/src/dawn/wire/client/Device.cpp b/src/dawn/wire/client/Device.cpp index 39cd0bc..320e8eb 100644 --- a/src/dawn/wire/client/Device.cpp +++ b/src/dawn/wire/client/Device.cpp
@@ -191,6 +191,7 @@ // The uncaptured error and logging callbacks are spontaneous and must not be called // after we call the device lost's |mCallback| below, so we clear them and wait for them to // be no longer referenced before moving forwards. + mDevice->mIsLost = true; mDevice->mCallbackInfos.Clear(); void* userdata1 = mUserdata1.ExtractAsDangling(); @@ -228,8 +229,12 @@ return ObjectType::Device; } -bool Device::IsAlive() const { - return mIsAlive; +bool Device::IsDestroyed() const { + return mIsDestroyed; +} + +bool Device::IsKnownLost() const { + return mIsLost; } Queue* Device::GetQueue() { @@ -254,6 +259,7 @@ } void Device::WillDropLastExternalRef() { + mIsDestroyed = true; if (IsRegistered()) { HandleDeviceLost(WGPUDeviceLostReason_Destroyed, ToOutputStringView("Device was destroyed.")); @@ -298,7 +304,6 @@ FutureID futureID = APIGetLostFuture().id; auto wireStatus = GetEventManager().SetFutureReady<DeviceLostEvent>(futureID, reason, message); DAWN_CHECK(wireStatus == WireResult::Success); - mIsAlive = false; } WGPUFuture Device::APIGetLostFuture() { @@ -312,7 +317,7 @@ } void Device::APISetLoggingCallback(const WGPULoggingCallbackInfo& callbackInfo) { - if (mIsAlive) { + if (!mIsLost) { mCallbackInfos.SetLoggingCallbackInfo(callbackInfo); } } @@ -443,7 +448,7 @@ cmd.self = ToAPI(this); GetClient()->SerializeCommand(cmd); - mIsAlive = false; + mIsDestroyed = true; } } // namespace dawn::wire::client
diff --git a/src/dawn/wire/client/Device.h b/src/dawn/wire/client/Device.h index ca09b42..88d0cca 100644 --- a/src/dawn/wire/client/Device.h +++ b/src/dawn/wire/client/Device.h
@@ -59,7 +59,8 @@ void SetLimits(const WGPULimits* limits); void SetFeatures(const WGPUFeatureName* features, uint32_t featuresCount); - bool IsAlive() const; + bool IsDestroyed() const; + bool IsKnownLost() const; Queue* GetQueue(); const LimitsAndFeatures& GetLimitsAndFeatures() const; @@ -110,7 +111,16 @@ Ref<Adapter> mAdapter; Ref<Queue> mQueue; - bool mIsAlive = true; + + // Note that we differentiate between destroyed and lost in that destroyed is a client-side + // state that is immediately set once `APIDestroy()` is called, whereas lost is a server-side + // state that is updated only once our lost callback has been completed. The destroyed state is, + // as of writing, only really needed for buffer mapping because device.Destroy() is supposed to + // explicitly unmap all buffers, but don't currently handle that exactly in the wire, so the + // destroyed state is used to help simulate that. Pretty much everything else should be relying + // on the lost state. + bool mIsDestroyed = false; + bool mIsLost = false; }; } // namespace dawn::wire::client
diff --git a/src/dawn/wire/client/Surface.cpp b/src/dawn/wire/client/Surface.cpp index 28f8e31..cc4b5b3 100644 --- a/src/dawn/wire/client/Surface.cpp +++ b/src/dawn/wire/client/Surface.cpp
@@ -142,13 +142,6 @@ return; } - if (!mConfiguredDevice->IsAlive()) { - surfaceTexture->status = WGPUSurfaceGetCurrentTextureStatus_SuccessOptimal; - surfaceTexture->texture = - Texture::CreateError(mConfiguredDevice.Get(), &mTextureDescriptor); - return; - } - // Assume texture creation will work in the server and return a new texture proxy. Client* wireClient = GetClient(); Ref<Texture> texture = wireClient->Make<Texture>(mConfiguredDevice.Get(), &mTextureDescriptor);