Changeset 223699 in webkit


Ignore:
Timestamp:
Oct 19, 2017 11:55:16 AM (7 years ago)
Author:
dbates@webkit.org
Message:

Share logic in InlineTextBox to compute selection rect
​https://bugs.webkit.org/show_bug.cgi?id=178232
<rdar://problem/34963452>

Reviewed by Zalan Bujtas.

Currently each paint routine in InlineTextBox duplicates similar code to compute the selection
rect it will paint. This change consolidates all the duplication into localSelectionRectWithClampedPositions()
and writes all of the paint operations, except for paintCompositionUnderline(), in terms of it.
We will write paintCompositionUnderline() in terms of localSelectionRectWithClampedPositions()
in a subsequent patch.

We also write localSelectionRect() in terms of localSelectionRectWithClampedPositions(). Ideally
we would have one way to compute the selection rect. However, localSelectionRect() and paintDocumentMarker()
currently expect the enclosing integral rectangle of the selection rectangle. The function
paintDocumentMarker() needs the enclosing integral rectangle to avoid truncating the dot pattern
drawn under marked words (e.g. a spelling error) on Cocoa platforms. With regards to localSelectionRect()
we should look to have it return the actual selection rectangle. See <​https://bugs.webkit.org/show_bug.cgi?id=138913>
for more details.

  • rendering/InlineTextBox.cpp:

(WebCore::InlineTextBox::localSelectionRect const): Move logic in common with paintSelection() into
localSelectionRectWithClampedPositions() and modified code to use it.
(WebCore::InlineTextBox::localSelectionRectWithClampedPositions const): Added.
(WebCore::InlineTextBox::paint): Store the local paint offset as a LayoutPoint as it is the canonical
data type for representing an offset when painting. Pass the local paint offset instead of the analagous boxOrigin value.
(WebCore::InlineTextBox::paintSelection): Write in terms of localSelectionRectWithClampedPositions().
(WebCore::InlineTextBox::paintTextSubrangeBackground): Ditto.
(WebCore::InlineTextBox::paintCompositionBackground): Ditto.
(WebCore::InlineTextBox::paintTextMatchMarker): Ditto.
(WebCore::InlineTextBox::paintDocumentMarker): Ditto.
(WebCore::InlineTextBox::paintDocumentMarkers): Pass paint offset instead of the analogous boxOrigin value.

  • rendering/InlineTextBox.h:
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r223697 r223699  
     12017-10-19  Daniel Bates  <dabates@apple.com>
     2
     3        Share logic in InlineTextBox to compute selection rect
     4        https://bugs.webkit.org/show_bug.cgi?id=178232
     5        <rdar://problem/34963452>
     6
     7        Reviewed by Zalan Bujtas.
     8
     9        Currently each paint routine in InlineTextBox duplicates similar code to compute the selection
     10        rect it will paint. This change consolidates all the duplication into localSelectionRectWithClampedPositions()
     11        and writes all of the paint operations, except for paintCompositionUnderline(), in terms of it.
     12        We will write paintCompositionUnderline() in terms of localSelectionRectWithClampedPositions()
     13        in a subsequent patch.
     14
     15        We also write localSelectionRect() in terms of localSelectionRectWithClampedPositions(). Ideally
     16        we would have one way to compute the selection rect. However, localSelectionRect() and paintDocumentMarker()
     17        currently expect the enclosing integral rectangle of the selection rectangle. The function
     18        paintDocumentMarker() needs the enclosing integral rectangle to avoid truncating the dot pattern
     19        drawn under marked words (e.g. a spelling error) on Cocoa platforms. With regards to localSelectionRect()
     20        we should look to have it return the actual selection rectangle. See <https://bugs.webkit.org/show_bug.cgi?id=138913>
     21        for more details.
     22
     23        * rendering/InlineTextBox.cpp:
     24        (WebCore::InlineTextBox::localSelectionRect const): Move logic in common with paintSelection() into
     25        localSelectionRectWithClampedPositions() and modified code to use it.
     26        (WebCore::InlineTextBox::localSelectionRectWithClampedPositions const): Added.
     27        (WebCore::InlineTextBox::paint): Store the local paint offset as a LayoutPoint as it is the canonical
     28        data type for representing an offset when painting. Pass the local paint offset instead of the analagous boxOrigin value.
     29        (WebCore::InlineTextBox::paintSelection): Write in terms of localSelectionRectWithClampedPositions().
     30        (WebCore::InlineTextBox::paintTextSubrangeBackground): Ditto.
     31        (WebCore::InlineTextBox::paintCompositionBackground): Ditto.
     32        (WebCore::InlineTextBox::paintTextMatchMarker): Ditto.
     33        (WebCore::InlineTextBox::paintDocumentMarker): Ditto.
     34        (WebCore::InlineTextBox::paintDocumentMarkers): Pass paint offset instead of the analogous boxOrigin value.
     35        * rendering/InlineTextBox.h:
     36
    1372017-10-19  Daniel Bates  <dabates@apple.com>
    238

  • trunk/Source/WebCore/rendering/InlineTextBox.cpp

    r223553 r223699  
    186186}
    187187
    188 // FIXME: Share more code with paintSelection().
    189 LayoutRect InlineTextBox::localSelectionRect(unsigned startPos, unsigned endPos) const
    190 {
    191     unsigned sPos = clampedOffset(startPos);
    192     unsigned ePos = clampedOffset(endPos);
    193 
    194     if (sPos >= ePos && !(startPos == endPos && startPos >= start() && startPos <= (start() + len())))
     188// FIXME: This function should return the same rectangle as localSelectionRectWithClampedPositions(..., ..., SelectionRectRounding::None),
     189// which represents the rectange of the selected text on the page. We need to update all callers to expect this change.
     190// See <https://bugs.webkit.org/show_bug.cgi?id=138913> for more details.
     191LayoutRect InlineTextBox::localSelectionRect(unsigned startPosition, unsigned endPosition) const
     192{
     193    if (startPosition > endPosition || endPosition < m_start || startPosition > end() + 1)
    195194        return { };
    196 
    197     LayoutUnit selectionTop = this->selectionTop();
    198     LayoutUnit selectionHeight = this->selectionHeight();
     195    return localSelectionRectWithClampedPositions(clampedOffset(startPosition), clampedOffset(endPosition), SelectionRectRounding::EnclosingIntRect);
     196}
     197
     198LayoutRect InlineTextBox::localSelectionRectWithClampedPositions(unsigned clampedStartPosition, unsigned clampedEndPosition, SelectionRectRounding roundingStrategy) const
     199{
     200    if (clampedStartPosition > clampedEndPosition)
     201        return { };
     202
     203    auto selectionTop = root().selectionTopAdjustedForPrecedingBlock();
     204    auto selectionBottom = this->selectionBottom();
    199205
    200206    auto text = this->text();
    201207    TextRun textRun = createTextRun(text);
    202208
    203     LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(m_logicalWidth, selectionHeight));
     209    // Use same y positioning and height as used for selected text on the page, so that when some or all of this
     210    // subrange is selected on the page there are no pieces sticking out.
     211    LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom - logicalBottom() : logicalTop() - selectionTop;
     212    auto selectionHeight = root().selectionHeightAdjustedForPrecedingBlock();
     213    LayoutRect selectionRect { LayoutPoint { logicalLeft(), logicalTop() - deltaY }, LayoutSize { m_logicalWidth, selectionHeight } };
     214
    204215    // Avoid computing the font width when the entire line box is selected as an optimization.
    205     if (sPos || ePos != textRun.length())
    206         lineFont().adjustSelectionRectForText(textRun, selectionRect, sPos, ePos);
    207     // FIXME: The computation of the snapped selection rect differs from the computation of this rect
    208     // in paintSelection(). See <https://bugs.webkit.org/show_bug.cgi?id=138913>.
    209     IntRect snappedSelectionRect = enclosingIntRect(selectionRect);
    210     LayoutUnit logicalWidth = snappedSelectionRect.width();
    211     if (snappedSelectionRect.x() > logicalRight())
    212         logicalWidth  = 0;
    213     else if (snappedSelectionRect.maxX() > logicalRight())
    214         logicalWidth = logicalRight() - snappedSelectionRect.x();
    215 
    216     LayoutPoint topPoint = isHorizontal() ? LayoutPoint(snappedSelectionRect.x(), selectionTop) : LayoutPoint(selectionTop, snappedSelectionRect.x());
    217     LayoutUnit width = isHorizontal() ? logicalWidth : selectionHeight;
    218     LayoutUnit height = isHorizontal() ? selectionHeight : logicalWidth;
    219 
    220     return LayoutRect(topPoint, LayoutSize(width, height));
     216    if (clampedStartPosition || clampedEndPosition != textRun.length())
     217        lineFont().adjustSelectionRectForText(textRun, selectionRect, clampedStartPosition, clampedEndPosition);
     218
     219    LayoutUnit selectionLeft;
     220    LayoutUnit selectionWidth;
     221    if (roundingStrategy == SelectionRectRounding::EnclosingIntRect) {
     222        auto snappedSelectionRect = enclosingIntRect(selectionRect);
     223        selectionLeft = snappedSelectionRect.x();
     224        selectionWidth = snappedSelectionRect.width();
     225        if (snappedSelectionRect.x() > logicalRight())
     226            selectionWidth = 0;
     227        else if (snappedSelectionRect.maxX() > logicalRight())
     228            selectionWidth = logicalRight() - snappedSelectionRect.x();
     229    } else {
     230        selectionLeft = selectionRect.x();
     231        selectionWidth = selectionRect.width();
     232    }
     233
     234    LayoutPoint location;
     235    LayoutSize size;
     236    if (isHorizontal()) {
     237        location = { selectionLeft, selectionTop };
     238        size = { selectionWidth, selectionHeight };
     239    } else {
     240        location = { selectionTop, selectionLeft };
     241        size = { selectionHeight, selectionWidth };
     242    }
     243
     244    return { location, size };
    221245}
    222246
    … …  
    406430    LayoutUnit paintStart = isHorizontal() ? paintInfo.rect.x() : paintInfo.rect.y();
    407431   
    408     FloatPoint localPaintOffset(paintOffset);
     432    LayoutPoint localPaintOffset { paintOffset };
    409433   
    410434    if (logicalStart >= paintEnd || logicalStart + logicalExtent <= paintStart)
    … …  
    473497    if (paintInfo.phase != PaintPhaseSelection && paintInfo.phase != PaintPhaseTextClip && !isPrinting) {
    474498        if (containsComposition && !useCustomUnderlines)
    475             paintCompositionBackground(context, boxOrigin);
    476 
    477         paintDocumentMarkers(context, boxOrigin, true);
     499            paintCompositionBackground(context, localPaintOffset);
     500
     501        paintDocumentMarkers(context, localPaintOffset, true);
    478502
    479503        if (haveSelection && !useCustomUnderlines)
    480             paintSelection(context, boxOrigin, selectionPaintStyle.fillColor);
     504            paintSelection(context, localPaintOffset, selectionPaintStyle.fillColor);
    481505    }
    482506
    … …  
    580604
    581605    if (paintInfo.phase == PaintPhaseForeground) {
    582         paintDocumentMarkers(context, boxOrigin, false);
     606        paintDocumentMarkers(context, localPaintOffset, false);
    583607
    584608        if (useCustomUnderlines)
    … …  
    624648}
    625649
    626 void InlineTextBox::paintSelection(GraphicsContext& context, const FloatPoint& boxOrigin, const Color& textColor)
     650void InlineTextBox::paintSelection(GraphicsContext& context, const LayoutPoint& paintOffset, const Color& textColor)
    627651{
    628652#if ENABLE(TEXT_SELECTION)
    … …  
    651675    // Note that if the text is truncated, we let the thing being painted in the truncation
    652676    // draw its own highlight.
    653     auto text = this->text();
    654     TextRun textRun = createTextRun(text);
    655 
    656     const RootInlineBox& rootBox = root();
    657     LayoutUnit selectionBottom = rootBox.selectionBottom();
    658     LayoutUnit selectionTop = rootBox.selectionTopAdjustedForPrecedingBlock();
    659 
    660     LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom - logicalBottom() : logicalTop() - selectionTop;
    661     LayoutUnit selectionHeight = std::max<LayoutUnit>(0, selectionBottom - selectionTop);
    662 
    663     LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_logicalWidth, selectionHeight);
    664     lineFont().adjustSelectionRectForText(textRun, selectionRect, selectionStart, selectionEnd);
     677    auto selectionRect = localSelectionRectWithClampedPositions(selectionStart, selectionEnd);
     678    selectionRect.moveBy(paintOffset);
    665679
    666680    // FIXME: Support painting combined text.
    667     context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), c);
     681    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), isLeftToRightDirection()), c);
    668682#else
    669683    UNUSED_PARAM(context);
    670     UNUSED_PARAM(boxOrigin);
     684    UNUSED_PARAM(paintOffset);
    671685    UNUSED_PARAM(textColor);
    672686#endif
    673687}
    674688
    675 inline void InlineTextBox::paintTextSubrangeBackground(GraphicsContext& context, const FloatPoint& boxOrigin, const Color& color, unsigned startOffset, unsigned endOffset)
     689inline void InlineTextBox::paintTextSubrangeBackground(GraphicsContext& context, const LayoutPoint& paintOffset, const Color& color, unsigned startOffset, unsigned endOffset)
    676690{
    677691    startOffset = clampedOffset(startOffset);
    … …  
    683697    updateGraphicsContext(context, TextPaintStyle { color }); // Don't draw text at all!
    684698
    685     // Use same y positioning and height as for selection, so that when the selection and this subrange are on
    686     // the same word there are no pieces sticking out.
    687     LayoutUnit deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
    688     LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, 0, selectionHeight());
    689 
    690     auto text = this->text();
    691     TextRun textRun = createTextRun(text);
    692     lineFont().adjustSelectionRectForText(textRun, selectionRect, startOffset, endOffset);
     699    auto selectionRect = localSelectionRectWithClampedPositions(startOffset, endOffset);
     700    selectionRect.moveBy(paintOffset);
    693701
    694702    // FIXME: Support painting combined text.
    695     context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), color);
    696 }
    697 
    698 void InlineTextBox::paintCompositionBackground(GraphicsContext& context, const FloatPoint& boxOrigin)
    699 {
    700     paintTextSubrangeBackground(context, boxOrigin, renderer().frame().editor().compositionStart(), renderer().frame().editor().compositionEnd(), Color::compositionFill);
    701 }
    702 
    703 void InlineTextBox::paintTextMatchMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const MarkerSubrange& subrange, bool isActiveMatch)
     703    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), isLeftToRightDirection()), color);
     704}
     705
     706void InlineTextBox::paintCompositionBackground(GraphicsContext& context, const LayoutPoint& paintOffset)
     707{
     708    paintTextSubrangeBackground(context, paintOffset, renderer().frame().editor().compositionStart(), renderer().frame().editor().compositionEnd(), Color::compositionFill);
     709}
     710
     711void InlineTextBox::paintTextMatchMarker(GraphicsContext& context, const LayoutPoint& paintOffset, const MarkerSubrange& subrange, bool isActiveMatch)
    704712{
    705713    if (!renderer().frame().editor().markedTextMatchesAreHighlighted())
    706714        return;
    707715    auto highlightColor = isActiveMatch ? renderer().theme().platformActiveTextSearchHighlightColor() : renderer().theme().platformInactiveTextSearchHighlightColor();
    708     paintTextSubrangeBackground(context, boxOrigin, highlightColor, subrange.startOffset, subrange.endOffset);
     716    paintTextSubrangeBackground(context, paintOffset, highlightColor, subrange.startOffset, subrange.endOffset);
    709717}
    710718
    … …  
    759767}
    760768
    761 void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const FloatPoint& boxOrigin, const MarkerSubrange& subrange)
     769void InlineTextBox::paintDocumentMarker(GraphicsContext& context, const LayoutPoint& paintOffset, const MarkerSubrange& subrange)
    762770{
    763771    // Never print spelling/grammar markers (5327887)
    … …  
    767775    if (m_truncation == cFullTruncation)
    768776        return;
    769 
    770     float start = 0; // start of line to draw, relative to tx
    771     float width = m_logicalWidth; // how much line to draw
    772777
    773778    // Determine whether we need to measure text
    … …  
    780785        markerSpansWholeBox = false;
    781786
    782     if (!markerSpansWholeBox) {
     787    FloatPoint location;
     788    float width;
     789    if (markerSpansWholeBox) {
     790        location = locationIncludingFlipping();
     791        width = m_logicalWidth;
     792    } else {
    783793        unsigned startPosition = clampedOffset(subrange.startOffset);
    784794        unsigned endPosition = clampedOffset(subrange.endOffset);
    785795
    786         // Calculate start & width
    787         int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
    788         int selHeight = selectionHeight();
    789         FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
    790         auto text = this->text();
    791         TextRun run = createTextRun(text);
    792 
    793         LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
    794         lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
    795         IntRect markerRect = enclosingIntRect(selectionRect);
    796         start = markerRect.x() - startPoint.x();
    797         width = markerRect.width();
    798     }
    799    
     796        // We round to the nearest enclosing integral rect to avoid truncating the dot decoration on Cocoa platforms.
     797        // FIXME: Find a better place to ensure that the Cocoa dots are not truncated.
     798        auto selectionRect = localSelectionRectWithClampedPositions(startPosition, endPosition, SelectionRectRounding::EnclosingIntRect);
     799        location = selectionRect.location();
     800        width = selectionRect.width();
     801    }
     802    location.moveBy(paintOffset);
     803
    800804    auto lineStyleForSubrangeType = [] (MarkerSubrange::Type type) {
    801805        switch (type) {
    … …  
    837841    }
    838842    // FIXME: Support painting combined text.
    839     context.drawLineForDocumentMarker(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, lineStyleForSubrangeType(subrange.type));
    840 }
    841 
    842 void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin, bool background)
     843    location.move(0, underlineOffset);
     844    context.drawLineForDocumentMarker(location, width, lineStyleForSubrangeType(subrange.type));
     845}
     846
     847void InlineTextBox::paintDocumentMarkers(GraphicsContext& context, const LayoutPoint& paintOffset, bool background)
    843848{
    844849    if (!renderer().textNode())
    … …  
    934939    for (auto& subrange : subdivide(subranges, OverlapStrategy::Frontmost)) {
    935940        if (subrange.type == MarkerSubrange::TextMatch)
    936             paintTextMatchMarker(context, boxOrigin, subrange, subrange.marker->isActiveMatch());
     941            paintTextMatchMarker(context, paintOffset, subrange, subrange.marker->isActiveMatch());
    937942        else
    938             paintDocumentMarker(context, boxOrigin, subrange);
     943            paintDocumentMarker(context, paintOffset, subrange);
    939944    }
    940945}

  • trunk/Source/WebCore/rendering/InlineTextBox.h

    r223552 r223699  
    111111    FloatRect calculateBoundaries() const override { return FloatRect(x(), y(), width(), height()); }
    112112
    113     virtual LayoutRect localSelectionRect(unsigned startPos, unsigned endPos) const;
     113    virtual LayoutRect localSelectionRect(unsigned startPosition, unsigned endPosition) const;
    114114    bool isSelected(unsigned startPosition, unsigned endPosition) const;
    115115    std::pair<unsigned, unsigned> selectionStartEnd() const;
    … …  
    153153    void paintDecoration(GraphicsContext&, const TextRun&, const FloatPoint& textOrigin, const FloatRect& boxRect,
    154154        TextDecoration, TextPaintStyle, const ShadowData*, const FloatRect& clipOutRect);
    155     void paintSelection(GraphicsContext&, const FloatPoint& boxOrigin, const Color&);
     155    void paintSelection(GraphicsContext&, const LayoutPoint& paintOffset, const Color&);
    156156
    157     void paintDocumentMarker(GraphicsContext&, const FloatPoint& boxOrigin, const MarkerSubrange&);
    158     void paintDocumentMarkers(GraphicsContext&, const FloatPoint& boxOrigin, bool background);
    159     void paintTextMatchMarker(GraphicsContext&, const FloatPoint& boxOrigin, const MarkerSubrange&, bool isActiveMatch);
     157    void paintDocumentMarker(GraphicsContext&, const LayoutPoint& paintOffset, const MarkerSubrange&);
     158    void paintDocumentMarkers(GraphicsContext&, const LayoutPoint& paintOffset, bool background);
     159    void paintTextMatchMarker(GraphicsContext&, const LayoutPoint& paintOffset, const MarkerSubrange&, bool isActiveMatch);
    160160
    161     void paintCompositionBackground(GraphicsContext&, const FloatPoint& boxOrigin);
     161    void paintCompositionBackground(GraphicsContext&, const LayoutPoint& paintOffset);
    162162    void paintCompositionUnderlines(GraphicsContext&, const FloatPoint& boxOrigin) const;
    163163    void paintCompositionUnderline(GraphicsContext&, const FloatPoint& boxOrigin, const CompositionUnderline&) const;
    164164
    165     void paintTextSubrangeBackground(GraphicsContext&, const FloatPoint& boxOrigin, const Color&, unsigned startOffset, unsigned endOffset);
     165    void paintTextSubrangeBackground(GraphicsContext&, const LayoutPoint& paintOffset, const Color&, unsigned startOffset, unsigned endOffset);
     166
     167    enum class SelectionRectRounding { None, EnclosingIntRect };
     168    LayoutRect localSelectionRectWithClampedPositions(unsigned clampedStartPosition, unsigned clampedEndPosition, SelectionRectRounding = SelectionRectRounding::None) const;
    166169
    167170    const RenderCombineText* combinedText() const;
Note: See TracChangeset for help on using the changeset viewer.