Refine cull rect for SVGInlineTextBox painting

third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp

Index: third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
diff --git a/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp b/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
index 93f106836dfb6959eb96f3cbe10e6f858e91f1eb..00cb35fee1cc2919f8070243346beb34338d8578 100644
--- a/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
+++ b/third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.cpp
@@ -33,6 +33,11 @@ static inline bool textShouldBePainted(const LayoutSVGInlineText& textLayoutObje
     return textLayoutObject.scaledFont().getFontDescription().computedPixelSize();
 }
 
+static bool paintsMarkerHighlights(const LayoutObject& layoutObject)

[pdr., 2016/07/06 21:06:46]

Make InlineTextBoxPainter's paintsMarkerHighlights a protected member function
and use it here too?

[fs, 2016/07/06 21:32:41]

SVGInlineTextBoxPainter are not related to InlineTextBoxPainter by inheritance
(only by name and function), but I can add it as a public (static) entry point -
or a protected one with a friend declaration (I think I prefer the former.) We
could probably add an inheritance relation-ship here, but that might call for an
unproportional amount of extra work. Other options?

+{
+    return layoutObject.node() && layoutObject.document().markers().hasMarkers(layoutObject.node());
+}
+
 bool SVGInlineTextBoxPainter::shouldPaintSelection(const PaintInfo& paintInfo) const
 {
     return !paintInfo.isPrinting() && m_svgInlineTextBox.getSelectionState() != SelectionNone;
@@ -49,7 +54,8 @@ void SVGInlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoin
     // We're explicitly not supporting composition & custom underlines and custom highlighters -- unlike InlineTextBox.
     // If we ever need that for SVG, it's very easy to refactor and reuse the code.
 
-    if (paintInfo.phase == PaintPhaseSelection && !shouldPaintSelection(paintInfo))
+    bool haveSelection = shouldPaintSelection(paintInfo);
+    if (!haveSelection && paintInfo.phase == PaintPhaseSelection)
         return;
 
     LayoutSVGInlineText& textLayoutObject = toLayoutSVGInlineText(*LineLayoutAPIShim::layoutObjectFrom(m_svgInlineTextBox.getLineLayoutItem()));
@@ -61,8 +67,19 @@ void SVGInlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoin
         LayoutObject& parentLayoutObject = *LineLayoutAPIShim::layoutObjectFrom(m_svgInlineTextBox.parent()->getLineLayoutItem());
         const ComputedStyle& style = parentLayoutObject.styleRef();
 
-        // TODO(chrishtr): passing the cull rect is incorrect.
-        DrawingRecorder recorder(paintInfo.context, m_svgInlineTextBox, displayItemType, FloatRect(paintInfo.cullRect().m_rect));
+        // We compute the paint rect with what looks like the logical values, to match the

[chrishtr, 2016/07/06 20:53:20]

Factor all this into a method saying it's computing the cull rect for painting?

[fs, 2016/07/06 21:32:41]

Will do.

[fs, 2016/07/06 23:33:30]

Moved into a boundsFor... method.

+        // computation in SVGInlineTextBox::calculateBoundaries, and the fact that vertical (etc)
+        // layouts are handled by SVGTextLayoutEngine.
+        LayoutRect paintRect(
+            m_svgInlineTextBox.topLeft(),
+            LayoutSize(m_svgInlineTextBox.logicalWidth(), m_svgInlineTextBox.logicalHeight()));

[chrishtr, 2016/07/06 20:53:20]

Physical, not logical.

[fs, 2016/07/06 21:32:41]

Per the comment above this code, this is intentionally using logical* (because
"weirdness and legacy" presumably). I can make sure the helper method I'll add
starts with 'physical' though.

+        if (paintInfo.phase != PaintPhaseSelection && (haveSelection || paintsMarkerHighlights(textLayoutObject))) {
+            paintRect.unite(m_svgInlineTextBox.localSelectionRect(
+                m_svgInlineTextBox.start(), m_svgInlineTextBox.start() + m_svgInlineTextBox.len()));
+        }
+        paintRect.moveBy(paintOffset);
+
+        DrawingRecorder recorder(paintInfo.context, m_svgInlineTextBox, displayItemType, FloatRect(paintRect));
         InlineTextBoxPainter(m_svgInlineTextBox).paintDocumentMarkers(
             paintInfo, paintOffset, style,
             textLayoutObject.scaledFont(), DocumentMarkerPaintPhase::Background);