Bug 214515

Summary: [CG] Flushing ImageBuffer IOSurface cached image can happen when creating an image for encoding only
Product: WebKit Reporter: Said Abou-Hallawa <sabouhallawa>
Component: CanvasAssignee: Said Abou-Hallawa <sabouhallawa>
Status: RESOLVED FIXED    
Severity: Normal CC: dino, simon.fraser, thorton, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

Said Abou-Hallawa
Reported 2020-07-17 23:55:01 PDT
This is related to bug 65767 and bug 157966ز Flushing the IOSurface of the ImageBuffer after calling putImageData() is expensive. Drawing the ImageBuffer to a GraphicsContext causes the cached image of the IOSurface to be flushed. But we should flush the cached image when getting the CFData of the ImageBuffer. There is no direct command to flush the cached image of an IOSurface. We do this by drawing an empty rectangle to the context of the IOSurface.
Attachments
Patch (4.27 KB, patch)
2020-07-17 23:56 PDT, Said Abou-Hallawa
no flags
Patch (4.55 KB, patch)
2020-07-19 14:45 PDT, Said Abou-Hallawa
no flags
Patch (4.59 KB, patch)
2020-07-19 14:52 PDT, Said Abou-Hallawa
no flags
Patch (4.82 KB, patch)
2020-07-20 11:33 PDT, Said Abou-Hallawa
no flags
Said Abou-Hallawa
Comment 1 2020-07-17 23:56:58 PDT
Said Abou-Hallawa
Comment 2 2020-07-19 14:45:10 PDT
Said Abou-Hallawa
Comment 3 2020-07-19 14:49:11 PDT
This patch gives a 15% progression to the Images test on MacBookPro15,2. I would expect all macOS and iOS devices will see similar progression.
Said Abou-Hallawa
Comment 4 2020-07-19 14:49:30 PDT
Said Abou-Hallawa
Comment 5 2020-07-19 14:52:04 PDT
Simon Fraser (smfr)
Comment 6 2020-07-20 10:16:05 PDT
Comment on attachment 404688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404688&action=review > Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h:71 > + mutable bool m_pendingFlushCachedImage { false }; I think this needs a better name. Maybe m_requiresDrawAfterPutImageData or something. It's not really a "pending" (that's used for "something is going to happen in future"). Can we set m_pendingFlushCachedImage to false whenever any other CG drawing takes place as well?
Said Abou-Hallawa
Comment 7 2020-07-20 11:33:19 PDT
Said Abou-Hallawa
Comment 8 2020-07-20 11:39:20 PDT
Comment on attachment 404688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404688&action=review >> Source/WebCore/platform/graphics/cg/ImageBufferIOSurfaceBackend.h:71 >> + mutable bool m_pendingFlushCachedImage { false }; > > I think this needs a better name. Maybe m_requiresDrawAfterPutImageData or something. It's not really a "pending" (that's used for "something is going to happen in future"). > > Can we set m_pendingFlushCachedImage to false whenever any other CG drawing takes place as well? Setting m_pendingFlushCachedImage after any other CG drawing takes place will be a little bit involving because we have to hack all the CanvasRenderingContext2D drawing functions. I think we can look at this later.
EWS
Comment 9 2020-07-20 13:18:34 PDT
Committed r264615: <https://trac.webkit.org/changeset/264615> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404732 [details].
Note You need to log in before you can comment on or make changes to this bug.