Bug 190615 - Tiling CSS gradients is slow
Summary: Tiling CSS gradients is slow
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-16 06:46 PDT by Antti Koivisto
Modified: 2018-10-21 02:55 PDT (History)
6 users (show)

See Also:


Attachments
patch (14.83 KB, patch)
2018-10-16 07:00 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
simpler patch (3.76 KB, patch)
2018-10-17 05:15 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
simpler patch (4.54 KB, patch)
2018-10-17 05:57 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
simpler patch (4.58 KB, patch)
2018-10-17 06:11 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Antti Koivisto 2018-10-16 06:46:12 PDT
Painting blocks the main thread on CG rendering queue to make copy of the backing store:

kevent_id
_dispatch_event_loop_wait_for_ownership
__DISPATCH_WAIT_FOR_QUEUE__
_dispatch_sync_f_slow
CA::CG::IOSurfaceDrawable::copy_cgimage()
WebCore::IOSurface::createImage()
WebCore::ImageBuffer::copyNativeImage(WebCore::BackingStoreCopy) const
WebCore::ImageBuffer::copyImage(WebCore::BackingStoreCopy, WebCore::PreserveResolution) const
WebCore::ImageBuffer::drawPattern(WebCore::GraphicsContext&, WebCore::FloatRect const&, WebCore::FloatRect const&, WebCore::AffineTransform const&, WebCore::FloatPoint const&, WebCore::FloatSize const&, WebCore::CompositeOperator, WebCore::BlendMode)
WebCore::GradientImage::drawPattern(WebCore::GraphicsContext&, WebCore::FloatRect const&, WebCore::FloatRect const&, WebCore::AffineTransform const&, WebCore::FloatPoint const&, WebCore::FloatSize const&, WebCore::CompositeOperator, WebCore::BlendMode)
WebCore::Image::drawTiled(WebCore::GraphicsContext&, WebCore::FloatRect const&, WebCore::FloatPoint const&, WebCore::FloatSize const&, WebCore::FloatSize const&, WebCore::CompositeOperator, WebCore::BlendMode, WebCore::DecodingMode)
Comment 1 Antti Koivisto 2018-10-16 07:00:39 PDT
Created attachment 352448 [details]
patch
Comment 2 Tim Horton 2018-10-16 10:55:12 PDT
Comment on attachment 352448 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352448&action=review

> Source/WebCore/platform/graphics/cocoa/IOSurface.h:111
> +    WEBCORE_EXPORT RetainPtr<CGImageRef> imageReference();

I don't love it, the sinkIntoImage approach made it much harder to do The Wrong Thing. But I guess you need to keep the surface around?
Comment 3 Antti Koivisto 2018-10-16 11:14:05 PDT
> I don't love it, the sinkIntoImage approach made it much harder to do The
> Wrong Thing. But I guess you need to keep the surface around?

I could throw away the IOSurface but presumably drawing IOSurface backed CGImage to an accelerated context is faster?
Comment 4 Antti Koivisto 2018-10-16 11:32:32 PDT
Hmm, maybe we can just sink it.
Comment 5 Simon Fraser (smfr) 2018-10-16 14:13:58 PDT
Comment on attachment 352448 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=352448&action=review

> Source/WebCore/platform/graphics/ImageBuffer.h:84
> +    void makeImmutable() { m_isImmutable = true; }

Would be nice to have the corresponding const getter.
Comment 6 Antti Koivisto 2018-10-17 05:15:35 PDT
Created attachment 352561 [details]
simpler patch

This ditches the "immutable ImageBuffer" concept and simply sinks the gradient ImageBuffer into an Image and uses that for caching instead.
Comment 7 Antti Koivisto 2018-10-17 05:57:43 PDT
Created attachment 352563 [details]
simpler patch
Comment 8 EWS Watchlist 2018-10-17 06:00:15 PDT
Attachment 352563 [details] did not pass style-queue:


ERROR: Source/WebCore/ChangeLog:10:  Need whitespace between colon and description  [changelog/filechangedescriptionwhitespace] [5]
Total errors found: 1 in 4 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Antti Koivisto 2018-10-17 06:11:34 PDT
Created attachment 352564 [details]
simpler patch
Comment 10 Tim Horton 2018-10-17 10:06:16 PDT
Comment on attachment 352564 [details]
simpler patch

Nice! Thank you, that makes me much happier. Is it still a PLT win (I assume?)?
Comment 11 Antti Koivisto 2018-10-17 11:35:23 PDT
> Nice! Thank you, that makes me much happier. Is it still a PLT win (I
> assume?)?

Hope so. Bots will tell.
Comment 12 WebKit Commit Bot 2018-10-17 12:00:11 PDT
Comment on attachment 352564 [details]
simpler patch

Clearing flags on attachment: 352564

Committed r237230: <https://trac.webkit.org/changeset/237230>
Comment 13 WebKit Commit Bot 2018-10-17 12:00:13 PDT
All reviewed patches have been landed.  Closing bug.
Comment 14 Radar WebKit Bug Importer 2018-10-17 12:01:47 PDT
<rdar://problem/45346201>
Comment 15 Antti Koivisto 2018-10-21 02:55:29 PDT
Bots indicate that this was >1% progression

https://perf-safari.apple.com/v3/#/charts?paneList=((886-1158))&since=1539416881058

Most PLT pages don't have gradients, the ones that were affected got 5-6% faster.