Summary: | [WebGPU] Start preparing for serializing commands to the GPU process | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Myles C. Maxfield <mmaxfield> | ||||
Component: | New Bugs | Assignee: | Myles C. Maxfield <mmaxfield> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | achristensen, ross.kirsling, webkit-bug-importer | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Myles C. Maxfield
2021-11-16 01:19:27 PST
Created attachment 444358 [details]
Patch
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? 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. Committed r285881 (244307@main): <https://commits.webkit.org/244307@main> This broke WinCairo, as your cq- shows... :( |