Find In Page hides the text when text color matches text search hightlight color.

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 "config.h"
 #include "core/paint/SVGInlineTextBoxPainter.h"
 
 #include "core/editing/Editor.h"
 #include "core/editing/markers/DocumentMarkerController.h"
 #include "core/editing/markers/RenderedDocumentMarker.h"
@@ skipping 47 matching lines @@
         return;
 
     DisplayItem::Type displayItemType = DisplayItem::paintPhaseToDrawingType(paintInfo.phase);
     if (!DrawingRecorder::useCachedDrawingIfPossible(paintInfo.context, m_svgInlineTextBox, displayItemType)) {
         LayoutObject& parentLayoutObject = *LineLayoutPaintShim::layoutObjectFrom(m_svgInlineTextBox.parent()->lineLayoutItem());
         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));
         InlineTextBoxPainter(m_svgInlineTextBox).paintDocumentMarkers(
-            paintInfo.context, paintOffset, style,
-            textLayoutObject.scaledFont(), true);
+            paintInfo, paintOffset, style,
+            textLayoutObject.scaledFont(), InlineTextBoxPainter::DocumentMarkerPaintPhase::Background);
 
         if (!m_svgInlineTextBox.textFragments().isEmpty())
             paintTextFragments(paintInfo, parentLayoutObject);
+
+        InlineTextBoxPainter(m_svgInlineTextBox).paintDocumentMarkers(
+            paintInfo, paintOffset, style,
+            textLayoutObject.scaledFont(), InlineTextBoxPainter::DocumentMarkerPaintPhase::Foreground);
     }
 }
 
 void SVGInlineTextBoxPainter::paintTextFragments(const PaintInfo& paintInfo, LayoutObject& parentLayoutObject)
 {
     const ComputedStyle& style = parentLayoutObject.styleRef();
     const SVGComputedStyle& svgStyle = style.svgStyle();
 
     bool hasFill = svgStyle.hasFill();
     bool hasVisibleStroke = svgStyle.hasVisibleStroke();
@@ skipping 79 matching lines @@
 
     LayoutSVGInlineText& textLayoutObject = toLayoutSVGInlineText(*LineLayoutPaintShim::layoutObjectFrom(m_svgInlineTextBox.lineLayoutItem()));
     if (!textShouldBePainted(textLayoutObject))
         return;
 
     const ComputedStyle& style = m_svgInlineTextBox.parent()->lineLayoutItem().styleRef();
 
     int startPosition, endPosition;
     m_svgInlineTextBox.selectionStartEnd(startPosition, endPosition);
 
-    int fragmentStartPosition = 0;
-    int fragmentEndPosition = 0;
     AffineTransform fragmentTransform;
-    unsigned textFragmentsSize = m_svgInlineTextBox.textFragments().size();
-    for (unsigned i = 0; i < textFragmentsSize; ++i) {
-        const SVGTextFragment& fragment = m_svgInlineTextBox.textFragments().at(i);
-
-        fragmentStartPosition = startPosition;
-        fragmentEndPosition = endPosition;
-        if (!m_svgInlineTextBox.mapStartEndPositionsIntoFragmentCoordinates(fragment, fragmentStartPosition, fragmentEndPosition))
-            continue;
-
+    Vector<SVGTextFragmentWithRange> fragmentInfoList = collectFragmentsInRange(startPosition, endPosition);
+    for (const SVGTextFragmentWithRange& fragmentWithRange : fragmentInfoList) {
+        const SVGTextFragment& fragment = fragmentWithRange.fragment;
         GraphicsContextStateSaver stateSaver(paintInfo.context);
         fragment.buildFragmentTransform(fragmentTransform);
         if (!fragmentTransform.isIdentity())
             paintInfo.context.concatCTM(fragmentTransform);
 
         paintInfo.context.setFillColor(backgroundColor);
-        paintInfo.context.fillRect(m_svgInlineTextBox.selectionRectForTextFragment(fragment, fragmentStartPosition, fragmentEndPosition, style), backgroundColor);
+        paintInfo.context.fillRect(m_svgInlineTextBox.selectionRectForTextFragment(fragment, fragmentWithRange.startPosition, fragmentWithRange.endPosition, style), backgroundColor);
     }
 }
 
 static inline LayoutObject* findLayoutObjectDefininingTextDecoration(InlineFlowBox* parentBox)
 {
     // Lookup first layout object in parent hierarchy which has text-decoration set.
     LayoutObject* layoutObject = 0;
     while (parentBox) {
         layoutObject = LineLayoutPaintShim::layoutObjectFrom(parentBox->lineLayoutItem());
 
@@ skipping 214 matching lines @@
     }
 
     // Eventually draw text using regular style from the end position of the selection to the end of the current chunk part
     if (endPosition < static_cast<int>(fragment.length) && !paintSelectedTextOnly) {
         SkPaint paint;
         if (setupTextPaint(paintInfo, style, resourceMode, paint))
             paintText(paintInfo, textRun, fragment, endPosition, fragment.length, paint);
     }
 }
 
-void SVGInlineTextBoxPainter::paintTextMatchMarker(GraphicsContext& context, const LayoutPoint&, DocumentMarker* marker, const ComputedStyle& style, const Font& font)
+Vector<SVGTextFragmentWithRange> SVGInlineTextBoxPainter::collectTextMatches(const PaintInfo& paintInfo, DocumentMarker* marker)

[fs, 2015/12/18 11:35:17]

|paintInfo| isn't used.

[ramya.v, 2015/12/21 08:13:49]

Done.

 {
-    // SVG is only interested in the TextMatch markers.
+    Vector<SVGTextFragmentWithRange> textMatchInfoList;
+
+    // SVG does not support grammar or spellcheck markers, so skip anything but TextMatch.
     if (marker->type() != DocumentMarker::TextMatch)
+        return textMatchInfoList;
+
+    if (!LineLayoutPaintShim::layoutObjectFrom(m_svgInlineTextBox.lineLayoutItem())->frame()->editor().markedTextMatchesAreHighlighted())
+        return textMatchInfoList;
+
+    int markerStartPosition = std::max<int>(marker->startOffset() - m_svgInlineTextBox.start(), 0);

[pdr., 2015/12/19 02:39:45]

Can these be negative? If not, lets use unsigned (similarly for
collectFragmentsInRange).

[ramya.v, 2015/12/21 08:13:49]

This would depend on collectFragmentsInRange,
mapStartEndPositionsIntoFragmentCoordinates, selectionStartEnd etc.. which are
used in various places.

Something similar was done in webkit in
http://trac.webkit.org/changeset/170337 and reverted in 
http://trac.webkit.org/changeset/170947

Probably might need to check case by case. As of now added a TODO in
collectFragmentsInRange since that's a common function.

[fs, 2015/12/21 12:00:01]

I believe this one can - the only guarantee (made by
InlineTextBoxPainter::paintDocumentMarkers) is that there is overlap. (It's
worth noting that both of the operands to the subtraction is unsigned though so
we're relying on wrapping and unsigned -> int doing the right thing [which it
should in practice].) A different formulation would lend itself better to an
unsigned version though.

+    int markerEndPosition = std::min<int>(marker->endOffset() - m_svgInlineTextBox.start(), m_svgInlineTextBox.len());
+
+    if (markerStartPosition >= markerEndPosition)
+        return textMatchInfoList;
+
+    textMatchInfoList = collectFragmentsInRange(markerStartPosition, markerEndPosition);

[fs, 2015/12/18 11:35:17]

Could just make this a tail-call ("return collectFragmentsInRange(...") and mark
|textMatchInfoList| const - maybe renaming it emptyTextMatchList or something.

[ramya.v, 2015/12/21 08:13:49]

Done.

+    return textMatchInfoList;
+}
+
+Vector<SVGTextFragmentWithRange> SVGInlineTextBoxPainter::collectFragmentsInRange(int startPosition, int endPosition)
+{
+    Vector<SVGTextFragmentWithRange> fragmentInfoList;
+    const Vector<SVGTextFragment>& fragments = m_svgInlineTextBox.textFragments();
+    unsigned textFragmentsSize = fragments.size();
+    for (unsigned i = 0; i < textFragmentsSize; ++i) {

[fs, 2015/12/18 11:35:17]

Use range-based for-loop.

[ramya.v, 2015/12/21 08:13:49]

Done.

+        const SVGTextFragment& fragment = fragments.at(i);
+
+        int fragmentStartPosition = startPosition;
+        int fragmentEndPosition = endPosition;
+        if (!m_svgInlineTextBox.mapStartEndPositionsIntoFragmentCoordinates(fragment, fragmentStartPosition, fragmentEndPosition))
+            continue;
+
+        fragmentInfoList.append(SVGTextFragmentWithRange(fragment, fragmentStartPosition, fragmentEndPosition));
+    }
+    return fragmentInfoList;
+}
+
+void SVGInlineTextBoxPainter::paintTextMatchMarkerForeground(const PaintInfo& paintInfo, const LayoutPoint& point, DocumentMarker* marker, const ComputedStyle& style, const Font& font)
+{
+    Vector<SVGTextFragmentWithRange> textMatchInfoList = collectTextMatches(paintInfo, marker);
+    if (textMatchInfoList.isEmpty())
         return;
 
-    LayoutSVGInlineText& textLayoutObject = toLayoutSVGInlineText(*LineLayoutPaintShim::layoutObjectFrom(m_svgInlineTextBox.lineLayoutItem()));
+    Color textColor = LayoutTheme::theme().platformTextSearchColor(marker->activeMatch());
+    AffineTransform fragmentTransform;
+    for (const SVGTextFragmentWithRange& textMatchInfo : textMatchInfoList) {
+        const SVGTextFragment& fragment = textMatchInfo.fragment;
+        GraphicsContextStateSaver stateSaver(paintInfo.context);
+        fragment.buildFragmentTransform(fragmentTransform);
+        if (!fragmentTransform.isIdentity())
+            paintInfo.context.concatCTM(fragmentTransform);
 
-    AffineTransform fragmentTransform;
-    for (InlineTextBox* box = textLayoutObject.firstTextBox(); box; box = box->nextTextBox()) {
-        if (!box->isSVGInlineTextBox())
-            continue;
-
-        SVGInlineTextBox* textBox = toSVGInlineTextBox(box);
-
-        int markerStartPosition = std::max<int>(marker->startOffset() - textBox->start(), 0);
-        int markerEndPosition = std::min<int>(marker->endOffset() - textBox->start(), textBox->len());
-
-        if (markerStartPosition >= markerEndPosition)
-            continue;
-
-        const Vector<SVGTextFragment>& fragments = textBox->textFragments();
-        unsigned textFragmentsSize = fragments.size();
-        for (unsigned i = 0; i < textFragmentsSize; ++i) {
-            const SVGTextFragment& fragment = fragments.at(i);
-
-            int fragmentStartPosition = markerStartPosition;
-            int fragmentEndPosition = markerEndPosition;
-            if (!textBox->mapStartEndPositionsIntoFragmentCoordinates(fragment, fragmentStartPosition, fragmentEndPosition))
-                continue;
-
-            FloatRect fragmentRect = textBox->selectionRectForTextFragment(fragment, fragmentStartPosition, fragmentEndPosition, style);
-            fragment.buildFragmentTransform(fragmentTransform);
-
-            // Draw the marker highlight.
-            if (LineLayoutPaintShim::layoutObjectFrom(m_svgInlineTextBox.lineLayoutItem())->frame()->editor().markedTextMatchesAreHighlighted()) {
-                Color color = marker->activeMatch() ?
-                    LayoutTheme::theme().platformActiveTextSearchHighlightColor() :
-                    LayoutTheme::theme().platformInactiveTextSearchHighlightColor();
-                GraphicsContextStateSaver stateSaver(context);
-                if (!fragmentTransform.isIdentity())
-                    context.concatCTM(fragmentTransform);
-                context.setFillColor(color);
-                context.fillRect(fragmentRect, color);
-            }
+        TextRun textRun = m_svgInlineTextBox.constructTextRun(style, fragment);
+        SkPaint paint;
+        if (setupTextPaint(paintInfo, style, ApplyToFillMode, paint)) {

[fs, 2015/12/18 11:35:17]

Since this does not depend on anything within the loop, it should be possible to
hoist - including the setColor etc.

Also, this can return false if 'fill' is 'none' or otherwise invalid (in general
if it should not produce any "ink"). IIUC, that's not an effect you want here?

Additionally, as long only ApplyToFillMode is used here you could just do
something like:

SkPaint fillPaint;
fillPaint.setColor(textColor.rgb());
fillPaint.setAntiAlias(true);

and then use that to paint. Which seems simpler. And when adding
ApplyToStrokeMode you'll likely need to do that because of how setupTextPaint
returns early when there's no paint to be applied.

Speaking of stroke - maybe you should add that to one of the (pixel) tests.

[ramya.v, 2015/12/21 08:13:49]

Moved setupPaint out of loop. As with other browsers even if fill is none, and
only stroke is specified both fill and stroke are painted. So for fill created a
generic paint as mentioned by you. For stroke used setupPaint and only if it is
successful painting stoke inside loop. Linux seems to be fine, Win and Mac seems
like fill not getting painted. (text-match-highlight-expected.png) Do I have to
change anything else? Please let me know your opinion

[fs, 2015/12/21 12:00:01]

That's odd... It does look like it happens for the non-highlight case too though
(lots of drop-outs noticable), so presumably this is a separate issue. Please
make a TC from that and file a bug. From the PoV of this CL we should hopefully
be fine though.

[ramya.v, 2015/12/21 13:01:55]

To file a bug, same test case, if I directly load in chrome browser in windows
its painting fine. Test results are showing this way.

[fs, 2015/12/21 13:13:24]

Ok, so it's in the test-runner only... better make sure to make that clear in
the bug report then.

+            paint.setColor(textColor.rgb());
+            paintText(paintInfo, textRun, fragment, textMatchInfo.startPosition, textMatchInfo.endPosition, paint);
         }
     }
 }
 
+void SVGInlineTextBoxPainter::paintTextMatchMarkerBackground(const PaintInfo& paintInfo, const LayoutPoint& point, DocumentMarker* marker, const ComputedStyle& style, const Font& font)
+{
+    Vector<SVGTextFragmentWithRange> textMatchInfoList = collectTextMatches(paintInfo, marker);
+    if (textMatchInfoList.isEmpty())
+        return;
+
+    Color color = LayoutTheme::theme().platformTextSearchHighlightColor(marker->activeMatch());
+    AffineTransform fragmentTransform;
+    for (const SVGTextFragmentWithRange& textMatchInfo : textMatchInfoList) {
+        const SVGTextFragment& fragment = textMatchInfo.fragment;
+        GraphicsContextStateSaver stateSaver(paintInfo.context);
+        fragment.buildFragmentTransform(fragmentTransform);
+        if (!fragmentTransform.isIdentity())
+            paintInfo.context.concatCTM(fragmentTransform);
+        FloatRect fragmentRect = m_svgInlineTextBox.selectionRectForTextFragment(fragment, textMatchInfo.startPosition, textMatchInfo.endPosition, style);
+        paintInfo.context.setFillColor(color);
+        paintInfo.context.fillRect(fragmentRect, color);

[fs, 2015/12/18 11:35:17]

Passing color to fillRect is redundant with setFillColor, so you can drop one or
the other. (If the latter, then it would be possible to make the state-saver
conditional.)

[ramya.v, 2015/12/21 08:13:49]

Done.

+    }
+}
+
 } // namespace blink