Summary: | [Web GPU] Vertex Buffers/Input State API updates for MVP | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Justin Fan <justin_fan> | ||||||||||
Component: | WebGPU | Assignee: | Justin Fan <justin_fan> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | commit-queue, darin, ews-watchlist, mmaxfield, rniwa, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Other | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
Attachments: |
|
Description
Justin Fan
2019-02-04 17:07:30 PST
Created attachment 370829 [details]
Patch
Comment on attachment 370829 [details] Patch Attachment 370829 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/12315373 New failing tests: http/wpt/beacon/cors/cors-preflight-cookie.html webgpu/whlsl-dereference-pointer-should-type-check.html Created attachment 370832 [details]
Archive of layout-test-results from ews104 for mac-highsierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-highsierra-wk2 Platform: Mac OS X 10.13.6
Comment on attachment 370829 [details]
Patch
Looks like the test webgpu/whlsl-dereference-pointer-should-type-check.html needs to be fixed.
(In reply to Darin Adler from comment #5) > Comment on attachment 370829 [details] > Patch > > Looks like the test webgpu/whlsl-dereference-pointer-should-type-check.html > needs to be fixed. Strange, this one must have been introduced when I rebased ToT. Created attachment 370873 [details]
Patch
Comment on attachment 370873 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=370873&action=review > Source/WebCore/ChangeLog:3 > + [Web GPU] Vertex Buffers/Input State API updates for MVP Nit: remove "for MVP" It's not like we're going to revert this patch after the MVP. > Source/WebCore/Modules/webgpu/GPUVertexBufferDescriptor.idl:2 > + * Copyright (C) 2018 Apple Inc. All rights reserved. 2019 > Source/WebCore/platform/graphics/gpu/GPUVertexBufferDescriptor.h:2 > + * Copyright (C) 2018 Apple Inc. All rights reserved. Ditto for all the other files > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:205 > + if (attributes.size() + attributeIndex > 16) { magic constant. At least make it a constexpr variable with a name > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:216 > + auto convertedBufferIndex = WHLSL::Metal::calculateVertexBufferIndex(index); You can probably move this out of WHLSL. It doesn't need to be there. > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:226 > + LOG(WebGPU, "%s: Invalid shaderLocation %u for vertex attribute!", functionName, attribute.shaderLocation); Shouldn't this say "duplicate" instead of "invalid"? > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:246 > + auto mtlAttributeDesc = retainPtr([attributeArray objectAtIndexedSubscript:attributeIndex]); > + [mtlAttributeDesc setFormat:mtlVertexFormatForGPUVertexFormat(attribute.format)]; > + [mtlAttributeDesc setOffset:offset]; > + [mtlAttributeDesc setBufferIndex:convertedBufferIndex]; > + END_BLOCK_OBJC_EXCEPTIONS; > + > + if (whlslDescriptor) > + whlslDescriptor->vertexAttributes.append({ convertVertexFormat(attribute.format), attribute.shaderLocation, attributeIndex }); > + > + ++attributeIndex; Cool. This is quite elegant. > Source/WebCore/platform/graphics/gpu/cocoa/GPURenderPipelineMetal.mm:573 > + // FIXME: depthStencilAttachmentDescriptor isn't implemented yet for WHLSL compiler. Can we have a new bugzilla bug and link here? Created attachment 370973 [details]
Patch for landing
Comment on attachment 370973 [details] Patch for landing Clearing flags on attachment: 370973 Committed r245905: <https://trac.webkit.org/changeset/245905> All reviewed patches have been landed. Closing bug. |