Summary: | Introduce RemoteDisplayListRecorderProxy and RemoteDisplayListRecorder | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Wenson Hsieh <wenson_hsieh> | ||||||||||||
Component: | WebKit Misc. | Assignee: | Wenson Hsieh <wenson_hsieh> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | annulen, ews-watchlist, gyuyoung.kim, mmaxfield, ryuan.choi, sabouhallawa, sergio, simon.fraser, thorton, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=231980 | ||||||||||||||
Bug Depends on: | |||||||||||||||
Bug Blocks: | 230860 | ||||||||||||||
Attachments: |
|
Description
Wenson Hsieh
2021-10-09 17:06:47 PDT
Created attachment 440739 [details]
For EWS
Created attachment 440740 [details]
For EWS
Created attachment 440745 [details]
For EWS
Created attachment 440746 [details]
Fix typo
Comment on attachment 440746 [details] Fix typo View in context: https://bugs.webkit.org/attachment.cgi?id=440746&action=review > Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:46 > + static Ref<RemoteDisplayListRecorder> create(WebCore::ImageBuffer& imageBuffer, QualifiedRenderingResourceIdentifier imageBufferIdentifier, RemoteRenderingBackend& renderingBackend) Can this ImageBuffer type be template-qualified so that we know at compile time that it's the right kind of image buffer? > Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:52 > + void startListeningForIPC(); > + void stopListeningForIPC(); These feel a bit weird on this API. > Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h:42 > + RemoteDisplayListRecorderProxy(WebCore::ImageBuffer&, RemoteRenderingBackendProxy&, const WebCore::FloatRect& initialClip, const WebCore::AffineTransform&, WebCore::DrawGlyphsRecorder::DeconstructDrawGlyphs = WebCore::DrawGlyphsRecorder::DeconstructDrawGlyphs::Yes); Same question: can ImageBuffer be specialized? It's a concrete image buffer here, right? > Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h:45 > + ~RemoteDisplayListRecorderProxy() { } = default? Comment on attachment 440746 [details] Fix typo View in context: https://bugs.webkit.org/attachment.cgi?id=440746&action=review Thanks for the review! >> Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:46 >> + static Ref<RemoteDisplayListRecorder> create(WebCore::ImageBuffer& imageBuffer, QualifiedRenderingResourceIdentifier imageBufferIdentifier, RemoteRenderingBackend& renderingBackend) > > Can this ImageBuffer type be template-qualified so that we know at compile time that it's the right kind of image buffer? I think it's possible, but then we'll need to templatize RemoteDisplayListRecorder with an ImageBufferBackendType as well, since the remote image buffer could be a AcceleratedRemoteImageBuffer or UnacceleratedRemoteImageBuffer. This means we'll need to teach the IPC message receiver generator to take template arguments for a given message receiver, such that the generated message receiver implementation can take this form: ``` template <typename ImageBufferBackendType> void RemoteDisplayListRecorder<ImageBufferBackendType>::didReceiveStreamMessage(IPC::StreamServerConnectionBase& connection, IPC::Decoder& decoder) ``` Let's tackle this separately in a followup bug. >> Source/WebKit/GPUProcess/graphics/RemoteDisplayListRecorder.h:52 >> + void stopListeningForIPC(); > > These feel a bit weird on this API. I believe `(start|stop)ListeningForIPC` is the established pattern for setting up and tearing down IPC message receivers in the GPU Process; but now that I think more about it, we only need to expose `stopListeningForIPC()` here to make it work with ScopedActiveMessageReceiveQueue, since we should always `startListeningForIPC()` right after creating this object anyways. I'll adjust this so that `startListeningForIPC()` is just a private helper method that's invoked in `RemoteDisplayListRecorder::create`, which is consistent with `RemoteRenderingBackend::create`. >> Source/WebKit/WebProcess/GPU/graphics/RemoteDisplayListRecorderProxy.h:45 >> + ~RemoteDisplayListRecorderProxy() { } > > = default? Fixed! Created attachment 440824 [details]
For EWS
Committed r283941 (242800@main): <https://commits.webkit.org/242800@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 440824 [details]. |