Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2203933002:
Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect (Closed)
|
Created:
4 years, 4 months ago by Xianzhu Modified:
4 years, 4 months ago CC:
blink-reviews, blink-reviews-layout_chromium.org, chromium-reviews, eae+blinkwatch, jchaffraix+rendering, leviw+renderwatch, pdr+renderingwatchlist_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. |
DescriptionReplace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect
We don't need to check if the layout is laid out because of children,
but just check if it will paint anything and if its painting is
affected by geometry change.
Removed neededLayoutBecauseOfChildren logic and replaced
skipInvalidationWhenLaiedOutChildren() with
paintedOutputOfObjectHasNoEffect().
Also cleaned up selection paint invalidation.
BUG=451090
Committed: https://crrev.com/bdd41f4e784486fd7522793283cb5c376a057f0a
Cr-Commit-Position: refs/heads/master@{#409965}
Patch Set 1 #Patch Set 2 : canSkipPaintInvalidation #Patch Set 3 : canSkipPaintInvalidation #Patch Set 4 : - #Patch Set 5 #
Total comments: 17
Patch Set 6 : - #
Total comments: 7
Patch Set 7 : - #
Total comments: 2
Patch Set 8 : - #Patch Set 9 : Fix first line invalidation issue #Messages
Total messages: 58 (35 generated)
The CQ bit was checked by wangxianzhu@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...
The CQ bit was checked by wangxianzhu@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: cast_shell_linux on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/cast_shell_linu...)
The CQ bit was checked by wangxianzhu@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by wangxianzhu@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...
The CQ bit was checked by wangxianzhu@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...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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 wangxianzhu@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...
Description was changed from ========== Don't invalidate object that mayNeedPaintInvalidation but paints nothing This was done with LayoutObject::skipInvalidationWhenLaidOutChildren() but actually we don't need to check if the layout is laid out because of children, but just check if it will paint anything. Removed neededLayoutBecauseOfChildren logic and renamed skipInvalidationWhenLaiedOutChildren() to isKnownToPaintNothing(). BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with canSkipUnforcedPaintInvalidation We don't need to check if the layout is laid out because of children, but just check if it will paint anything. Removed neededLayoutBecauseOfChildren logic and renamed replaced skipInvalidationWhenLaiedOutChildren() with canSkipUnforcedPaintInvalidation(). Also cleaned up selection paint invalidation. BUG=451090 ==========
wangxianzhu@chromium.org changed reviewers: + chrishtr@chromium.org, trchen@chromium.org
https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Layo... File third_party/WebKit/LayoutTests/css3/flexbox/repaint-on-margin-change.html (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Layo... third_party/WebKit/LayoutTests/css3/flexbox/repaint-on-margin-change.html:17: background-color: green; This change is for the original intent of the test. Otherwise this test will invalidate nothing because all elements paint nothing. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (left): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.cpp:695: return false; The "isLayoutBlockFlow() && toLayoutBlockFlow(this)->firstLineBox()" condition is removed because LayoutBlockFlow no longer invalidate for line boxes, but LayoutInline and LayoutText do. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.h (left): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.h:361: } This is now directly inlined in the new LayoutBox::canSkipUnforcedPaintInvalidation(). https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutView.cpp (left): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutView.cpp:580: } Blocks no longer paint selection gaps. This change is included in this CL because this affects how we check if a LayoutBox has selection in LayoutBox::canSkipUnforcedPaintInvalidation(). https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/svg/LayoutSVGHiddenContainer.cpp (left): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/svg/LayoutSVGHiddenContainer.cpp:56: The above implementations are moved into the header because it looks clearer for that group of functions under "LayoutSVGHiddenContainer paints nothing" comment.
Description was changed from ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with canSkipUnforcedPaintInvalidation We don't need to check if the layout is laid out because of children, but just check if it will paint anything. Removed neededLayoutBecauseOfChildren logic and renamed replaced skipInvalidationWhenLaiedOutChildren() with canSkipUnforcedPaintInvalidation(). Also cleaned up selection paint invalidation. BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with canSkipUnforcedPaintInvalidation We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and renamed replaced skipInvalidationWhenLaiedOutChildren() with canSkipUnforcedPaintInvalidation(). Also cleaned up selection paint invalidation. BUG=451090 ==========
https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2039: if (getSelectionState() != SelectionNone) What's the (brief) explanation for this? https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2047: if (hasBoxDecorationBackground() || styleRef().hasVisualOverflowingEffect()) ditto https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2050: if (hasClipRelatedProperty() || hasControlClip()) ditto https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1426: if (canSkipUnforcedPaintInvalidation()) What is the main use case of this optimization? Is it avoiding invalidation on the body element when resizing? https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.h (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.h:1124: // An object needs to check unforced paint invalidation if it's not set shouldDoFullPaintInvalidation, Maybe rewrite as: "unforced paint invalidation is when shouldDoFullPaintInvalidation is false, but mayNeedPaintInvalidation or childShouldCheckForPaintInvalidation is true. This returns true if the object has unforced paint invalidation bits set, but they can be skipped because its painting geometry will not change for some reason"
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)
https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2039: if (getSelectionState() != SelectionNone) On 2016/08/03 20:33:50, chrishtr wrote: > What's the (brief) explanation for this? Done. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2047: if (hasBoxDecorationBackground() || styleRef().hasVisualOverflowingEffect()) On 2016/08/03 20:33:50, chrishtr wrote: > ditto Done. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2050: if (hasClipRelatedProperty() || hasControlClip()) On 2016/08/03 20:33:50, chrishtr wrote: > ditto Done. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1426: if (canSkipUnforcedPaintInvalidation()) On 2016/08/03 20:33:51, chrishtr wrote: > What is the main use case of this optimization? Is it avoiding invalidation on > the body element when resizing? It avoids invalidation if the geometry change affects nothing of this object's painting. This includes the case of resizing body element (having no own content). Added a comment. https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.h (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.h:1124: // An object needs to check unforced paint invalidation if it's not set shouldDoFullPaintInvalidation, On 2016/08/03 20:33:51, chrishtr wrote: > Maybe rewrite as: > > "unforced paint invalidation is when shouldDoFullPaintInvalidation is false, but > mayNeedPaintInvalidation or childShouldCheckForPaintInvalidation is true. This > returns > true if the object has unforced paint invalidation bits set, but they can be > skipped because its painting geometry will not change for some reason" Done.
https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1426: if (canSkipUnforcedPaintInvalidation()) On 2016/08/03 at 21:30:35, Xianzhu wrote: > On 2016/08/03 20:33:51, chrishtr wrote: > > What is the main use case of this optimization? Is it avoiding invalidation on > > the body element when resizing? > > It avoids invalidation if the geometry change affects nothing of this object's painting. This includes the case of resizing body element (having no own content). > > Added a comment. How about restricting the optimization to the body element?
https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/80001/third_party/WebKit/Sour... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1426: if (canSkipUnforcedPaintInvalidation()) On 2016/08/03 21:34:56, chrishtr wrote: > On 2016/08/03 at 21:30:35, Xianzhu wrote: > > On 2016/08/03 20:33:51, chrishtr wrote: > > > What is the main use case of this optimization? Is it avoiding invalidation > on > > > the body element when resizing? > > > > It avoids invalidation if the geometry change affects nothing of this object's > painting. This includes the case of resizing body element (having no own > content). > > > > Added a comment. > > How about restricting the optimization to the body element? This optimization also reduces unnecessary rasterization invalidations for other big empty objects or big objects that have children but paint nothing for itself. For example, <body> <div id="container"> ... </div> </body> container.appendChild(someChild with small width); we only need to invalidate the area of the new child. Without the optimization, we also need to invalidate the container (width-of-body x height-of-new-child).
https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2037: bool LayoutBox::canSkipUnforcedPaintInvalidation() const In what case are the conditions in this method different than "this object doesn't paint anything"? There most be something, but don't know yet.. https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2048: // If the box has clip, we need to repaint the changed part when the box got resized, Changed part of children
https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2048: // If the box has clip, we need to repaint the changed part when the box got resized, On 2016/08/03 at 23:13:15, chrishtr wrote: > Changed part of children Meaning: it's the childrens' paint output that would be clipped.
https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2037: bool LayoutBox::canSkipUnforcedPaintInvalidation() const On 2016/08/03 23:13:15, chrishtr wrote: > In what case are the conditions in this method different than "this object > doesn't paint anything"? There > most be something, but don't know yet.. For now there is no other cases. However, clip is a case not to skip paint invalidation even if the object paints nothing. https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2048: // If the box has clip, we need to repaint the changed part when the box got resized, On 2016/08/03 23:13:43, chrishtr wrote: > On 2016/08/03 at 23:13:15, chrishtr wrote: > > Changed part of children > > Meaning: it's the childrens' paint output that would be clipped. Done. (I think there is a bug about ancestor clip change. In theory the children should invalidate themselves on ancestor clip change, but this is missing for now. The condition hides the bug in most cases, but the bug still occur in the following scenario: Frame 1: Ancestor expands clip which should a child's visual rect. Frame 2: The child moves. There will be under-invalidation in Frame 2 for the child's exposed area in Frame 1 because the child's previousPaintInvalidationRect is not updated during Frame 1. I believe the bug has existed since the first of repaint-after-layout, but it's so rare that no one reported it and no layout test covers it. It seems not easy to fix it in spv1 without hurting performance, so I would like to treat it as a "fundamental paint invalidation bug" for spv1 and not to fix it for now. This will be very different in spv2 because we'll use unclipped visual rects, apply clip in later stages, and also handle rasterization invalidation on clip property change differently.)
On 2016/08/04 at 00:02:14, wangxianzhu wrote: > https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... > File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): > > https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/layout/LayoutBox.cpp:2037: bool LayoutBox::canSkipUnforcedPaintInvalidation() const > On 2016/08/03 23:13:15, chrishtr wrote: > > In what case are the conditions in this method different than "this object > > doesn't paint anything"? There > > most be something, but don't know yet.. > > For now there is no other cases. > > However, clip is a case not to skip paint invalidation even if the object paints nothing. > > https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... > third_party/WebKit/Source/core/layout/LayoutBox.cpp:2048: // If the box has clip, we need to repaint the changed part when the box got resized, > On 2016/08/03 23:13:43, chrishtr wrote: > > On 2016/08/03 at 23:13:15, chrishtr wrote: > > > Changed part of children > > > > Meaning: it's the childrens' paint output that would be clipped. > > Done. > > (I think there is a bug about ancestor clip change. In theory the children should invalidate themselves on ancestor clip change, but this is missing for now. The condition hides the bug in most cases, but the bug still occur in the following scenario: > > Frame 1: Ancestor expands clip which should a child's visual rect. > Frame 2: The child moves. > > There will be under-invalidation in Frame 2 for the child's exposed area in Frame 1 because the child's previousPaintInvalidationRect is not updated during Frame 1. > > I believe the bug has existed since the first of repaint-after-layout, but it's so rare that no one reported it and no layout test covers it. > > It seems not easy to fix it in spv1 without hurting performance, so I would like to treat it as a "fundamental paint invalidation bug" for spv1 and not to fix it for now. > > This will be very different in spv2 because we'll use unclipped visual rects, apply clip in later stages, and also handle rasterization invalidation on clip property change differently.) Ok please note down this bug somewhere though.
https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2037: bool LayoutBox::canSkipUnforcedPaintInvalidation() const On 2016/08/04 at 00:02:14, Xianzhu wrote: > On 2016/08/03 23:13:15, chrishtr wrote: > > In what case are the conditions in this method different than "this object > > doesn't paint anything"? There > > most be something, but don't know yet.. > > For now there is no other cases. > > However, clip is a case not to skip paint invalidation even if the object paints nothing. Right. Then is it correct to say that this method could be named "doesNotAffectPaintedOutput"? Then this optimization would be reframed as "skip paint invalidation for objects whose painted output has no effect". And given that definition, why invalidate paint for it ever? Why not early out from painting it also?
https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutBox.cpp (right): https://codereview.chromium.org/2203933002/diff/100001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutBox.cpp:2037: bool LayoutBox::canSkipUnforcedPaintInvalidation() const On 2016/08/04 00:57:47, chrishtr wrote: > On 2016/08/04 at 00:02:14, Xianzhu wrote: > > On 2016/08/03 23:13:15, chrishtr wrote: > > > In what case are the conditions in this method different than "this object > > > doesn't paint anything"? There > > > most be something, but don't know yet.. > > > > For now there is no other cases. > > > > However, clip is a case not to skip paint invalidation even if the object > paints nothing. > > Right. Then is it correct to say that this method could be named > "doesNotAffectPaintedOutput"? Then this optimization would > be reframed as "skip paint invalidation for objects whose painted output has no > effect". The name "doesNotAffectPaintedOutput" seems not clear about what does not affect painted output. In the context, it's "geometry change without forced paint invalidation". Also we should state "unforced paint invalidation". We can't skip forced paint invalidation even if currently the object will paint nothing, in case that the object painted something in the previous frame. So canSkipUnforcedPaintInvalidation() seems better describe the result (but not the reason). > And given that definition, why invalidate paint for it ever? Previously the optimization existed in LayoutObject::skipInvalidationWhenLaidOutChildren(). This CL make the optimization not depend on whether we laid-out children. Created this CL because I felt skipInvalidationWhenLaidOutChildren() is not a good function to export from layout/ to paint/ when I was moving paint invalidation code under paint/. > Why not early out from painting it also? We do early out from painting, but the conditions are not at a single place in painting code. For example, we early out from background painting if there is no background. Actually this function is a conservative collection of early-out conditions in painting.
https://codereview.chromium.org/2203933002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1427: if (canSkipUnforcedPaintInvalidation()) If it was called paintedOutputOfObjectHasNoEffect() I think this would be clearer, and avoid the need for the comment on 426. The reader would see clearly why the optimization should work.
An alternation of this CL is to still use LayoutObject::skipInvalidationWhenLaidOutChildren() from paint invalidation code. It will be discarded for spv2 because - object invalidation for such objects doesn't matter because repainting of the objects are cheap; - we'll issue rasterization invalidation after painting, so if an object painted nothing we'll not issue rasterization invalidation for it.
On 2016/08/04 at 16:48:33, wangxianzhu wrote: > An alternation of this CL is to still use LayoutObject::skipInvalidationWhenLaidOutChildren() from paint invalidation code. > > It will be discarded for spv2 because > - object invalidation for such objects doesn't matter because repainting of the objects are cheap; > - we'll issue rasterization invalidation after painting, so if an object painted nothing we'll not issue rasterization invalidation for it. I like the current CL, just suggesting on the name. :)
The CQ bit was checked by wangxianzhu@chromium.org to run a CQ dry run
https://codereview.chromium.org/2203933002/diff/120001/third_party/WebKit/Sou... File third_party/WebKit/Source/core/layout/LayoutObject.cpp (right): https://codereview.chromium.org/2203933002/diff/120001/third_party/WebKit/Sou... third_party/WebKit/Source/core/layout/LayoutObject.cpp:1427: if (canSkipUnforcedPaintInvalidation()) On 2016/08/04 16:47:54, chrishtr wrote: > If it was called paintedOutputOfObjectHasNoEffect() I think this would be > clearer, and avoid the need for the > comment on 426. The reader would see clearly why the optimization should work. Good suggestion. Done.
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with canSkipUnforcedPaintInvalidation We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and renamed replaced skipInvalidationWhenLaiedOutChildren() with canSkipUnforcedPaintInvalidation(). Also cleaned up selection paint invalidation. BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ==========
Description was changed from ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ==========
lgtm
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: 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 wangxianzhu@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/2203933002/#ps160001 (title: "Fix first line invalidation issue")
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: linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by wangxianzhu@chromium.org
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.
Description was changed from ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ==========
Message was sent while issue was closed.
Committed patchset #9 (id:160001)
Message was sent while issue was closed.
Description was changed from ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 ========== to ========== Replace LayoutObject::skipInvalidationWhenLaidOutChildren() with paintedOutputOfObjectHasNoEffect We don't need to check if the layout is laid out because of children, but just check if it will paint anything and if its painting is affected by geometry change. Removed neededLayoutBecauseOfChildren logic and replaced skipInvalidationWhenLaiedOutChildren() with paintedOutputOfObjectHasNoEffect(). Also cleaned up selection paint invalidation. BUG=451090 Committed: https://crrev.com/bdd41f4e784486fd7522793283cb5c376a057f0a Cr-Commit-Position: refs/heads/master@{#409965} ==========
Message was sent while issue was closed.
Patchset 9 (id:??) landed as https://crrev.com/bdd41f4e784486fd7522793283cb5c376a057f0a Cr-Commit-Position: refs/heads/master@{#409965}
Message was sent while issue was closed.
A revert of this CL (patchset #9 id:160001) has been created in https://codereview.chromium.org/2211283003/ by wangxianzhu@chromium.org. The reason for reverting is: With the CL, LayoutBox::getPaintInvalidationReason() sometimes returns PaintInvalidationIncremental when paintedOutputOfObjectHasNoEffect() is true.. |
