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

Issue 1907273002: Remove unnecessary full-subtree layouts for filtered SVG containers (Closed)

Created:
4 years, 8 months ago by fs
Modified:
4 years, 8 months ago
Reviewers:
pdr.
CC:
fs, blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, krit, eae+blinkwatch, f(malita), gyuyoung2, jchaffraix+rendering, kouhei+svg_chromium.org, leviw+renderwatch, pdr+renderingwatchlist_chromium.org, pdr+svgwatchlist_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

Remove unnecessary full-subtree layouts for filtered SVG containers The presence of a filter on a container (or the SVG root) would force a layout of the entire subtree if any child needed a layout. This used to serve the purpose of making sure that the entire subtree would get repainted (an artifact of how 'filter' is handled on SVG elements.) With the current paint invalidation system, this should however make no practical difference, since the layout of the subtree will only end up marking descendant LayoutObjects as "maybe needing paint invalidation" - which the ones that didn't actually change don't - and hence for these nothing will be invalidated. The container itself will be invalidated via the call to SVGResourcesCache::clientLayoutChanged, which will mark it for paint invalidation if it has instantiated a filter resource. BUG=603956 Committed: https://crrev.com/b6a48975dfe23cf5721c0712052057112055cabb Cr-Commit-Position: refs/heads/master@{#389283}

Patch Set 1 #

Patch Set 2 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -12 lines) Patch
M third_party/WebKit/Source/core/layout/svg/LayoutSVGContainer.cpp View 1 1 chunk +1 line, -5 lines 0 comments Download
M third_party/WebKit/Source/core/layout/svg/LayoutSVGRoot.cpp View 1 2 chunks +2 lines, -7 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
fs
Now I've managed to convince myself (by tests and code-stepping) that these conditions are not ...
4 years, 8 months ago (2016-04-22 15:48:28 UTC) #2
pdr.
On 2016/04/22 at 15:48:28, fs wrote: > Now I've managed to convince myself (by tests ...
4 years, 8 months ago (2016-04-22 22:47:41 UTC) #3
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1907273002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1907273002/20001
4 years, 8 months ago (2016-04-22 22:55:22 UTC) #5
commit-bot: I haz the power
Committed patchset #2 (id:20001)
4 years, 8 months ago (2016-04-22 23:00:16 UTC) #6
commit-bot: I haz the power
4 years, 8 months ago (2016-04-22 23:01:47 UTC) #8
Message was sent while issue was closed.
Patchset 2 (id:??) landed as
https://crrev.com/b6a48975dfe23cf5721c0712052057112055cabb
Cr-Commit-Position: refs/heads/master@{#389283}

Powered by Google App Engine
This is Rietveld 408576698