Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2498763002:
Allow subsequence caching for SVG roots that have PaintLayers. (Closed)
DescriptionAllow subsequence caching for SVG roots that have PaintLayers.
SVG subtrees paint atomically, which is all that is needed to get
this feature.
BUG=663953
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Committed: https://crrev.com/a77dfb5865c1fc52c2094acba37fea629e26ccf5
Cr-Commit-Position: refs/heads/master@{#431973}
Patch Set 1 #Patch Set 2 : none #
Total comments: 2
Patch Set 3 : none #Patch Set 4 : Merge branch 'master' into svgroot #Patch Set 5 : none #
Messages
Total messages: 23 (13 generated)
Description was changed from ========== none none BUG= ========== to ========== none none BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
Description was changed from ========== none none BUG= CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Allow subsequence caching for SVG roots that have PaintLayers. SVG subtrees paint atomically, which is all that is needed to get this feature. BUG=663953 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ==========
chrishtr@chromium.org changed reviewers: + pdr@chromium.org, wkorman@chromium.org
The CQ bit was checked by chrishtr@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
lgtm This seems worth doing in any case but I am curious re: the in person discussion you and pdr@ were having a day or two ago, the janky case from http://crbug.com/656320 looked like it was an: img src="foo.svg" and pdr@ seemed to think that meant that subsequence caching would not help with jank. What is our updated thinking/conclusion? https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right): https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:155: !paintLayer.layoutObject()->isSVGRoot()) Consider adding comment above this block re: why we allow for SVGRoot here and below.
On 2016/11/12 at 00:05:51, wkorman wrote: > lgtm > > This seems worth doing in any case but I am curious re: the in person discussion you and pdr@ were having a day or two ago, the janky case from http://crbug.com/656320 looked like it was an: img src="foo.svg" and pdr@ seemed to think that meant that subsequence caching would not help with jank. > > What is our updated thinking/conclusion? That case is solved by https://codereview.chromium.org/2500533003. That use case doesn't require this patch (though it doesn't hurt), because the LayoutView of the SVGImage "document" has subsequence caching. I don't remember a conclusion about it still being janky no matter what; pdr do you remember? > > https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... > File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right): > > https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... > third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:155: !paintLayer.layoutObject()->isSVGRoot()) > Consider adding comment above this block re: why we allow for SVGRoot here and below.
The CQ bit was checked by chrishtr@chromium.org
https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp (right): https://codereview.chromium.org/2498763002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/core/paint/PaintLayerPainter.cpp:155: !paintLayer.layoutObject()->isSVGRoot()) On 2016/11/12 at 00:05:51, wkorman wrote: > Consider adding comment above this block re: why we allow for SVGRoot here and below. Done.
The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/2498763002/#ps40001 (title: "none")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by chrishtr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/2498763002/#ps60001 (title: "Merge branch 'master' into svgroot")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by chrishtr@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from wkorman@chromium.org Link to the patchset: https://codereview.chromium.org/2498763002/#ps80001 (title: "none")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #5 (id:80001)
Message was sent while issue was closed.
Description was changed from ========== Allow subsequence caching for SVG roots that have PaintLayers. SVG subtrees paint atomically, which is all that is needed to get this feature. BUG=663953 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 ========== to ========== Allow subsequence caching for SVG roots that have PaintLayers. SVG subtrees paint atomically, which is all that is needed to get this feature. BUG=663953 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Committed: https://crrev.com/a77dfb5865c1fc52c2094acba37fea629e26ccf5 Cr-Commit-Position: refs/heads/master@{#431973} ==========
Message was sent while issue was closed.
Patchset 5 (id:??) landed as https://crrev.com/a77dfb5865c1fc52c2094acba37fea629e26ccf5 Cr-Commit-Position: refs/heads/master@{#431973} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
