Bug 170851

Summary: GraphicsLayerCA::recursiveCommitChanges should not descend into subtrees without changes
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: Layout and RenderingAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, simon.fraser, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 170876    
Bug Blocks: 170928, 170938    
Attachments:
Description Flags
patch
none
patch
simon.fraser: review+
patch
none
patch
none
patch none

Antti Koivisto
Reported 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.
Attachments
patch (11.45 KB, patch)
2017-04-14 11:04 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (8.72 KB, patch)
2017-04-14 11:14 PDT, Antti Koivisto 		simon.fraser: review+
DetailsFormatted DiffDiff
patch (11.62 KB, patch)
2017-04-15 04:49 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (14.89 KB, patch)
2017-04-17 06:22 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
patch (16.55 KB, patch)
2017-04-17 06:40 PDT, Antti Koivisto 		no flags
DetailsFormatted DiffDiff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.
Antti Koivisto
Comment 1 2017-04-14 11:04:39 PDT
Created attachment 307124 [details] patch
Antti Koivisto
Comment 2 2017-04-14 11:14:05 PDT
Created attachment 307125 [details] patch
Radar WebKit Bug Importer
Comment 3 2017-04-14 11:18:14 PDT
<rdar://problem/31631307>
Simon Fraser (smfr)
Comment 4 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.
Antti Koivisto
Comment 5 2017-04-15 04:49:51 PDT
Created attachment 307197 [details] patch
Antti Koivisto
Comment 6 2017-04-15 06:16:04 PDT
https://trac.webkit.org/r215393
WebKit Commit Bot
Comment 7 2017-04-15 11:13:54 PDT
Re-opened since this is blocked by bug 170876
Antti Koivisto
Comment 8 2017-04-17 06:22:27 PDT
Created attachment 307266 [details] patch Enable the optimisation for Mac only for now.
Antti Koivisto
Comment 9 2017-04-17 06:40:35 PDT
Created attachment 307268 [details] patch
WebKit Commit Bot
Comment 10 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>
WebKit Commit Bot
Comment 11 2017-04-17 08:00:38 PDT
All reviewed patches have been landed. Closing bug.
Simon Fraser (smfr)
Comment 12 2017-04-17 13:54:31 PDT
That's unfortunate. Do we understand the iOS issue?
Antti Koivisto
Comment 13 2017-04-18 00:12:55 PDT
I think so. I just wanted to do some more testing on iOS before enabling it.
Note You need to log in before you can comment on or make changes to this bug.