Bug 130768 – [iOS WebKit2] Flush all surfaces after painting into all of them, instead of after painting into each one

• Component: WebKit2
• Status: RESOLVED FIXED

[Tim Horton]

The remote layer tree is effectively painting layers serially at the moment, as RemoteLayerBackingStore::drawInContext first paints into the layer, then flushes it.

To increase parallelism and hopefully increase painting performance, we should first paint all of the changed backing stores, then flush them all.

Ideally in the future we'd move that flush off onto another thread so we can move forward with the next frame while flushing, but that's for another patch.

<rdar://problem/16421471>

[Tim Horton]

Created attachment 227832 [details]
patch

[Tim Horton]

http://trac.webkit.org/changeset/166296

[WebKit Commit Bot]

Re-opened since this is blocked by bug 130822

[Tim Horton]

Created attachment 227934 [details]
patch

[Tim Horton]

I need to double check what happens with clones.

[Tim Horton]

Comment on attachment 227934 [details]
patch

Clones are broken.

[Tim Horton]

Created attachment 228210 [details]
patch (will break clones but fix in a follow-up)

[Tim Horton]

Created attachment 228213 [details]
maybe rebase

[Benjamin Poulain]

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

> Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h:46
>  class RemoteLayerBackingStore {
> +    WTF_MAKE_NONCOPYABLE(RemoteLayerBackingStore);

How come this is not Fast Allocated?

> Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm:337
> +    if (!m_frontContextPendingFlush)
> +        return;
> +
> +    CGContextFlush(m_frontContextPendingFlush.get());
> +    m_frontContextPendingFlush = nullptr;

Maybe?

if (m_frontContextPendingFlush) {
    CGContextFlush(m_frontContextPendingFlush.get());
    m_frontContextPendingFlush = nullptr;
}

> Source/WebKit2/Shared/mac/RemoteLayerTreeTransaction.mm:418
> +    for (const auto& layer : m_changedLayers) {

"auto" sucks here, no way to guess the type.

> Source/WebKit2/WebProcess/WebPage/mac/PlatformCALayerRemote.h:154
> +    RemoteLayerTreeTransaction::LayerProperties& properties() { return m_properties; }

You could have a const version.

> Source/WebKit2/WebProcess/WebPage/mac/RemoteLayerTreeDrawingArea.mm:310
> +    for (auto& layer : transaction.changedLayers()) {

auto sucks here too :(

[Benjamin Poulain]

Comment on attachment 228213 [details]
maybe rebase

r+ with the comments from the previous patch.

[Tim Horton]

http://trac.webkit.org/changeset/166545