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

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

Issue 1934863002: Revert of 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 096ca917ca303a9fb32a65c0f77489cfe3cdedf3..574c7c5e3f56c1e4ab967e40520181f1abf42668 100644
--- a/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
+++ b/third_party/WebKit/Source/core/layout/PaintInvalidationState.cpp
@@ -14,7 +14,53 @@
#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)
{
@@ -42,9 +88,6 @@
, m_enclosingSelfPaintingLayer(*layoutView.layer())
#if ENABLE(ASSERT)
, m_didUpdateForChildren(false)
-#endif
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
- , m_canCheckFastPathSlowPathEquality(layoutView == m_paintInvalidationContainer)
#endif
{
if (!supportsCachedOffsets(layoutView)) {
@@ -77,9 +120,6 @@
, m_enclosingSelfPaintingLayer(parentState.enclosingSelfPaintingLayer(currentObject))
#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) {
@@ -161,9 +201,6 @@
m_clipped = false; // Will be updated in updateForChildren().
m_paintOffset = LayoutSize();
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
- m_canCheckFastPathSlowPathEquality = true;
-#endif
return;
}
@@ -198,12 +235,7 @@
// 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.
- if (m_clipped) {
- m_clipped = false;
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
- m_canCheckFastPathSlowPathEquality = false;
-#endif
- }
+ m_clipped = false;
return;
}
@@ -316,7 +348,7 @@
if (m_currentObject.isSVG() && !m_currentObject.isSVGRoot())
point = m_svgTransform.mapPoint(point);
point += FloatPoint(m_paintOffset);
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
// TODO(wangxianzhu): We can't enable this ASSERT for now because of crbug.com/597745.
// ASSERT(point == slowLocalOriginToAncestorPoint(m_currentObject, m_paintInvalidationContainer, FloatPoint());
#endif
@@ -352,9 +384,9 @@
rect.move(m_paintOffset);
if (m_clipped)
rect.intersect(m_clipRect);
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
LayoutRect slowPathRect = SVGLayoutSupport::clippedOverflowRectForPaintInvalidation(m_currentObject, *m_paintInvalidationContainer);
- assertFastPathAndSlowPathRectsEqual(rect, slowPathRect);
+ assertRectsEqual(m_currentObject, m_paintInvalidationContainer, rect, slowPathRect);
#endif
} else {
// TODO(wangxianzhu): Sometimes m_cachedOffsetsEnabled==false doesn't mean we can't use cached
@@ -380,15 +412,15 @@
ASSERT(!m_didUpdateForChildren);
if (m_cachedOffsetsEnabled) {
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
+#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
LayoutRect slowPathRect(rect);
slowMapToVisualRectInAncestorSpace(m_currentObject, *m_paintInvalidationContainer, slowPathRect);
#endif
rect.move(m_paintOffset);
if (m_clipped)
rect.intersect(m_clipRect);
-#ifdef CHECK_FAST_PATH_SLOW_PATH_EQUALITY
- assertFastPathAndSlowPathRectsEqual(rect, slowPathRect);
+#if ASSERT_SAME_RESULT_SLOW_AND_FAST_PATH
+ assertRectsEqual(m_currentObject, *m_paintInvalidationContainer, rect, slowPathRect);
#endif
} else {
slowMapToVisualRectInAncestorSpace(m_currentObject, *m_paintInvalidationContainer, rect);
@@ -423,56 +455,4 @@
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