Bug 195518 - [Web GPU] Update GPURenderPipelineDescriptor and add GPUColorStateDescriptor.format
Summary: [Web GPU] Update GPURenderPipelineDescriptor and add GPUColorStateDescriptor....
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGPU (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Justin Fan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-03-09 04:08 PST by Justin Fan
Modified: 2019-03-12 18:33 PDT (History)
3 users (show)

See Also:


Attachments
Patch (141.87 KB, patch)
2019-03-12 12:43 PDT, Justin Fan
no flags Details | Formatted Diff | Diff
Patch for landing (149.31 KB, patch)
2019-03-12 17:55 PDT, Justin Fan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.