- Revision
- 239944
- Author
- justin_...@apple.com
- Date
- 2019-01-14 13:56:07 -0800 (Mon, 14 Jan 2019)
Log Message
[WebGPU] Map WebGPUBindGroupLayoutBindings from the BindGroupLayoutDescriptor for error checking and later referencing
https://bugs.webkit.org/show_bug.cgi?id=193405
Reviewed by Dean Jackson.
Source/WebCore:
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):
LayoutTests:
Small fixes that do not alter behavior.
* webgpu/bind-groups.html:
* webgpu/pipeline-layouts.html:
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (239943 => 239944)
--- trunk/LayoutTests/ChangeLog 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/LayoutTests/ChangeLog 2019-01-14 21:56:07 UTC (rev 239944)
@@ -1,3 +1,15 @@
+2019-01-14 Justin Fan <justin_...@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 Dean Jackson.
+
+ Small fixes that do not alter behavior.
+
+ * webgpu/bind-groups.html:
+ * webgpu/pipeline-layouts.html:
+
2019-01-14 Zalan Bujtas <za...@apple.com>
[LFC][BFC] Add basic box-sizing support.
Modified: trunk/LayoutTests/webgpu/bind-groups.html (239943 => 239944)
--- trunk/LayoutTests/webgpu/bind-groups.html 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/LayoutTests/webgpu/bind-groups.html 2019-01-14 21:56:07 UTC (rev 239944)
@@ -17,7 +17,7 @@
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 };
Modified: trunk/LayoutTests/webgpu/pipeline-layouts.html (239943 => 239944)
--- trunk/LayoutTests/webgpu/pipeline-layouts.html 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/LayoutTests/webgpu/pipeline-layouts.html 2019-01-14 21:56:07 UTC (rev 239944)
@@ -22,7 +22,7 @@
}, "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");
Modified: trunk/Source/WebCore/ChangeLog (239943 => 239944)
--- trunk/Source/WebCore/ChangeLog 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/Source/WebCore/ChangeLog 2019-01-14 21:56:07 UTC (rev 239944)
@@ -1,3 +1,30 @@
+2019-01-14 Justin Fan <justin_...@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 Dean Jackson.
+
+ 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-14 Myles C. Maxfield <mmaxfi...@apple.com>
[WHLSL] Assorted cleanup
Modified: trunk/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp (239943 => 239944)
--- trunk/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/Source/WebCore/Modules/webgpu/WebGPUDevice.cpp 2019-01-14 21:56:07 UTC (rev 239944)
@@ -97,10 +97,15 @@
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);
+ }
+
auto bindingResourceVisitor = WTF::makeVisitor([] (RefPtr<WebGPUTextureView> view) -> Optional<GPUBindingResource> {
if (view)
return static_cast<GPUBindingResource>(view->texture());
@@ -115,11 +120,16 @@
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);
}
}
Modified: trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h (239943 => 239944)
--- trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroup.h 2019-01-14 21:56:07 UTC (rev 239944)
@@ -42,7 +42,7 @@
static Ref<GPUBindGroup> create(GPUBindGroupDescriptor&&);
private:
- GPUBindGroup(GPUBindGroupDescriptor&&);
+ explicit GPUBindGroup(GPUBindGroupDescriptor&&);
Ref<GPUBindGroupLayout> m_layout;
Vector<GPUBindGroupBinding> m_bindings;
Modified: trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h (239943 => 239944)
--- trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/Source/WebCore/platform/graphics/gpu/GPUBindGroupLayout.h 2019-01-14 21:56:07 UTC (rev 239944)
@@ -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 @@
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;
};
Modified: trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm (239943 => 239944)
--- trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm 2019-01-14 21:44:46 UTC (rev 239943)
+++ trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPUBindGroupLayoutMetal.mm 2019-01-14 21:56:07 UTC (rev 239944)
@@ -76,8 +76,14 @@
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 @@
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 @@
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))
{
}
