[S.P.] Fix cull rects for InlineTextBox to account for vertical text.

Source/core/paint/InlineTextBoxPainter.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/InlineTextBoxPainter.h"
 
 #include "core/dom/DocumentMarkerController.h"
 #include "core/dom/RenderedDocumentMarker.h"
 #include "core/editing/CompositionUnderline.h"
@@ skipping 31 matching lines @@
 }
 
 void InlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoint& paintOffset)
 {
     if (m_inlineTextBox.isLineBreak() || !paintInfo.shouldPaintWithinRoot(&m_inlineTextBox.renderer()) || m_inlineTextBox.renderer().style()->visibility() != VISIBLE
         || m_inlineTextBox.truncation() == cFullTruncation || paintInfo.phase == PaintPhaseOutline || !m_inlineTextBox.len())
         return;
 
     ASSERT(paintInfo.phase != PaintPhaseSelfOutline && paintInfo.phase != PaintPhaseChildOutlines);
 
-    LayoutRect logicalVisualOverflow = m_inlineTextBox.logicalOverflowRect();
-    LayoutUnit logicalStart = logicalVisualOverflow.x() + (m_inlineTextBox.isHorizontal() ? paintOffset.x() : paintOffset.y());
-    LayoutUnit logicalExtent = logicalVisualOverflow.width();
-
-    LayoutUnit paintEnd = m_inlineTextBox.isHorizontal() ? paintInfo.rect.maxX() : paintInfo.rect.maxY();
-    LayoutUnit paintStart = m_inlineTextBox.isHorizontal() ? paintInfo.rect.x() : paintInfo.rect.y();
+    const LayoutStyle& styleToUse = m_inlineTextBox.renderer().styleRef(m_inlineTextBox.isFirstLineStyle());
+    GraphicsContext* context = paintInfo.context;
 
     // We round the y-axis to ensure consistent line heights.
     LayoutPoint adjustedPaintOffset = LayoutPoint(paintOffset.x(), paintOffset.y().round());
 
-    if (logicalStart >= paintEnd || logicalStart + logicalExtent <= paintStart)
-        return;
+    FloatPoint boxOrigin = m_inlineTextBox.locationIncludingFlipping().toFloatPoint();
+    boxOrigin.move(adjustedPaintOffset.x().toFloat(), adjustedPaintOffset.y().toFloat());
+    FloatRect boxRect(boxOrigin, FloatSize(m_inlineTextBox.logicalWidth(), m_inlineTextBox.logicalHeight()));

[Xianzhu, 2015/03/06 23:17:06]

It seems that we used overflowRect to check for early return, now we use
location/width/height. Is this change intended?

[chrishtr, 2015/03/06 23:19:22]

Yes. Part of this CL is unifying that code, since boxRect should be the same as
overflowRect was previously. The one exception is when it's adjusted for text
combine on line 71, which I also did on purpose to make early-outing more
precise in that use case and further unify the code.

[Xianzhu, 2015/03/06 23:31:33]

Based on InlineBox.cpp, it seems that
logicalFrameRect/logicalWidth/logicalHeight (from which boxRect is calculated)
and logicalOverflowRect are different. The former values seem not to include
overflow.

[chrishtr, 2015/03/07 00:52:35]

Sigh. Yes it does. Oh well. Going to make this CL less aggressive.

+    bool shouldRotate = false;
+    LayoutTextCombine* combinedText = nullptr;
+    if (!m_inlineTextBox.isHorizontal()) {
+        if (styleToUse.hasTextCombine() && m_inlineTextBox.renderer().isCombineText()) {
+            combinedText = &toLayoutTextCombine(m_inlineTextBox.renderer());
+            if (!combinedText->isCombined())
+                combinedText = nullptr;
+        }
+        if (combinedText) {
+            combinedText->updateFont();
+            boxRect.setWidth(combinedText->inlineWidthForLayout());
+        } else {
+            shouldRotate = true;
+        }
+    }
+
+    if (m_inlineTextBox.isHorizontal()) {
+        if (boxRect.x() >= paintInfo.rect.maxX() || boxRect.x() + boxRect.width() <= paintInfo.rect.x())
+            return;
+    } else {
+        if (boxRect.y() >= paintInfo.rect.maxY() || boxRect.y() + boxRect.width() <= paintInfo.rect.y())
+            return;
+    }
 
     bool isPrinting = m_inlineTextBox.renderer().document().printing();
 
     // Determine whether or not we're selected.
     bool haveSelection = !isPrinting && paintInfo.phase != PaintPhaseTextClip && m_inlineTextBox.selectionState() != LayoutObject::SelectionNone;
     if (!haveSelection && paintInfo.phase == PaintPhaseSelection) {
         // When only painting the selection, don't bother to paint if there is none.
         return;
     }
 
     // The text clip phase already has a DrawingRecorder. Text clips are initiated only in BoxPainter::paintLayerExtended, which is already
     // within a DrawingRecorder.
     OwnPtr<DrawingRecorder> drawingRecorder;
     if (RuntimeEnabledFeatures::slimmingPaintEnabled() && paintInfo.phase != PaintPhaseTextClip) {
-        LayoutRect paintRect = logicalVisualOverflow;
-        paintRect.moveBy(adjustedPaintOffset);
-        drawingRecorder = adoptPtr(new DrawingRecorder(paintInfo.context, m_inlineTextBox.displayItemClient(), DisplayItem::paintPhaseToDrawingType(paintInfo.phase), paintRect));
+        drawingRecorder = adoptPtr(new DrawingRecorder(paintInfo.context, m_inlineTextBox.displayItemClient(), DisplayItem::paintPhaseToDrawingType(paintInfo.phase), boxRect));

[Xianzhu, 2015/03/06 23:17:06]

Ditto.

         if (drawingRecorder->canUseCachedDrawing())
             return;
     }
 
+    if (shouldRotate)
+        context->concatCTM(TextPainter::rotation(boxRect, TextPainter::Clockwise));
+
+
     if (m_inlineTextBox.truncation() != cNoTruncation) {
         if (m_inlineTextBox.renderer().containingBlock()->style()->isLeftToRightDirection() != m_inlineTextBox.isLeftToRightDirection()) {
             // Make the visible fragment of text hug the edge closest to the rest of the run by moving the origin
             // at which we start drawing text.
             // e.g. In the case of LTR text truncated in an RTL Context, the correct behavior is:
             // |Hello|CBA| -> |...He|CBA|
             // In order to draw the fragment "He" aligned to the right edge of it's box, we need to start drawing
             // farther to the right.
             // NOTE: WebKit's behavior differs from that of IE which appears to just overlay the ellipsis on top of the
             // truncated string i.e.  |Hello|CBA| -> |...lo|CBA|
             LayoutUnit widthOfVisibleText = m_inlineTextBox.renderer().width(m_inlineTextBox.start(), m_inlineTextBox.truncation(), m_inlineTextBox.textPos(), m_inlineTextBox.isLeftToRightDirection() ? LTR : RTL, m_inlineTextBox.isFirstLineStyle());
             LayoutUnit widthOfHiddenText = m_inlineTextBox.logicalWidth() - widthOfVisibleText;
             // FIXME: The hit testing logic also needs to take this translation into account.
             LayoutSize truncationOffset(m_inlineTextBox.isLeftToRightDirection() ? widthOfHiddenText : -widthOfHiddenText, 0);
             adjustedPaintOffset.move(m_inlineTextBox.isHorizontal() ? truncationOffset : truncationOffset.transposedSize());
         }
     }
 
-    GraphicsContext* context = paintInfo.context;
-    const LayoutStyle& styleToUse = m_inlineTextBox.renderer().styleRef(m_inlineTextBox.isFirstLineStyle());
-
-    FloatPoint boxOrigin = m_inlineTextBox.locationIncludingFlipping().toFloatPoint();
-    boxOrigin.move(adjustedPaintOffset.x().toFloat(), adjustedPaintOffset.y().toFloat());
-    FloatRect boxRect(boxOrigin, FloatSize(m_inlineTextBox.logicalWidth(), m_inlineTextBox.logicalHeight()));
-
-    bool shouldRotate = false;
-    LayoutTextCombine* combinedText = nullptr;
-    if (!m_inlineTextBox.isHorizontal()) {
-        if (styleToUse.hasTextCombine() && m_inlineTextBox.renderer().isCombineText()) {
-            combinedText = &toLayoutTextCombine(m_inlineTextBox.renderer());
-            if (!combinedText->isCombined())
-                combinedText = nullptr;
-        }
-        if (combinedText) {
-            combinedText->updateFont();
-            boxRect.setWidth(combinedText->inlineWidthForLayout());
-        } else {
-            shouldRotate = true;
-            context->concatCTM(TextPainter::rotation(boxRect, TextPainter::Clockwise));
-        }
-    }
-
     // Determine whether or not we have composition underlines to draw.
     bool containsComposition = m_inlineTextBox.renderer().node() && m_inlineTextBox.renderer().frame()->inputMethodController().compositionNode() == m_inlineTextBox.renderer().node();
     bool useCustomUnderlines = containsComposition && m_inlineTextBox.renderer().frame()->inputMethodController().compositionUsesCustomUnderlines();
 
     // Determine text colors.
     TextPainter::Style textStyle = TextPainter::textPaintingStyle(m_inlineTextBox.renderer(), styleToUse, paintInfo.forceBlackText(), isPrinting);
     TextPainter::Style selectionStyle = TextPainter::selectionPaintingStyle(m_inlineTextBox.renderer(), haveSelection, paintInfo.forceBlackText(), isPrinting, textStyle);
     bool paintSelectedTextOnly = (paintInfo.phase == PaintPhaseSelection);
     bool paintSelectedTextSeparately = !paintSelectedTextOnly && textStyle != selectionStyle;
 
@@ skipping 678 matching lines @@
             LayoutTheme::theme().platformActiveTextSearchHighlightColor() :
             LayoutTheme::theme().platformInactiveTextSearchHighlightColor();
         GraphicsContextStateSaver stateSaver(*pt);
         pt->clip(FloatRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_inlineTextBox.logicalWidth(), selHeight));
         pt->drawHighlightForText(font, run, FloatPoint(boxOrigin.x(), boxOrigin.y() - deltaY), selHeight, color, sPos, ePos);
     }
 }
 
 
 } // namespace blink