Modified: trunk/Source/WebCore/ChangeLog (294093 => 294094)
--- trunk/Source/WebCore/ChangeLog 2022-05-12 04:49:28 UTC (rev 294093)
+++ trunk/Source/WebCore/ChangeLog 2022-05-12 05:14:05 UTC (rev 294094)
@@ -1,3 +1,28 @@
+2022-05-11 Simon Fraser <[email protected]>
+
+ Some RenderStyle::diff() optimizations
+ https://bugs.webkit.org/show_bug.cgi?id=240323
+
+ Reviewed by Alan Bujtas.
+
+ Some LengthBox operator== stuff was showing up on profiles, which we can avoid by
+ first testing m_surroundData pointer equality.
+
+ scrollPadding and scrollSnapType live in rareNonInheritedData, so we can move their
+ comparisons into rareNonInheritedDataChangeRequiresLayout().
+
+ changeRequiresRepaint() can check for m_backgroundData and m_surroundData pointer
+ equality before doing more expensive tests.
+
+ These changes reduce the time in RenderStyle::diff() in the MotionMark Design
+ subtest by ~40%.
+
+ * rendering/style/RenderStyle.cpp:
+ (WebCore::rareNonInheritedDataChangeRequiresLayout):
+ (WebCore::RenderStyle::changeRequiresLayout const):
+ (WebCore::RenderStyle::changeRequiresLayerRepaint const):
+ (WebCore::RenderStyle::changeRequiresRepaint const):
+
2022-05-11 Wenson Hsieh <[email protected]>
ImageAnalysisQueue should extract and analyze images inside of subframes
Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (294093 => 294094)
--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2022-05-12 04:49:28 UTC (rev 294093)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp 2022-05-12 05:14:05 UTC (rev 294094)
@@ -741,6 +741,10 @@
if (!arePointingToEqualData(first.boxReflect, second.boxReflect))
return true;
+ // If the counter directives change, trigger a relayout to re-calculate counter values and rebuild the counter node tree.
+ if (!arePointingToEqualData(first.counterDirectives, second.counterDirectives))
+ return true;
+
if (first.multiCol != second.multiCol)
return true;
@@ -816,6 +820,12 @@
|| first.effectiveContainment().contains(Containment::InlineSize) != second.effectiveContainment().contains(Containment::InlineSize))
return true;
+ if (first.scrollPadding != second.scrollPadding)
+ return true;
+
+ if (first.scrollSnapType != second.scrollSnapType)
+ return true;
+
return false;
}
@@ -904,12 +914,35 @@
return true;
}
- if (m_surroundData->margin != other.m_surroundData->margin)
- return true;
+ if (m_surroundData.ptr() != other.m_surroundData.ptr()) {
+ if (m_surroundData->margin != other.m_surroundData->margin)
+ return true;
- if (m_surroundData->padding != other.m_surroundData->padding)
- return true;
+ if (m_surroundData->padding != other.m_surroundData->padding)
+ return true;
+ // If our border widths change, then we need to layout. Other changes to borders only necessitate a repaint.
+ if (borderLeftWidth() != other.borderLeftWidth()
+ || borderTopWidth() != other.borderTopWidth()
+ || borderBottomWidth() != other.borderBottomWidth()
+ || borderRightWidth() != other.borderRightWidth())
+ return true;
+
+ if (position() != PositionType::Static) {
+ if (m_surroundData->offset != other.m_surroundData->offset) {
+ // FIXME: We would like to use SimplifiedLayout for relative positioning, but we can't quite do that yet.
+ // We need to make sure SimplifiedLayout can operate correctly on RenderInlines (we will need
+ // to add a selfNeedsSimplifiedLayout bit in order to not get confused and taint every line).
+ if (position() != PositionType::Absolute)
+ return true;
+
+ // Optimize for the case where a positioned layer is moving but not changing size.
+ if (!positionChangeIsMovementOnly(m_surroundData->offset, other.m_surroundData->offset, m_boxData->width()))
+ return true;
+ }
+ }
+ }
+
// FIXME: We should add an optimized form of layout that just recomputes visual overflow.
if (changeAffectsVisualOverflow(other))
return true;
@@ -985,38 +1018,13 @@
|| m_nonInheritedFlags.overflowY != other.m_nonInheritedFlags.overflowY)
return true;
- // If our border widths change, then we need to layout. Other changes to borders
- // only necessitate a repaint.
- if (borderLeftWidth() != other.borderLeftWidth()
- || borderTopWidth() != other.borderTopWidth()
- || borderBottomWidth() != other.borderBottomWidth()
- || borderRightWidth() != other.borderRightWidth())
- return true;
-
- // If the counter directives change, trigger a relayout to re-calculate counter values and rebuild the counter node tree.
- if (!arePointingToEqualData(m_rareNonInheritedData->counterDirectives, other.m_rareNonInheritedData->counterDirectives))
- return true;
-
if ((visibility() == Visibility::Collapse) != (other.visibility() == Visibility::Collapse))
return true;
- if (position() != PositionType::Static) {
- if (m_surroundData->offset != other.m_surroundData->offset) {
- // FIXME: We would like to use SimplifiedLayout for relative positioning, but we can't quite do that yet.
- // We need to make sure SimplifiedLayout can operate correctly on RenderInlines (we will need
- // to add a selfNeedsSimplifiedLayout bit in order to not get confused and taint every line).
- if (position() != PositionType::Absolute)
- return true;
-
- // Optimize for the case where a positioned layer is moving but not changing size.
- if (!positionChangeIsMovementOnly(m_surroundData->offset, other.m_surroundData->offset, m_boxData->width()))
- return true;
- }
- }
-
bool hasFirstLineStyle = hasPseudoStyle(PseudoId::FirstLine);
if (hasFirstLineStyle != other.hasPseudoStyle(PseudoId::FirstLine))
return true;
+
if (hasFirstLineStyle) {
auto* firstLineStyle = getCachedPseudoStyle(PseudoId::FirstLine);
if (!firstLineStyle)
@@ -1029,9 +1037,6 @@
return true;
}
- if (scrollPadding() != other.scrollPadding() || scrollSnapType() != other.scrollSnapType())
- return true;
-
return false;
}
@@ -1083,13 +1088,17 @@
bool RenderStyle::changeRequiresLayerRepaint(const RenderStyle& other, OptionSet<StyleDifferenceContextSensitiveProperty>& changedContextSensitiveProperties) const
{
// Style::Resolver has ensured that zIndex is non-auto only if it's applicable.
- if (m_boxData->usedZIndex() != other.m_boxData->usedZIndex() || m_boxData->hasAutoUsedZIndex() != other.m_boxData->hasAutoUsedZIndex())
- return true;
+ if (m_boxData.ptr() != other.m_boxData.ptr()) {
+ if (m_boxData->usedZIndex() != other.m_boxData->usedZIndex() || m_boxData->hasAutoUsedZIndex() != other.m_boxData->hasAutoUsedZIndex())
+ return true;
+ }
if (position() != PositionType::Static) {
- if (m_visualData->clip != other.m_visualData->clip || m_visualData->hasClip != other.m_visualData->hasClip) {
- changedContextSensitiveProperties.add(StyleDifferenceContextSensitiveProperty::ClipRect);
- return true;
+ if (m_visualData.ptr() != other.m_visualData.ptr()) {
+ if (m_visualData->clip != other.m_visualData->clip || m_visualData->hasClip != other.m_visualData->hasClip) {
+ changedContextSensitiveProperties.add(StyleDifferenceContextSensitiveProperty::ClipRect);
+ return true;
+ }
}
}
@@ -1198,16 +1207,24 @@
if (!requiresPainting(*this) && !requiresPainting(other))
return false;
- bool currentColorDiffers = m_inheritedData->color != other.m_inheritedData->color;
-
if (m_inheritedFlags.visibility != other.m_inheritedFlags.visibility
|| m_inheritedFlags.printColorAdjust != other.m_inheritedFlags.printColorAdjust
|| m_inheritedFlags.insideLink != other.m_inheritedFlags.insideLink
- || m_inheritedFlags.insideDefaultButton != other.m_inheritedFlags.insideDefaultButton
- || m_surroundData->border != other.m_surroundData->border
- || !m_backgroundData->isEquivalentForPainting(*other.m_backgroundData, currentColorDiffers))
+ || m_inheritedFlags.insideDefaultButton != other.m_inheritedFlags.insideDefaultButton)
return true;
+
+ if (m_backgroundData.ptr() != other.m_backgroundData.ptr()) {
+ bool currentColorDiffers = m_inheritedData->color != other.m_inheritedData->color;
+ if (!m_backgroundData->isEquivalentForPainting(*other.m_backgroundData, currentColorDiffers))
+ return true;
+ }
+
+ if (m_surroundData.ptr() != other.m_surroundData.ptr()) {
+ if (m_surroundData->border != other.m_surroundData->border)
+ return true;
+ }
+
if (m_rareNonInheritedData.ptr() != other.m_rareNonInheritedData.ptr()
&& rareNonInheritedDataChangeRequiresRepaint(*m_rareNonInheritedData, *other.m_rareNonInheritedData, changedContextSensitiveProperties))
return true;
