[WebGPU] Fix and add validation to WebGPURenderPipeline and MTLVertexDescriptor
<rdar://problem/47327648>
<rdar://problem/47612384>
<rdar://problem/47612385>
Created attachment 360380 [details] Patch
Comment on attachment 360380 [details] 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
Comment on attachment 360380 [details] 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? Our style guide doesn't mention ObjC dot vs bracket. I use [object methodCall] and object.property. >> 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. I could process the vertex buffer layouts first to determine the stride of the attribute's buffer. However, we won't know the size of the vertex buffer, so I'm not sure how to validate stride, because it has the same issue as the offset value here. >> Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:219 >> + NSError *error = [NSError errorWithDomain:@"com.apple.WebKit.GPU" code:1 userInfo:nil]; > > wat error is an autoreleased NSError * created via the way we did it in WebMetal ¯\_(ツ)_/¯. Useful for debugging.
Created attachment 360485 [details] Patch for landing
Comment on attachment 360485 [details] Patch for landing Clearing flags on attachment: 360485 Committed r240672: <https://trac.webkit.org/changeset/240672>
All reviewed patches have been landed. Closing bug.