No need to store the backend parameters in RemoteImageBufferProxy. They can be accessed from the current ImageBufferBackend via the ImageBuffer before the backend is cleared.
Created attachment 414293 [details] Patch
Comment on attachment 414293 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414293&action=review > Source/WebKit/WebProcess/GPU/graphics/RemoteImageBufferProxy.h:187 > + void commitFlushDisplayList(WebCore::DisplayList::FlushIdentifier flushIdentifier) override We need to stop using "commit" and "flush" in ways we don't all understand.
IMO (but obv. up for discussion): "submit" for "send display list/work to GPUP", like Wenson fixed elsewhere "flush" is "wait till all submitted work has actually been completed" "commit" should be just for the layer tree transactions (but is effectively the same as "submit" for a transaction) and no combination of the above means anything so we shouldn't see them adjacent to each other :)
Created attachment 414861 [details] Patch
Created attachment 414879 [details] Patch
Created attachment 414880 [details] Patch
Created attachment 414885 [details] Patch
<rdar://problem/71747762>
Created attachment 414912 [details] Patch
Created attachment 414935 [details] Patch
Comment on attachment 414935 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=414935&action=review > Source/WebCore/platform/graphics/ConcreteImageBuffer.h:297 > + std::unique_ptr<ImageBufferBackend::Parameters> m_parameters; Why not just store a copy of the tiny struct? Much simpler. > Source/WebCore/platform/graphics/ImageBufferBackend.h:166 > + const Parameters& m_parameters; Who owns the original data? Would be safer to just store by value.
Created attachment 415043 [details] Patch
Created attachment 415045 [details] Patch
Created attachment 415046 [details] Patch
Created attachment 415050 [details] Patch
ImageBufferBackend::Parameters are saved by value in ImageBufferBackend and ConcreteImageBuffer. Renaming the flush/commit/flush functions was removed from this patch and will be addressed in a different patch.
Created attachment 415395 [details] Patch
Comment on attachment 415395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415395&action=review > Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:124 > + auto backendSize = this->backendSize(); Doesn't seem worthwhile storing this in a local. > Source/WebCore/platform/graphics/cg/ImageBufferCGBitmapBackend.cpp:108 > + return { static_cast<int>(CGBitmapContextGetWidth(context().platformContext())), static_cast<int>(CGBitmapContextGetHeight(context().platformContext())) }; Let's not fetch context().platformContext() twice.
Created attachment 415434 [details] Patch
Created attachment 415445 [details] Patch
Comment on attachment 415395 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=415395&action=review >> Source/WebCore/platform/graphics/cg/ImageBufferCGBackend.cpp:124 >> + auto backendSize = this->backendSize(); > > Doesn't seem worthwhile storing this in a local. Actually it is worthy. sinkIntoNativeImage() destroys the backend. So we can't call backendSize() after calling sinkIntoNativeImage(). I put it back and I added a comment.
Committed r270458: <https://trac.webkit.org/changeset/270458> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415445 [details].