Bug 214515 - [CG] Flushing ImageBuffer IOSurface cached image can happen when creating an image for encoding only
Summary: [CG] Flushing ImageBuffer IOSurface cached image can happen when creating an ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Canvas (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Said Abou-Hallawa
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-07-17 23:55 PDT by Said Abou-Hallawa
Modified: 2020-07-20 13:18 PDT (History)
4 users (show)

See Also:


Attachments
Patch (4.27 KB, patch)
2020-07-17 23:56 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.55 KB, patch)
2020-07-19 14:45 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.59 KB, patch)
2020-07-19 14:52 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff
Patch (4.82 KB, patch)
2020-07-20 11:33 PDT, Said Abou-Hallawa
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Said Abou-Hallawa 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.
Comment 1 Said Abou-Hallawa 2020-07-17 23:56:58 PDT
Created attachment 404637 [details]
Patch
Comment 2 Said Abou-Hallawa 2020-07-19 14:45:10 PDT
Created attachment 404687 [details]
Patch
Comment 3 Said Abou-Hallawa 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.
Comment 4 Said Abou-Hallawa 2020-07-19 14:49:30 PDT
<rdar://problem/65735991>
Comment 5 Said Abou-Hallawa 2020-07-19 14:52:04 PDT
Created attachment 404688 [details]
Patch
Comment 6 Simon Fraser (smfr) 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?
Comment 7 Said Abou-Hallawa 2020-07-20 11:33:19 PDT
Created attachment 404732 [details]
Patch
Comment 8 Said Abou-Hallawa 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.
Comment 9 EWS 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].