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

Source/core/paint/InlineTextBoxPainter.cpp

Index: Source/core/paint/InlineTextBoxPainter.cpp
diff --git a/Source/core/paint/InlineTextBoxPainter.cpp b/Source/core/paint/InlineTextBoxPainter.cpp
index 994f4e8475323386ee3b30d51b4cb309831cbb08..d0c8d5537efb530a31ce3c2c391343e7426efaa7 100644
--- a/Source/core/paint/InlineTextBoxPainter.cpp
+++ b/Source/core/paint/InlineTextBoxPainter.cpp
@@ -49,18 +49,38 @@ void InlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoint&
 
     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();
 
@@ -75,13 +95,15 @@ void InlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoint&
     // 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
@@ -100,30 +122,6 @@ void InlineTextBoxPainter::paint(const PaintInfo& paintInfo, const LayoutPoint&
         }
     }
 
-    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();