Bug 233358 - [WebGPU] Implement GPU process's message receivers
Summary: [WebGPU] Implement GPU process's message receivers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-11-19 04:26 PST by Myles C. Maxfield
Modified: 2021-11-19 18:34 PST (History)
2 users (show)

See Also:


Attachments
Patch (181.72 KB, patch)
2021-11-19 04:34 PST, Myles C. Maxfield
dino: review+
Details | Formatted Diff | Diff
Patch for committing (187.02 KB, patch)
2021-11-19 14:56 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Patch for committing (187.08 KB, patch)
2021-11-19 15:13 PST, Myles C. Maxfield
mmaxfield: commit-queue-
Details | Formatted Diff | Diff
Patch for committing (190.46 KB, patch)
2021-11-19 16:31 PST, Myles C. Maxfield
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Myles C. Maxfield 2021-11-19 04:26:00 PST
[WebGPU] Implement GPU process's message receivers
Comment 1 Myles C. Maxfield 2021-11-19 04:34:06 PST
Created attachment 444800 [details]
Patch
Comment 2 Dean Jackson 2021-11-19 10:03:15 PST
Comment on attachment 444800 [details]
Patch

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

> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteBuffer.cpp:52
> +    // FIXME: Implement this.

ASSERT_NOT_IMPLEMENTED()?

> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteCommandEncoder.cpp:72
> +    auto computePassEncoder = m_backing->beginComputePass(*convertedDescriptor);

What happens if convertedDescriptor still doesn't have a value at this point?

> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteCommandEncoder.cpp:88
> +    auto convertedSource = m_objectRegistry.convertBufferFromBacking(source);
> +    ASSERT(convertedSource);
> +    auto convertedDestination = m_objectRegistry.convertBufferFromBacking(source);
> +    ASSERT(convertedDestination);
> +    m_backing->copyBufferToBuffer(*convertedSource, sourceOffset, *convertedDestination, destinationOffset, size);

Should this (and the other similar code) actually test that the converted buffers exist rather than just ASSERT?
Comment 3 Myles C. Maxfield 2021-11-19 13:14:32 PST
Comment on attachment 444800 [details]
Patch

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

>> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteCommandEncoder.cpp:72
>> +    auto computePassEncoder = m_backing->beginComputePass(*convertedDescriptor);
> 
> What happens if convertedDescriptor still doesn't have a value at this point?

RemoteComputePassEncoder::create takes an optional. If the input was nullopt, we should be passing nullopt to the consumer, but if the input wasn't nullopt, we should verify that it could be converted successfully.

>> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteCommandEncoder.cpp:88
>> +    m_backing->copyBufferToBuffer(*convertedSource, sourceOffset, *convertedDestination, destinationOffset, size);
> 
> Should this (and the other similar code) actually test that the converted buffers exist rather than just ASSERT?

oh, you mean only call the backing function if the lookup was successful? I'll update the patch to do that.
Comment 4 Myles C. Maxfield 2021-11-19 14:56:22 PST
Created attachment 444858 [details]
Patch for committing
Comment 5 Myles C. Maxfield 2021-11-19 15:07:19 PST
Comment on attachment 444800 [details]
Patch

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

>> Source/WebKit/GPUProcess/graphics/WebGPU/RemoteBuffer.cpp:52
>> +    // FIXME: Implement this.
> 
> ASSERT_NOT_IMPLEMENTED()?

It looks like doing this forces adding [[ noreturn ]] to the function signature in the header, which I think is pollution. I'll just implement this ASAP.
Comment 6 Myles C. Maxfield 2021-11-19 15:13:32 PST
Created attachment 444862 [details]
Patch for committing
Comment 7 Myles C. Maxfield 2021-11-19 16:31:35 PST
Created attachment 444867 [details]
Patch for committing
Comment 8 EWS 2021-11-19 18:33:50 PST
Committed r286087 (244474@main): <https://commits.webkit.org/244474@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 444867 [details].
Comment 9 Radar WebKit Bug Importer 2021-11-19 18:34:23 PST
<rdar://problem/85627098>