Attachment 357964 Details for Bug 192990 – Patch

[patch] Patch

bug-192990-20181221120929.patch (text/plain), 17.00 KB, created by Justin Fan on 2018-12-21 12:09:30 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Fan

Created: 2018-12-21 12:09:30 PST

Size: 17.00 KB

patch

obsolete

>Subversion Revision: 239507
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d79908336129502d1f309ed4092b0225cc4f93d3..592c59f69d684ae2607cac27b5e7f9689575f020 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,40 @@
>+2018-12-21  Justin Fan  <justin_fan@apple.com>
>+
>+        [WebGPU] GPUBindGroupLayout refactoring: no HashMap, and failure logging
>+        https://bugs.webkit.org/show_bug.cgi?id=192990
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Refactor away the unnecessary HashMaps when creating MTLArgumentEncoders in GPUBindGroupLayout creation.
>+        Also update GPUBindGroupLayout::create -> tryCreate, in order to better handle Objective-C exceptions.
>+
>+        No new tests; no change in behavior.
>+
>+        * Modules/webgpu/WebGPUBindGroupLayout.cpp:
>+        (WebCore::WebGPUBindGroupLayout::create):
>+        (WebCore::WebGPUBindGroupLayout::WebGPUBindGroupLayout):
>+        * Modules/webgpu/WebGPUBindGroupLayout.h:
>+        (WebCore::WebGPUBindGroupLayout::bindGroupLayout const):
>+        * Modules/webgpu/WebGPUDevice.cpp:
>+        (WebCore::WebGPUDevice::createBindGroupLayout const):
>+        * platform/graphics/gpu/GPUBindGroupLayout.h:
>+        * platform/graphics/gpu/GPUDevice.cpp:
>+        (WebCore::GPUDevice::tryCreateBindGroupLayout const): Renamed from ::create*. Now returning a RefPtr. 
>+        (WebCore::GPUDevice::createBindGroupLayout const): Deleted.
>+        * platform/graphics/gpu/GPUDevice.h:
>+        * platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm:
>+        (WebCore::appendArgumentToArray):
>+        (WebCore::newEncoder):
>+        (WebCore::GPUBindGroupLayout::tryCreate): Renamed from ::create. Now returning a RefPtr.
>+        (WebCore::GPUBindGroupLayout::GPUBindGroupLayout):
>+        (WebCore::appendArgumentToArrayInMap): Deleted.
>+        (WebCore::GPUBindGroupLayout::create): Deleted.
>+
>+        Deleted unneeded GPUBindGroupLayout.cpp:
>+        * Sources.txt:
>+        * WebCore.xcodeproj/project.pbxproj:
>+        * platform/graphics/gpu/GPUBindGroupLayout.cpp: Removed.
>+
> 2018-12-21  Justin Michaud  <justin_michaud@apple.com>
> 
>         Repeated background images with zero size should display the background color
>diff --git a/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.cpp b/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.cpp
>index d987bf33267844ed5075e83e265f6c2b64dfabef..d3d2f879014e563df9153605bb0e305f63fd1752 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.cpp
>+++ b/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.cpp
>@@ -30,12 +30,12 @@
> 
> namespace WebCore {
> 
>-Ref<WebGPUBindGroupLayout> WebGPUBindGroupLayout::create(Ref<GPUBindGroupLayout>&& layout)
>+Ref<WebGPUBindGroupLayout> WebGPUBindGroupLayout::create(RefPtr<GPUBindGroupLayout>&& layout)
> {
>     return adoptRef(*new WebGPUBindGroupLayout(WTFMove(layout)));
> }
> 
>-WebGPUBindGroupLayout::WebGPUBindGroupLayout(Ref<GPUBindGroupLayout>&& layout)
>+WebGPUBindGroupLayout::WebGPUBindGroupLayout(RefPtr<GPUBindGroupLayout>&& layout)
>     : m_bindGroupLayout(WTFMove(layout))
> {
> }
>diff --git a/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.h b/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.h
>index be29203a3ee2458a6f9cb477849e21b48e3791bc..f9c0f83050f6791737c2f53a092200a5fdea6aef 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.h
>+++ b/Source/WebCore/Modules/webgpu/WebGPUBindGroupLayout.h
>@@ -36,14 +36,14 @@ namespace WebCore {
> 
> class WebGPUBindGroupLayout : public RefCounted<WebGPUBindGroupLayout> {
> public:
>-    static Ref<WebGPUBindGroupLayout> create(Ref<GPUBindGroupLayout>&&);
>+    static Ref<WebGPUBindGroupLayout> create(RefPtr<GPUBindGroupLayout>&&);
> 
>-    RefPtr<GPUBindGroupLayout> bindGroupLayout() const { return m_bindGroupLayout.copyRef(); }
>+    RefPtr<GPUBindGroupLayout> bindGroupLayout() const { return m_bindGroupLayout; }
> 
> private:
>-    explicit WebGPUBindGroupLayout(Ref<GPUBindGroupLayout>&&);
>+    explicit WebGPUBindGroupLayout(RefPtr<GPUBindGroupLayout>&&);
> 
>-    Ref<GPUBindGroupLayout> m_bindGroupLayout;
>+    RefPtr<GPUBindGroupLayout> m_bindGroupLayout;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp b/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>index 9415c35fb1b14b53521018d3d7b16063c1f2c33b..73a1aa615369bf1cce3d93b1fa79704677b37687 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>+++ b/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp
>@@ -69,7 +69,7 @@ RefPtr<WebGPUBuffer> WebGPUDevice::createBuffer(WebGPUBufferDescriptor&& descrip
> 
> Ref<WebGPUBindGroupLayout> WebGPUDevice::createBindGroupLayout(WebGPUBindGroupLayoutDescriptor&& descriptor) const
> {
>-    auto layout = m_device->createBindGroupLayout(GPUBindGroupLayoutDescriptor { descriptor.bindings });
>+    auto layout = m_device->tryCreateBindGroupLayout(GPUBindGroupLayoutDescriptor { descriptor.bindings });
>     return WebGPUBindGroupLayout::create(WTFMove(layout));
> }
> 
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index a8c473728aa887863f6744c761cd0ffa98bdda64..4ef700445530fc21b42f789eb5413d31340dcfea 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -1743,7 +1743,6 @@ platform/graphics/filters/SourceAlpha.cpp
> platform/graphics/filters/SourceGraphic.cpp
> platform/graphics/filters/SpotLightSource.cpp
> 
>-platform/graphics/gpu/GPUBindGroupLayout.cpp
> platform/graphics/gpu/GPUDevice.cpp
> platform/graphics/gpu/GPUPipelineLayout.cpp
> platform/graphics/gpu/legacy/GPULegacyBuffer.cpp
>diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>index 1df8d7e1301d7a00c169039d3228e67b0b2d109b..5c5cc1f2d07b6ff2c275ee9a6a136fdbe8583d91 100644
>--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>@@ -13738,7 +13738,6 @@
> 		D01A27AC10C9BFD800026A42 /* SpaceSplitString.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = SpaceSplitString.h; sourceTree = "<group>"; };
> 		D0232B5821CB49B7009483B9 /* GPUBindGroupLayoutMetal.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = GPUBindGroupLayoutMetal.mm; sourceTree = "<group>"; };
> 		D02454D021C4A41C00B73628 /* GPUBindGroupLayout.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GPUBindGroupLayout.h; sourceTree = "<group>"; };
>-		D02454D121C4A41C00B73628 /* GPUBindGroupLayout.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = GPUBindGroupLayout.cpp; sourceTree = "<group>"; };
> 		D02B83ED21C8397A00F85473 /* WebGPUBindGroupLayoutDescriptor.idl */ = {isa = PBXFileReference; lastKnownFileType = text; path = WebGPUBindGroupLayoutDescriptor.idl; sourceTree = "<group>"; };
> 		D02C26912181416D00D818E4 /* WebGPUAdapterDescriptor.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WebGPUAdapterDescriptor.h; sourceTree = "<group>"; };
> 		D02C26922181416D00D818E4 /* WebGPUAdapterDescriptor.idl */ = {isa = PBXFileReference; lastKnownFileType = text; path = WebGPUAdapterDescriptor.idl; sourceTree = "<group>"; };
>@@ -18064,7 +18063,6 @@
> 			children = (
> 				D087CE3721ACA94200BDE174 /* cocoa */,
> 				312FF8CE21A4C33F00EB199D /* legacy */,
>-				D02454D121C4A41C00B73628 /* GPUBindGroupLayout.cpp */,
> 				D02454D021C4A41C00B73628 /* GPUBindGroupLayout.h */,
> 				D0B8BB0121C46E78000C7681 /* GPUBindGroupLayoutBinding.h */,
> 				D083D98421C48050008E8EFF /* GPUBindGroupLayoutDescriptor.h */,
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.cpp b/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.cpp
>deleted file mode 100644
>index 729c062834a03ca83d8197c4a001e0b221e9860c..0000000000000000000000000000000000000000
>--- a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.cpp
>+++ /dev/null
>@@ -1,36 +0,0 @@
>-/*
>- * Copyright (C) 2018 Apple Inc. All rights reserved.
>- *
>- * Redistribution and use in source and binary forms, with or without
>- * modification, are permitted provided that the following conditions
>- * are met:
>- * 1. Redistributions of source code must retain the above copyright
>- *    notice, this list of conditions and the following disclaimer.
>- * 2. Redistributions in binary form must reproduce the above copyright
>- *    notice, this list of conditions and the following disclaimer in the
>- *    documentation and/or other materials provided with the distribution.
>- *
>- * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
>- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
>- * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
>- * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
>- * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>- * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>- * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
>- * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
>- * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
>- * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
>- * THE POSSIBILITY OF SUCH DAMAGE.
>- */
>-
>-#include "config.h"
>-#include "GPUBindGroupLayout.h"
>-
>-#if ENABLE(WEBGPU)
>-
>-namespace WebCore {
>-
>-} // namespace WebCore
>-
>-#endif // ENABLE(WEBGPU)
>-
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h b/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>index 1c0a91590bf163d8679f788d12e5753a4b157c11..db2e21950c80a6da9eb7239ec69a5f571fe566c9 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>+++ b/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h
>@@ -29,11 +29,9 @@
> 
> #include "GPUBindGroupLayoutDescriptor.h"
> 
>-#include <wtf/HashMap.h>
>-#include <wtf/Ref.h>
> #include <wtf/RefCounted.h>
>+#include <wtf/RefPtr.h>
> #include <wtf/RetainPtr.h>
>-#include <wtf/Vector.h>
> 
> OBJC_PROTOCOL(MTLArgumentEncoder);
> 
>@@ -43,14 +41,18 @@ class GPUDevice;
> 
> class GPUBindGroupLayout : public RefCounted<GPUBindGroupLayout> {
> public:
>-    using ArgumentsMap = HashMap<GPUShaderStageFlags, RetainPtr<MTLArgumentEncoder>>;
>+    struct ArgumentEncoders {
>+        RetainPtr<MTLArgumentEncoder> vertex;
>+        RetainPtr<MTLArgumentEncoder> fragment;
>+        RetainPtr<MTLArgumentEncoder> compute;
>+    };
> 
>-    static Ref<GPUBindGroupLayout> create(const GPUDevice&, GPUBindGroupLayoutDescriptor&&);
>+    static RefPtr<GPUBindGroupLayout> tryCreate(const GPUDevice&, GPUBindGroupLayoutDescriptor&&);
> 
> private:
>-    GPUBindGroupLayout(ArgumentsMap&&);
>+    GPUBindGroupLayout(ArgumentEncoders&&);
> 
>-    ArgumentsMap m_argumentsMap;
>+    ArgumentEncoders m_argumentEncoders;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUDevice.cpp b/Source/WebCore/platform/graphics/gpu/GPUDevice.cpp
>index 8097d590c6c544cd2db230c86104b98be22450c8..11329ee85e51b02bd31c81b3fe2a2139432d74f3 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPUDevice.cpp
>+++ b/Source/WebCore/platform/graphics/gpu/GPUDevice.cpp
>@@ -46,9 +46,9 @@ RefPtr<GPUBuffer> GPUDevice::createBuffer(GPUBufferDescriptor&& descriptor) cons
>     return GPUBuffer::create(*this, WTFMove(descriptor));
> }
> 
>-Ref<GPUBindGroupLayout> GPUDevice::createBindGroupLayout(GPUBindGroupLayoutDescriptor&& descriptor) const
>+RefPtr<GPUBindGroupLayout> GPUDevice::tryCreateBindGroupLayout(GPUBindGroupLayoutDescriptor&& descriptor) const
> {
>-    return GPUBindGroupLayout::create(*this, WTFMove(descriptor));
>+    return GPUBindGroupLayout::tryCreate(*this, WTFMove(descriptor));
> }
> 
> Ref<GPUPipelineLayout> GPUDevice::createPipelineLayout(GPUPipelineLayoutDescriptor&& descriptor) const
>diff --git a/Source/WebCore/platform/graphics/gpu/GPUDevice.h b/Source/WebCore/platform/graphics/gpu/GPUDevice.h
>index 5c9d309a321c613a78502db9c561ebac50a78ba7..376d0fe62fbd33fbf7e2e16c064bdb22a04ff497 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPUDevice.h
>+++ b/Source/WebCore/platform/graphics/gpu/GPUDevice.h
>@@ -59,7 +59,7 @@ public:
> 
>     RefPtr<GPUBuffer> createBuffer(GPUBufferDescriptor&&) const;
> 
>-    Ref<GPUBindGroupLayout> createBindGroupLayout(GPUBindGroupLayoutDescriptor&&) const;
>+    RefPtr<GPUBindGroupLayout> tryCreateBindGroupLayout(GPUBindGroupLayoutDescriptor&&) const;
>     Ref<GPUPipelineLayout> createPipelineLayout(GPUPipelineLayoutDescriptor&&) const;
> 
>     RefPtr<GPUShaderModule> createShaderModule(GPUShaderModuleDescriptor&&) const;
>diff --git a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>index 4fc34773c1a1c982b015beefb7a15f758c64aa3f..04640ca7754e079d023bb21bedd13bb6182b4fb6 100644
>--- a/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>+++ b/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm
>@@ -29,7 +29,9 @@
> #if ENABLE(WEBGPU)
> 
> #import "GPUDevice.h"
>+#import "Logging.h"
> 
>+#import <Foundation/Foundation.h>
> #import <Metal/Metal.h>
> #import <wtf/BlockObjCExceptions.h>
> 
>@@ -48,57 +50,74 @@ static MTLDataType MTLDataTypeForBindingType(GPUBindGroupLayoutBinding::BindingT
>     }
> }
> 
>-using ArgumentArraysMap = HashMap<GPUShaderStageFlags, RetainPtr<NSMutableArray<MTLArgumentDescriptor *>>>;
>-static void appendArgumentToArrayInMap(ArgumentArraysMap& map, GPUShaderStageFlags stage, RetainPtr<MTLArgumentDescriptor> argument)
>+using ArgumentArray = RetainPtr<NSMutableArray<MTLArgumentDescriptor *>>;
>+static void appendArgumentToArray(ArgumentArray array, RetainPtr<MTLArgumentDescriptor> argument)
> {
>-    auto iterator = map.find(stage);
>-    if (iterator == map.end()) {
>-        BEGIN_BLOCK_OBJC_EXCEPTIONS;
>-        map.set(stage, [[NSMutableArray alloc] initWithObjects:argument.get(), nil]);
>-        END_BLOCK_OBJC_EXCEPTIONS;
>-    } else
>-        [iterator->value addObject:argument.get()];
>+    if (!array)
>+        array = [[NSMutableArray alloc] initWithObjects:argument.get(), nil];
>+    else
>+        [array addObject:argument.get()];
> }
> 
>-Ref<GPUBindGroupLayout> GPUBindGroupLayout::create(const GPUDevice& device, GPUBindGroupLayoutDescriptor&& descriptor)
>+static RetainPtr<MTLArgumentEncoder> newEncoder(const GPUDevice& device, ArgumentArray array)
>+{
>+    RetainPtr<MTLArgumentEncoder> encoder;
>+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
>+    encoder = adoptNS([device.platformDevice() newArgumentEncoderWithArguments:array.get()]);
>+    END_BLOCK_OBJC_EXCEPTIONS;
>+    if (!encoder)
>+        LOG(WebGPU, "GPUBindGroupLayout::tryCreate(): Unable to create MTLArgumentEncoder!");
>+
>+    return encoder;
>+};
>+
>+RefPtr<GPUBindGroupLayout> GPUBindGroupLayout::tryCreate(const GPUDevice& device, GPUBindGroupLayoutDescriptor&& descriptor)
> {
>-    ArgumentArraysMap argumentArraysMap;
>+    ArgumentArray vertexArguments, fragmentArguments, computeArguments;
> 
>     for (const auto& binding : descriptor.bindings) {
>-
>         RetainPtr<MTLArgumentDescriptor> mtlArgument;
> 
>         BEGIN_BLOCK_OBJC_EXCEPTIONS;
>         mtlArgument = adoptNS([MTLArgumentDescriptor new]);
>         END_BLOCK_OBJC_EXCEPTIONS;
> 
>+        if (!mtlArgument) {
>+            LOG(WebGPU, "GPUBindGroupLayout::tryCreate(): Unable to create MTLArgumentDescriptor!");
>+            return nullptr;
>+        }
>+
>         mtlArgument.get().dataType = MTLDataTypeForBindingType(binding.type);
>         mtlArgument.get().index = binding.binding;
> 
>         if (binding.visibility & GPUShaderStageBit::VERTEX)
>-            appendArgumentToArrayInMap(argumentArraysMap, GPUShaderStageBit::VERTEX, mtlArgument);
>+            appendArgumentToArray(vertexArguments, mtlArgument);
>         if (binding.visibility & GPUShaderStageBit::FRAGMENT)
>-            appendArgumentToArrayInMap(argumentArraysMap, GPUShaderStageBit::FRAGMENT, mtlArgument);
>+            appendArgumentToArray(fragmentArguments, mtlArgument);
>         if (binding.visibility & GPUShaderStageBit::COMPUTE)
>-            appendArgumentToArrayInMap(argumentArraysMap, GPUShaderStageBit::COMPUTE, mtlArgument);
>+            appendArgumentToArray(computeArguments, mtlArgument);
>     }
> 
>-    ArgumentsMap argumentsMap;
>-
>-    BEGIN_BLOCK_OBJC_EXCEPTIONS;
>+    ArgumentEncoders encoders;
> 
>-    for (const auto& argumentArrayPair : argumentArraysMap) {
>-        auto mtlArgumentEncoder = adoptNS([device.platformDevice() newArgumentEncoderWithArguments:argumentArrayPair.value.get()]);
>-        argumentsMap.set(argumentArrayPair.key, mtlArgumentEncoder);
>+    if (vertexArguments) {
>+        if (!(encoders.vertex = newEncoder(device, vertexArguments)))
>+            return nullptr;
>+    }
>+    if (fragmentArguments) {
>+        if (!(encoders.fragment = newEncoder(device, fragmentArguments)))
>+            return nullptr;
>+    }
>+    if (computeArguments) {
>+        if (!(encoders.compute = newEncoder(device, computeArguments)))
>+            return nullptr;
>     }
> 
>-    END_BLOCK_OBJC_EXCEPTIONS;
>-
>-    return adoptRef(*new GPUBindGroupLayout(WTFMove(argumentsMap)));
>+    return adoptRef(new GPUBindGroupLayout(WTFMove(encoders)));
> }
> 
>-GPUBindGroupLayout::GPUBindGroupLayout(ArgumentsMap&& map)
>-    : m_argumentsMap(map)
>+GPUBindGroupLayout::GPUBindGroupLayout(ArgumentEncoders&& encoders)
>+    : m_argumentEncoders(WTFMove(encoders))
> {
> }
>

Actions: View | Formatted Diff | Diff

Attachments on bug 192990: 357964 | 357965