[webkit-reviews] review granted: [Bug 191084] [WebGPU] Experimental prototype for MSL shaders and render pipelines : [Attachment 353669] Patch

[bugzilla-daemon at webkit.org]

Dean Jackson <dino at apple.com> has granted Justin Fan <justin_fan at apple.com>'s
request for review:
Bug 191084: [WebGPU] Experimental prototype for MSL shaders and render
pipelines
https://bugs.webkit.org/show_bug.cgi?id=191084

Attachment 353669: Patch

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




--- Comment #11 from Dean Jackson <dino at apple.com> ---
Comment on attachment 353669
  --> https://bugs.webkit.org/attachment.cgi?id=353669
Patch

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

We should probably add a test with bad shader code to ensure our promise is
rejected.

I think the sooner you make small tests for each bit of the library, the
better.

> Source/WebCore/ChangeLog:3
> +	   [WebGPU] Experimental prototype for MSL shaders and render pipelines

Needs to be accurate.

> Source/WebCore/Modules/webgpu/GPUDevice.h:34
> +#if ENABLE(WEBGPU)

Typo: METAL

> Source/WebCore/Modules/webgpu/WebGPUDevice.h:46
> +    static RefPtr<WebGPUDevice> create(ScriptExecutionContext&,
Ref<WebGPUAdapter>&&);

Why do we need the ScriptExecutionContext?

> Source/WebCore/Modules/webgpu/cocoa/GPUDeviceMetal.mm:68
> +GPUDevice::GPUDevice(PlatformDeviceSmartPtr&& device)
> +    : m_platformDevice(WTFMove(device))
> +{
> +    UNUSED_PARAM(m_platformDevice);
> +}
> +
> +RefPtr<GPUShaderModule>
GPUDevice::createShaderModule(GPUShaderModuleDescriptor&& descriptor) const
> +{
> +    return GPUShaderModule::create(*this, WTFMove(descriptor));
> +}

Technically these bits could go in a GPUDevice.cpp file, but we can do that
later.