Attachment 359068 Details for Bug 193405 – Patch

[patch] Patch

bug-193405-20190114123654.patch (text/plain), 10.38 KB, created by Justin Fan on 2019-01-14 12:36:55 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Fan

Created: 2019-01-14 12:36:55 PST

Size: 10.38 KB

patch

obsolete

>Subversion Revision: 239864
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ad3517d49683c9af32531dbc27d0d0ff00794c6e..bd06a07c884186d7f03b780baeaee53ca243997d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-01-14  Justin Fan  <justin_fan@apple.com>
>+
>+        [WebGPU] Map WebGPUBindGroupLayoutBindings from the BindGroupLayoutDescriptor for error checking and later referencing
>+        https://bugs.webkit.org/show_bug.cgi?id=193405
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When creating a WebGPUBindGroupLayout, cache the WebGPUBindGroupLayoutDescriptor's list of BindGroupLayoutBindings
>+        in a HashMap, keyed by binding number, for quick reference during the WebGPUProgrammablePassEncoder::setBindGroups 
>+        implementation to follow. Also add error-checking e.g. detecting duplicate binding numbers in the same WebGPUBindGroupLayout
>+        and non-existent binding numbers when creating the WebGPUBindGroup.
>+
>+        No new tests. BindGroups and BindGroupLayouts reflect the (canonical?) strategy of returning empty 
>+        objects upon creation failure and reporting errors elswhere. Since error reporting is not yet implemented, 
>+        the error checks aren't testable from LayoutTests right now. Expected behavior unchanged and covered by existing tests.
>+
>+        * Modules/webgpu/WebGPUDevice.cpp:
>+        (WebCore::WebGPUDevice::createBindGroup const): 
>+                Number of bindings must be consistent between bindings and layout bindings.
>+                BindGroupBindings should only refer to existing BindGroupLayoutBindings.
>+        * platform/graphics/gpu/GPUBindGroup.h: 
>+        * platform/graphics/gpu/GPUBindGroupLayout.h:
>+        (WebCore::GPUBindGroupLayout::bindingsMap const): Added. Cache map of BindGroupLayoutBindings.
>+        * platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm: Disallow duplicate binding numbers in BindGroupLayoutBindings.
>+        (WebCore::GPUBindGroupLayout::tryCreate):
>+        (WebCore::GPUBindGroupLayout::GPUBindGroupLayout):
>+
> 2019-01-09  Justin Fan  <justin_fan@apple.com>
> 
>         [WebGPU] Fix vertex-buffer-triangle-strip test and small update to GPURenderPipeline
>diff --git a/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp b/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>index 3c4c0eb251ac7c82307faa73ff7af903ab3d75c4..70adec8306fea23377528dade7bb88ab2c2bd072 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>+++ b/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>@@ -97,7 +97,12 @@ Ref<WebGPUPipelineLayout> WebGPUDevice::createPipelineLayout(WebGPUPipelineLayou
> Ref<WebGPUBindGroup> WebGPUDevice::createBindGroup(WebGPUBindGroupDescriptor&& descriptor) const
> {
>     if (!descriptor.layout || !descriptor.layout->bindGroupLayout()) {
>-        LOG(WebGPU, "WebGPUDevice::createBindGroup: Invalid WebGPUBindGroupLayout!");
>+        LOG(WebGPU, "WebGPUDevice::createBindGroup(): Invalid WebGPUBindGroupLayout!");
>+        return WebGPUBindGroup::create(nullptr);
>+    }
>+
>+    if (descriptor.bindings.size() != descriptor.layout->bindGroupLayout()->bindingsMap().size()) {
>+        LOG(WebGPU, "WebGPUDevice::createBindGroup(): Mismatched number of WebGPUBindGroupLayoutBindings and WebGPUBindGroupBindings!");
>         return WebGPUBindGroup::create(nullptr);
>     }
> 
>@@ -115,11 +120,16 @@ Ref<WebGPUBindGroup> WebGPUDevice::createBindGroup(WebGPUBindGroupDescriptor&& d
>     bindGroupBindings.reserveCapacity(descriptor.bindings.size());
> 
>     for (const auto& binding : descriptor.bindings) {
>+        if (!descriptor.layout->bindGroupLayout()->bindingsMap().contains(binding.binding)) {
>+            LOG(WebGPU, "WebGPUDevice::createBindGroup(): WebGPUBindGroupBinding %lu not found in WebGPUBindGroupLayout!", binding.binding);
>+            return WebGPUBindGroup::create(nullptr);
>+        }
>+
>         auto bindingResource = WTF::visit(bindingResourceVisitor, binding.resource);
>         if (bindingResource)
>             bindGroupBindings.uncheckedAppend(GPUBindGroupBinding { binding.binding, WTFMove(bindingResource.value()) });
>         else {
>-            LOG(WebGPU, "WebGPUDevice::createBindGroup: Invalid WebGPUBindingResource in WebGPUBindGroupBindings!");
>+            LOG(WebGPU, "WebGPUDevice::createBindGroup(): Invalid WebGPUBindingResource for binding %lu in WebGPUBindGroupBindings!", binding.binding);
>             return WebGPUBindGroup::create(nullptr);
>         }
>     }
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h b/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h
>index eb7f29f4e127b02a7f935063000dfce24deb96f6..806138367971de6bad5cb65af19f75510cc33092 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h
>+++ b/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h
>@@ -42,7 +42,7 @@ public:
>     static Ref<GPUBindGroup> create(GPUBindGroupDescriptor&&);
> 
> private:
>-    GPUBindGroup(GPUBindGroupDescriptor&&);
>+    explicit GPUBindGroup(GPUBindGroupDescriptor&&);
> 
>     Ref<GPUBindGroupLayout> m_layout;
>     Vector<GPUBindGroupBinding> m_bindings;
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h b/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>index db2e21950c80a6da9eb7239ec69a5f571fe566c9..f2ad89e70df0f31ed93e6684216501c671d37a3e 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>+++ b/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>@@ -29,6 +29,7 @@
> 
> #include "GPUBindGroupLayoutDescriptor.h"
> 
>+#include <wtf/HashMap.h>
> #include <wtf/RefCounted.h>
> #include <wtf/RefPtr.h>
> #include <wtf/RetainPtr.h>
>@@ -49,9 +50,13 @@ public:
> 
>     static RefPtr<GPUBindGroupLayout> tryCreate(const GPUDevice&, GPUBindGroupLayoutDescriptor&&);
> 
>+    using BindingsMapType = HashMap<unsigned long long, GPUBindGroupLayoutBinding, WTF::IntHash<unsigned long long>, WTF::UnsignedWithZeroKeyHashTraits<unsigned long long>>;
>+    const BindingsMapType& bindingsMap() const { return m_bindingsMap; }
>+
> private:
>-    GPUBindGroupLayout(ArgumentEncoders&&);
>+    GPUBindGroupLayout(BindingsMapType&&, ArgumentEncoders&&);
> 
>+    const BindingsMapType m_bindingsMap;
>     ArgumentEncoders m_argumentEncoders;
> };
> 
>diff --git a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>index 5a19e64178c3587dde499621ea9ee1782df98ef7..f8931ce6629b150c6a9853a1da8ec457b9e75b47 100644
>--- a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>+++ b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>@@ -76,8 +76,14 @@ static RetainPtr<MTLArgumentEncoder> newEncoder(const GPUDevice& device, Argumen
> RefPtr<GPUBindGroupLayout> GPUBindGroupLayout::tryCreate(const GPUDevice& device, GPUBindGroupLayoutDescriptor&& descriptor)
> {
>     ArgumentArray vertexArguments, fragmentArguments, computeArguments;
>+    BindingsMapType bindingsMap;
> 
>     for (const auto& binding : descriptor.bindings) {
>+        if (!bindingsMap.add(binding.binding, binding)) {
>+            LOG(WebGPU, "GPUBindGroupLayout::tryCreate(): Duplicate binding %lu found in WebGPUBindGroupLayoutDescriptor!", binding.binding);
>+            return nullptr;
>+        }
>+
>         RetainPtr<MTLArgumentDescriptor> mtlArgument;
> 
>         BEGIN_BLOCK_OBJC_EXCEPTIONS;
>@@ -85,7 +91,7 @@ RefPtr<GPUBindGroupLayout> GPUBindGroupLayout::tryCreate(const GPUDevice& device
>         END_BLOCK_OBJC_EXCEPTIONS;
> 
>         if (!mtlArgument) {
>-            LOG(WebGPU, "GPUBindGroupLayout::tryCreate(): Unable to create MTLArgumentDescriptor!");
>+            LOG(WebGPU, "GPUBindGroupLayout::tryCreate(): Unable to create MTLArgumentDescriptor for binding %lu!", binding.binding);
>             return nullptr;
>         }
> 
>@@ -115,11 +121,12 @@ RefPtr<GPUBindGroupLayout> GPUBindGroupLayout::tryCreate(const GPUDevice& device
>             return nullptr;
>     }
> 
>-    return adoptRef(new GPUBindGroupLayout(WTFMove(encoders)));
>+    return adoptRef(new GPUBindGroupLayout(WTFMove(bindingsMap), WTFMove(encoders)));
> }
> 
>-GPUBindGroupLayout::GPUBindGroupLayout(ArgumentEncoders&& encoders)
>-    : m_argumentEncoders(WTFMove(encoders))
>+GPUBindGroupLayout::GPUBindGroupLayout(BindingsMapType&& bindingsMap, ArgumentEncoders&& encoders)
>+    : m_bindingsMap(WTFMove(bindingsMap))
>+    , m_argumentEncoders(WTFMove(encoders))
> {
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index fdedb854898e8e0833df987c81175b990d33b23d..322aea3c7d48d8460bd5ad69d7c94fcd8a296dfa 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,15 @@
>+2019-01-14  Justin Fan  <justin_fan@apple.com>
>+
>+        [WebGPU] Map WebGPUBindGroupLayoutBindings from the BindGroupLayoutDescriptor for error checking and later referencing
>+        https://bugs.webkit.org/show_bug.cgi?id=193405
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Small fixes that do not alter behavior.
>+
>+        * webgpu/bind-groups.html:
>+        * webgpu/pipeline-layouts.html:
>+
> 2019-01-09  Justin Fan  <justin_fan@apple.com>
> 
>         [WebGPU] Fix vertex-buffer-triangle-strip test and small update to GPURenderPipeline
>diff --git a/LayoutTests/webgpu/bind-groups.html b/LayoutTests/webgpu/bind-groups.html
>index 18a8ebf6752f4a0f893c5cbdab3eadeb9a5e04d1..6f52f661cc123f1fc5e8c3046adf1c3cc5be596f 100644
>--- a/LayoutTests/webgpu/bind-groups.html
>+++ b/LayoutTests/webgpu/bind-groups.html
>@@ -17,7 +17,7 @@ promise_test(async () => {
>         type: "storageBuffer"
>     };
> 
>-    const bindGroupLayout = device.createBindGroupLayout({ bindings: [bufferLayoutBinding]});
>+    const bindGroupLayout = device.createBindGroupLayout({ bindings: [bufferLayoutBinding] });
> 
>     const buffer = device.createBuffer({ size: 16, usage: WebGPUBufferUsage.STORAGE });
>     const bufferBinding = { buffer: buffer, offset: 0, size: 16 };
>diff --git a/LayoutTests/webgpu/pipeline-layouts.html b/LayoutTests/webgpu/pipeline-layouts.html
>index 2167f6def2dd26d7233622cb41b4c50e46afbf4e..6ba14db54c1a62584851254dcc5e9bf1ecb66260 100644
>--- a/LayoutTests/webgpu/pipeline-layouts.html
>+++ b/LayoutTests/webgpu/pipeline-layouts.html
>@@ -22,7 +22,7 @@ test(() => {
> }, "Create a basic WebGPUBindGroupLayoutDescriptor."); 
> 
> promise_test(async () => {
>-    const device = await window.getBasicDevice();
>+    const device = await getBasicDevice();
>     const bindGroupLayout = device.createBindGroupLayout({ bindings: [createBindGroupLayoutBinding()] });
>     assert_true(bindGroupLayout instanceof WebGPUBindGroupLayout, "createBindGroupLayout returned a WebGPUBindGroupLayout");
>

Actions: View | Formatted Diff | Diff

Attachments on bug 193405: 359068