Diff
Modified: trunk/LayoutTests/ChangeLog (240671 => 240672)
--- trunk/LayoutTests/ChangeLog 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/LayoutTests/ChangeLog 2019-01-29 20:50:17 UTC (rev 240672)
@@ -1,3 +1,15 @@
+2019-01-29 Justin Fan <[email protected]>
+
+ [WebGPU] Fix and add validation to WebGPURenderPipeline and MTLVertexDescriptor
+ https://bugs.webkit.org/show_bug.cgi?id=193926
+ <rdar://problem/47327648>
+
+ Reviewed by Myles C. Maxfield.
+
+ Updated test for new vertex input logic. Now provides color as a vertex attribute.
+
+ * webgpu/vertex-buffer-triangle-strip.html:
+
2019-01-29 Devin Rousso <[email protected]>
Web Inspector: provide a way to edit page WebRTC settings on a remote target
Modified: trunk/LayoutTests/webgpu/vertex-buffer-triangle-strip.html (240671 => 240672)
--- trunk/LayoutTests/webgpu/vertex-buffer-triangle-strip.html 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/LayoutTests/webgpu/vertex-buffer-triangle-strip.html 2019-01-29 20:50:17 UTC (rev 240672)
@@ -15,11 +15,13 @@
struct VertexIn
{
float4 position [[attribute(0)]];
+ float green [[attribute(1)]];
};
struct VertexOut
{
float4 position [[position]];
+ float4 color;
};
vertex VertexOut vertex_main(VertexIn vertexIn [[stage_in]])
@@ -26,42 +28,34 @@
{
VertexOut vOut;
vOut.position = vertexIn.position;
+ vOut.color = float4(0, vertexIn.green, 0, 1);
return vOut;
}
-fragment float4 fragment_main()
+fragment float4 fragment_main(VertexOut v [[stage_in]])
{
- return float4(0.0, 1.0, 0.0, 1.0);
+ return v.color;
}
`
function createVertexBuffer(device) {
- const bufferSize = 4 * 4 * 4;
+ const bufferSize = 4 * 5 * 4;
const buffer = device.createBuffer({ size: bufferSize, usage: WebGPUBufferUsage.MAP_WRITE });
- let arrayBuffer = buffer.mapping;
- let floatArray = new Float32Array(arrayBuffer);
+ let floatArray = new Float32Array(buffer.mapping);
- floatArray[0] = -1;
- floatArray[1] = 1;
- floatArray[2] = 0;
- floatArray[3] = 1;
+ const vertices = [
+ // float4 xyzw, float g
+ -1, 1, 0, 1, 1,
+ -1, -1, 0, 1, 1,
+ 1, 1, 0, 1, 1,
+ 1, -1, 0, 1, 1
+ ];
- floatArray[4] = -1;
- floatArray[5] = -1;
- floatArray[6] = 0;
- floatArray[7] = 1;
+ for (let i = 0; i < vertices.length; ++i) {
+ floatArray[i] = vertices[i];
+ }
- floatArray[8] = 1;
- floatArray[9] = 1;
- floatArray[10] = 0;
- floatArray[11] = 1;
-
- floatArray[12] = 1;
- floatArray[13] = -1;
- floatArray[14] = 0;
- floatArray[15] = 1;
-
return buffer;
}
@@ -73,9 +67,15 @@
inputSlot: 0,
offset: 0,
format: WebGPUVertexFormat.FLOAT_R32_G32_B32_A32
+ }, {
+ shaderLocation: 1,
+ inputSlot: 0,
+ offset: 4 * 4,
+ format: WebGPUVertexFormat.FLOAT_R32
}],
inputs: [{
- stride: 4 * 4,
+ inputSlot: 0,
+ stride: 4 * 5,
stepMode: WebGPUInputStepMode.VERTEX
}]
}
Modified: trunk/Source/WebCore/ChangeLog (240671 => 240672)
--- trunk/Source/WebCore/ChangeLog 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/Source/WebCore/ChangeLog 2019-01-29 20:50:17 UTC (rev 240672)
@@ -1,3 +1,21 @@
+2019-01-29 Justin Fan <[email protected]>
+
+ [WebGPU] Fix and add validation to WebGPURenderPipeline and MTLVertexDescriptor
+ https://bugs.webkit.org/show_bug.cgi?id=193926
+ <rdar://problem/47327648>
+
+ Reviewed by Myles C. Maxfield.
+
+ Update vertex input to properly utilize inputSlot and shaderLocation fields, and add some validation.
+
+ Test: webgpu/vertex-buffer-triangle-strip.html
+
+ * Modules/webgpu/WebGPUVertexInputDescriptor.idl:
+ * platform/graphics/gpu/GPUVertexInputDescriptor.h:
+ * platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:
+ (WebCore::setInputStateForPipelineDescriptor): Properly retain Metal types.
+ (WebCore::GPURenderPipeline::create): Provide error logging for MTLRenderPipelineState creation.
+
2019-01-29 Keith Rollin <[email protected]>
Add .xcfilelists to Run Script build phases
Modified: trunk/Source/WebCore/Modules/webgpu/WebGPUVertexInputDescriptor.idl (240671 => 240672)
--- trunk/Source/WebCore/Modules/webgpu/WebGPUVertexInputDescriptor.idl 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/Source/WebCore/Modules/webgpu/WebGPUVertexInputDescriptor.idl 2019-01-29 20:50:17 UTC (rev 240672)
@@ -31,6 +31,7 @@
Conditional=WEBGPU,
EnabledAtRuntime=WebGPU
] dictionary WebGPUVertexInputDescriptor {
+ u32 inputSlot;
u32 stride;
WebGPUInputStepModeEnum stepMode;
};
Modified: trunk/Source/WebCore/platform/graphics/gpu/GPUVertexInputDescriptor.h (240671 => 240672)
--- trunk/Source/WebCore/platform/graphics/gpu/GPUVertexInputDescriptor.h 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/Source/WebCore/platform/graphics/gpu/GPUVertexInputDescriptor.h 2019-01-29 20:50:17 UTC (rev 240672)
@@ -42,6 +42,7 @@
};
struct GPUVertexInputDescriptor {
+ unsigned long inputSlot;
unsigned long stride;
GPUInputStepModeEnum stepMode;
};
Modified: trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm (240671 => 240672)
--- trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm 2019-01-29 20:37:02 UTC (rev 240671)
+++ trunk/Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm 2019-01-29 20:50:17 UTC (rev 240672)
@@ -118,42 +118,56 @@
#endif
auto mtlVertexDescriptor = adoptNS([MTLVertexDescriptor new]);
- // Populate vertex attributes, if any.
const auto& attributes = descriptor.inputState.attributes;
- // FIXME: What kind of validation is needed here?
- MTLVertexAttributeDescriptorArray *attributeArray = mtlVertexDescriptor.get().attributes;
+ auto attributeArray = retainPtr(mtlVertexDescriptor.get().attributes);
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) {
+ LOG(WebGPU, "%s: Invalid shaderLocation %lu for vertex attribute!", functionName, location);
+ return false;
+ }
+ // Maximum number of vertex buffers supported.
+ if (attributes[i].inputSlot > 16) {
+ LOG(WebGPU, "%s: Invalid inputSlot %lu for vertex attribute %lu!", functionName, attributes[i].inputSlot, location);
+ return false;
+ }
auto mtlFormat = validateAndConvertVertexFormatToMTLVertexFormat(attributes[i].format);
if (!mtlFormat) {
- LOG(WebGPU, "%s: Invalid WebGPUVertexFormatEnum for vertex attribute!", functionName);
+ LOG(WebGPU, "%s: Invalid WebGPUVertexFormatEnum for vertex attribute %lu!", functionName, location);
return false;
}
- MTLVertexAttributeDescriptor *mtlAttributeDesc = [attributeArray objectAtIndexedSubscript:i];
- mtlAttributeDesc.format = *mtlFormat;
- mtlAttributeDesc.offset = attributes[i].offset;
- mtlAttributeDesc.bufferIndex = attributes[i].inputSlot;
- [mtlVertexDescriptor.get().attributes setObject:mtlAttributeDesc atIndexedSubscript:i];
+ 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;
+ [mtlVertexDescriptor.get().attributes setObject:mtlAttributeDesc.get() atIndexedSubscript:location];
}
- // Populate vertex buffer layouts, if any.
const auto& inputs = descriptor.inputState.inputs;
- MTLVertexBufferLayoutDescriptorArray *layoutArray = mtlVertexDescriptor.get().layouts;
+ auto layoutArray = retainPtr(mtlVertexDescriptor.get().layouts);
for (size_t j = 0; j < inputs.size(); ++j) {
+ auto slot = inputs[j].inputSlot;
+ if (inputs[j].inputSlot > 16) {
+ LOG(WebGPU, "%s: Invalid inputSlot %d for vertex buffer!", functionName, slot);
+ return false;
+ }
+
auto mtlStepFunction = validateAndConvertStepModeToMTLStepFunction(inputs[j].stepMode);
if (!mtlStepFunction) {
- LOG(WebGPU, "%s: Invalid WebGPUInputStepMode for vertex input!", functionName);
+ LOG(WebGPU, "%s: Invalid WebGPUInputStepMode for vertex buffer at slot %lu!", functionName, slot);
return false;
}
- MTLVertexBufferLayoutDescriptor *mtlLayoutDesc = [layoutArray objectAtIndexedSubscript:j];
- mtlLayoutDesc.stepFunction = *mtlStepFunction;
- mtlLayoutDesc.stride = inputs[j].stride;
- [mtlVertexDescriptor.get().layouts setObject:mtlLayoutDesc atIndexedSubscript:j];
+ auto mtlLayoutDesc = retainPtr([layoutArray objectAtIndexedSubscript:slot]);
+ mtlLayoutDesc.get().stepFunction = *mtlStepFunction;
+ mtlLayoutDesc.get().stride = inputs[j].stride;
+ [mtlVertexDescriptor.get().layouts setObject:mtlLayoutDesc.get() atIndexedSubscript:slot];
}
mtlDescriptor.vertexDescriptor = mtlVertexDescriptor.get();
@@ -183,8 +197,16 @@
return nullptr;
}
- if (!setFunctionsForPipelineDescriptor(functionName, mtlDescriptor.get(), descriptor)
- || !setInputStateForPipelineDescriptor(functionName, mtlDescriptor.get(), descriptor))
+ bool didSetFunctions = false, didSetInputState = false;
+
+ BEGIN_BLOCK_OBJC_EXCEPTIONS;
+
+ didSetFunctions = setFunctionsForPipelineDescriptor(functionName, mtlDescriptor.get(), descriptor);
+ didSetInputState = setInputStateForPipelineDescriptor(functionName, mtlDescriptor.get(), descriptor);
+
+ END_BLOCK_OBJC_EXCEPTIONS;
+
+ if (!didSetFunctions || !didSetInputState)
return nullptr;
// FIXME: Get the pixelFormat as configured for the context/CAMetalLayer.
@@ -194,14 +216,15 @@
BEGIN_BLOCK_OBJC_EXCEPTIONS;
- pipeline = adoptNS([device.platformDevice() newRenderPipelineStateWithDescriptor:mtlDescriptor.get() error:nil]);
+ NSError *error = [NSError errorWithDomain:@"com.apple.WebKit.GPU" code:1 userInfo:nil];
+ pipeline = adoptNS([device.platformDevice() newRenderPipelineStateWithDescriptor:mtlDescriptor.get() error:&error]);
+ if (!pipeline)
+ LOG(WebGPU, "%s: %s!", functionName, error.localizedDescription.UTF8String);
END_BLOCK_OBJC_EXCEPTIONS;
- if (!pipeline) {
- LOG(WebGPU, "%s: Error creating MTLRenderPipelineState!", functionName);
+ if (!pipeline)
return nullptr;
- }
return adoptRef(new GPURenderPipeline(WTFMove(pipeline), WTFMove(descriptor)));
}
