Improve outline bounds estimation in InlinePainter

Source/core/paint/InlinePainter.cpp

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "config.h"
 #include "core/paint/InlinePainter.h"
 
 #include "core/layout/LayoutBlock.h"
 #include "core/layout/LayoutInline.h"
 #include "core/layout/LayoutTheme.h"
@@ skipping 11 matching lines @@
 void InlinePainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     // FIXME: When Skia supports annotation rect covering (https://code.google.com/p/skia/issues/detail?id=3872),
     // this rect may be covered by foreground and descendant drawings. Then we may need a dedicated paint phase.
     if (paintInfo.phase == PaintPhaseForeground && paintInfo.context->printing())
         ObjectPainter(m_layoutInline).addPDFURLRectIfNeeded(paintInfo, paintOffset);
 
     LineBoxListPainter(*m_layoutInline.lineBoxes()).paint(&m_layoutInline, paintInfo, paintOffset);
 }
 
+LayoutRect InlinePainter::outlinePaintRect(const LayoutPoint& paintOffset) const
+{
+    const ComputedStyle& style = m_layoutInline.styleRef();
+    int outlineOutset;

[chrishtr, 2015/06/04 17:08:12]

Why does linesVisualOverflowBoundingBox() not already include the bounding box?
If not then this seems to imply to me that this code is wrong also, since it
should include outline:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

So include outline in linesVisualOverflowBoundingBox()?

[fs, 2015/06/05 10:19:52]

Hmm, yes... the inflate(outlineSize) in LayoutInline::clippedOverflowRect
confused me - now it looks rather redundant to me.

+    if (style.outlineStyleIsAuto())
+        outlineOutset = GraphicsContext::focusRingOutsetExtent(style.outlineOffset(), style.outlineWidth());
+    else
+        outlineOutset = style.outlineSize();
+    LayoutRect outlineRect = m_layoutInline.linesVisualOverflowBoundingBox();
+    outlineRect.inflate(outlineOutset);
+    outlineRect.moveBy(paintOffset);
+    return outlineRect;
+}
+
 void InlinePainter::paintOutline(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     const ComputedStyle& styleToUse = m_layoutInline.styleRef();
     if (!styleToUse.hasOutline())
         return;
 
     if (styleToUse.outlineStyleIsAuto()) {
         if (LayoutTheme::theme().shouldDrawDefaultFocusRing(&m_layoutInline)) {
             Vector<LayoutRect> focusRingRects;
             m_layoutInline.addFocusRingRects(focusRingRects, paintOffset);
-            LayoutRect focusRingBoundingRect;
-            for (const auto& rect : focusRingRects)
-                focusRingBoundingRect.unite(rect);
 
-            LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutInline, paintInfo.phase, focusRingBoundingRect);
+            LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutInline, paintInfo.phase, outlinePaintRect(paintOffset));
             if (recorder.canUseCachedDrawing())
                 return;
 
             // Only paint the focus ring by hand if the theme isn't able to draw the focus ring.
             ObjectPainter(m_layoutInline).paintFocusRing(paintInfo, styleToUse, focusRingRects);
         }
         return;
     }
 
     if (styleToUse.outlineStyle() == BNONE)
         return;
 
     Vector<LayoutRect> rects;
 
     rects.append(LayoutRect());
     for (InlineFlowBox* curr = m_layoutInline.firstLineBox(); curr; curr = curr->nextLineBox()) {
         RootInlineBox& root = curr->root();
         LayoutUnit top = std::max<LayoutUnit>(root.lineTop(), curr->logicalTop());
         LayoutUnit bottom = std::min<LayoutUnit>(root.lineBottom(), curr->logicalBottom());
         rects.append(LayoutRect(curr->x(), top, curr->logicalWidth(), bottom - top));
     }
     rects.append(LayoutRect());
 
     Color outlineColor = m_layoutInline.resolveColor(styleToUse, CSSPropertyOutlineColor);
     bool useTransparencyLayer = outlineColor.hasAlpha();
 
-    int outlineWidth = styleToUse.outlineWidth();
-    LayoutRect bounds;
-    for (const auto& rect : rects) {
-        LayoutRect rectCopy(rect);
-        rectCopy.expand(LayoutSize(outlineWidth, outlineWidth));
-        bounds.unite(rectCopy);
-    }
-    bounds.moveBy(paintOffset);
-
-    LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutInline, paintInfo.phase, bounds);
+    LayoutObjectDrawingRecorder recorder(*paintInfo.context, m_layoutInline, paintInfo.phase, outlinePaintRect(paintOffset));
     if (recorder.canUseCachedDrawing())
         return;
 
     GraphicsContext* graphicsContext = paintInfo.context;
     if (useTransparencyLayer) {
         graphicsContext->beginLayer(static_cast<float>(outlineColor.alpha()) / 255);
         outlineColor = Color(outlineColor.red(), outlineColor.green(), outlineColor.blue());
     }
 
     for (unsigned i = 1; i < rects.size() - 1; i++)
@@ skipping 117 matching lines @@
             pixelSnappedBox.maxX() + outlineWidth,
             pixelSnappedBox.maxY() + outlineWidth,
             BSBottom, outlineColor, outlineStyle,
             outlineWidth,
             outlineWidth,
             antialias);
     }
 }
 
 } // namespace blink