[webkit-changes] [222528] trunk/Source/WebCore

Tue, 26 Sep 2017 15:29:12 -0700

Title: [222528] trunk/Source/WebCore
Revision
222528
Author
[email protected]
Date
2017-09-26 15:28:43 -0700 (Tue, 26 Sep 2017)

Log Message

Extract out combined text query into a member function
https://bugs.webkit.org/show_bug.cgi?id=177493

Reviewed by Zalan Bujtas.

Towards removing duplication throughout the paint code extract the
querying for the combined text of an inline text box into a member
function.

For a similar reason, update functions to query lineStyle() instead
of passing this information an argument.

No functionality changed. So, no new tests.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::paint): Extract logic to query for combined text
from here to InlineTextBox::combinedText() and modify code to call this
member function.
(WebCore::InlineTextBox::paintTextSubrangeBackground): Query for line style
instead of taking it as an argument.
(WebCore::InlineTextBox::paintCompositionBackground): Ditto.
(WebCore::InlineTextBox::paintTextMatchMarker): Ditto.
(WebCore::InlineTextBox::paintDecoration): Update code to query combinedText()
instead of taking RenderCombineText as an argument. While I am here make use of
GraphicsContextStateSaver to save and restore the graphics context state when
we have a non-empty clip rect.
(WebCore::InlineTextBox::paintDocumentMarker): Query for line style instead of
taking it as an argument.
(WebCore::InlineTextBox::paintDocumentMarkers): Ditto.
(WebCore::InlineTextBox::combinedText const): Added; extracted from InlineTextBox::paint().
* rendering/InlineTextBox.h:
* rendering/TextPainter.h:
(WebCore::TextPainter::setEmphasisMark): Modified to take a const RenderCombineText*
as opposed to a pointer to a non-const RenderCombineText.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (222527 => 222528)


--- trunk/Source/WebCore/ChangeLog	2017-09-26 22:19:12 UTC (rev 222527)
+++ trunk/Source/WebCore/ChangeLog	2017-09-26 22:28:43 UTC (rev 222528)
@@ -1,3 +1,40 @@
+2017-09-26  Daniel Bates  <[email protected]>
+
+        Extract out combined text query into a member function
+        https://bugs.webkit.org/show_bug.cgi?id=177493
+
+        Reviewed by Zalan Bujtas.
+
+        Towards removing duplication throughout the paint code extract the
+        querying for the combined text of an inline text box into a member
+        function.
+
+        For a similar reason, update functions to query lineStyle() instead
+        of passing this information an argument.
+
+        No functionality changed. So, no new tests.
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::paint): Extract logic to query for combined text
+        from here to InlineTextBox::combinedText() and modify code to call this
+        member function.
+        (WebCore::InlineTextBox::paintTextSubrangeBackground): Query for line style
+        instead of taking it as an argument.
+        (WebCore::InlineTextBox::paintCompositionBackground): Ditto.
+        (WebCore::InlineTextBox::paintTextMatchMarker): Ditto.
+        (WebCore::InlineTextBox::paintDecoration): Update code to query combinedText()
+        instead of taking RenderCombineText as an argument. While I am here make use of
+        GraphicsContextStateSaver to save and restore the graphics context state when
+        we have a non-empty clip rect.
+        (WebCore::InlineTextBox::paintDocumentMarker): Query for line style instead of
+        taking it as an argument.
+        (WebCore::InlineTextBox::paintDocumentMarkers): Ditto.
+        (WebCore::InlineTextBox::combinedText const): Added; extracted from InlineTextBox::paint().
+        * rendering/InlineTextBox.h:
+        * rendering/TextPainter.h:
+        (WebCore::TextPainter::setEmphasisMark): Modified to take a const RenderCombineText*
+        as opposed to a pointer to a non-const RenderCombineText.
+
 2017-09-26  Joanmarie Diggs  <[email protected]>
 
         AX: ARIA grids claim to be multiselectable even with aria-multiselectable is set to false

Modified: trunk/Source/WebCore/rendering/InlineTextBox.cpp (222527 => 222528)


--- trunk/Source/WebCore/rendering/InlineTextBox.cpp	2017-09-26 22:19:12 UTC (rev 222527)
+++ trunk/Source/WebCore/rendering/InlineTextBox.cpp	2017-09-26 22:28:43 UTC (rev 222528)
@@ -461,7 +461,7 @@
     boxOrigin.moveBy(localPaintOffset);
     FloatRect boxRect(boxOrigin, FloatSize(logicalWidth(), logicalHeight()));
 
-    RenderCombineText* combinedText = lineStyle.hasTextCombine() && is<RenderCombineText>(renderer()) && downcast<RenderCombineText>(renderer()).isCombined() ? &downcast<RenderCombineText>(renderer()) : nullptr;
+    auto* combinedText = this->combinedText();
 
     bool shouldRotate = !isHorizontal() && !combinedText;
     if (shouldRotate)
@@ -488,9 +488,9 @@
     // and composition underlines.
     if (paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseTextClip && !isPrinting) {
         if (containsComposition && !useCustomUnderlines)
-            paintCompositionBackground(context, boxOrigin, lineStyle, font);
+            paintCompositionBackground(context, boxOrigin, font);
 
-        paintDocumentMarkers(context, boxOrigin, lineStyle, font, true);
+        paintDocumentMarkers(context, boxOrigin, font, true);
 
         if (haveSelection && !useCustomUnderlines)
             paintSelection(context, boxOrigin, lineStyle, font, selectionPaintStyle.fillColor);
@@ -600,11 +600,11 @@
                 textDecorationSelectionClipOutRect.setWidth(logicalSelectionWidth);
             }
         }
-        paintDecoration(context, font, combinedText, textRun, textOrigin, boxRect, textDecorations, textPaintStyle, textShadow, textDecorationSelectionClipOutRect);
+        paintDecoration(context, font, textRun, textOrigin, boxRect, textDecorations, textPaintStyle, textShadow, textDecorationSelectionClipOutRect);
     }
 
     if (paintInfo.phase == PaintPhaseForeground) {
-        paintDocumentMarkers(context, boxOrigin, lineStyle, font, false);
+        paintDocumentMarkers(context, boxOrigin, font, false);
 
         if (useCustomUnderlines) {
             const Vector<CompositionUnderline>& underlines = renderer().frame().editor().customCompositionUnderlines();
@@ -715,7 +715,7 @@
 #endif
 }
 
-inline void InlineTextBox::paintTextSubrangeBackground(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, const Color& color, unsigned startOffset, unsigned endOffset)
+inline void InlineTextBox::paintTextSubrangeBackground(GraphicsContext& context, const FloatPoint& boxOrigin, const FontCascade& font, const Color& color, unsigned startOffset, unsigned endOffset)
 {
     startOffset = clampedOffset(startOffset);
     endOffset = clampedOffset(endOffset);
@@ -731,22 +731,22 @@
     LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, 0, selectionHeight());
 
     // FIXME: Adjust text run for combined text and hyphenation.
-    TextRun textRun = constructTextRun(style);
+    TextRun textRun = constructTextRun(lineStyle());
     font.adjustSelectionRectForText(textRun, selectionRect, startOffset, endOffset);
     context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), color);
 }
 
-void InlineTextBox::paintCompositionBackground(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font)
+void InlineTextBox::paintCompositionBackground(GraphicsContext& context, const FloatPoint& boxOrigin, const FontCascade& font)
 {
-    paintTextSubrangeBackground(context, boxOrigin, style, font, renderer().frame().editor().compositionStart(), renderer().frame().editor().compositionEnd(), Color::compositionFill);
+    paintTextSubrangeBackground(context, boxOrigin, font, renderer().frame().editor().compositionStart(), renderer().frame().editor().compositionEnd(), Color::compositionFill);
 }
 
-void InlineTextBox::paintTextMatchMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, const MarkerSubrange& subrange, bool isActiveMatch)
+void InlineTextBox::paintTextMatchMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const FontCascade& font, const MarkerSubrange& subrange, bool isActiveMatch)
 {
     if (!renderer().frame().editor().markedTextMatchesAreHighlighted())
         return;
     auto highlightColor = isActiveMatch ? renderer().theme().platformActiveTextSearchHighlightColor() : renderer().theme().platformInactiveTextSearchHighlightColor();
-    paintTextSubrangeBackground(context, boxOrigin, style, font, highlightColor, subrange.startOffset, subrange.endOffset);
+    paintTextSubrangeBackground(context, boxOrigin, font, highlightColor, subrange.startOffset, subrange.endOffset);
 }
 
 static inline void mirrorRTLSegment(float logicalWidth, TextDirection direction, float& start, float width)
@@ -756,7 +756,7 @@
     start = logicalWidth - width - start;
 }
 
-void InlineTextBox::paintDecoration(GraphicsContext& context, const FontCascade& font, RenderCombineText* combinedText, const TextRun& textRun, const FloatPoint& textOrigin,
+void InlineTextBox::paintDecoration(GraphicsContext& context, const FontCascade& font, const TextRun& textRun, const FloatPoint& textOrigin,
     const FloatRect& boxRect, TextDecoration decoration, TextPaintStyle textPaintStyle, const ShadowData* shadow, const FloatRect& clipOutRect)
 {
     if (m_truncation == cFullTruncation)
@@ -763,7 +763,9 @@
         return;
 
     updateGraphicsContext(context, textPaintStyle);
-    if (combinedText)
+
+    bool isCombinedText = combinedText();
+    if (isCombinedText)
         context.concatCTM(rotation(boxRect, Clockwise));
 
     float start = 0;
@@ -784,21 +786,20 @@
     FloatPoint localOrigin = boxRect.location();
     localOrigin.move(start, 0);
 
-    if (!clipOutRect.isEmpty()) {
-        context.save();
-        context.clipOut(clipOutRect);
+    {
+        GraphicsContextStateSaver stateSaver { context, false };
+        if (!clipOutRect.isEmpty()) {
+            stateSaver.save();
+            context.clipOut(clipOutRect);
+        }
+        decorationPainter.paintTextDecoration(textRun, textOrigin, localOrigin);
     }
 
-    decorationPainter.paintTextDecoration(textRun, textOrigin, localOrigin);
-
-    if (!clipOutRect.isEmpty())
-        context.restore();
-
-    if (combinedText)
+    if (isCombinedText)
         context.concatCTM(rotation(boxRect, Counterclockwise));
 }
 
-void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, const MarkerSubrange& subrange)
+void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const FontCascade& font, const MarkerSubrange& subrange)
 {
     // Never print spelling/grammar markers (5327887)
     if (renderer().document().printing())
@@ -831,7 +832,7 @@
         int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
         int selHeight = selectionHeight();
         FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
-        TextRun run = constructTextRun(style);
+        TextRun run = constructTextRun(lineStyle());
 
         LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
         font.adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
@@ -881,7 +882,7 @@
     context.drawLineForDocumentMarker(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, lineStyleForSubrangeType(subrange.type));
 }
 
-void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin, const RenderStyle& style, const FontCascade& font, bool background)
+void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin, const FontCascade& font, bool background)
 {
     if (!renderer().textNode())
         return;
@@ -975,9 +976,9 @@
 
     for (auto& subrange : subdivide(subranges, OverlapStrategy::Frontmost)) {
         if (subrange.type == MarkerSubrange::TextMatch)
-            paintTextMatchMarker(context, boxOrigin, style, font, subrange, subrange.marker->isActiveMatch());
+            paintTextMatchMarker(context, boxOrigin, font, subrange, subrange.marker->isActiveMatch());
         else
-            paintDocumentMarker(context, boxOrigin, style, font, subrange);
+            paintDocumentMarker(context, boxOrigin, font, subrange);
     }
 }
 
@@ -1111,6 +1112,11 @@
     return run;
 }
 
+const RenderCombineText* InlineTextBox::combinedText() const
+{
+    return lineStyle().hasTextCombine() && is<RenderCombineText>(renderer()) && downcast<RenderCombineText>(renderer()).isCombined() ? &downcast<RenderCombineText>(renderer()) : nullptr;
+}
+
 ExpansionBehavior InlineTextBox::expansionBehavior() const
 {
     ExpansionBehavior leadingBehavior;

Modified: trunk/Source/WebCore/rendering/InlineTextBox.h (222527 => 222528)


--- trunk/Source/WebCore/rendering/InlineTextBox.h	2017-09-26 22:19:12 UTC (rev 222527)
+++ trunk/Source/WebCore/rendering/InlineTextBox.h	2017-09-26 22:28:43 UTC (rev 222528)
@@ -155,19 +155,21 @@
     virtual float positionForOffset(unsigned offset) const;
 
 private:
-    void paintDecoration(GraphicsContext&, const FontCascade&, RenderCombineText*, const TextRun&, const FloatPoint& textOrigin, const FloatRect& boxRect,
+    void paintDecoration(GraphicsContext&, const FontCascade&, const TextRun&, const FloatPoint& textOrigin, const FloatRect& boxRect,
         TextDecoration, TextPaintStyle, const ShadowData*, const FloatRect& clipOutRect);
     void paintSelection(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&, const Color&);
 
-    void paintDocumentMarker(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&, const MarkerSubrange&);
-    void paintDocumentMarkers(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&, bool background);
-    void paintTextMatchMarker(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&, const MarkerSubrange&, bool isActiveMatch);
+    void paintDocumentMarker(GraphicsContext&, const FloatPoint& boxOrigin, const FontCascade&, const MarkerSubrange&);
+    void paintDocumentMarkers(GraphicsContext&, const FloatPoint& boxOrigin, const FontCascade&, bool background);
+    void paintTextMatchMarker(GraphicsContext&, const FloatPoint& boxOrigin, const FontCascade&, const MarkerSubrange&, bool isActiveMatch);
 
-    void paintCompositionBackground(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&);
+    void paintCompositionBackground(GraphicsContext&, const FloatPoint& boxOrigin, const FontCascade&);
     void paintCompositionUnderline(GraphicsContext&, const FloatPoint& boxOrigin, const CompositionUnderline&);
 
-    void paintTextSubrangeBackground(GraphicsContext&, const FloatPoint& boxOrigin, const RenderStyle&, const FontCascade&, const Color&, unsigned startOffset, unsigned endOffset);
+    void paintTextSubrangeBackground(GraphicsContext&, const FloatPoint& boxOrigin, const FontCascade&, const Color&, unsigned startOffset, unsigned endOffset);
 
+    const RenderCombineText* combinedText() const;
+
     ExpansionBehavior expansionBehavior() const;
 
     void behavesLikeText() const = delete;

Modified: trunk/Source/WebCore/rendering/TextPainter.h (222527 => 222528)


--- trunk/Source/WebCore/rendering/TextPainter.h	2017-09-26 22:19:12 UTC (rev 222527)
+++ trunk/Source/WebCore/rendering/TextPainter.h	2017-09-26 22:28:43 UTC (rev 222528)
@@ -59,7 +59,7 @@
 
     void setIsHorizontal(bool isHorizontal) { m_textBoxIsHorizontal = isHorizontal; }
 
-    void setEmphasisMark(const AtomicString& mark, float offset, RenderCombineText*);
+    void setEmphasisMark(const AtomicString& mark, float offset, const RenderCombineText*);
 
     void paintRange(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned start, unsigned end);
     void paint(const TextRun&, unsigned length, const FloatRect& boxRect, const FloatPoint& textOrigin,
@@ -80,12 +80,12 @@
     const ShadowData* m_shadow { nullptr };
     const ShadowData* m_selectionShadow { nullptr };
     AtomicString m_emphasisMark;
-    RenderCombineText* m_combinedText { nullptr };
+    const RenderCombineText* m_combinedText { nullptr };
     float m_emphasisMarkOffset { 0 };
     bool m_textBoxIsHorizontal { true };
 };
 
-inline void TextPainter::setEmphasisMark(const AtomicString& mark, float offset, RenderCombineText* combinedText)
+inline void TextPainter::setEmphasisMark(const AtomicString& mark, float offset, const RenderCombineText* combinedText)
 {
     m_emphasisMark = mark;
     m_emphasisMarkOffset = offset;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to