[WebGPU] Curl noise is showing more dark / shadows in Safari / WebKit…

… compared to Chrome Canary

https://bugs.webkit.org/show_bug.cgi?id=274174
<rdar://problem/128060711>

Reviewed by Dan Glastonbury.

Auto-generated layouts had a bug where it was assumed a given resource
had the same offset in the Metal argument buffer across vertex, fragment,
and compute stages.

This is not necessarily the case, we need to store seperate indices for
each stage.

* Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp:
(WebCore::WebGPU::DeviceImpl::createBindGroupLayout):
* Source/WebGPU/WebGPU/BindGroupLayout.mm:
(WebGPU::Device::createBindGroupLayout):
* Source/WebGPU/WebGPU/RenderPipeline.mm:
(WebGPU::Device::addPipelineLayouts):
* Source/WebGPU/WebGPU/WebGPU.h:

Canonical link: https://commits.webkit.org/278846@main

Source/WebCore/Modules/WebGPU/Implementation/WebGPUDeviceImpl.cpp

@@ -189,7 +189,7 @@ RefPtr<BindGroupLayout> DeviceImpl::createBindGroupLayout(const BindGroupLayoutD
         return WGPUBindGroupLayoutEntry {
             .nextInChain = nullptr,
             .binding = entry.binding,
-            .metalBinding = entry.binding,
+            .metalBinding = { entry.binding, entry.binding, entry.binding },
             .visibility = m_convertToBackingContext->convertShaderStageFlagsToBacking(entry.visibility),
             .buffer = {
                 nullptr,

Source/WebGPU/WebGPU/BindGroupLayout.mm

@@ -189,18 +189,12 @@ static void reportErrorInCreateBindGroupLayout(NSString* errorMessage, bool isAu
     }
 
     Vector<WGPUBindGroupLayoutEntry> descriptorEntries(std::span { descriptor.entries, descriptor.entryCount });
-    std::sort(descriptorEntries.begin(), descriptorEntries.end(), [](const WGPUBindGroupLayoutEntry& a, const WGPUBindGroupLayoutEntry& b) {
-        return a.metalBinding < b.metalBinding;
-    });
-
     BindGroupLayout::EntriesContainer bindGroupLayoutEntries;
     size_t sizeOfDynamicOffsets[stageCount] = { 0, 0, 0 };
     uint32_t bindingOffset[stageCount] = { 0, 0, 0 };
     uint32_t bufferCounts[stageCount] = { 0, 0, 0 };
     HashMap<uint32_t, uint64_t, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> slotForEntry;
-    bool hasCompilerGeneratedArrayLengths = false;
     const auto maxBindingIndex = limits().maxBindingsPerBindGroup;
-    uint32_t bindingIndices[stageCount] = { 0, 0, 0 };
     HashSet<uint32_t, DefaultHash<uint32_t>, WTF::UnsignedWithZeroKeyHashTraits<uint32_t>> usedBindingSlots;
     uint32_t dynamicUniformBuffers = 0;
     uint32_t dynamicStorageBuffers = 0;
@@ -263,7 +257,6 @@ static void reportErrorInCreateBindGroupLayout(NSString* errorMessage, bool isAu
             bindingLayout = WGPUExternalTextureBindingLayout();
         } else if (entry.buffer.type == static_cast<WGPUBufferBindingType>(WGPUBufferBindingType_ArrayLength)) {
             slotForEntry.set(entry.buffer.bufferSizeForBinding, entry.binding);
-            hasCompilerGeneratedArrayLengths = true;
             continue;
         } else {
             if (!processBindingLayout(entry.buffer)) {
@@ -296,7 +289,7 @@ static void reportErrorInCreateBindGroupLayout(NSString* errorMessage, bool isAu
                 if (containsStage(entry.visibility, stage)) {
                     indicesForBinding.add(makeKey(entry.binding, stage), descriptors[0].access);
                     auto renderStage = stages[stage];
-                    auto argumentBufferBindingIndex = isAutoGenerated ? bindingIndices[stage] : entry.binding;
+                    auto argumentBufferBindingIndex = isAutoGenerated ? entry.metalBinding[stage] : entry.binding;
                     argumentBufferIndices[renderStage] = isAutoGenerated ? argumentBufferBindingIndex : (argumentBufferBindingIndex + bindingOffset[stage]);
                     if (entry.buffer.hasDynamicOffset) {
                         dynamicOffsets[stage] = sizeOfDynamicOffsets[stage];
@@ -338,10 +331,9 @@ static void reportErrorInCreateBindGroupLayout(NSString* errorMessage, bool isAu
                     }
 
                     for (int descriptorIndex = 0; descriptorIndex < maxGeneratedDescriptors; ++descriptorIndex) {
-                        if (MTLArgumentDescriptor *descriptor = descriptors[descriptorIndex]) {
+                        if (MTLArgumentDescriptor *descriptor = descriptors[descriptorIndex])
                             addDescriptor(arguments[stage], descriptor, *argumentBufferIndices[renderStage] + descriptorIndex);
-                            ++bindingIndices[stage];
-                        } else
+                        else
                             break;
                     }
 
@@ -402,7 +394,7 @@ static void reportErrorInCreateBindGroupLayout(NSString* errorMessage, bool isAu
             NSUInteger maxIndex = [arguments[stage] objectAtIndex:arguments[stage].count - 1].index;
             for (auto& entry : bindGroupLayoutEntries) {
                 if (entry.value.bufferSizeArgumentBufferIndices[renderStage]) {
-                    if (!hasCompilerGeneratedArrayLengths)
+                    if (!isAutoGenerated)
                         *entry.value.bufferSizeArgumentBufferIndices[renderStage] += maxIndex;
                     else if (auto it = slotForEntry.find(entry.value.binding); it != slotForEntry.end())
                         entry.value.bufferSizeArgumentBufferIndices[renderStage] = it->value;

Source/WebGPU/WebGPU/RenderPipeline.mm

@@ -795,11 +795,14 @@ static WGPUTextureFormat convertFormat(WGSL::TexelFormat format)
         auto& entries = pipelineEntries[bindGroupLayout.group];
         HashMap<String, uint64_t> entryMap;
         for (auto& entry : bindGroupLayout.entries) {
+            auto visibility = convertVisibility(entry.visibility);
+            auto stage = visibility / 2;
             // FIXME: https://bugs.webkit.org/show_bug.cgi?id=265204 - use a set instead
             if (auto existingBindingIndex = entries.findIf([&](const WGPUBindGroupLayoutEntry& e) {
                 return e.binding == entry.webBinding;
             }); existingBindingIndex != notFound) {
-                entries[existingBindingIndex].visibility |= convertVisibility(entry.visibility);
+                entries[existingBindingIndex].visibility |= visibility;
+                entries[existingBindingIndex].metalBinding[stage] = entry.binding;
                 continue;
             }
             WGPUBindGroupLayoutEntry newEntry = { };
@@ -816,8 +819,8 @@ static WGPUTextureFormat convertFormat(WGSL::TexelFormat format)
             }
 
             newEntry.binding = entry.webBinding;
-            newEntry.metalBinding = entry.binding;
-            newEntry.visibility = convertVisibility(entry.visibility);
+            newEntry.metalBinding[stage] = entry.binding;
+            newEntry.visibility = visibility;
             WTF::switchOn(entry.bindingMember, [&](const WGSL::BufferBindingLayout& bufferBinding) {
                 newEntry.buffer = WGPUBufferBindingLayout {
                     .nextInChain = nullptr,

Source/WebGPU/WebGPU/WebGPU.h

@@ -1146,7 +1146,7 @@ typedef struct WGPUBindGroupDescriptor {
 typedef struct WGPUBindGroupLayoutEntry {
     WGPUChainedStruct const * nextInChain;
     uint32_t binding;
-    uint32_t metalBinding;
+    uint32_t metalBinding[WGPUShaderStage_Compute / 2 + 1];
     WGPUShaderStageFlags visibility;
     WGPUBufferBindingLayout buffer;
     WGPUSamplerBindingLayout sampler;