Attachment 360811 Details for Bug 194125 – Patch

[patch] Patch

bug-194125-20190131171824.patch (text/plain), 28.77 KB, created by Justin Fan on 2019-01-31 17:18:24 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Justin Fan

Created: 2019-01-31 17:18:24 PST

Size: 28.77 KB

patch

obsolete

>Subversion Revision: 240822
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index fa1ebfb3b6e44b690211998a1c3112ab88a1cc53..0ddfbd2cc4ad01f67aa9a7ba9bba4b6fd5a5bb24 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,35 @@
>+2019-01-31  Justin Fan  <justin_fan@apple.com>
>+
>+        [Web GPU] Fix GPURenderPassEncoder::setVertexBuffers and allow overlapping indices with GPUBindGroups
>+        https://bugs.webkit.org/show_bug.cgi?id=194125
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        GPURenderPassEncoder::setVertexBuffers is now actually written to set all buffers provided. In addition,
>+        shift vertex input buffer indices so that any resource bindings can bind vertex buffer resources to the same indices. 
>+
>+        Existing tests cover setVertexBuffers. Updated buffer-resource-triangles to assign bind groups and vertex buffers to the same index.
>+
>+        * Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.cpp: Added.
>+        (WebCore::WHLSL::Metal::calculateVertexBufferIndex): Simple shifting function for vertex input buffer indices.
>+        * Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.h: Added.
>+        * Modules/webgpu/WebGPUProgrammablePassEncoder.cpp:
>+        (WebCore::WebGPUProgrammablePassEncoder::setBindGroup const): Limit maximum bind group indices to 0 to 3.
>+        * Modules/webgpu/WebGPURenderPassEncoder.cpp: 
>+        (WebCore::WebGPURenderPassEncoder::setVertexBuffers): Limit vertex input indices to 0 to 15.
>+        * Modules/webgpu/WebGPURenderPassEncoder.h: Move IDL/bindings bug note to IDL file.
>+        * Modules/webgpu/WebGPURenderPassEncoder.idl: Ditto.
>+        * platform/graphics/gpu/GPULimits.h: Added. Home for Web GPU limits constants shared between files.
>+        * platform/graphics/gpu/GPURenderPassEncoder.h: Change IDL/bindings bug workaround to unsigned long long to prevent narrowing compared to spec.
>+        * platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm:
>+        (WebCore::GPURenderPassEncoder::setVertexBuffers): Now properly calls Metal's setVertexBuffers. 
>+        * platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:
>+        (WebCore::setInputStateForPipelineDescriptor): Fix validation checks for vertex attribute numbers and vertex buffer indices. 
>+
>+        Add symbols to project:
>+        * Sources.txt:
>+        * WebCore.xcodeproj/project.pbxproj:
>+
> 2019-01-31  Jer Noble  <jer.noble@apple.com>
> 
>         [Mac] Requesting PiP from two different WebViews gets PiP window "stuck"
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.cpp b/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.cpp
>new file mode 100644
>index 0000000000000000000000000000000000000000..e26831e552dfdda194305e9400bafe89abbcb852
>--- /dev/null
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.cpp
>@@ -0,0 +1,49 @@
>+/*
>+ * Copyright (C) 2019 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 "WHLSLVertexBufferIndexCalculator.h"
>+
>+#if ENABLE(WEBGPU)
>+
>+namespace WebCore {
>+
>+namespace WHLSL {
>+
>+namespace Metal {
>+
>+unsigned long calculateVertexBufferIndex(unsigned long index)
>+{
>+    // Reserve the first few MTLBuffer slots for argument buffers for GPUBindGroups.
>+    return index + 8;
>+}
>+
>+} // namespace Metal
>+
>+} // namespace WHLSL
>+
>+} // namespace WebCore
>+
>+#endif // ENABLE(WEBGPU)
>diff --git a/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.h b/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..bb55b89f4a301992ecdba6aec914ab16ca350399
>--- /dev/null
>+++ b/Source/WebCore/Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.h
>@@ -0,0 +1,44 @@
>+/*
>+ * Copyright (C) 2019 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.
>+ */
>+
>+#pragma once
>+
>+#if ENABLE(WEBGPU)
>+
>+namespace WebCore {
>+
>+namespace WHLSL {
>+
>+namespace Metal {
>+
>+unsigned long calculateVertexBufferIndex(unsigned long);
>+
>+} // namespace Metal
>+
>+} // namespace WHLSL
>+
>+} // namespace WebCore
>+
>+#endif // ENABLE(WEBGPU)
>diff --git a/Source/WebCore/Modules/webgpu/WebGPUProgrammablePassEncoder.cpp b/Source/WebCore/Modules/webgpu/WebGPUProgrammablePassEncoder.cpp
>index f6f5ea6d334657719cab51b78fd3e001f82ac336..c391520b63d1688cc334374e0c953c085fec1b62 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPUProgrammablePassEncoder.cpp
>+++ b/Source/WebCore/Modules/webgpu/WebGPUProgrammablePassEncoder.cpp
>@@ -47,6 +47,11 @@ Ref<WebGPUCommandBuffer> WebGPUProgrammablePassEncoder::endPass()
> 
> void WebGPUProgrammablePassEncoder::setBindGroup(unsigned long index, const WebGPUBindGroup& bindGroup) const
> {
>+    // Maximum number of bind groups supported in Web GPU.
>+    if (index >= 4) {
>+        LOG(WebGPU, "WebGPUProgrammablePassEncoder::setBindGroup(): Invalid index!");
>+        return;
>+    }
>     if (!bindGroup.bindGroup()) {
>         LOG(WebGPU, "WebGPUProgrammablePassEncoder::setBindGroup(): Invalid WebGPUBindGroup!");
>         return;
>diff --git a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp
>index daf1b08227d4656039c372d3531c0ff98647b4d1..03fb31bbc39cbb00f14b0c8a696df5fa6a3d3790 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp
>+++ b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.cpp
>@@ -28,6 +28,7 @@
> 
> #if ENABLE(WEBGPU)
> 
>+#include "GPULimits.h"
> #include "GPUProgrammablePassEncoder.h"
> #include "GPURenderPassEncoder.h"
> #include "Logging.h"
>@@ -46,18 +47,22 @@ WebGPURenderPassEncoder::WebGPURenderPassEncoder(Ref<WebGPUCommandBuffer>&& crea
> {
> }
> 
>-void WebGPURenderPassEncoder::setVertexBuffers(unsigned long startSlot, Vector<RefPtr<WebGPUBuffer>>&& buffers, Vector<unsigned>&& offsets)
>+void WebGPURenderPassEncoder::setVertexBuffers(unsigned long startSlot, Vector<RefPtr<WebGPUBuffer>>&& buffers, Vector<unsigned long long>&& offsets)
> {
>     if (buffers.isEmpty() || buffers.size() != offsets.size()) {
>         LOG(WebGPU, "WebGPURenderPassEncoder::setVertexBuffers: Invalid number of buffers or offsets!");
>         return;
>     }
> 
>+    if (startSlot + buffers.size() > maxVertexBuffers) {
>+        LOG(WebGPU, "WebGPURenderPassEncoder::setVertexBuffers: Invalid startSlot %lu for %lu buffers!", startSlot, buffers.size());
>+        return;
>+    }
>+
>     auto gpuBuffers = buffers.map([] (const auto& buffer) -> Ref<const GPUBuffer> {
>         return buffer->buffer();
>     });
> 
>-    // FIXME: Use startSlot properly.
>     m_passEncoder->setVertexBuffers(startSlot, WTFMove(gpuBuffers), WTFMove(offsets));
> }
> 
>diff --git a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.h b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.h
>index 2b9a72302dc1eee771bce9d1c056e94e79192153..d0de0d7df4c55bd514240d83740b88b5a34a9320 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.h
>+++ b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.h
>@@ -42,8 +42,7 @@ class WebGPURenderPassEncoder final : public WebGPUProgrammablePassEncoder {
> public:
>     static Ref<WebGPURenderPassEncoder> create(Ref<WebGPUCommandBuffer>&&, Ref<GPURenderPassEncoder>&&);
> 
>-    // FIXME: Last argument should be Vector<unsigned long>. Why is the generated code incorrectly assuming the IDL wants a sequence<unsigned int>?
>-    void setVertexBuffers(unsigned long, Vector<RefPtr<WebGPUBuffer>>&&, Vector<unsigned>&&);
>+    void setVertexBuffers(unsigned long, Vector<RefPtr<WebGPUBuffer>>&&, Vector<unsigned long long>&&);
>     void draw(unsigned long vertexCount, unsigned long instanceCount, unsigned long firstVertex, unsigned long firstInstance);
> 
> private:
>diff --git a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.idl b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.idl
>index d162ec528edbe1d6805446bea94faba62091e988..91aceb1373f6fcc0d91ad6bbed15e3efedd2cd50 100644
>--- a/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.idl
>+++ b/Source/WebCore/Modules/webgpu/WebGPURenderPassEncoder.idl
>@@ -31,7 +31,8 @@ typedef unsigned long u32;
>     EnabledAtRuntime=WebGPU,
>     JSGenerateToJSObject
> ] interface WebGPURenderPassEncoder : WebGPUProgrammablePassEncoder {
>-    void setVertexBuffers(u32 startSlot, sequence<WebGPUBuffer> buffers, sequence<u32> offsets);
>+    // FIXME: (<rdar://problem/47717832>) Last argument should be sequence<unsigned long>, but bindings generates Vector<unsigned int>.
>+    void setVertexBuffers(u32 startSlot, sequence<WebGPUBuffer> buffers, sequence<unsigned long long> offsets);
> 
>     void draw(u32 vertexCount, u32 instanceCount, u32 firstVertex, u32 firstInstance);
> 
>diff --git a/Source/WebCore/Sources.txt b/Source/WebCore/Sources.txt
>index 378c0653b8090cf01c3b0dadca3959130e7f8584..3f4296d5242f423e590a386d02b2b929a8d8e6a1 100644
>--- a/Source/WebCore/Sources.txt
>+++ b/Source/WebCore/Sources.txt
>@@ -332,6 +332,7 @@ Modules/webgpu/WHLSL/Metal/WHLSLMetalCodeGenerator.cpp
> Modules/webgpu/WHLSL/Metal/WHLSLNativeFunctionWriter.cpp
> Modules/webgpu/WHLSL/Metal/WHLSLEntryPointScaffolding.cpp
> Modules/webgpu/WHLSL/Metal/WHLSLNativeTypeWriter.cpp
>+Modules/webgpu/WHLSL/Metal/WHLSLVertexBufferIndexCalculator.cpp
> Modules/webgpu/WHLSL/WHLSLFunctionStageChecker.cpp
> Modules/webgpu/WHLSL/AST/WHLSLTypeArgument.cpp
> Modules/webgpu/WHLSL/AST/WHLSLBuiltInSemantic.cpp
>diff --git a/Source/WebCore/WebCore.xcodeproj/project.pbxproj b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>index 88cf902a69b039b06f04eb09634184474308b2d0..d11367d3cc30035b60eccf4b52780437f213a8c7 100644
>--- a/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>+++ b/Source/WebCore/WebCore.xcodeproj/project.pbxproj
>@@ -14148,6 +14148,9 @@
> 		D0EACFAE219E30FD000FA75C /* WebGPUTextureFormatEnum.idl */ = {isa = PBXFileReference; lastKnownFileType = text; path = WebGPUTextureFormatEnum.idl; sourceTree = "<group>"; };
> 		D0EDA772143E303C0028E383 /* CachedRawResource.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = CachedRawResource.cpp; sourceTree = "<group>"; };
> 		D0EDA773143E303C0028E383 /* CachedRawResource.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = CachedRawResource.h; sourceTree = "<group>"; };
>+		D0F75598220391C300118058 /* WHLSLVertexBufferIndexCalculator.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = WHLSLVertexBufferIndexCalculator.h; sourceTree = "<group>"; };
>+		D0F75599220391C300118058 /* WHLSLVertexBufferIndexCalculator.cpp */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.cpp; path = WHLSLVertexBufferIndexCalculator.cpp; sourceTree = "<group>"; };
>+		D0F7559F2203BA1400118058 /* GPULimits.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = GPULimits.h; sourceTree = "<group>"; };
> 		D0FF2A5B11F8C45A007E74E0 /* PingLoader.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = PingLoader.cpp; sourceTree = "<group>"; };
> 		D0FF2A5C11F8C45A007E74E0 /* PingLoader.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = PingLoader.h; sourceTree = "<group>"; };
> 		D2CDB5ED638F43AF86F07AA2 /* JSErrorEventCustom.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = JSErrorEventCustom.cpp; sourceTree = "<group>"; };
>@@ -17210,6 +17213,8 @@
> 				1CECB3C721F59C8700F44542 /* WHLSLNativeTypeWriter.h */,
> 				1CECB3B121F2B98500F44542 /* WHLSLTypeNamer.cpp */,
> 				1CECB3B021F2B98500F44542 /* WHLSLTypeNamer.h */,
>+				D0F75599220391C300118058 /* WHLSLVertexBufferIndexCalculator.cpp */,
>+				D0F75598220391C300118058 /* WHLSLVertexBufferIndexCalculator.h */,
> 			);
> 			path = Metal;
> 			sourceTree = "<group>";
>@@ -18452,6 +18457,7 @@
> 				312FF8BF21A4C2F100EB199D /* GPUDevice.cpp */,
> 				312FF8BE21A4C2F100EB199D /* GPUDevice.h */,
> 				D0D8649921BA1B1F003C983C /* GPUInputStateDescriptor.h */,
>+				D0F7559F2203BA1400118058 /* GPULimits.h */,
> 				312FF8C421A4C2F400EB199D /* GPUPipelineDescriptorBase.h */,
> 				D003288721C9A4E500622AA6 /* GPUPipelineLayout.cpp */,
> 				D003288621C9A4E500622AA6 /* GPUPipelineLayout.h */,
>diff --git a/Source/WebCore/platform/graphics/gpu/GPULimits.h b/Source/WebCore/platform/graphics/gpu/GPULimits.h
>new file mode 100644
>index 0000000000000000000000000000000000000000..5dd7bfcb126d3d7b55287c17d2bc2cf7f2c164dd
>--- /dev/null
>+++ b/Source/WebCore/platform/graphics/gpu/GPULimits.h
>@@ -0,0 +1,36 @@
>+/*
>+ * Copyright (C) 2019 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.
>+ */
>+
>+#pragma once
>+
>+#if ENABLE(WEBGPU)
>+
>+namespace WebCore {
>+
>+const unsigned long maxVertexBuffers = 16;
>+
>+} // namespace WebCore
>+
>+#endif // ENABLE(WEBGPU)
>diff --git a/Source/WebCore/platform/graphics/gpu/GPURenderPassEncoder.h b/Source/WebCore/platform/graphics/gpu/GPURenderPassEncoder.h
>index 1edbb76e036040c983d740fde18d85b0015412c2..feb5c37b8855e7b036e33883ba35aac46c438438 100644
>--- a/Source/WebCore/platform/graphics/gpu/GPURenderPassEncoder.h
>+++ b/Source/WebCore/platform/graphics/gpu/GPURenderPassEncoder.h
>@@ -53,7 +53,7 @@ public:
> 
>     void setPipeline(Ref<GPURenderPipeline>&&) final;
> 
>-    void setVertexBuffers(unsigned long, Vector<Ref<const GPUBuffer>>&&, Vector<unsigned>&&);
>+    void setVertexBuffers(unsigned long, Vector<Ref<const GPUBuffer>>&&, Vector<unsigned long long>&&);
>     void draw(unsigned long vertexCount, unsigned long instanceCount, unsigned long firstVertex, unsigned long firstInstance);
> 
> private:
>diff --git a/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm b/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm
>index aec40e479d71a8c8082c609b6d231c516c28fdb9..5a8c8d2b3ee0d05f08dad6195d18275856218e4d 100644
>--- a/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm
>+++ b/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPassEncoderMetal.mm
>@@ -33,7 +33,8 @@
> #import "GPURenderPassDescriptor.h"
> #import "GPURenderPipeline.h"
> #import "Logging.h"
>-
>+#import "WHLSLVertexBufferIndexCalculator.h"
>+#import <Foundation/Foundation.h>
> #import <Metal/Metal.h>
> #import <wtf/BlockObjCExceptions.h>
> 
>@@ -92,12 +93,21 @@ void GPURenderPassEncoder::setPipeline(Ref<GPURenderPipeline>&& pipeline)
>     m_pipeline = WTFMove(pipeline);
> }
> 
>-void GPURenderPassEncoder::setVertexBuffers(unsigned long index, Vector<Ref<const GPUBuffer>>&& buffers, Vector<unsigned>&& offsets) 
>+void GPURenderPassEncoder::setVertexBuffers(unsigned long index, Vector<Ref<const GPUBuffer>>&& buffers, Vector<unsigned long long>&& offsets)
> {
>     ASSERT(buffers.size() && offsets.size() == buffers.size());
>-    // FIXME: Only worry about the first buffer for now, and treat startSlot as the index.
>-    // FIXME: Replace with MTLRenderPassEncoder::setVertexBuffers.
>-    [m_platformRenderPassEncoder setVertexBuffer:buffers[0]->platformBuffer() offset:offsets[0] atIndex:index];
>+
>+    BEGIN_BLOCK_OBJC_EXCEPTIONS;
>+
>+    auto mtlBuffers = buffers.map([] (const auto& buffer) {
>+        return buffer->platformBuffer();
>+    });
>+
>+    auto indexRanges = NSMakeRange(WHLSL::Metal::calculateVertexBufferIndex(index), buffers.size());
>+
>+    [m_platformRenderPassEncoder setVertexBuffers:mtlBuffers.data() offsets:(const NSUInteger *)offsets.data() withRange:indexRanges];
>+
>+    END_BLOCK_OBJC_EXCEPTIONS;
> }
> 
> static MTLPrimitiveType primitiveTypeForGPUPrimitiveTopology(PrimitiveTopology type)
>diff --git a/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm b/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm
>index 58860eba5c8bb70d0ecf276815e2b826cf5b668d..b77ee9a52bcec2838db7edfbf033a8041986b7ab 100644
>--- a/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm
>+++ b/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm
>@@ -28,8 +28,9 @@
> 
> #if ENABLE(WEBGPU)
> 
>+#import "GPULimits.h"
> #import "Logging.h"
>-
>+#import "WHLSLVertexBufferIndexCalculator.h"
> #import <Metal/Metal.h>
> #import <wtf/BlockObjCExceptions.h>
> #import <wtf/Optional.h>
>@@ -194,12 +195,11 @@ static bool setInputStateForPipelineDescriptor(const char* const functionName, M
>     for (size_t i = 0; i < attributes.size(); ++i) {
>         auto location = attributes[i].shaderLocation;
>         // Maximum number of vertex attributes to be supported by Web GPU.
>-        if (location > 16) {
>+        if (location >= 16) {
>             LOG(WebGPU, "%s: Invalid shaderLocation %lu for vertex attribute!", functionName, location);
>             return false;
>         }
>-        // Maximum number of vertex buffers supported.
>-        if (attributes[i].inputSlot > 16) {
>+        if (attributes[i].inputSlot >= maxVertexBuffers) {
>             LOG(WebGPU, "%s: Invalid inputSlot %lu for vertex attribute %lu!", functionName, attributes[i].inputSlot, location);
>             return false;
>         }
>@@ -212,7 +212,7 @@ static bool setInputStateForPipelineDescriptor(const char* const functionName, M
>         auto mtlAttributeDesc = retainPtr([attributeArray objectAtIndexedSubscript:location]);
>         mtlAttributeDesc.get().format = *mtlFormat;
>         mtlAttributeDesc.get().offset = attributes[i].offset; // FIXME: After adding more vertex formats, ensure offset < buffer's stride + format's data size.
>-        mtlAttributeDesc.get().bufferIndex = attributes[i].inputSlot;
>+        mtlAttributeDesc.get().bufferIndex = WHLSL::Metal::calculateVertexBufferIndex(attributes[i].inputSlot);
>         [mtlVertexDescriptor.get().attributes setObject:mtlAttributeDesc.get() atIndexedSubscript:location];
>     }
> 
>@@ -222,7 +222,7 @@ static bool setInputStateForPipelineDescriptor(const char* const functionName, M
> 
>     for (size_t j = 0; j < inputs.size(); ++j) {
>         auto slot = inputs[j].inputSlot;
>-        if (inputs[j].inputSlot > 16) {
>+        if (slot >= maxVertexBuffers) {
>             LOG(WebGPU, "%s: Invalid inputSlot %d for vertex buffer!", functionName, slot);
>             return false;
>         }
>@@ -233,10 +233,11 @@ static bool setInputStateForPipelineDescriptor(const char* const functionName, M
>             return false;
>         }
> 
>-        auto mtlLayoutDesc = retainPtr([layoutArray objectAtIndexedSubscript:slot]);
>+        auto convertedSlot = WHLSL::Metal::calculateVertexBufferIndex(slot);
>+        auto mtlLayoutDesc = retainPtr([layoutArray objectAtIndexedSubscript:convertedSlot]);
>         mtlLayoutDesc.get().stepFunction = *mtlStepFunction;
>         mtlLayoutDesc.get().stride = inputs[j].stride;
>-        [mtlVertexDescriptor.get().layouts setObject:mtlLayoutDesc.get() atIndexedSubscript:slot];
>+        [mtlVertexDescriptor.get().layouts setObject:mtlLayoutDesc.get() atIndexedSubscript:convertedSlot];
>     }
> 
>     mtlDescriptor.vertexDescriptor = mtlVertexDescriptor.get();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 01a678bea7cf604ec1c08f193006515e96a203bf..ee33c1665ed54a670bd6dd91d3bcdcb569872457 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-01-31  Justin Fan  <justin_fan@apple.com>
>+
>+        [WebGPU] Fix GPURenderPassEncoder::setVertexBuffers and allow overlapping indices with GPUBindGroups
>+        https://bugs.webkit.org/show_bug.cgi?id=194125
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Updated buffer-resource-triangles to assign bind groups and vertex buffers to the same index.
>+
>+        * webgpu/buffer-resource-triangles.html:
>+
> 2019-01-31  Chris Dumez  <cdumez@apple.com>
> 
>         [ MacOS ] Layout Test performance-api/performance-observer-callback-after-gc.html is flaky
>diff --git a/LayoutTests/webgpu/buffer-resource-triangles.html b/LayoutTests/webgpu/buffer-resource-triangles.html
>index d7115ba6cba2c7850a3be7b9629dd8381c4bc658..996579ef16c19b2dfb19ffb7cb41f2955a529ac6 100644
>--- a/LayoutTests/webgpu/buffer-resource-triangles.html
>+++ b/LayoutTests/webgpu/buffer-resource-triangles.html
>@@ -12,6 +12,10 @@ const shaderCode = `
>     
> using namespace metal;
> 
>+struct VertexInput {
>+    float4 position [[attribute(0)]];
>+};
>+
> struct Vertex {
>     float4 position [[position]];
> };
>@@ -23,18 +27,26 @@ struct VertexArguments {
> };
> 
> vertex Vertex vertex_main(
>+    VertexInput input [[stage_in]],
>     const device VertexArguments& args0 [[buffer(0)]],
>     const device VertexArguments& args1 [[buffer(1)]],
>     uint vid [[vertex_id]])
> {
>     switch (vid)
>     {
>-        case 0: return args0.v0[0];
>-        case 1: return args0.v1[0];
>-        case 2: return args0.v2[0];
>-        case 3: return args1.v0[0];
>-        case 4: return args1.v1[0];
>-        default: return args1.v2[0];
>+        case 0:
>+        case 1:
>+        case 2: {
>+            Vertex out;
>+            out.position = input.position;
>+            return out;
>+        }
>+        case 3: return *args0.v0;
>+        case 4: return *args0.v1;
>+        case 5: return *args0.v2;
>+        case 6: return *args1.v0;
>+        case 7: return *args1.v1;
>+        default: return *args1.v2;
>     }
> }
> 
>@@ -65,11 +77,27 @@ function createUniformBufferBindGroupLayout(bindNum, stage = WebGPUShaderStageBi
>     };
> }
> 
>-const bufferSize = 4 * 4;
>+const vertexSize = 4 * 4;
>+const verticesBufferSize = vertexSize * 3;
> 
> // FIXME: Keep up to date with buffer upload decisions.
>+function createVerticesBuffer(device) {
>+    const buffer = device.createBuffer({ size:verticesBufferSize, usage: WebGPUBufferUsage.VERTEX });
>+
>+    const vertices = [
>+        0, 1, 0, 1,
>+        -1, -1, 0, 1,
>+        1, -1, 0, 1
>+    ];
>+
>+    const mappedArray = new Float32Array(buffer.mapping);
>+    mappedArray.set(vertices);
>+
>+    return buffer;
>+}
>+
> function createFloat4Buffer(device, a, b) {
>-    const buffer = device.createBuffer({ size: bufferSize, usage: WebGPUBufferUsage.UNIFORM });
>+    const buffer = device.createBuffer({ size: vertexSize, usage: WebGPUBufferUsage.UNIFORM });
>     
>     const arrayBuffer = buffer.mapping;
>     const floatArray = new Float32Array(arrayBuffer);
>@@ -86,7 +114,7 @@ function createBufferBinding(buffer) {
>     return {
>         buffer: buffer,
>         offset: 0,
>-        size: bufferSize
>+        size: vertexSize
>     };
> }
> 
>@@ -98,6 +126,8 @@ async function test() {
>     const shaderModule = device.createShaderModule({ code: shaderCode });
> 
>     // Create vertex data WebGPUBuffers.
>+    const verticesBuffer = createVerticesBuffer(device);
>+
>     const upperLeft = createFloat4Buffer(device, -1, 1);
>     const upperMiddle = createFloat4Buffer(device, 0, 1);
>     const upperRight = createFloat4Buffer(device, 1, 1);
>@@ -107,6 +137,22 @@ async function test() {
>     // Color data buffer.
>     const green = createFloat4Buffer(device, 0, 1);
> 
>+    // Create vertex input state.
>+    const inputState = {
>+        indexFormat: WebGPUIndexFormat.UINT32,
>+        attributes: [{
>+            shaderLocation: 0,
>+            inputSlot: 0,
>+            offset: 0,
>+            format: WebGPUVertexFormat.FLOAT_R32_G32_B32_A32
>+        }],
>+        inputs: [{
>+            inputSlot: 0,
>+            stride: vertexSize,
>+            stepMode: WebGPUInputStepMode.VERTEX
>+        }]
>+    };
>+
>     // Create buffer WebGPUBindGroupLayoutBindings.
>     const layoutUL = createUniformBufferBindGroupLayout(bindingNums.UL);
>     const layoutUM = createUniformBufferBindGroupLayout(bindingNums.UM);
>@@ -118,11 +164,10 @@ async function test() {
>     // WebGPUBindGroupLayouts
>     const leftTriangleBGLayout = device.createBindGroupLayout({ bindings: [layoutUL, layoutUM, layoutLL, layoutG] });
>     const rightTriangleBGLayout = device.createBindGroupLayout({ bindings: [layoutUR, layoutUM, layoutLR] });
>-    const middleTriangleBGLayout = device.createBindGroupLayout({ bindings: [layoutUM, layoutLL, layoutLR, layoutG] });
> 
>     // WebGPUPipelineLayout and WebGPURenderPipeline
>-    const pipelineLayout = device.createPipelineLayout({ bindGroupLayouts: [leftTriangleBGLayout, middleTriangleBGLayout, rightTriangleBGLayout] });
>-    const pipeline = createBasicPipeline(shaderModule, device, pipelineLayout, null, "triangleList");
>+    const pipelineLayout = device.createPipelineLayout({ bindGroupLayouts: [leftTriangleBGLayout, rightTriangleBGLayout] });
>+    const pipeline = createBasicPipeline(shaderModule, device, pipelineLayout, inputState, "triangleList");
> 
>     // WebGPUBufferBindings
>     const bindingUL = createBufferBinding(upperLeft);
>@@ -149,22 +194,17 @@ async function test() {
>         layout: rightTriangleBGLayout,
>         bindings: [bgBindingUR, bgBindingUM, bgBindingLR]
>     });
>-    const middleTriangleBG = device.createBindGroup({
>-        layout: middleTriangleBGLayout,
>-        bindings: [bgBindingUM, bgBindingLL, bgBindingLR, bgBindingG]
>-    });
> 
>     const commandBuffer = device.createCommandBuffer();
>     const passEncoder = beginBasicRenderPass(context, commandBuffer);
>     passEncoder.setPipeline(pipeline);
> 
>-    // Draw upper triangles.
>+    // Vertex data for upper triangles.
>     passEncoder.setBindGroup(0, leftTriangleBG);
>     passEncoder.setBindGroup(1, rightTriangleBG);
>-    passEncoder.draw(6, 1, 0, 0);
>-    // Draw lower triangle.
>-    passEncoder.setBindGroup(0, middleTriangleBG);
>-    passEncoder.draw(3, 1, 0, 0);
>+    // Lower triangle.
>+    passEncoder.setVertexBuffers(0, [verticesBuffer], [0]);
>+    passEncoder.draw(9, 1, 0, 0);
> 
>     const endCommandBuffer = passEncoder.endPass();
>     const queue = device.getQueue();

Actions: View | Formatted Diff | Diff

Attachments on bug 194125: 360811