Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1825193002:
Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. (Closed)
|
Created:
4 years, 9 months ago by wkorman Modified:
4 years, 9 months ago Reviewers:
chrishtr CC:
blink-reviews, blink-reviews-layout_chromium.org, blink-reviews-paint_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), jbroman, jchaffraix+rendering, Justin Novosad, kinuko+watch, leviw+renderwatch, pdr+graphicswatchlist_chromium.org, pdr+renderingwatchlist_chromium.org, rwlbuis, Stephen Chennney, slimming-paint-reviews_chromium.org, 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. |
DescriptionMake GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor.
Currently, the DisplayItemClient is LayoutScrollbarPart. This has two
problems: the mapping between the LayoutScrollbarPart and its backing
is hard to reverse-engineer given the way the scrollbar code is set
up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the
fact that LayoutScrollbarPart can be painted in both non-composited
and composited modes. This distinction is important for computation
of correct visual rects.
This can be cleaned up by replaying the painted scrollbar content
with the GraphicsLayer backing as the DisplayItemClient, which
has the correct visualRect() (i.e. bounds of GraphicsLayer).
BUG=529938
Committed: https://crrev.com/8460ef8fc6125185abc1486eb61b0ca7fc9298ad
Cr-Commit-Position: refs/heads/master@{#382778}
Patch Set 1 #
Total comments: 5
Patch Set 2 : Code review feedback. #Patch Set 3 : Fix context for scroll corner. #Patch Set 4 : Sync to head. #Patch Set 5 : Disable caching. #Patch Set 6 : Use cached drawings when possible. #
Messages
Total messages: 40 (21 generated)
wkorman@chromium.org changed reviewers: + chrishtr@chromium.org
The CQ bit was checked by wkorman@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/1 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/1
Better description: Make GraphicsLayer the DisplayItemClient for scrollbars which are composited through PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of currect visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing it as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer).
Description was changed from ========== Wrap composited scrollbar in an SkPicture in graphics layer space. To allow for eventual querying via an rtree in cc. BUG=529938 ========== to ========== Make GraphicsLayer the DisplayItemClient for scrollbars which are composited through PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ==========
Description was changed from ========== Make GraphicsLayer the DisplayItemClient for scrollbars which are composited through PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ========== to ========== Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ==========
Description was changed from ========== Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ========== to ========== Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ==========
Updated description.
https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp (right): https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:802: SkPictureBuilder pictureBuilder(layerBounds, nullptr, &context); Factor the SkPictureBuilder into the calling function in order to: - Fix the comment on line 832 - Avoid errors about choosing the correct GraphicsContext. https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:817: pictureBuilder.endRecording()->playback(context.canvas()); Add a comment about why there is replaying into a parent context. https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp (right): https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/paint/PaintInvalidationCapableScrollableArea.cpp:70: graphicsLayer->invalidateDisplayItemClient(*graphicsLayer, PaintInvalidationScroll); Document.
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... File third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp (right): https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:802: SkPictureBuilder pictureBuilder(layerBounds, nullptr, &context); On 2016/03/22 at 19:45:47, chrishtr wrote: > Factor the SkPictureBuilder into the calling function in order to: > > - Fix the comment on line 832 > - Avoid errors about choosing the correct GraphicsContext. Done. https://codereview.chromium.org/1825193002/diff/1/third_party/WebKit/Source/c... third_party/WebKit/Source/core/layout/compositing/PaintLayerCompositor.cpp:817: pictureBuilder.endRecording()->playback(context.canvas()); On 2016/03/22 at 19:45:47, chrishtr wrote: > Add a comment about why there is replaying into a parent context. Done.
lgtm
The CQ bit was checked by wkorman@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/20001
The CQ bit was unchecked by wkorman@chromium.org
The CQ bit was checked by wkorman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1825193002/#ps40001 (title: "Fix context for scroll corner.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)
The CQ bit was checked by wkorman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1825193002/#ps60001 (title: "Sync to head.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_blink_oilpan_rel on tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_blink_oil...)
The CQ bit was checked by wkorman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1825193002/#ps80001 (title: "Disable caching.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/80001
The CQ bit was unchecked by wkorman@chromium.org
The CQ bit was checked by wkorman@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/80001
The CQ bit was checked by wkorman@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from chrishtr@chromium.org Link to the patchset: https://codereview.chromium.org/1825193002/#ps100001 (title: "Use cached drawings when possible.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1825193002/100001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1825193002/100001
Message was sent while issue was closed.
Committed patchset #6 (id:100001)
Message was sent while issue was closed.
Description was changed from ========== Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 ========== to ========== Make GraphicsLayer the DisplayItemClient for scrollbars composited via PaintLayerCompositor. Currently, the DisplayItemClient is LayoutScrollbarPart. This has two problems: the mapping between the LayoutScrollbarPart and its backing is hard to reverse-engineer given the way the scrollbar code is set up (LayoutScrollbarPart vs Scrollbar vs ScrollableArea), and the fact that LayoutScrollbarPart can be painted in both non-composited and composited modes. This distinction is important for computation of correct visual rects. This can be cleaned up by replaying the painted scrollbar content with the GraphicsLayer backing as the DisplayItemClient, which has the correct visualRect() (i.e. bounds of GraphicsLayer). BUG=529938 Committed: https://crrev.com/8460ef8fc6125185abc1486eb61b0ca7fc9298ad Cr-Commit-Position: refs/heads/master@{#382778} ==========
Message was sent while issue was closed.
Patchset 6 (id:??) landed as https://crrev.com/8460ef8fc6125185abc1486eb61b0ca7fc9298ad Cr-Commit-Position: refs/heads/master@{#382778}
Message was sent while issue was closed.
A revert of this CL (patchset #6 id:100001) has been created in https://codereview.chromium.org/1839533002/ by nednguyen@google.com. The reason for reverting is: Speculative revert: this may cause crash for rasterize_and_record_micro benchmark. BUG=597391. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
