Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(752)

Unified Diff: third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp

Issue 1859833002: Don't assert rect mapping result if impossible, and enable assert in debug build (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@SVGRootClip
Patch Set: Created 4 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « third_party/WebKit/Source/core/layout/PaintInvalidationState.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
diff --git a/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp b/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
index 574c7c5e3f56c1e4ab967e40520181f1abf42668..096ca917ca303a9fb32a65c0f77489cfe3cdedf3 100644
--- a/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
+++ b/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
@@ -14,54 +14,8 @@
#include "core/layout/svg/SVGLayoutSupport.h"
#include "core/paint/PaintLayer.h"
-// We can't enable this by default because saturated operations of LayoutUnit
-// don't conform commutative law for overflowing results, preventing us from
-// making fast path and slow path always return the same result.
-#define ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH (0 && ENABLE(ASSERT))
-
namespace blink {
-#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
-// Make sure that the fast path and the slow path generate the same rect.
-void assertRectsEqual(const LayoutObject& object, const LayoutBoxModelObject& ancestor, const LayoutRect& rect, const LayoutRect& slowPathRect)
-{
- // TODO(wangxianzhu): This is for cases that a sub-frame creates a root PaintInvalidationState
- // which doesn't inherit clip from ancestor frames.
- // Remove the condition when we eliminate the latter case of PaintInvalidationState(const LayoutView&, ...).
- if (object.isLayoutView())
- return;
-
- // We ignore ancestor clipping for FixedPosition in fast path.
- if (object.styleRef().position() == FixedPosition)
- return;
-
- // TODO(crbug.com/597903): Fast path and slow path should generate equal empty rects.
- if (rect.isEmpty() && slowPathRect.isEmpty())
- return;
-
- if (rect == slowPathRect)
- return;
-
- // Tolerate the difference between the two paths when crossing frame boundaries.
- if (object.view() != ancestor.view()) {
- LayoutRect inflatedRect = rect;
- inflatedRect.inflate(1);
- if (inflatedRect.contains(slowPathRect))
- return;
- LayoutRect inflatedSlowPathRect = slowPathRect;
- inflatedSlowPathRect.inflate(1);
- if (inflatedSlowPathRect.contains(rect))
- return;
- }
-
-#ifndef NDEBUG
- WTFLogAlways("Fast path paint invalidation rect differs from slow path: %s vs %s", rect.toString().ascii().data(), slowPathRect.toString().ascii().data());
- showLayoutTree(&object);
-#endif
- ASSERT_NOT_REACHED();
-}
-#endif
-
static bool supportsCachedOffsets(const LayoutObject& object)
{
return !object.hasTransformRelatedProperty()
@@ -89,6 +43,9 @@ PaintInvalidationState::PaintInvalidationState(const LayoutView& layoutView, Vec
#if ENABLE(ASSERT)
, m_didUpdateForChildren(false)
#endif
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+ , m_canCheckFastPathSlowPathEquality(layoutView == m_paintInvalidationContainer)
+#endif
{
if (!supportsCachedOffsets(layoutView)) {
m_cachedOffsetsEnabled = false;
@@ -121,6 +78,9 @@ PaintInvalidationState::PaintInvalidationState(const PaintInvalidationState& par
#if ENABLE(ASSERT)
, m_didUpdateForChildren(false)
#endif
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+ , m_canCheckFastPathSlowPathEquality(parentState.m_canCheckFastPathSlowPathEquality)
+#endif
{
if (currentObject == parentState.m_currentObject) {
// Sometimes we create a new PaintInvalidationState from parentState on the same object
@@ -201,6 +161,9 @@ PaintInvalidationState::PaintInvalidationState(const PaintInvalidationState& par
m_clipped = false; // Will be updated in updateForChildren().
m_paintOffset = LayoutSize();
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+ m_canCheckFastPathSlowPathEquality = true;
+#endif
return;
}
@@ -235,7 +198,12 @@ void PaintInvalidationState::updateForCurrentObject(const PaintInvalidationState
// In the above way to get paint offset, we can't get accurate clip rect, so just assume no clip.
// Clip on fixed-position is rare, in case that paintInvalidationContainer crosses frame boundary
// and the LayoutView is clipped by something in owner document.
- m_clipped = false;
+ if (m_clipped) {
+ m_clipped = false;
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+ m_canCheckFastPathSlowPathEquality = false;
+#endif
+ }
return;
}
@@ -348,7 +316,7 @@ LayoutPoint PaintInvalidationState::computePositionFromPaintInvalidationBacking(
if (m_currentObject.isSVG() && !m_currentObject.isSVGRoot())
point = m_svgTransform.mapPoint(point);
point += FloatPoint(m_paintOffset);
-#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
// TODO(wangxianzhu): We can't enable this ASSERT for now because of crbug.com/597745.
// ASSERT(point == slowLocalOriginToAncestorPoint(m_currentObject, m_paintInvalidationContainer, FloatPoint());
#endif
@@ -384,9 +352,9 @@ LayoutRect PaintInvalidationState::computePaintInvalidationRectInBackingForSVG()
rect.move(m_paintOffset);
if (m_clipped)
rect.intersect(m_clipRect);
-#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
LayoutRect slowPathRect = SVGLayoutSupport::clippedOverflowRectForPaintInvalidation(m_currentObject, *m_paintInvalidationContainer);
- assertRectsEqual(m_currentObject, m_paintInvalidationContainer, rect, slowPathRect);
+ assertFastPathAndSlowPathRectsEqual(rect, slowPathRect);
#endif
} else {
// TODO(wangxianzhu): Sometimes m_cachedOffsetsEnabled==false doesn't mean we can't use cached
@@ -412,15 +380,15 @@ void PaintInvalidationState::mapLocalRectToPaintInvalidationContainer(LayoutRect
ASSERT(!m_didUpdateForChildren);
if (m_cachedOffsetsEnabled) {
-#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
LayoutRect slowPathRect(rect);
slowMapToVisualRectInAncestorSpace(m_currentObject, *m_paintInvalidationContainer, slowPathRect);
#endif
rect.move(m_paintOffset);
if (m_clipped)
rect.intersect(m_clipRect);
-#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
- assertRectsEqual(m_currentObject, *m_paintInvalidationContainer, rect, slowPathRect);
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+ assertFastPathAndSlowPathRectsEqual(rect, slowPathRect);
#endif
} else {
slowMapToVisualRectInAncestorSpace(m_currentObject, *m_paintInvalidationContainer, rect);
@@ -455,4 +423,56 @@ PaintLayer& PaintInvalidationState::enclosingSelfPaintingLayer(const LayoutObjec
return m_enclosingSelfPaintingLayer;
}
+#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+
+static bool mayHaveBeenSaturated(LayoutUnit value)
+{
+ // This is not accurate, just to avoid too big values.
+ return value.abs() >= LayoutUnit::max() / 2;
+}
+
+static bool mayHaveBeenSaturated(const LayoutRect& rect)
+{
+ return mayHaveBeenSaturated(rect.x()) || mayHaveBeenSaturated(rect.y()) || mayHaveBeenSaturated(rect.width()) || mayHaveBeenSaturated(rect.height());
+}
+
+void PaintInvalidationState::assertFastPathAndSlowPathRectsEqual(const LayoutRect& fastPathRect, const LayoutRect& slowPathRect) const
+{
+ if (!m_canCheckFastPathSlowPathEquality)
+ return;
+
+ // TODO(crbug.com/597903): Fast path and slow path should generate equal empty rects.
+ if (fastPathRect.isEmpty() && slowPathRect.isEmpty())
+ return;
+
+ if (fastPathRect == slowPathRect)
+ return;
+
+ // LayoutUnit uses saturated arithmetic operations. If any interim or final result is saturated,
+ // the same operations in different order produce different results. Don't compare results
+ // if any of them may have been saturated.
+ if (mayHaveBeenSaturated(fastPathRect) || mayHaveBeenSaturated(slowPathRect))
+ return;
+
+ // Tolerate the difference between the two paths when crossing frame boundaries.
+ if (m_currentObject.view() != m_paintInvalidationContainer->view()) {
+ LayoutRect inflatedFastPathRect = fastPathRect;
+ inflatedFastPathRect.inflate(1);
+ if (inflatedFastPathRect.contains(slowPathRect))
+ return;
+ LayoutRect inflatedSlowPathRect = slowPathRect;
+ inflatedSlowPathRect.inflate(1);
+ if (inflatedSlowPathRect.contains(fastPathRect))
+ return;
+ }
+
+ WTFLogAlways("Fast path paint invalidation rect differs from slow path: fast: %s vs slow: %s",
+ fastPathRect.toString().ascii().data(), slowPathRect.toString().ascii().data());
+ showLayoutTree(&m_currentObject);
+
+ ASSERT_NOT_REACHED();
+}
+
+#endif // CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+
} // namespace blink
« no previous file with comments | « third_party/WebKit/Source/core/layout/PaintInvalidationState.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698