Bug 191291 - [WebGPU] Experimental prototype for WebGPURenderPipeline and WebGPUSwapChain
Summary: [WebGPU] Experimental prototype for WebGPURenderPipeline and WebGPUSwapChain
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: 2018-11-05 18:32 PST by Justin Fan
Modified: 2018-11-06 20:37 PST (History)
4 users (show)

See Also:


Attachments
Patch (82.22 KB, patch)
2018-11-05 18:49 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (84.21 KB, patch)
2018-11-06 12:59 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (86.20 KB, patch)
2018-11-06 14:26 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (86.20 KB, patch)
2018-11-06 15:32 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (86.25 KB, patch)
2018-11-06 15:40 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch (85.79 KB, patch)
2018-11-06 17:05 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch for landing (95.17 KB, patch)
2018-11-06 18:58 PST, Justin Fan
no flags Details | Formatted Diff | Diff
Patch for landing (93.88 KB, patch)
2018-11-06 19:01 PST, 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 2018-11-05 18:32:38 PST
[WebGPU] Experimental prototype for WebGPURenderPipeline and WebGPUSwapChain
Comment 1 Justin Fan 2018-11-05 18:49:42 PST
Created attachment 353940 [details]
Patch
Comment 2 Radar WebKit Bug Importer 2018-11-05 18:51:16 PST
<rdar://problem/45829392>
Comment 3 EWS Watchlist 2018-11-05 18:52:28 PST
Attachment 353940 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 41 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 4 Justin Fan 2018-11-05 18:54:28 PST
For the style failure: IDL and binding code are expecting all-caps enums, at least in the current version of WebGPU.
Comment 5 Justin Fan 2018-11-06 12:59:38 PST
Created attachment 353993 [details]
Patch
Comment 6 EWS Watchlist 2018-11-06 13:04:41 PST
Attachment 353993 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 7 Justin Fan 2018-11-06 14:26:18 PST
Created attachment 354005 [details]
Patch
Comment 8 EWS Watchlist 2018-11-06 14:29:43 PST
Attachment 354005 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Justin Fan 2018-11-06 15:32:33 PST
Created attachment 354018 [details]
Patch
Comment 10 EWS Watchlist 2018-11-06 15:35:38 PST
Attachment 354018 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Justin Fan 2018-11-06 15:40:26 PST
Created attachment 354019 [details]
Patch
Comment 12 EWS Watchlist 2018-11-06 15:42:06 PST
Attachment 354019 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 45 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 13 Justin Fan 2018-11-06 17:05:18 PST
Created attachment 354034 [details]
Patch
Comment 14 EWS Watchlist 2018-11-06 17:07:41 PST
Attachment 354034 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 44 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 15 Myles C. Maxfield 2018-11-06 18:32:36 PST
Comment on attachment 354034 [details]
Patch

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

> Source/WebCore/Modules/webgpu/GPUPipelineDescriptorBase.h:37
> +    Vector<GPUPipelineStageDescriptor> stages;

Are you sure this is the right internal design? We need to differentiate between the vertex shader and the fragment shader when we compile the state. That shouldn't have to iterate through a list of things just to find which item is for the fragment shader.

> Source/WebCore/Modules/webgpu/GPUPipelineStageDescriptor.h:37
> +    const GPUShaderModule& module;

What happens when the module gets garbage collected? Will this be dangling?

> Source/WebCore/Modules/webgpu/GPURenderPipelineDescriptor.h:37
> +struct GPURenderPipelineDescriptor : GPUPipelineDescriptorBase {

Why struct?

> Source/WebCore/Modules/webgpu/GPURenderPipelineDescriptor.h:52
> +    int primitiveTopology;

This shouldn't be an int.

> Source/WebCore/Modules/webgpu/WebGPUPipelineStageDescriptor.h:39
> +    const WebGPUShaderModule* module = nullptr;

Ditto about the garbage collector

> Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:36
> +    enum {

enum class

> Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39
> +        VERTEX = 0,
> +        FRAGMENT = 1,
> +        COMPUTE = 2

Can we use different internal names for these? Our style is not to use ALL CAPS.

> Source/WebCore/Modules/webgpu/cocoa/GPURenderPipeline.h:50
> +#else
> +using PlatformRenderPipeline = void;
> +using PlatformRenderPipelineSmartPtr = RefPtr<void>;
> +#endif

I think it's fine to assume we're not using metal for now. We can break that assumption one day when the linux ports want WebGPU.

> Source/WebCore/Modules/webgpu/cocoa/GPURenderPipelineMetal.mm:88
> +    BEGIN_BLOCK_OBJC_EXCEPTIONS;

We should be deliberate about where we use these.

> Source/WebCore/bindings/js/WebCoreBuiltinNames.h:182
> +    macro(WebGPURenderPipeline) \
> +    macro(WebGPUShaderStage) \

Why aren't the other objects in here?

> LayoutTests/webgpu/render-pipelines.html:5
> +<script id="library" type="x-shader/x-metal">

😐

> LayoutTests/webgpu/webgpu-basics.html:129
> +    // let commandBuffer = defaultDevice.createCommandBuffer();
> +    // if (!commandBuffer) {
> +    //     testFailed("Could not create WebGPUCommandBuffer!");
> +    //     return;
> +    // }
> +
> +    // let texture = context.getNextTexture();
> +    // if (!texture) {
> +    //     testFailed("Could not get next WebGPUTexture!");
> +    //     return;
> +    // }
> +
> +    // let textureView = context.createTextureView();
> +    // if (!textureView) {
> +    //     testFailed("Could not create WebGPUTextureView!");
> +    //     return;
> +    // }
> +
> +    // // FIXME: Flesh out the rest of WebGPURenderPassDescriptor. Default loadOp, storeOp, clearColor for now.
> +    // let renderPassDescriptor = {
> +    //     attachment : textureView
> +    // };
> +
> +    // let renderPassEncoder = commandBuffer.beginRenderPass(renderPassDescriptor);
> +    // renderPassEncoder.setPipeline(renderPipeline);
> +
> +    // // Note that we didn't attach any buffers - the shader is generating the vertices for us.
> +    // renderPassEncoder.draw(3, 1, 0, 0);
> +    // renderPassEncoder.endPass();
> +
> +    // let queue = device.getQueue();
> +    // if (!queue) {
> +    //     testFailed("Unable to create default WebGPUQueue!");
> +    //     return;
> +    // }
> +    // queue.submit([commandBuffer]);
> +
> +    // context.present();

We don't leave code commented in WebKit.
Comment 16 Justin Fan 2018-11-06 18:58:05 PST
Created attachment 354047 [details]
Patch for landing
Comment 17 Justin Fan 2018-11-06 19:01:01 PST
Created attachment 354048 [details]
Patch for landing
Comment 18 Justin Fan 2018-11-06 19:02:02 PST
I'm removing the commented code from my test for now, but I will submit another patch to clean up the implementation after I get a demo working.
Comment 19 EWS Watchlist 2018-11-06 19:07:39 PST
Attachment 354048 [details] did not pass style-queue:


ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:37:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:38:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
ERROR: Source/WebCore/Modules/webgpu/WebGPUShaderStage.h:39:  enum members should use InterCaps with an initial capital letter or initial 'k' for C-style enums.  [readability/enum_casing] [4]
Total errors found: 3 in 56 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 20 WebKit Commit Bot 2018-11-06 20:37:12 PST
Comment on attachment 354048 [details]
Patch for landing

Clearing flags on attachment: 354048

Committed r237912: <https://trac.webkit.org/changeset/237912>
Comment 21 WebKit Commit Bot 2018-11-06 20:37:13 PST
All reviewed patches have been landed.  Closing bug.