Combine outline and focus ring code paths

Source/core/paint/ObjectPainter.cpp

Index: Source/core/paint/ObjectPainter.cpp
diff --git a/Source/core/paint/ObjectPainter.cpp b/Source/core/paint/ObjectPainter.cpp
index 3e96ff3dd80573bd7b68ca47a84bda586d44ee73..226bc1ee5d08c1684ae0a59cb16358831415b441 100644
--- a/Source/core/paint/ObjectPainter.cpp
+++ b/Source/core/paint/ObjectPainter.cpp
@@ -5,6 +5,7 @@
 #include "config.h"
 #include "core/paint/ObjectPainter.h"
 
+#include "core/layout/LayoutInline.h"
 #include "core/layout/LayoutObject.h"
 #include "core/layout/LayoutTheme.h"
 #include "core/paint/BoxBorderPainter.h"
@@ -16,16 +17,172 @@
 
 namespace blink {
 
-void ObjectPainter::paintFocusRing(const PaintInfo& paintInfo, const ComputedStyle& style, const Vector<LayoutRect>& focusRingRects)
+namespace {
+
+void paintNonAutoOutlineFastPath(const PaintInfo& paintInfo, const IntRect& rect, const ComputedStyle& style, const Color& color)

[pdr., 2015/08/27 22:59:33]

How "outline:auto" affects this was confusing to me at first. Can you add a
comment above here describing this a little more? Maybe something like:
// Fast path for painting a single outline.

We may want to change the name as well. Maybe just "paintSingleOutlineFastPath"?

[Xianzhu, 2015/08/28 00:26:04]

Renamed it to paintSingleRectangleOutline, and paintNonAutoOutlineSlowPath to
paintComplexOutline.

+{
+    LayoutRect inner(rect);

[pdr., 2015/08/27 22:59:33]

Can you assert that the style is not auto here?

[Xianzhu, 2015/08/28 00:26:04]

Done.

This seems less confusing to include "NonAuto" in the function names.

+    inner.inflate(style.outlineOffset());
+    LayoutRect outer(inner);
+    outer.inflate(style.outlineWidth());
+    const BorderEdge commonEdgeInfo(style.outlineWidth(), color, style.outlineStyle());
+    BoxBorderPainter(style, outer, inner, commonEdgeInfo).paintBorder(paintInfo, outer);
+}
+
+struct OutlineEdgeInfo {
+    int x1;
+    int y1;
+    int x2;
+    int y2;
+    BoxSide side;
+};
+
+// Adjust length of edges if needed. Returns the width of the joint.
+int adjustJoint(int outlineWidth, OutlineEdgeInfo& edge1, OutlineEdgeInfo& edge2)
 {
-    ASSERT(style.outlineStyleIsAuto());
-    Vector<IntRect> focusRingIntRects;
-    for (size_t i = 0; i < focusRingRects.size(); ++i)
-        focusRingIntRects.append(pixelSnappedIntRect(focusRingRects[i]));
-    paintInfo.context->drawFocusRing(focusRingIntRects, style.outlineWidth(), style.outlineOffset(), m_layoutObject.resolveColor(style, CSSPropertyOutlineColor));
+    // A clockwise joint:
+    // - needs no adjustment of edge length because our edges are along the clockwise outer edge of the outline;
+    // - needs a positive adjacent joint width (required by drawLineForBoxSide).
+    // A counterclockwise joint:
+    // - needs to increase the edge length to include the joint;
+    // - needs a negative adjacent joint width (required by drawLineForBoxSide).
+    switch (edge1.side) {
+    case BSTop:
+        switch (edge2.side) {
+        case BSRight: // Clockwise
+            return outlineWidth;
+        case BSLeft: // Counterclockwise
+            edge1.x2 += outlineWidth;
+            edge2.y2 += outlineWidth;
+            return -outlineWidth;
+        default: // Same side or no joint.
+            return 0;
+        }
+    case BSRight:
+        switch (edge2.side) {
+        case BSBottom: // Clockwise
+            return outlineWidth;
+        case BSTop: // Counterclockwise
+            edge1.y2 += outlineWidth;
+            edge2.x1 -= outlineWidth;
+            return -outlineWidth;
+        default: // Same side or no joint.
+            return 0;
+        }
+    case BSBottom:
+        switch (edge2.side) {
+        case BSLeft: // Clockwise
+            return outlineWidth;
+        case BSRight: // Counterclockwise
+            edge1.x1 -= outlineWidth;
+            edge2.y1 -= outlineWidth;
+            return -outlineWidth;
+        default: // Same side or no joint.
+            return 0;
+        }
+    case BSLeft:
+        switch (edge2.side) {
+        case BSTop: // Clockwise
+            return outlineWidth;
+        case BSBottom: // Counterclockwise
+            edge1.y1 -= outlineWidth;
+            edge2.x2 += outlineWidth;
+            return -outlineWidth;
+        default: // Same side or no joint.
+            return 0;
+        }
+    default:
+        ASSERT_NOT_REACHED();
+        return 0;
+    }
+}
+
+void paintNonAutoOutlineSlowPath(GraphicsContext& graphicsContext, const Vector<IntRect> rects, const ComputedStyle& style, const Color& color)

[pdr., 2015/08/27 22:59:33]

Same comments here as above.

[Xianzhu, 2015/08/28 00:26:04]

Done.

+{
+    // Construct a clockwise path along the outer edge of the outline.
+    SkRegion region;
+    int width = style.outlineWidth();
+    int outset = style.outlineOffset() + style.outlineWidth();
+    for (auto& r : rects) {
+        IntRect rect = r;
+        rect.inflate(outset);
+        region.op(rect, SkRegion::kUnion_Op);
+    }
+    SkPath path;
+    if (!region.getBoundaryPath(&path))
+        return;
+
+    Vector<OutlineEdgeInfo, 4> edges;
+
+    SkPath::Iter iter(path, false);
+    SkPoint points[4];
+    size_t count = 0;
+    for (SkPath::Verb verb = iter.next(points, false); verb != SkPath::kDone_Verb; verb = iter.next(points, false)) {
+        if (verb != SkPath::kLine_Verb)
+            continue;
+
+        edges.grow(++count);
+        OutlineEdgeInfo& edge = edges.last();
+        edge.x1 = SkScalarTruncToInt(points[0].x());
+        edge.y1 = SkScalarTruncToInt(points[0].y());
+        edge.x2 = SkScalarTruncToInt(points[1].x());
+        edge.y2 = SkScalarTruncToInt(points[1].y());
+        if (edge.x1 == edge.x2) {
+            if (edge.y1 < edge.y2) {
+                edge.x1 -= width;
+                edge.side = BSRight;
+            } else {
+                std::swap(edge.y1, edge.y2);
+                edge.x2 += width;
+                edge.side = BSLeft;
+            }
+        } else {
+            ASSERT(edge.y1 == edge.y2);
+            if (edge.x1 < edge.x2) {
+                edge.y2 += width;
+                edge.side = BSTop;
+            } else {
+                std::swap(edge.x1, edge.x2);
+                edge.y1 -= width;
+                edge.side = BSBottom;
+            }
+        }
+    }
+
+    if (!count)
+        return;
+
+    Color outlineColor = color;
+    bool useTransparencyLayer = color.hasAlpha();
+    if (useTransparencyLayer) {
+        graphicsContext.beginLayer(static_cast<float>(color.alpha()) / 255);
+        outlineColor = Color(outlineColor.red(), outlineColor.green(), outlineColor.blue());
+    }
+
+    ASSERT(count >= 4 && edges.size() == count);
+    int firstAdjacentWidth = adjustJoint(width, edges.last(), edges.first());
+
+    // The width of the angled part of starting and ending joint of the current edge.
+    int adjacentWidthStart = firstAdjacentWidth;
+    int adjacentWidthEnd;
+    for (size_t i = 0; i < count; ++i) {
+        OutlineEdgeInfo& edge = edges[i];
+        adjacentWidthEnd = i == count - 1 ? firstAdjacentWidth : adjustJoint(width, edge, edges[i + 1]);
+        int adjacentWidth1 = adjacentWidthStart;
+        int adjacentWidth2 = adjacentWidthEnd;
+        if (edge.side == BSLeft || edge.side == BSBottom)
+            std::swap(adjacentWidth1, adjacentWidth2);
+        ObjectPainter::drawLineForBoxSide(&graphicsContext, edge.x1, edge.y1, edge.x2, edge.y2, edge.side, outlineColor, style.outlineStyle(), adjacentWidth1, adjacentWidth2, false);
+        adjacentWidthStart = adjacentWidthEnd;
+    }
+
+    if (useTransparencyLayer)
+        graphicsContext.endLayer();
 }
 
-void ObjectPainter::paintOutline(const PaintInfo& paintInfo, const LayoutRect& visualOverflowRect, const LayoutSize& objectSize, const LayoutPoint& paintOffset)
+} // namespace
+
+void ObjectPainter::paintOutline(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     const ComputedStyle& styleToUse = m_layoutObject.styleRef();
     if (!styleToUse.hasOutline())
@@ -34,34 +191,54 @@ void ObjectPainter::paintOutline(const PaintInfo& paintInfo, const LayoutRect& v
     if (LayoutObjectDrawingRecorder::useCachedDrawingIfPossible(*paintInfo.context, m_layoutObject, paintInfo.phase))
         return;
 
-    LayoutRect visualOverflowBounds(visualOverflowRect);
-    visualOverflowBounds.moveBy(paintOffset);
-    LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutObject, paintInfo.phase, visualOverflowBounds);
+    // Only paint the focus ring by hand if the theme isn't able to draw the focus ring.
+    bool paintFocusRing = styleToUse.outlineStyleIsAuto();
+    if (paintFocusRing && !LayoutTheme::theme().shouldDrawDefaultFocusRing(&m_layoutObject))
+        return;
 
+    Vector<LayoutRect> outlineRects;
+    m_layoutObject.addOutlineRects(outlineRects, paintOffset);
+
+    Vector<IntRect> pixelSnappedOutlineRects;
+    for (auto& r : outlineRects)
+        pixelSnappedOutlineRects.append(pixelSnappedIntRect(r));
+
+    IntRect unitedOutlineRect = unionRect(pixelSnappedOutlineRects);
+    if (unitedOutlineRect.isEmpty()) {
+        // We need to draw an outline around an empty point.
+        IntRect emptyPointRect = pixelSnappedIntRect(LayoutRect(paintOffset, LayoutSize()));
+        pixelSnappedOutlineRects.append(emptyPointRect);
+        unitedOutlineRect = emptyPointRect;
+    }
+
+    IntRect bounds = unitedOutlineRect;
+    bounds.inflate(m_layoutObject.styleRef().outlineOutsetExtent());
+    LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutObject, paintInfo.phase, bounds);
+
+    Color color = m_layoutObject.resolveColor(styleToUse, CSSPropertyOutlineColor);
     if (styleToUse.outlineStyleIsAuto()) {
-        if (LayoutTheme::theme().shouldDrawDefaultFocusRing(&m_layoutObject)) {
-            // Only paint the focus ring by hand if the theme isn't able to draw the focus ring.
-            Vector<LayoutRect> focusRingRects;
-            m_layoutObject.addOutlineRects(focusRingRects, paintOffset);
-            paintFocusRing(paintInfo, styleToUse, focusRingRects);
-        }
+        paintInfo.context->drawFocusRing(pixelSnappedOutlineRects, styleToUse.outlineWidth(), styleToUse.outlineOffset(), color);
         return;
     }
 
-    if (styleToUse.outlineStyle() == BNONE)
+    if (unitedOutlineRect == pixelSnappedOutlineRects[0]) {
+        paintNonAutoOutlineFastPath(paintInfo, unitedOutlineRect, styleToUse, color);
         return;
+    }
+    paintNonAutoOutlineSlowPath(*paintInfo.context, pixelSnappedOutlineRects, styleToUse, color);
+}
 
-    LayoutRect inner(paintOffset, objectSize);
-    inner = LayoutRect(pixelSnappedIntRect(inner));
-    inner.inflate(styleToUse.outlineOffset());
+void ObjectPainter::paintInlineChildrenOutlines(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
+{
+    ASSERT(paintInfo.phase == PaintPhaseOutline || paintInfo.phase == PaintPhaseChildOutlines);
 
-    LayoutRect outer(inner);
-    int outlineWidth = styleToUse.outlineWidth();
-    outer.inflate(outlineWidth);
+    PaintInfo childPaintInfo(paintInfo);
+    childPaintInfo.phase = paintInfo.phase == PaintPhaseChildOutlines ? PaintPhaseOutline : paintInfo.phase;
 
-    const BorderEdge commonEdgeInfo(outlineWidth,
-        m_layoutObject.resolveColor(styleToUse, CSSPropertyOutlineColor), styleToUse.outlineStyle());
-    BoxBorderPainter(styleToUse, outer, inner, commonEdgeInfo).paintBorder(paintInfo, outer);
+    for (LayoutObject* child = m_layoutObject.slowFirstChild(); child; child = child->nextSibling()) {
+        if (child->isLayoutInline() && !toLayoutInline(child)->hasSelfPaintingLayer())
+            child->paint(childPaintInfo, paintOffset);
+    }
 }
 
 void ObjectPainter::addPDFURLRectIfNeeded(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)