Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(41)

Issue 1996543002: Avoid using forced layout to trigger paint invalidation for SVG containers (Closed)

Created:
4 years, 7 months ago by fs
Modified:
4 years, 6 months ago
Reviewers:
chrishtr, Xianzhu, pdr.
CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-platform-graphics_chromium.org, Rik, chromium-reviews, danakj+watch_chromium.org, dshwang, drott+blinkwatch_chromium.org, krit, eae+blinkwatch, f(malita), gyuyoung2, jbroman, jchaffraix+rendering, Justin Novosad, kinuko+watch, kouhei+svg_chromium.org, leviw+renderwatch, pdr+svgwatchlist_chromium.org, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rwlbuis, Stephen Chennney, szager+layoutwatch_chromium.org, zoltan1
Base URL:
https://chromium.googlesource.com/chromium/src.git@master
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Avoid using forced layout to trigger paint invalidation for SVG containers Currently, SVG containers in the LayoutObject hierarchy force layout of their children if the transform changes. The main reason for this is to trigger paint invalidation of the subtree. In some cases - changes to the scale factor - there are other reasons to trigger layout, like computing a new scale factor for <text> or re-layout nodes with non-scaling stroke. Compute a "scale-factor change" in addition to the "transform change" already computed, then use this new signal to determine if layout should be forced for the subtree. Trigger paint invalidation using the LayoutObject flags instead. The downside to this is that paint invalidation will walk into "hidden" containers which rarely require repaint (since they are not technically visible). This will hopefully be rectified in a follow-up CL. For the testcase from 603850, this essentially eliminates the cost of layout (from ~350ms to ~0ms on authors machine; layout cost is related to text metrics recalculation), bumping frame rate significantly. BUG=603956, 603850 Committed: https://crrev.com/44f1431b20c16d8f8da0ce8ff7bbf2adddcdd785 Cr-Commit-Position: refs/heads/master@{#400950}

Patch Set 1 #

Patch Set 2 : Rebased and changed scale-change detection #

Patch Set 3 : Rebase #

Patch Set 4 : NeedsRebaseline -> NeedsManualRebaseline #

Patch Set 5 : Rebase #

Total comments: 5

Patch Set 6 : Use enum instead of bitfield #

Total comments: 2

Patch Set 7 : Add comment #

Patch Set 8 : TE updates #

Unified diffs Side-by-side diffs Delta from patch set Stats (+81 lines, -21 lines) Patch
M third_party/WebKit/LayoutTests/TestExpectations View 1 2 3 4 5 6 7 2 chunks +9 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h View 1 2 3 4 5 6 1 chunk +10 lines, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp View 1 2 3 4 5 2 chunks +4 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceMarker.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGResourceMarker.cpp View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGTransformableContainer.h View 1 2 3 4 5 1 chunk +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGTransformableContainer.cpp View 1 2 3 4 5 2 chunks +20 lines, -4 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGViewportContainer.h View 1 2 3 4 5 2 chunks +2 lines, -2 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGViewportContainer.cpp View 1 2 3 4 5 1 chunk +16 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/SVGLayoutSupport.cpp View 1 2 3 4 5 6 7 1 chunk +1 line, -1 line 0 comments Download
M third_party/WebKit/Source/platform/transforms/AffineTransform.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/transforms/AffineTransform.cpp View 1 1 chunk +12 lines, -2 lines 0 comments Download

Messages

Total messages: 17 (6 generated)
fs
4 years, 6 months ago (2016-06-14 15:16:38 UTC) #4
Xianzhu
Is there try result of a patch set showing the change of test expectations? Or ...
4 years, 6 months ago (2016-06-14 16:52:37 UTC) #5
fs
On 2016/06/14 at 16:52:37, wangxianzhu wrote: > Is there try result of a patch set ...
4 years, 6 months ago (2016-06-14 18:02:23 UTC) #6
pdr.
Super cool https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h (right): https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h#newcode73 third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h:73: enum TransformChangeFlags { Returning just ScaleChange from ...
4 years, 6 months ago (2016-06-15 12:15:17 UTC) #7
fs
https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h (right): https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h#newcode73 third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h:73: enum TransformChangeFlags { On 2016/06/15 at 12:15:17, pdr. wrote: ...
4 years, 6 months ago (2016-06-15 22:10:33 UTC) #8
fs
https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h (right): https://codereview.chromium.org/1996543002/diff/80001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h#newcode73 third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h:73: enum TransformChangeFlags { On 2016/06/15 at 22:10:33, fs wrote: ...
4 years, 6 months ago (2016-06-20 11:48:49 UTC) #9
pdr.
LGTM with or without a comment. https://codereview.chromium.org/1996543002/diff/100001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h (right): https://codereview.chromium.org/1996543002/diff/100001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h#newcode73 third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h:73: enum class TransformChange ...
4 years, 6 months ago (2016-06-21 00:13:04 UTC) #10
fs
https://codereview.chromium.org/1996543002/diff/100001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h File third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h (right): https://codereview.chromium.org/1996543002/diff/100001/third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h#newcode73 third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.h:73: enum class TransformChange { On 2016/06/21 at 00:13:04, pdr. ...
4 years, 6 months ago (2016-06-21 08:24:45 UTC) #11
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1996543002/140001
4 years, 6 months ago (2016-06-21 11:05:11 UTC) #14
commit-bot: I haz the power
Committed patchset #8 (id:140001)
4 years, 6 months ago (2016-06-21 11:11:42 UTC) #15
commit-bot: I haz the power
4 years, 6 months ago (2016-06-21 11:13:01 UTC) #17
Message was sent while issue was closed.
Patchset 8 (id:??) landed as
https://crrev.com/44f1431b20c16d8f8da0ce8ff7bbf2adddcdd785
Cr-Commit-Position: refs/heads/master@{#400950}

Powered by Google App Engine
This is Rietveld 408576698