[webkit-reviews] review granted: [Bug 195518] [Web GPU] Update GPURenderPipelineDescriptor and add GPUColorStateDescriptor.format : [Attachment 364426] Patch

[bugzilla-daemon at webkit.org]

Myles C. Maxfield <mmaxfield at apple.com> has granted Justin Fan
<justin_fan at apple.com>'s request for review:
Bug 195518: [Web GPU] Update GPURenderPipelineDescriptor and add
GPUColorStateDescriptor.format
https://bugs.webkit.org/show_bug.cgi?id=195518

Attachment 364426: Patch

https://bugs.webkit.org/attachment.cgi?id=364426&action=review




--- Comment #3 from Myles C. Maxfield <mmaxfield at apple.com> ---
Comment on attachment 364426
  --> https://bugs.webkit.org/attachment.cgi?id=364426
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364426&action=review

> Source/WebCore/Modules/webgpu/GPUVertexAttributeDescriptor.idl:35
> +enum GPUVertexFormat {
> +    "float",
> +    "float2",
> +    "float3",
> +    "float4"
> +};

Why not add the rest of the vertex formats?

> Source/WebCore/Modules/webgpu/WebGPURenderPipelineDescriptor.cpp:41
> +Optional<GPURenderPipelineDescriptor>
WebGPURenderPipelineDescriptor::asGPURenderPipelineDescriptor() const

"as" implies this is idempotent, but this appears to create a new object every
time it's called. Why not "create...()"?

> Tools/DumpRenderTree/DerivedSources-input.xcfilelist:205
> +$(PROJECT_DIR)/dom/Event.idl
> +$(PROJECT_DIR)/dom/EventInit.idl
> +$(PROJECT_DIR)/dom/EventModifierInit.idl
> +$(PROJECT_DIR)/dom/EventTarget.idl
> +$(PROJECT_DIR)/dom/FocusEvent.idl
> +$(PROJECT_DIR)/dom/GlobalEventHandlers.idl
> +$(PROJECT_DIR)/dom/HashChangeEvent.idl
> +$(PROJECT_DIR)/dom/InputEvent.idl
> +$(PROJECT_DIR)/dom/KeyboardEvent.idl
> +$(PROJECT_DIR)/dom/MessageChannel.idl
> +$(PROJECT_DIR)/dom/MessageEvent.idl
> +$(PROJECT_DIR)/dom/MessagePort.idl
> +$(PROJECT_DIR)/dom/MouseEvent.idl
> +$(PROJECT_DIR)/dom/MouseEventInit.idl
> +$(PROJECT_DIR)/dom/MutationCallback.idl

What do these have to do with your patch?