[webkit-reviews] review granted: [Bug 237755] [WebGPU] Allow for scheduling asynchronous work : [Attachment 454624] Patch

[bugzilla-daemon at webkit.org]

Kimmo Kinnunen <kkinnunen at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 237755: [WebGPU] Allow for scheduling asynchronous work
https://bugs.webkit.org/show_bug.cgi?id=237755

Attachment 454624: Patch

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




--- Comment #8 from Kimmo Kinnunen <kkinnunen at apple.com> ---
Comment on attachment 454624
  --> https://bugs.webkit.org/attachment.cgi?id=454624
Patch

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

> Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUImpl.cpp:48
> +	   scheduleWorkFunction(makeBlockPtr(workItem));

WGPUWorkItem is a block, so it shouldn't need to be wrapped in a block?
Also, the call could move the `workItem`..

> Source/WebCore/PAL/pal/graphics/WebGPU/Impl/WebGPUImpl.h:44
> +    using ScheduleWorkFunction = Function<void(WorkItem&&)>;

> The reason is because I prefer using C++ lambdas wherever possible because
they are more expressive than C blocks (e.g. lambdas can be moved from, etc.).
So, internally to the framework, I'm using lambdas, and they get converted to
block pointers at framework boundaries.

The ScheduleWorkFunction will not move anything from WorkItem (which
wrapsWGPUWorkItem) . It won't move anything out of WGPUWorkItem either, as that
is just a block pointer (opaque function with context.) WorkItem cannot be used
in any other way than called.

So you are redundantly wrapping the WGPUWorkItem call inside a redundant call.

I think it's ok if your requirements are:
1) Not expose that blocks exist in this layer of the SW stack (sw layer ==
WebKit WebGPU client)
2) Not expose WGPU* types  in this layer of the SW stack

However, since both of these are given forever in WebKit, having the extra
indirection forever sounds silly.
I'd imagine WGPU* types are like uint32_t types: they're the fundamental types
this particular implementation is being built on.