Bug 195518

Summary: [Web GPU] Update GPURenderPipelineDescriptor and add GPUColorStateDescriptor.format
Product: WebKit Reporter: Justin Fan <justin_fan>
Component: WebGPUAssignee: Justin Fan <justin_fan>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Justin Fan 2019-03-09 04:08:21 PST
Currently Web GPU is hard-coded to provide the required pixel format for the first color attachment, and no others. Add GPUColorStateDescriptor from the API in order to provide the desired texture format to all Web GPU color attachments. Blend states will follow after MVP.
Comment 1 Justin Fan 2019-03-11 10:12:59 PDT
<rdar://problem/46322356>
Comment 2 Justin Fan 2019-03-12 12:43:27 PDT
Created attachment 364426 [details]
Patch
Comment 3 Myles C. Maxfield 2019-03-12 17:14:59 PDT
Comment on attachment 364426 [details]
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?
Comment 4 Justin Fan 2019-03-12 17:54:12 PDT
(In reply to Myles C. Maxfield from comment #3)
> Comment on attachment 364426 [details]
> 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?

There's currently an issue raised where we haven't finalized the list of supported formats across all backend APIs. I can add more in a new patch, but these are enough for now imo.
Comment 5 Justin Fan 2019-03-12 17:55:30 PDT
Created attachment 364487 [details]
Patch for landing
Comment 6 WebKit Commit Bot 2019-03-12 18:33:18 PDT
Comment on attachment 364487 [details]
Patch for landing

Clearing flags on attachment: 364487

Committed r242839: <https://trac.webkit.org/changeset/242839>
Comment 7 WebKit Commit Bot 2019-03-12 18:33:19 PDT
All reviewed patches have been landed.  Closing bug.