d3d12: Signal fence after PIX Present

Calling ID3D12SharingContract::Present issues GPU work on the command
queue which needs to be synchronized with resource deallocation. This
seems to work now perhaps due to the keyed mutex semantics keeping the
D3D11 texture alive for longer than necessary. With fences, this missing
synchronization causes the validation layers to complain about early
deallocation of the ID3D12Resource.

Moving the Present to SynchronizeImportTextureBeforeUse ensures that it
happens before NextSerial and hence the signal fence that's recorded
will include any GPU work issued by Present. Also, resource deallocation
will happen after this work. However, this has the side-effect of PIX
seeing more frames, once per ExecuteCommandLists, but it could be argued
that's more accurate and useful.

Bug: dawn:1544
Change-Id: I1b417049045a812837f67072d7f09ac47bc18125
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/103841
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org>

src/dawn/native/d3d12/TextureD3D12.cpp

@@ -656,53 +656,37 @@ Texture::~Texture() {}
 void Texture::DestroyImpl() {
     TextureBase::DestroyImpl();
 
-    Device* device = ToBackend(GetDevice());
+    ToBackend(GetDevice())->DeallocateMemory(mResourceAllocation);
 
-    // In PIX's D3D12-only mode, there is no way to determine frame boundaries
+    // Set mSwapChainTexture to false to prevent ever calling ID3D12SharingContract::Present again.
-    // for WebGPU since Dawn does not manage DXGI swap chains. Without assistance,
-    // PIX will wait forever for a present that never happens.
-    // If we know we're dealing with a swapbuffer texture, inform PIX we've
-    // "presented" the texture so it can determine frame boundaries and use its
-    // contents for the UI.
-    if (mSwapChainTexture) {
-        ID3D12SharingContract* d3dSharingContract = device->GetSharingContract();
-        if (d3dSharingContract != nullptr) {
-            d3dSharingContract->Present(mResourceAllocation.GetD3D12Resource(), 0, 0);
-        }
-    }
-
-    device->DeallocateMemory(mResourceAllocation);
-
-    // Now that we've deallocated the memory, the texture is no longer a swap chain texture.
-    // We can set mSwapChainTexture to false to avoid passing a nullptr to
-    // ID3D12SharingContract::Present.
     mSwapChainTexture = false;
 
-    // Now that the texture has been destroyed. It should release the refptr of the d3d11on12
+    // Now that the texture has been destroyed, it should release the d3d11on12 resource refptr.
-    // resource and the fence.
     mD3D11on12Resource = nullptr;
 }
 
 ResultOrError<ExecutionSerial> Texture::EndAccess() {
     ASSERT(mD3D11on12Resource == nullptr);
 
+    Device* device = ToBackend(GetDevice());
+
     // Synchronize if texture access wasn't synchronized already due to ExecuteCommandLists.
     if (!mSignalFenceValue.has_value()) {
         // Needed to ensure that command allocator doesn't get destroyed before pending commands
         // are submitted due to calling NextSerial(). No-op if there are no pending commands.
-        DAWN_TRY(ToBackend(GetDevice())->ExecutePendingCommandContext());
+        DAWN_TRY(device->ExecutePendingCommandContext());
         // If there were pending commands that used this texture mSignalFenceValue will be set,
         // but if it's still not set, generate a signal fence after waiting on wait fences.
         if (!mSignalFenceValue.has_value()) {
             DAWN_TRY(SynchronizeImportedTextureBeforeUse());
             DAWN_TRY(SynchronizeImportedTextureAfterUse());
         }
-        DAWN_TRY(ToBackend(GetDevice())->NextSerial());
+        DAWN_TRY(device->NextSerial());
         ASSERT(mSignalFenceValue.has_value());
     }
 
     ExecutionSerial ret = mSignalFenceValue.value();
-    ASSERT(ret <= GetDevice()->GetLastSubmittedCommandSerial());
+    ASSERT(ret <= device->GetLastSubmittedCommandSerial());
     // Explicitly call reset() since std::move() on optional doesn't make it std::nullopt.
     mSignalFenceValue.reset();
     return ret;
@@ -757,11 +741,24 @@ MaybeError Texture::SynchronizeImportedTextureBeforeUse() {
 }
 
 MaybeError Texture::SynchronizeImportedTextureAfterUse() {
+    // In PIX's D3D12-only mode, there is no way to determine frame boundaries
+    // for WebGPU since Dawn does not manage DXGI swap chains. Without assistance,
+    // PIX will wait forever for a present that never happens.
+    // If we know we're dealing with a swapbuffer texture, inform PIX we've
+    // "presented" the texture so it can determine frame boundaries and use its
+    // contents for the UI.
+    Device* device = ToBackend(GetDevice());
+    if (mSwapChainTexture) {
+        ID3D12SharingContract* d3dSharingContract = device->GetSharingContract();
+        if (d3dSharingContract != nullptr) {
+            d3dSharingContract->Present(mResourceAllocation.GetD3D12Resource(), 0, 0);
+        }
+    }
     if (mD3D11on12Resource != nullptr) {
         DAWN_TRY(mD3D11on12Resource->ReleaseKeyedMutex());
     } else {
         // NextSerial() will be called after this - this is also checked in EndAccess().
-        mSignalFenceValue = ToBackend(GetDevice())->GetPendingCommandSerial();
+        mSignalFenceValue = device->GetPendingCommandSerial();
     }
     return {};
 }