Bug 190615 – Tiling CSS gradients is slow

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

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)

[Antti Koivisto]

Created attachment 352448 [details]
patch

[Tim Horton]

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?

[Antti Koivisto]

> 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?

[Antti Koivisto]

Hmm, maybe we can just sink it.

[Simon Fraser (smfr)]

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.

[Antti Koivisto]

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.

[Antti Koivisto]

Created attachment 352563 [details]
simpler patch

[EWS Watchlist]

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.

[Antti Koivisto]

Created attachment 352564 [details]
simpler patch

[Tim Horton]

Comment on attachment 352564 [details]
simpler patch

Nice! Thank you, that makes me much happier. Is it still a PLT win (I assume?)?

[Antti Koivisto]

> Nice! Thank you, that makes me much happier. Is it still a PLT win (I
> assume?)?

Hope so. Bots will tell.

[WebKit Commit Bot]

Comment on attachment 352564 [details]
simpler patch

Clearing flags on attachment: 352564

Committed r237230: <https://trac.webkit.org/changeset/237230>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45346201>

[Antti Koivisto]

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.