Refine cull rect for SVGInlineTextBox painting

third_party/WebKit/Source/core/paint/SVGInlineTextBoxPainter.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 "core/paint/SVGInlineTextBoxPainter.h"
 
 #include "core/editing/Editor.h"
 #include "core/editing/markers/DocumentMarkerController.h"
 #include "core/editing/markers/RenderedDocumentMarker.h"
 #include "core/frame/LocalFrame.h"
@@ skipping 15 matching lines @@
 
 namespace blink {
 
 static inline bool textShouldBePainted(const LayoutSVGInlineText& textLayoutObject)
 {
     // Font::pixelSize(), returns FontDescription::computedPixelSize(), which returns "int(x + 0.5)".
     // If the absolute font size on screen is below x=0.5, don't render anything.
     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;
 }
 
 void SVGInlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     ASSERT(paintInfo.phase == PaintPhaseForeground || paintInfo.phase == PaintPhaseSelection);
     ASSERT(m_svgInlineTextBox.truncation() == cNoTruncation);
 
     if (m_svgInlineTextBox.getLineLayoutItem().style()->visibility() != VISIBLE)
         return;
 
     // 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()));
     if (!textShouldBePainted(textLayoutObject))
         return;
 
     DisplayItem::Type displayItemType = DisplayItem::paintPhaseToDrawingType(paintInfo.phase);
     if (!DrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, m_svgInlineTextBox, displayItemType)) {
         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);
 
         if (!m_svgInlineTextBox.textFragments().isEmpty())
             paintTextFragments(paintInfo, parentLayoutObject);
 
         InlineTextBoxPainter(m_svgInlineTextBox).paintDocumentMarkers(
             paintInfo, paintOffset, style,
             textLayoutObject.scaledFont(), DocumentMarkerPaintPhase::Foreground);
@@ skipping 420 matching lines @@
             stateSaver.save();
             paintInfo.context.concatCTM(fragment.buildFragmentTransform());
         }
         FloatRect fragmentRect = m_svgInlineTextBox.selectionRectForTextFragment(fragment, textMatchInfo.startPosition, textMatchInfo.endPosition, style);
         paintInfo.context.setFillColor(color);
         paintInfo.context.fillRect(fragmentRect);
     }
 }
 
 } // namespace blink