Skip to content

GitLab

T
tangled
Commit 1ea99b02 authored by wangxianzhu's avatar wangxianzhu Committed by Commit bot
Browse files
Options

[SPInvalidation] Update paint properties on css clip change 

Add StyleDifference::needsPaintPropertyUpdate() to indicate a style
change directly needs paint property update, and set the flag on
changes of css clip, transform, etc.

Move two tests from their original directory into paint/invalidation.

BUG=645667
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2

Review-Url: https://codereview.chromium.org/2570463004
Cr-Commit-Position: refs/heads/master@{#438068}
parent 67cb47ca
Hide whitespace changes
Inline Side-by-side
Showing with 141 additions and 46 deletions
third_party/WebKit/LayoutTests/FlagExpectations/enable-slimming-paint-v2
View file @ 1ea99b02
......@@ -2159,10 +2159,6 @@ Bug(none) fast/overflow/003.xml [ Failure ]
Bug(none) fast/overflow/overflow-float-stacking.html [ Failure ]
Bug(none) fast/overflow/overflow-stacking.html [ Failure ]
# Paint property under-invalidation for clip changes needs investigation.
crbug.com/645667 compositing/clip-change.html [ Failure ]
crbug.com/645667 compositing/layer-creation/iframe-clip-removed.html [ Failure ]
# Need to write SPv2 version of this code.
crbug.com/157218 compositing/overflow/border-radius-styles-with-composited-child.html [ Failure ]
......
third_party/WebKit/LayoutTests/TestExpectations
View file @ 1ea99b02
......@@ -98,13 +98,10 @@ crbug.com/626748 virtual/spinvalidation/paint/invalidation/table/cached-change-t
crbug.com/646176 virtual/spinvalidation/ [ Skip ]
crbug.com/646176 virtual/spinvalidation/paint/invalidation/ [ Pass ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/outline-clip-change.html [ Crash Failure ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/filter-repaint-accelerated-on-accelerated-filter.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/position-change-keeping-geometry.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/should-not-repaint-composited-descendants.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/resize-repaint.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/shrink-layer.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/compositing/clipping-should-not-repaint-composited-descendants.html [ Crash ]
crbug.com/645667 virtual/spinvalidation/paint/invalidation/filter-repaint-on-accelerated-layer.html [ Crash ]
# --- End SlimmingPaintInvalidation Tests ---
......
third_party/WebKit/LayoutTests/paint/invalidation/compositing/absolute-to-static-with-clip-expected.html 0 → 100644
View file @ 1ea99b02
<!DOCTYPE html>
<div style="width: 100px; height: 100px; background-color: green"></div>
third_party/WebKit/LayoutTests/paint/invalidation/compositing/absolute-to-static-with-clip.html 0 → 100644
View file @ 1ea99b02
<!DOCTYPE html>
<div id="clipper" style="position: absolute; width: 100px; height: 100px;
background-color: green; clip: rect(25px 75px 75px 25px)">
</div>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script>
runAfterLayoutAndPaint(function() {
clipper.style.position = 'static';
}, true);
</script>
third_party/WebKit/LayoutTests/compositing/clip-change.html → third_party/WebKit/LayoutTests/paint/invalidation/compositing/clip-change.html
View file @ 1ea99b02
<!DOCTYPE html>
<html>
<head>
<script src="../resources/run-after-layout-and-paint.js"></script>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<style>
#indicator {
position: absolute;
......
third_party/WebKit/LayoutTests/compositing/layer-creation/iframe-clip-removed.html → third_party/WebKit/LayoutTests/paint/invalidation/compositing/iframe-clip-removed.html
View file @ 1ea99b02
......@@ -19,27 +19,16 @@
clip: rect(0px, 100px, 100px, 0px);
}
</style>
<script>
onload = function() {
// Render one frame with clipping, then remove the clip.
window.requestAnimationFrame(function() {
window.requestAnimationFrame(function() {
document.getElementById("clip").style.clip = "auto";
if (window.testRunner)
testRunner.notifyDone();
});
});
}
if (window.testRunner && window.internals) {
testRunner.waitUntilDone();
internals.settings.setPreferCompositingToLCDTextEnabled(true);
}
</script>
<div id="scroller">
<div id="clip">
<iframe id="subframe"></iframe>
</div>
</div>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script>
if (window.internals)
internals.settings.setPreferCompositingToLCDTextEnabled(true);
runAfterLayoutAndPaint(function() {
clip.style.clip = "auto";
}, true);
</script>
third_party/WebKit/LayoutTests/paint/invalidation/compositing/static-to-absolute-with-clip-expected.html 0 → 100644
View file @ 1ea99b02
<!DOCTYPE html>
<div style="position: relative; top: 25px; left: 25px;
width: 50px; height: 50px; background-color: green">
</div>
third_party/WebKit/LayoutTests/paint/invalidation/compositing/static-to-absolute-with-clip.html 0 → 100644
View file @ 1ea99b02
<!DOCTYPE html>
<div id="clipper" style="width: 100px; height: 100px;
background-color: green; clip: rect(25px 75px 75px 25px)">
</div>
<script src="../../../resources/run-after-layout-and-paint.js"></script>
<script>
runAfterLayoutAndPaint(function() {
clipper.style.position = 'absolute';
}, true);
</script>
third_party/WebKit/Source/core/layout/LayoutBoxModelObject.cpp
View file @ 1ea99b02
......@@ -226,13 +226,8 @@ void LayoutBoxModelObject::styleWillChange(StyleDifference diff,
FloatStateForStyleChange::setWasFloating(this, isFloating());
if (const ComputedStyle* oldStyle = style()) {
if (hasLayer() && diff.needsPaintInvalidationSubtree()) {
if (oldStyle->hasAutoClip() != newStyle.hasAutoClip() ||
oldStyle->clip() != newStyle.clip())
layer()->clipper().clearClipRectsIncludingDescendants();
}
}
if (hasLayer() && diff.cssClipChanged())
layer()->clipper().clearClipRectsIncludingDescendants();
LayoutObject::styleWillChange(diff, newStyle);
}
......
third_party/WebKit/Source/core/layout/LayoutObject.cpp
View file @ 1ea99b02
......@@ -1467,25 +1467,6 @@ StyleDifference LayoutObject::adjustStyleDifference(
diff.setNeedsFullLayout();
}
if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled()) {
// Text nodes share style with their parents but the checked styles don't
// apply to them, hence the !isText() check.
if (!isText() && (diff.transformChanged() || diff.opacityChanged() ||
diff.zIndexChanged() || diff.filterChanged() ||
diff.backdropFilterChanged())) {
// We don't need to invalidate paint of objects on SPv2 when only paint
// property or paint order change. Mark the painting layer needing repaint
// for changed paint property or paint order. Raster invalidation will be
// issued if needed during paint.
if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
ObjectPaintInvalidator(*this).slowSetPaintingLayerNeedsRepaint();
// When transform, opacity, etc. change, paint properties will also change
// so we need to mark this object as needing an update.
getMutableForPainting().setNeedsPaintPropertyUpdate();
}
}
if (!RuntimeEnabledFeatures::slimmingPaintV2Enabled()) {
// If transform changed, and the layer does not paint into its own separate
// backing, then we need to invalidate paints.
......@@ -1530,6 +1511,11 @@ StyleDifference LayoutObject::adjustStyleDifference(
}
}
// TODO(wangxianzhu): We may avoid subtree paint invalidation on CSS clip
// change for SPv2.
if (diff.cssClipChanged())
diff.setNeedsPaintInvalidationSubtree();
// Optimization: for decoration/color property changes, invalidation is only
// needed if we have style or text affected by these properties.
if (diff.textDecorationOrColorChanged() && !diff.needsPaintInvalidation()) {
......@@ -1738,6 +1724,20 @@ void LayoutObject::setStyle(PassRefPtr<ComputedStyle> style) {
else if (diff.needsPaintInvalidationObject() ||
updatedDiff.needsPaintInvalidationObject())
setShouldDoFullPaintInvalidation();
// Text nodes share style with their parents but the paint properties don't
// apply to them, hence the !isText() check.
if (RuntimeEnabledFeatures::slimmingPaintInvalidationEnabled() &&
diff.needsPaintPropertyUpdate() && !isText()) {
setNeedsPaintPropertyUpdate();
// We don't need to invalidate paint of objects on SPv2 when only paint
// property or paint order change. Mark the painting layer needing repaint
// for changed paint property or paint order. Raster invalidation will be
// issued if needed during paint.
if (RuntimeEnabledFeatures::slimmingPaintV2Enabled())
ObjectPaintInvalidator(*this).slowSetPaintingLayerNeedsRepaint();
}
}
void LayoutObject::styleWillChange(StyleDifference diff,
......
third_party/WebKit/Source/core/paint/PaintPropertyTreeBuilderTest.cpp
View file @ 1ea99b02
......@@ -3138,4 +3138,15 @@ TEST_P(PaintPropertyTreeBuilderTest,
!div->layoutObject()->paintProperties()->overflowClip());
}
// A basic sanity check for over-invalidation of paint properties.
TEST_P(PaintPropertyTreeBuilderTest, NoPaintPropertyUpdateOnBackgroundChange) {
setBodyInnerHTML("<div id='div' style='background-color: blue'>DIV</div>");
auto* div = document().getElementById("div");
document().view()->updateAllLifecyclePhases();
div->setAttribute(HTMLNames::styleAttr, "background-color: green");
document().view()->updateLifecycleToLayoutClean();
EXPECT_FALSE(div->layoutObject()->needsPaintPropertyUpdate());
}
} // namespace blink
third_party/WebKit/Source/core/style/ComputedStyle.cpp
View file @ 1ea99b02
......@@ -594,8 +594,12 @@ StyleDifference ComputedStyle::visualInvalidationDiff(
updatePropertySpecificDifferences(other, diff);
// The following conditions need to be at last, because they may depend on
// conditions in diff computed above.
if (scrollAnchorDisablingPropertyChanged(other, diff))
diff.setScrollAnchorDisablingPropertyChanged();
if (diffNeedsPaintPropertyUpdate(other, diff))
diff.setNeedsPaintPropertyUpdate();
// Cursors are not checked, since they will be set appropriately in response
// to mouse events, so they don't need to cause any paint invalidation or
......@@ -610,7 +614,7 @@ StyleDifference ComputedStyle::visualInvalidationDiff(
bool ComputedStyle::scrollAnchorDisablingPropertyChanged(
const ComputedStyle& other,
StyleDifference& diff) const {
const StyleDifference& diff) const {
if (m_nonInheritedData.m_position != other.m_nonInheritedData.m_position)
return true;
......@@ -932,11 +936,6 @@ bool ComputedStyle::diffNeedsFullLayout(const ComputedStyle& other) const {
bool ComputedStyle::diffNeedsPaintInvalidationSubtree(
const ComputedStyle& other) const {
if (position() != StaticPosition &&
(m_visual->clip != other.m_visual->clip ||
m_visual->hasAutoClip != other.m_visual->hasAutoClip))
return true;
if (m_rareNonInheritedData.get() != other.m_rareNonInheritedData.get()) {
if (m_rareNonInheritedData->m_effectiveBlendMode !=
other.m_rareNonInheritedData->m_effectiveBlendMode ||
......@@ -1132,6 +1131,24 @@ void ComputedStyle::updatePropertySpecificDifferences(
diff.setTextDecorationOrColorChanged();
}
}
bool hasClip = hasOutOfFlowPosition() && !m_visual->hasAutoClip;
bool otherHasClip =
other.hasOutOfFlowPosition() && !other.m_visual->hasAutoClip;
if (hasClip != otherHasClip ||
(hasClip && m_visual->clip != other.m_visual->clip))
diff.setCSSClipChanged();
}
bool ComputedStyle::diffNeedsPaintPropertyUpdate(
const ComputedStyle& other,
const StyleDifference& diff) const {
if (diff.transformChanged() || diff.opacityChanged() ||
diff.zIndexChanged() || diff.filterChanged() ||
diff.backdropFilterChanged() || diff.cssClipChanged())
return true;
return false;
}
void ComputedStyle::addPaintImage(StyleImage* image) {
......
third_party/WebKit/Source/core/style/ComputedStyle.h
View file @ 1ea99b02
......@@ -3971,7 +3971,7 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
TransformationMatrix&) const;
bool scrollAnchorDisablingPropertyChanged(const ComputedStyle& other,
StyleDifference&) const;
const StyleDifference&) const;
bool diffNeedsFullLayoutAndPaintInvalidation(
const ComputedStyle& other) const;
bool diffNeedsFullLayout(const ComputedStyle& other) const;
......@@ -3982,6 +3982,8 @@ class CORE_EXPORT ComputedStyle : public ComputedStyleBase,
const ComputedStyle& other) const;
void updatePropertySpecificDifferences(const ComputedStyle& other,
StyleDifference&) const;
bool diffNeedsPaintPropertyUpdate(const ComputedStyle& other,
const StyleDifference&) const;
bool requireTransformOrigin(ApplyTransformOrigin applyOrigin,
ApplyMotionPath) const;
......
third_party/WebKit/Source/core/style/StyleDifference.h
View file @ 1ea99b02
......@@ -20,9 +20,10 @@ class StyleDifference {
ZIndexChanged = 1 << 2,
FilterChanged = 1 << 3,
BackdropFilterChanged = 1 << 4,
CSSClipChanged = 1 << 5,
// The object needs to issue paint invalidations if it is affected by text
// decorations or properties dependent on color (e.g., border or outline).
TextDecorationOrColorChanged = 1 << 5,
TextDecorationOrColorChanged = 1 << 6,
// If you add a value here, be sure to update the number of bits on
// m_propertySpecificDifferences.
};
......@@ -32,11 +33,18 @@ class StyleDifference {
m_layoutType(NoLayout),
m_recomputeOverflow(false),
m_propertySpecificDifferences(0),
m_scrollAnchorDisablingPropertyChanged(false) {}
m_scrollAnchorDisablingPropertyChanged(false),
m_needsPaintPropertyUpdate(false) {}
bool hasDifference() const {
return m_paintInvalidationType || m_layoutType ||
m_propertySpecificDifferences;
bool result = m_paintInvalidationType || m_layoutType ||
m_propertySpecificDifferences;
// m_recomputeOverflow, m_scrollAnchorDisablingPropertyChanged and
// m_needsPaintPropertyUpdate are never set without other flags set.
DCHECK(result ||
(!m_recomputeOverflow && !m_scrollAnchorDisablingPropertyChanged &&
!m_needsPaintPropertyUpdate));
return result;
}
bool hasAtMostPropertySpecificDifferences(
......@@ -113,6 +121,11 @@ class StyleDifference {
m_propertySpecificDifferences |= BackdropFilterChanged;
}
bool cssClipChanged() const {
return m_propertySpecificDifferences & CSSClipChanged;
}
void setCSSClipChanged() { m_propertySpecificDifferences |= CSSClipChanged; }
bool textDecorationOrColorChanged() const {
return m_propertySpecificDifferences & TextDecorationOrColorChanged;
}
......@@ -127,6 +140,9 @@ class StyleDifference {
m_scrollAnchorDisablingPropertyChanged = true;
}
bool needsPaintPropertyUpdate() const { return m_needsPaintPropertyUpdate; }
void setNeedsPaintPropertyUpdate() { m_needsPaintPropertyUpdate = true; }
private:
enum PaintInvalidationType {
NoPaintInvalidation = 0,
......@@ -138,8 +154,9 @@ class StyleDifference {
enum LayoutType { NoLayout = 0, PositionedMovement, FullLayout };
unsigned m_layoutType : 2;
unsigned m_recomputeOverflow : 1;
unsigned m_propertySpecificDifferences : 6;
unsigned m_propertySpecificDifferences : 7;
unsigned m_scrollAnchorDisablingPropertyChanged : 1;
unsigned m_needsPaintPropertyUpdate : 1;
};
} // namespace blink
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment