Bug 170851 - GraphicsLayerCA::recursiveCommitChanges should not descend into subtrees without changes
Summary: GraphicsLayerCA::recursiveCommitChanges should not descend into subtrees with...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on: 170876
Blocks: 170928 170938
  Show dependency treegraph
 
Reported: 2017-04-14 10:38 PDT by Antti Koivisto
Modified: 2017-04-18 03:09 PDT (History)
6 users (show)

See Also:


Attachments
patch (11.45 KB, patch)
2017-04-14 11:04 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (8.72 KB, patch)
2017-04-14 11:14 PDT, Antti Koivisto
simon.fraser: review+
Details | Formatted Diff | Diff
patch (11.62 KB, patch)
2017-04-15 04:49 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (14.89 KB, patch)
2017-04-17 06:22 PDT, Antti Koivisto
no flags Details | Formatted Diff | Diff
patch (16.55 KB, patch)
2017-04-17 06:40 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 2017-04-14 10:38:17 PDT
With lots of layers this can be very slow as it always traverses the entire layer tree. We should track which branches actually have changes.
Comment 1 Antti Koivisto 2017-04-14 11:04:39 PDT
Created attachment 307124 [details]
patch
Comment 2 Antti Koivisto 2017-04-14 11:14:05 PDT
Created attachment 307125 [details]
patch
Comment 3 Radar WebKit Bug Importer 2017-04-14 11:18:14 PDT
<rdar://problem/31631307>
Comment 4 Simon Fraser (smfr) 2017-04-14 14:51:43 PDT
Comment on attachment 307125 [details]
patch

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

Nice, I always wanted to do this.

> Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:4033
> +        if (ancestorCA.m_hasDescendantsWithUncommittedChanges)
> +            return;
> +        ancestorCA.m_hasDescendantsWithUncommittedChanges = true;

I would prefer you go through a setter and not poke the member variable directly.
Comment 5 Antti Koivisto 2017-04-15 04:49:51 PDT
Created attachment 307197 [details]
patch
Comment 6 Antti Koivisto 2017-04-15 06:16:04 PDT
https://trac.webkit.org/r215393
Comment 7 WebKit Commit Bot 2017-04-15 11:13:54 PDT
Re-opened since this is blocked by bug 170876
Comment 8 Antti Koivisto 2017-04-17 06:22:27 PDT
Created attachment 307266 [details]
patch

Enable the optimisation for Mac only for now.
Comment 9 Antti Koivisto 2017-04-17 06:40:35 PDT
Created attachment 307268 [details]
patch
Comment 10 WebKit Commit Bot 2017-04-17 08:00:36 PDT
Comment on attachment 307268 [details]
patch

Clearing flags on attachment: 307268

Committed r215410: <http://trac.webkit.org/changeset/215410>
Comment 11 WebKit Commit Bot 2017-04-17 08:00:38 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Simon Fraser (smfr) 2017-04-17 13:54:31 PDT
That's unfortunate. Do we understand the iOS issue?
Comment 13 Antti Koivisto 2017-04-18 00:12:55 PDT
I think so. I just wanted to do some more testing on iOS before enabling it.