commit 4ddfcbeeff44c65fbe7c50f2865c4c848cec069a
author Jie Chen <jie.a.chen@intel.com> Wed Sep 18 08:10:48 2024 +0000
committer Dawn LUCI CQ <dawn-scoped@luci-project-accounts.iam.gserviceaccount.com> Wed Sep 18 08:10:48 2024 +0000
tree 0c09af12e7c6f33770e04370f817a313a423b6c4
parent 5bc64765c5e9fbde981eac9f79d34bc487ae9343

Fix nits in ClearWithDrawHelper

Using storage attachments of undefined format to create the pipeline is
not allowed. This fixes the issue by removing such attachments.
This also fixes the bug in 'ShouldApplyClearColorWithDraw', which
mistakenly makes 'clearWithDrawForBigInt' mask 'clearWithDraw'.

Bug: chromium:329702368
Change-Id: I9be332f848c71305839bf2e8e2009805b1dd9e61
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/206093
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Jie A Chen <jie.a.chen@intel.com>
Reviewed-by: Loko Kung <lokokung@google.com>

src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp

diff --git a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
index 1c1ed9e..295d04b 100644
--- a/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
+++ b/src/dawn/native/ApplyClearColorValueWithDrawHelper.cpp
@@ -388,7 +388,7 @@
     if (loadOp != wgpu::LoadOp::Clear) {
         return false;
     }
-    if (clearWithDrawForBigInt) {
+    if (!clearWithDraw && clearWithDrawForBigInt) {
         return NeedsBigIntClear(view->GetFormat(), clearValue);
     }
 
@@ -448,15 +448,17 @@
     if (const auto* pls = renderPassDescriptor.Get<RenderPassPixelLocalStorage>()) {
         key->hasPLS = true;
         key->totalPixelLocalStorageSize = pls->totalPixelLocalStorageSize;
-        key->plsAttachments = std::vector<wgpu::PipelineLayoutStorageAttachment>(
-            pls->totalPixelLocalStorageSize / kPLSSlotByteSize,
-            {nullptr, 0, wgpu::TextureFormat::Undefined});
-        for (size_t i = 0; i < pls->storageAttachmentCount; i++) {
-            size_t slot = pls->storageAttachments[i].offset / kPLSSlotByteSize;
-            key->plsAttachments[slot].format =
-                pls->storageAttachments[i].storage->GetFormat().format;
-            key->plsAttachments[slot].offset = pls->storageAttachments[i].offset;
+        for (size_t i = 0; i < pls->storageAttachmentCount; ++i) {
+            wgpu::PipelineLayoutStorageAttachment attachment{};
+            attachment.format = pls->storageAttachments[i].storage->GetFormat().format;
+            attachment.offset = pls->storageAttachments[i].offset;
+            key->plsAttachments.push_back(std::move(attachment));
         }
+        // Sort the PLS attachments by offset to make sure the order is deterministic.
+        std::sort(
+            key->plsAttachments.begin(), key->plsAttachments.end(),
+            [](const wgpu::PipelineLayoutStorageAttachment& a,
+               const wgpu::PipelineLayoutStorageAttachment& b) { return a.offset < b.offset; });
     }
 
     return true;