Remove block content clips where we can show that they are unnecessary.

This CL: 1. Modifies RenderBlock::computeOverflow to track, in addition to the visual overflow of the element itself, a bounding rect for the visual overflow of its contents. All call sites of addVisualOverflow are modified to declare whether the visual overflow is affected by the contents clip or not. 2. Modifies RenderBox::pushContentsClip to use this rect to determine whether the contents are visually contained by the clip, in which case it omits the clip. See measurements in my comment #3 on the bug: https://code.google.com/p/chromium/issues/detail?id=238732#c3 BUG=238732 [email protected] Review URL: https://codereview.chromium.org/16402019 git-svn-id: svn://svn.chromium.org/blink/trunk@155847 bbb929c8-8fbe-4397-9dbb-9b2b20218538
yoavweiss · Aug 9, 2013 · 70f2c26 · 70f2c26
1 parent 40bde9e
commit 70f2c26
Show file tree

Hide file tree

Showing 15 changed files with 383 additions and 30 deletions.
diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations
@@ -603,6 +603,9 @@ webkit.org/b/ [ Win7 ] http/tests/xmlhttprequest/redirect-cross-origin-post.html
 
 webkit.org/b/83647 [ Linux Win ] fast/multicol/cell-shrinkback.html [ ImageOnlyFailure ]
 
+crbug.com/238732 fast/forms/placeholder-position.html [ NeedsRebaseline ]
+crbug.com/238732 [ Mac ] editing/selection/select-across-readonly-input-3.html [ NeedsRebaseline ]
+
 # IETC grid layout
 webkit.org/b/83912 ietestcenter/css3/grid/grid-items-002.htm [ ImageOnlyFailure ]
 webkit.org/b/83913 ietestcenter/css3/grid/grid-items-003.htm [ ImageOnlyFailure ]

diff --git a/Source/core/core.gypi b/Source/core/core.gypi
@@ -3754,6 +3754,7 @@
             'platform/chromium/ClipboardChromiumTest.cpp',
             'platform/graphics/FontTest.cpp',
             'platform/graphics/GraphicsContextTest.cpp',
+            'platform/graphics/RoundedRectTest.cpp',
             'platform/graphics/UnitBezierTest.cpp',
             'platform/graphics/chromium/AnimationTranslationUtilTest.cpp',
             'platform/graphics/chromium/Canvas2DLayerBridgeTest.cpp',
@@ -3769,6 +3770,7 @@
             'platform/image-decoders/gif/GIFImageDecoderTest.cpp',
             'platform/image-decoders/webp/WEBPImageDecoderTest.cpp',
             'platform/text/DateTimeFormatTest.cpp',
+            'rendering/RenderOverflowTest.cpp',
             'tests/ArenaTestHelpers.h',
             'tests/FakeWebGraphicsContext3D.h',
             'tests/HTMLDimension.cpp',

diff --git a/Source/core/platform/graphics/RoundedRect.cpp b/Source/core/platform/graphics/RoundedRect.cpp
@@ -27,6 +27,7 @@
 
 #include "config.h"
 #include "core/platform/graphics/RoundedRect.h"
+#include "wtf/Assertions.h"
 
 #include <algorithm>
 
@@ -150,6 +151,16 @@ RoundedRect::RoundedRect(const IntRect& rect, const IntSize& topLeft, const IntS
 {
 }
 
+IntRect RoundedRect::radiusCenterRect() const
+{
+    ASSERT(isRenderable());
+    int minX = m_rect.x() + max(m_radii.topLeft().width(), m_radii.bottomLeft().width());
+    int minY = m_rect.y() + max(m_radii.topLeft().height(), m_radii.topRight().height());
+    int maxX = m_rect.maxX() - max(m_radii.topRight().width(), m_radii.bottomRight().width());
+    int maxY = m_rect.maxY() - max(m_radii.bottomLeft().height(), m_radii.bottomRight().height());
+    return IntRect(minX, minY, maxX - minX, maxY - minY);
+}
+
 void RoundedRect::includeLogicalEdges(const Radii& edges, bool isHorizontal, bool includeLogicalLeftEdge, bool includeLogicalRightEdge)
 {
     m_radii.includeLogicalEdges(edges, isHorizontal, includeLogicalLeftEdge, includeLogicalRightEdge);

diff --git a/Source/core/platform/graphics/RoundedRect.h b/Source/core/platform/graphics/RoundedRect.h
@@ -82,6 +82,9 @@ class RoundedRect {
     bool isRounded() const { return !m_radii.isZero(); }
     bool isEmpty() const { return m_rect.isEmpty(); }
 
+    // Returns a quickly computed rect enclosed by the rounded rect.
+    IntRect radiusCenterRect() const;
+
     void setRect(const IntRect& rect) { m_rect = rect; }
     void setRadii(const Radii& radii) { m_radii = radii; }
 

diff --git a/Source/core/platform/graphics/RoundedRectTest.cpp b/Source/core/platform/graphics/RoundedRectTest.cpp
@@ -0,0 +1,84 @@
+/*
+ * Copyright (C) 2013 Google Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are
+ * met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following disclaimer
+ * in the documentation and/or other materials provided with the
+ * distribution.
+ *     * Neither the name of Google Inc. nor the names of its
+ * contributors may be used to endorse or promote products derived from
+ * this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+#include "core/platform/graphics/RoundedRect.h"
+
+#include "core/platform/graphics/IntRect.h"
+
+#include <gtest/gtest.h>
+
+using namespace WebCore;
+
+namespace WebCore {
+
+// FIXME: Move this somewhere more generic.
+void PrintTo(const IntRect& rect, std::ostream* os)
+{
+    *os << "IntRect("
+        << rect.x() << ", "
+        << rect.y() << ", "
+        << rect.width() << ", "
+        << rect.height() << ")";
+}
+
+} // namespace WebCore
+
+namespace {
+
+TEST(RoundedRectTest, RadiusCenterRectZeroRadius)
+{
+    RoundedRect rr(100, 200, 300, 400);
+    EXPECT_TRUE(rr.rect().contains(rr.radiusCenterRect()));
+    EXPECT_EQ(IntRect(100, 200, 300, 400), rr.radiusCenterRect());
+}
+
+TEST(RoundedRectTest, RadiusCenterRectEqualRadius)
+{
+    RoundedRect rr(IntRect(100, 200, 300, 400), IntSize(10, 10), IntSize(10, 10), IntSize(10, 10), IntSize(10, 10));
+    EXPECT_TRUE(rr.rect().contains(rr.radiusCenterRect()));
+    EXPECT_EQ(IntRect(110, 210, 280, 380), rr.radiusCenterRect());
+}
+
+TEST(RoundedRectTest, RadiusCenterRectUnequalRadius)
+{
+    RoundedRect rr(IntRect(100, 200, 300, 400), IntSize(5, 5), IntSize(10, 10), IntSize(15, 15), IntSize(20, 20));
+    EXPECT_TRUE(rr.rect().contains(rr.radiusCenterRect()));
+    EXPECT_EQ(IntRect(115, 210, 265, 370), rr.radiusCenterRect());
+}
+
+TEST(RoundedRectTest, RadiusCenterRectElliptical)
+{
+    RoundedRect rr(IntRect(100, 200, 300, 400), IntSize(20, 10), IntSize(20, 10), IntSize(10, 20), IntSize(10, 20));
+    EXPECT_TRUE(rr.rect().contains(rr.radiusCenterRect()));
+    EXPECT_EQ(IntRect(120, 210, 260, 370), rr.radiusCenterRect());
+}
+
+} // namespace
diff --git a/Source/core/rendering/RenderBlock.cpp b/Source/core/rendering/RenderBlock.cpp
@@ -1714,8 +1714,7 @@ void RenderBlock::addOverflowFromChildren()
         if (columnCount(colInfo)) {
             LayoutRect lastRect = columnRectAt(colInfo, columnCount(colInfo) - 1);
             addLayoutOverflow(lastRect);
-            if (!hasOverflowClip())
-                addVisualOverflow(lastRect);
+            addContentsVisualOverflow(lastRect);
         }
     }
 }
@@ -1753,7 +1752,7 @@ void RenderBlock::computeOverflow(LayoutUnit oldClientAfterEdge, bool recomputeF
     if (textIndent < 0) {
         LayoutRect clientRect(noOverflowRect());
         LayoutRect rectToApply = LayoutRect(clientRect.x() + min<LayoutUnit>(0, textIndent), clientRect.y(), clientRect.width() - min<LayoutUnit>(0, textIndent), clientRect.height());
-        addVisualOverflow(rectToApply);
+        addContentsVisualOverflow(rectToApply);
     }
 
     // Add visual overflow from box-shadow and border-image-outset.
@@ -2904,7 +2903,13 @@ void RenderBlock::paint(PaintInfo& paintInfo, const LayoutPoint& paintOffset)
             return;
     }
 
-    bool pushedClip = pushContentsClip(paintInfo, adjustedPaintOffset);
+    // There are some cases where not all clipped visual overflow is accounted for.
+    // FIXME: reduce the number of such cases.
+    ContentsClipBehavior contentsClipBehavior = ForceContentsClip;
+    if (hasOverflowClip() && !hasControlClip() && !(shouldPaintSelectionGaps() && phase == PaintPhaseForeground) && !hasCaret())
+        contentsClipBehavior = SkipContentsClipIfPossible;
+
+    bool pushedClip = pushContentsClip(paintInfo, adjustedPaintOffset, contentsClipBehavior);
     paintObject(paintInfo, adjustedPaintOffset);
     if (pushedClip)
         popContentsClip(paintInfo, phase, adjustedPaintOffset);
@@ -3097,8 +3102,7 @@ void RenderBlock::paintChild(RenderBox* child, PaintInfo& paintInfo, const Layou
         child->paint(paintInfo, childPoint);
 }
 
-
-void RenderBlock::paintCaret(PaintInfo& paintInfo, const LayoutPoint& paintOffset, CaretType type)
+bool RenderBlock::hasCaret(CaretType type) const
 {
     // Paint the caret if the FrameSelection says so or if caret browsing is enabled
     bool caretBrowsing = frame()->settings() && frame()->settings()->caretBrowsingEnabled();
@@ -3111,13 +3115,18 @@ void RenderBlock::paintCaret(PaintInfo& paintInfo, const LayoutPoint& paintOffse
         caretPainter = frame()->page()->dragCaretController()->caretRenderer();
         isContentEditable = frame()->page()->dragCaretController()->isContentEditable();
     }
+    return caretPainter == this && (isContentEditable || caretBrowsing);
+}
 
-    if (caretPainter == this && (isContentEditable || caretBrowsing)) {
-        if (type == CursorCaret)
-            frame()->selection()->paintCaret(paintInfo.context, paintOffset, paintInfo.rect);
-        else
-            frame()->page()->dragCaretController()->paintDragCaret(frame(), paintInfo.context, paintOffset, paintInfo.rect);
-    }
+void RenderBlock::paintCaret(PaintInfo& paintInfo, const LayoutPoint& paintOffset, CaretType type)
+{
+    if (!hasCaret(type))
+        return;
+
+    if (type == CursorCaret)
+        frame()->selection()->paintCaret(paintInfo.context, paintOffset, paintInfo.rect);
+    else
+        frame()->page()->dragCaretController()->paintDragCaret(frame(), paintInfo.context, paintOffset, paintInfo.rect);
 }
 
 void RenderBlock::paintObject(PaintInfo& paintInfo, const LayoutPoint& paintOffset)

diff --git a/Source/core/rendering/RenderBlock.h b/Source/core/rendering/RenderBlock.h
@@ -881,6 +881,9 @@ class RenderBlock : public RenderBox {
     void paintSelection(PaintInfo&, const LayoutPoint&);
     void paintCaret(PaintInfo&, const LayoutPoint&, CaretType);
 
+    bool hasCaret() const { return hasCaret(CursorCaret) || hasCaret(DragCaret); }
+    bool hasCaret(CaretType) const;
+
     FloatingObject* insertFloatingObject(RenderBox*);
     void removeFloatingObject(RenderBox*);
     void removeFloatingObjectsBelow(FloatingObject*, int logicalOffset);

diff --git a/Source/core/rendering/RenderBlockLineLayout.cpp b/Source/core/rendering/RenderBlockLineLayout.cpp
@@ -3315,8 +3315,8 @@ void RenderBlock::addOverflowFromInlineChildren()
         endPadding = 1;
     for (RootInlineBox* curr = firstRootBox(); curr; curr = curr->nextRootBox()) {
         addLayoutOverflow(curr->paddedLayoutOverflowRect(endPadding));
-        if (!hasOverflowClip())
-            addVisualOverflow(curr->visualOverflowRect(curr->lineTop(), curr->lineBottom()));
+        LayoutRect visualOverflow = curr->visualOverflowRect(curr->lineTop(), curr->lineBottom());
+        addContentsVisualOverflow(visualOverflow);
     }
 }
 

diff --git a/Source/core/rendering/RenderBox.cpp b/Source/core/rendering/RenderBox.cpp
@@ -1517,7 +1517,7 @@ bool RenderBox::repaintLayerRectsForImage(WrappedImagePtr image, const FillLayer
     return false;
 }
 
-bool RenderBox::pushContentsClip(PaintInfo& paintInfo, const LayoutPoint& accumulatedOffset)
+bool RenderBox::pushContentsClip(PaintInfo& paintInfo, const LayoutPoint& accumulatedOffset, ContentsClipBehavior contentsClipBehavior)
 {
     if (paintInfo.phase == PaintPhaseBlockBackground || paintInfo.phase == PaintPhaseSelfOutline || paintInfo.phase == PaintPhaseMask)
         return false;
@@ -1528,18 +1528,38 @@ bool RenderBox::pushContentsClip(PaintInfo& paintInfo, const LayoutPoint& accumu
     if (!isControlClip && !isOverflowClip)
         return false;
 
+    LayoutRect clipRect = isControlClip ? controlClipRect(accumulatedOffset) : overflowClipRect(accumulatedOffset, paintInfo.renderRegion);
+    RoundedRect clipRoundedRect(0, 0, 0, 0);
+    bool hasBorderRadius = style()->hasBorderRadius();
+    if (hasBorderRadius)
+        clipRoundedRect = style()->getRoundedInnerBorderFor(LayoutRect(accumulatedOffset, size()));
+
+    if (contentsClipBehavior == SkipContentsClipIfPossible) {
+        LayoutRect contentsVisualOverflow = contentsVisualOverflowRect();
+        if (contentsVisualOverflow.isEmpty())
+            return false;
+
+        LayoutRect conservativeClipRect = clipRect;
+        if (hasBorderRadius)
+            conservativeClipRect.intersect(clipRoundedRect.radiusCenterRect());
+        conservativeClipRect.moveBy(-accumulatedOffset);
+        if (hasLayer())
+            conservativeClipRect.move(scrolledContentOffset());
+        if (conservativeClipRect.contains(contentsVisualOverflow))
+            return false;
+    }
+
     if (paintInfo.phase == PaintPhaseOutline)
         paintInfo.phase = PaintPhaseChildOutlines;
     else if (paintInfo.phase == PaintPhaseChildBlockBackground) {
         paintInfo.phase = PaintPhaseBlockBackground;
         paintObject(paintInfo, accumulatedOffset);
         paintInfo.phase = PaintPhaseChildBlockBackgrounds;
     }
-    IntRect clipRect = pixelSnappedIntRect(isControlClip ? controlClipRect(accumulatedOffset) : overflowClipRect(accumulatedOffset, paintInfo.renderRegion));
     paintInfo.context->save();
-    if (style()->hasBorderRadius())
-        paintInfo.context->clipRoundedRect(style()->getRoundedInnerBorderFor(LayoutRect(accumulatedOffset, size())));
-    paintInfo.context->clip(clipRect);
+    if (hasBorderRadius)
+        paintInfo.context->clipRoundedRect(clipRoundedRect);
+    paintInfo.context->clip(pixelSnappedIntRect(clipRect));
     return true;
 }
 
@@ -4210,7 +4230,8 @@ void RenderBox::addVisualEffectOverflow()
     }
 
     // Add in the final overflow with shadows and outsets combined.
-    addVisualOverflow(LayoutRect(overflowMinX, overflowMinY, overflowMaxX - overflowMinX, overflowMaxY - overflowMinY));
+    LayoutRect visualEffectOverflow(overflowMinX, overflowMinY, overflowMaxX - overflowMinX, overflowMaxY - overflowMinY);
+    addVisualOverflow(visualEffectOverflow);
 }
 
 void RenderBox::addOverflowFromChild(RenderBox* child, const LayoutSize& delta)
@@ -4229,11 +4250,11 @@ void RenderBox::addOverflowFromChild(RenderBox* child, const LayoutSize& delta)
     // Add in visual overflow from the child.  Even if the child clips its overflow, it may still
     // have visual overflow of its own set from box shadows or reflections.  It is unnecessary to propagate this
     // overflow if we are clipping our own overflow.
-    if (child->hasSelfPaintingLayer() || hasOverflowClip())
+    if (child->hasSelfPaintingLayer())
         return;
     LayoutRect childVisualOverflowRect = child->visualOverflowRectForPropagation(style());
     childVisualOverflowRect.move(delta);
-    addVisualOverflow(childVisualOverflowRect);
+    addContentsVisualOverflow(childVisualOverflowRect);
 }
 
 void RenderBox::addLayoutOverflow(const LayoutRect& rect)
@@ -4298,12 +4319,24 @@ void RenderBox::addVisualOverflow(const LayoutRect& rect)
     m_overflow->addVisualOverflow(rect);
 }
 
+void RenderBox::addContentsVisualOverflow(const LayoutRect& rect)
+{
+    if (!hasOverflowClip()) {
+        addVisualOverflow(rect);
+        return;
+    }
+
+    if (!m_overflow)
+        m_overflow = adoptPtr(new RenderOverflow(clientBoxRect(), borderBoxRect()));
+    m_overflow->addContentsVisualOverflow(rect);
+}
+
 void RenderBox::clearLayoutOverflow()
 {
     if (!m_overflow)
         return;
 
-    if (!hasVisualOverflow()) {
+    if (!hasVisualOverflow() && contentsVisualOverflowRect().isEmpty()) {
         m_overflow.clear();
         return;
     }

diff --git a/Source/core/rendering/RenderBox.h b/Source/core/rendering/RenderBox.h
@@ -40,6 +40,8 @@ enum OverlayScrollbarSizeRelevancy { IgnoreOverlayScrollbarSize, IncludeOverlayS
 
 enum ShouldComputePreferred { ComputeActual, ComputePreferred };
 
+enum ContentsClipBehavior { ForceContentsClip, SkipContentsClipIfPossible };
+
 class RenderBox : public RenderBoxModelObject {
 public:
     explicit RenderBox(ContainerNode*);
@@ -186,9 +188,14 @@ class RenderBox : public RenderBoxModelObject {
 
     LayoutRect overflowRectForPaintRejection() const;
 
+    LayoutRect contentsVisualOverflowRect() const { return m_overflow ? m_overflow->contentsVisualOverflowRect() : LayoutRect(); }
+
     void addLayoutOverflow(const LayoutRect&);
     void addVisualOverflow(const LayoutRect&);
 
+    // Clipped by the contents clip, if one exists.
+    void addContentsVisualOverflow(const LayoutRect&);
+
     void addVisualEffectOverflow();
     void addOverflowFromChild(RenderBox* child) { addOverflowFromChild(child, child->locationOffset()); }
     void addOverflowFromChild(RenderBox* child, const LayoutSize& delta);
@@ -470,7 +477,7 @@ class RenderBox : public RenderBoxModelObject {
     LayoutRect clipRect(const LayoutPoint& location, RenderRegion*);
     virtual bool hasControlClip() const { return false; }
     virtual LayoutRect controlClipRect(const LayoutPoint&) const { return LayoutRect(); }
-    bool pushContentsClip(PaintInfo&, const LayoutPoint& accumulatedOffset);
+    bool pushContentsClip(PaintInfo&, const LayoutPoint& accumulatedOffset, ContentsClipBehavior);
     void popContentsClip(PaintInfo&, PaintPhase originalPhase, const LayoutPoint& accumulatedOffset);
 
     virtual void paintObject(PaintInfo&, const LayoutPoint&) { ASSERT_NOT_REACHED(); }

diff --git a/Source/core/rendering/RenderListItem.cpp b/Source/core/rendering/RenderListItem.cpp
@@ -392,16 +392,16 @@ void RenderListItem::positionListMarker()
             bool propagateLayoutOverflow = true;
             do {
                 o = o->parentBox();
-                if (o->hasOverflowClip())
-                    propagateVisualOverflow = false;
                 if (o->isRenderBlock()) {
                     if (propagateVisualOverflow)
-                        toRenderBlock(o)->addVisualOverflow(markerRect);
+                        toRenderBlock(o)->addContentsVisualOverflow(markerRect);
                     if (propagateLayoutOverflow)
                         toRenderBlock(o)->addLayoutOverflow(markerRect);
                 }
-                if (o->hasOverflowClip())
+                if (o->hasOverflowClip()) {
                     propagateLayoutOverflow = false;
+                    propagateVisualOverflow = false;
+                }
                 if (o->hasSelfPaintingLayer())
                     propagateVisualOverflow = false;
                 markerRect.moveBy(-o->location());