Bug 233179 – [WebGPU] Start preparing for serializing commands to the GPU process

• Component: New Bugs
• Status: RESOLVED FIXED

[Myles C. Maxfield]

[WebGPU] Start preparing for serializing commands to the GPU process

[Myles C. Maxfield]

Created attachment 444358 [details]
Patch

[Alex Christensen]

Comment on attachment 444358 [details]
Patch

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

r=me with many nits.  Comments about namespaces and std::function apply several times in the patch.

> Source/WebKit/Shared/WebGPU/WebGPUBindGroupDescriptor.h:34
> +namespace WebKit {
> +namespace WebGPU {

namespace WebKit::WebGPU

> Source/WebKit/Shared/WebGPU/WebGPUBindGroupEntry.h:32
> +#include <utility>
> +#include <variant>

I don't see these used directly

> Source/WebKit/Shared/WebGPU/WebGPUBindGroupEntry.h:37
> +enum class BindingResourceType {

: uint8_t

> Source/WebKit/Shared/WebGPU/WebGPUConvertFromBackingContext.h:68
> +    virtual const PAL::WebGPU::Adapter* convertAdapterFromBacking(WebGPUIdentifier) = 0;

Do we want to return raw pointers here?  What will be the lifetime of these objects?

> Source/WebKit/Shared/WebGPU/WebGPUDepthStencilState.h:40
> +    bool depthWriteEnabled;

Do we want to have default initializers for these to avoid uninitialized memory?

> Source/WebKit/Shared/WebGPU/WebGPUDeviceDescriptor.h:40
> +    // Vector<KeyValuePair<String, uint64_t>> requiredLimits;

Can we avoid adding commented code like this?

> Source/WebKit/Shared/WebGPU/WebGPUOrigin3D.h:43
> +}

You have comments elsewhere indicating that this closes a namespace.

> Source/WebKit/WebProcess/GPU/graphics/WebGPU/RemoteAdapterProxy.h:62
> +    Deque<std::function<void(Ref<PAL::WebGPU::Device>)>> m_callbacks;

Can we use WTF::Function instead of std::function?

[Myles C. Maxfield]

Comment on attachment 444358 [details]
Patch

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

>> Source/WebKit/Shared/WebGPU/WebGPUConvertFromBackingContext.h:68
>> +    virtual const PAL::WebGPU::Adapter* convertAdapterFromBacking(WebGPUIdentifier) = 0;
> 
> Do we want to return raw pointers here?  What will be the lifetime of these objects?

These return non-owning nullable references.

This is used in the GPU Process. WebGPU objects will be owned by a registry, and the registry will receive messages to create / delete things. When the GPU Process receives a rendering command, that command will have one of the WebKit::WebGPU structs as its parameter, which will hold WebGPUIdentifiers inside it. There will be a function to convert a WebKit::WebGPU struct to a PAL::WebGPU struct, thereby replacing the WebGPUIdentifiers with references to objects inside the registry. That function will take one of these ConvertFromBackingContext objects as a parameter. This interface will be implemented by the registry, and it will receive the WebGPUIdentifier and return a non-owning reference to the WebGPU object it owns internally. Then, the PAL::WebGPU class complete with non-owning references will be passed down into WebGPU.framework where the rendering command will be executed.

>> Source/WebKit/Shared/WebGPU/WebGPUDepthStencilState.h:40
>> +    bool depthWriteEnabled;
> 
> Do we want to have default initializers for these to avoid uninitialized memory?

Yes. That's a big change, though, so I'll do it in a follow-up.

[Myles C. Maxfield]

Committed r285881 (244307@main): <https://commits.webkit.org/244307@main>

[Radar WebKit Bug Importer]

<rdar://problem/85472586>

[Ross Kirsling]

This broke WinCairo, as your cq- shows... :(

[Ross Kirsling]

Fixed cmake ENABLE(GPU_PROCESS) build in r285881.