Bug 237864

Created attachment 454646 [details]
Patch

Created attachment 454647 [details]
Patch

Comment on attachment 454647 [details]
Patch

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

> Source/WebGPU/WebGPU/Device.h:89
> +

Is there a reason this is duplicated across the whole hierarchy?
In the end there's only one item that can schedule work: the Instance.

Why are we holding device that holds adapter that holds instance instead of holding instance directly?

Comment on attachment 454647 [details]
Patch

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

>> Source/WebGPU/WebGPU/Device.h:89
>> +
> 
> Is there a reason this is duplicated across the whole hierarchy?
> In the end there's only one item that can schedule work: the Instance.
> 
> Why are we holding device that holds adapter that holds instance instead of holding instance directly?

Yeah, I think you're right that this can be more straightforward.

Objects are going to need the device pointer eventually, though, to implement the "valid to use with" check: https://gpuweb.github.io/gpuweb/#abstract-opdef-valid-to-use-with

So I think the chain can be Objects -> Device -> Instance, rather than Objects -> Other objects -> Device -> Adapter -> Instance.

Created attachment 454758 [details]
Patch

Comment on attachment 454758 [details]
Patch

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

> Source/WebGPU/WebGPU/Queue.h:61
> +    void scheduleWork(Instance::WorkItem&&);

Still, I think it's a bit strange to have the schedulework per class.


I'd imagine it'd be:
void Queue::someFunction()
{
    instance()->scheduleWork()
}

Queue::someFunction
  Instance::scheduleWork

instead of:
Queue::someFunction
  Queue::scheduleWork
    Device::scheduleWork
       Instance::scheduleWork

Comment on attachment 454758 [details]
Patch

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

>> Source/WebGPU/WebGPU/Queue.h:61
>> +    void scheduleWork(Instance::WorkItem&&);
> 
> Still, I think it's a bit strange to have the schedulework per class.
> 
> 
> I'd imagine it'd be:
> void Queue::someFunction()
> {
>     instance()->scheduleWork()
> }
> 
> Queue::someFunction
>   Instance::scheduleWork
> 
> instead of:
> Queue::someFunction
>   Queue::scheduleWork
>     Device::scheduleWork
>        Instance::scheduleWork

Right, makes sense. The Queue::scheduleWork() is valuable, I think, so I think the best option is in the middle:

Queue::someFunction
  Queue::scheduleWork
      Instance::scheduleWork

Committed r291365 (248497@trunk): <https://commits.webkit.org/248497@trunk>

<rdar://problem/90391171>