[webkit-reviews] review granted: [Bug 193926] [WebGPU] Fix and add validation to WebGPURenderPipeline and MTLVertexDescriptor : [Attachment 360380] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Mon Jan 28 19:03:21 PST 2019
Myles C. Maxfield <mmaxfield at apple.com> has granted Justin Fan
<justin_fan at apple.com>'s request for review:
Bug 193926: [WebGPU] Fix and add validation to WebGPURenderPipeline and
MTLVertexDescriptor
https://bugs.webkit.org/show_bug.cgi?id=193926
Attachment 360380: Patch
https://bugs.webkit.org/attachment.cgi?id=360380&action=review
--- Comment #6 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 360380
--> https://bugs.webkit.org/attachment.cgi?id=360380
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=360380&action=review
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:123
> + auto attributeArray = retainPtr(mtlVertexDescriptor.get().attributes);
Are you sure our style guide permits use of Objective-C dot syntax?
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:128
> + if (location > 30) {
I think this limit needs to be 16, not 30. It is/will be in the spec.
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:133
> + if (attributes[i].inputSlot > 30) {
Ditto.
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:145
> + mtlAttributeDesc.get().offset = attributes[i].offset; // FIXME:
Setting this to an invalidly large value causes pipelineState creation to
SIGABT.
This offset, plus the byte size of the data type, need to be less than or equal
to the buffer's stride.
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:156
> + if (inputs[j].inputSlot > 30) {
Ditto about 16
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:205
> + populateMtlDescriptorSuccess =
setFunctionsForPipelineDescriptor(functionName, mtlDescriptor.get(),
descriptor)
> + && setInputStateForPipelineDescriptor(functionName,
mtlDescriptor.get(), descriptor);
I would separate these out into separate statements.
> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:219
> + NSError *error = [NSError errorWithDomain:@"com.apple.WebKit.GPU" code:1
userInfo:nil];
wat
More information about the webkit-reviews
mailing list