[Web GPU] Implement API default values
Created attachment 367668 [details] Patch
Comment on attachment 367668 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=367668&action=review > Source/WebCore/ChangeLog:8 > + Add default values and 'required' qualifiers recently merged to the WebGPU API. It's pretty frightening that this patch doesn't update almost any .cpp or .mm files, which means we were happily accepting garbage values before. I would have hoped that before this patch, all the C++ code that received these values would have done more default value creation or exception/error throwing, and this patch could have deleted all that C++ code in favor of using the IDL to represent the same checks. > Source/WebCore/Modules/webgpu/WebGPUBufferBinding.h:37 > + uint64_t offset = 0; Shouldn't this be done in the IDL? > Source/WebCore/Modules/webgpu/WebGPURenderPipelineDescriptor.h:41 > + Optional<WebGPUPipelineStageDescriptor> fragmentStage; 👍 > Source/WebCore/platform/graphics/gpu/GPUBlendDescriptor.h:59 > + GPUBlendFactor srcFactor { GPUBlendFactor::One }; > + GPUBlendFactor dstFactor { GPUBlendFactor::Zero }; > + GPUBlendOperation operation { GPUBlendOperation::Add }; Doesn't the IDL take care of this for you? And ditto below. > LayoutTests/ChangeLog:24 > + * webgpu/bind-groups.html: > + * webgpu/blend-triangle-strip.html: > + * webgpu/blit-commands.html: > + * webgpu/buffer-command-buffer-races.html: > + * webgpu/buffer-resource-triangles.html: > + * webgpu/color-write-mask-triangle-strip.html: > + * webgpu/compute-squares.html: > + * webgpu/depth-enabled-triangle-strip.html: > + * webgpu/draw-indexed-triangles.html: > + * webgpu/js/webgpu-functions.js: > + (createBasicDepthTexture): > + * webgpu/texture-triangle-strip.html: > + * webgpu/textures-textureviews.html: > + * webgpu/vertex-buffer-triangle-strip.html: > + * webgpu/whlsl.html: Just deleting code from existing tests isn't very exhaustive. There should probably be a single new file that has a bunch of tests in it, where each test specifies every value except one, and then draws something, and we determine from the rendered content (or from some other mechanism) whether the default value was specified. If we made such a test we should consider contributing it to the WG. The WG is going to try to make a comprehensive test suite.
Created attachment 367761 [details] Patch for landing
Comment on attachment 367761 [details] Patch for landing Rejecting attachment 367761 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 367761, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit ChangeLog entry in LayoutTests/ChangeLog contains OOPS!. Full output: https://webkit-queues.webkit.org/results/11919784
Created attachment 367763 [details] Patch for landing
Comment on attachment 367763 [details] Patch for landing Clearing flags on attachment: 367763 Committed r244442: <https://trac.webkit.org/changeset/244442>
All reviewed patches have been landed. Closing bug.
<rdar://problem/50031486>