Changeset 223259 in webkit


Ignore:
Timestamp:
Oct 12, 2017 2:31:36 PM (7 years ago)
Author:
dbates@webkit.org
Message:

Teach InlineTextBox::clampOffset() about combined text and hyphenation
​https://bugs.webkit.org/show_bug.cgi?id=178032

Reviewed by Zalan Bujtas.

Treat combined text and the last character of a word halve plus hyphen as single units.

With regards to combined text, ideally we would allow arbitrary selection inside combined
text. Currently we do not support selection of combined text. To simplify the process of
adding support for selecting combined text we treat combined text as a single unit. Once
we are confident that we correctly implemented such support we can re-evaluate allowing
arbitrary selection of combined text.

With regards to treating the last character of a word halve plus hyphen as a single unit.
This patch extends the targeted fix made for document markers in r223013 to all code that
makes use of clamped offsets as a result the selection rect for inline boxes more accurately
reflect the rectangle(s) that make up the painted selection. This is a step towards reconciling
the difference between the computation of the rectangle that represents an arbitrary
selection and the code that paints the active selection as part of <​https://bugs.webkit.org/show_bug.cgi?id=138913>.

  • rendering/InlineTextBox.cpp:

(WebCore::InlineTextBox::localSelectionRect const): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Only measure the
text represented by the selection if the start position > 0 or the end position is not equal
to the length of the run.
(WebCore::InlineTextBox::paint): Remove unnecessary code to fix up the selection start and
end positions based on the truncation offset as this is done by clampedOffset(), called by
selectionStartEnd().
(WebCore::InlineTextBox::clampedOffset const): Modified to adjust the clamped offset with
respect to truncation as well as treat combined text or a trailing word halve plus hyphen
as single units. Assert that we are not fully truncated because it does not make sense to
be computing the clamped offset in such a situation since nothing should be painted.
(WebCore::InlineTextBox::selectionStartEnd const): Modified to compute the end of an inside
selection using clampedOffset() to account for truncation, combined text or a hyphen. We
already are using clampedOffset() when computing the start and end position for all other
selection states.
(WebCore::InlineTextBox::paintSelection): Compute text run, including combined text
or hyphens due to line wrapping now that specified start and end positions are clamped with
respect to combined text and hyphens (computed earlier in this function). Remove unnecessary
code to adjust selection end point with respect to truncation, combined text, or an added
hyphen now that selectionStartEnd() takes care of this (via clampedOffset()).
(WebCore::InlineTextBox::paintTextSubrangeBackground): Compute text run, including combined
text or hyphens due to line wrapping now that specified start and end positions are clamped
with respect to combined text and hyphens (computed earlier in this function).
(WebCore::InlineTextBox::paintDocumentMarker): Compute text run, including combined text now
that specified start and end positions are clamped with respect to combined text (computed earlier in this function).
Also remove unnecessary code to adjust end offset of the marker with respect to truncation
and length of the text run as clampedOffset() now does this for us.

Location:
trunk/Source/WebCore
Files:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r223254 r223259  
     12017-10-12  Daniel Bates  <dabates@apple.com>
     2
     3        Teach InlineTextBox::clampOffset() about combined text and hyphenation
     4        https://bugs.webkit.org/show_bug.cgi?id=178032
     5
     6        Reviewed by Zalan Bujtas.
     7
     8        Treat combined text and the last character of a word halve plus hyphen as single units.
     9
     10        With regards to combined text, ideally we would allow arbitrary selection inside combined
     11        text. Currently we do not support selection of combined text. To simplify the process of
     12        adding support for selecting combined text we treat combined text as a single unit. Once
     13        we are confident that we correctly implemented such support we can re-evaluate allowing
     14        arbitrary selection of combined text.
     15
     16        With regards to treating the last character of a word halve plus hyphen as a single unit.
     17        This patch extends the targeted fix made for document markers in r223013 to all code that
     18        makes use of clamped offsets as a result the selection rect for inline boxes more accurately
     19        reflect the rectangle(s) that make up the painted selection. This is a step towards reconciling
     20        the difference between the computation of the rectangle that represents an arbitrary
     21        selection and the code that paints the active selection as part of <https://bugs.webkit.org/show_bug.cgi?id=138913>.
     22
     23        * rendering/InlineTextBox.cpp:
     24        (WebCore::InlineTextBox::localSelectionRect const): Compute text run, including combined text
     25        or hyphens due to line wrapping now that specified start and end positions are clamped with
     26        respect to combined text and hyphens (computed earlier in this function). Only measure the
     27        text represented by the selection if the start position > 0 or the end position is not equal
     28        to the length of the run.
     29        (WebCore::InlineTextBox::paint): Remove unnecessary code to fix up the selection start and
     30        end positions based on the truncation offset as this is done by clampedOffset(), called by
     31        selectionStartEnd().
     32        (WebCore::InlineTextBox::clampedOffset const): Modified to adjust the clamped offset with
     33        respect to truncation as well as treat combined text or a trailing word halve plus hyphen
     34        as single units. Assert that we are not fully truncated because it does not make sense to
     35        be computing the clamped offset in such a situation since nothing should be painted.
     36        (WebCore::InlineTextBox::selectionStartEnd const): Modified to compute the end of an inside
     37        selection using clampedOffset() to account for truncation, combined text or a hyphen. We
     38        already are using clampedOffset() when computing the start and end position for all other
     39        selection states.
     40        (WebCore::InlineTextBox::paintSelection): Compute text run, including combined text
     41        or hyphens due to line wrapping now that specified start and end positions are clamped with
     42        respect to combined text and hyphens (computed earlier in this function). Remove unnecessary
     43        code to adjust selection end point with respect to truncation, combined text, or an added
     44        hyphen now that selectionStartEnd() takes care of this (via clampedOffset()).
     45        (WebCore::InlineTextBox::paintTextSubrangeBackground): Compute text run, including combined
     46        text or hyphens due to line wrapping now that specified start and end positions are clamped
     47        with respect to combined text and hyphens (computed earlier in this function).
     48        (WebCore::InlineTextBox::paintDocumentMarker): Compute text run, including combined text now
     49        that specified start and end positions are clamped with respect to combined text (computed earlier in this function).
     50        Also remove unnecessary code to adjust end offset of the marker with respect to truncation
     51        and length of the text run as clampedOffset() now does this for us.
     52
    1532017-10-11  Simon Fraser  <simon.fraser@apple.com>
    254

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

    r223198 r223259  
    198198    LayoutUnit selectionHeight = this->selectionHeight();
    199199
    200     // FIXME: Adjust selection rect with respect to combined text.
    201     bool ignoreCombinedText = true;
    202     auto text = this->text(ignoreCombinedText);
     200    auto text = this->text();
    203201    TextRun textRun = createTextRun(text);
    204     // FIXME: Shouldn't we adjust ePos to textRun.length() if ePos == (m_truncation != cNoTruncation ? m_truncation : m_len)
    205     // so that the selection spans the hypen/combined text?
    206202
    207203    LayoutRect selectionRect = LayoutRect(LayoutPoint(logicalLeft(), selectionTop), LayoutSize(m_logicalWidth, selectionHeight));
    208204    // Avoid computing the font width when the entire line box is selected as an optimization.
    209     if (sPos || ePos != m_len)
     205    if (sPos || ePos != textRun.length())
    210206        lineFont().adjustSelectionRectForText(textRun, selectionRect, sPos, ePos);
    211207    // FIXME: The computation of the snapped selection rect differs from the computation of this rect
    … …  
    501497    if (haveSelection && (paintSelectedTextOnly || paintSelectedTextSeparately))
    502498        std::tie(selectionStart, selectionEnd) = selectionStartEnd();
    503 
    504     if (m_truncation != cNoTruncation) {
    505         selectionStart = std::min(selectionStart, static_cast<unsigned>(m_truncation));
    506         selectionEnd = std::min(selectionEnd, static_cast<unsigned>(m_truncation));
    507     }
    508499
    509500    float emphasisMarkOffset = 0;
    … …  
    599590unsigned InlineTextBox::clampedOffset(unsigned x) const
    600591{
    601     return std::max(std::min(x, start() + len()), start()) - start();
     592    ASSERT(m_truncation != cFullTruncation);
     593    unsigned offset = std::max(std::min(x, m_start + m_len), m_start) - m_start;
     594    if (m_truncation != cNoTruncation)
     595        offset = std::min<unsigned>(offset, m_truncation);
     596    else if (offset == m_len) {
     597        // Fix up the offset if we are combined text or have a hyphen because we manage these embellishments.
     598        // That is, they are not reflected in renderer().text(). We treat combined text as a single unit.
     599        // We also treat the last codepoint in this box and the hyphen as a single unit.
     600        if (auto* combinedText = this->combinedText())
     601            offset = combinedText->combinedStringForRendering().length();
     602        else if (hasHyphen())
     603            offset += lineStyle().hyphenString().length();
     604    }
     605    return offset;
    602606}
    603607
    … …  
    606610    auto selectionState = renderer().selectionState();
    607611    if (selectionState == RenderObject::SelectionInside)
    608         return { 0, m_len };
     612        return { 0, clampedOffset(m_start + m_len) };
    609613   
    610614    auto start = renderer().view().selection().startPosition();
    … …  
    644648    // Note that if the text is truncated, we let the thing being painted in the truncation
    645649    // draw its own highlight.
    646 
    647     // FIXME: Adjust text run for combined text.
    648     bool ignoreCombinedText = true;
    649     auto text = this->text(ignoreCombinedText);
     650    auto text = this->text();
    650651    TextRun textRun = createTextRun(text);
    651     unsigned endOfLineIgnoringHyphenAndCombinedText = m_truncation != cNoTruncation ? m_truncation : m_len;
    652     if (selectionEnd == endOfLineIgnoringHyphenAndCombinedText)
    653         selectionEnd = textRun.length(); // Extend selection to include hyphen/combined text.
    654652
    655653    const RootInlineBox& rootBox = root();
    … …  
    662660    LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, m_logicalWidth, selectionHeight);
    663661    lineFont().adjustSelectionRectForText(textRun, selectionRect, selectionStart, selectionEnd);
     662
     663    // FIXME: Support painting combined text.
    664664    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), c);
    665665#else
    … …  
    685685    LayoutRect selectionRect = LayoutRect(boxOrigin.x(), boxOrigin.y() - deltaY, 0, selectionHeight());
    686686
    687     // FIXME: Adjust text run for combined text and hyphenation.
    688     bool ignoreCombinedText = true;
    689     bool ignoreHyphen = true;
    690     auto text = this->text(ignoreCombinedText, ignoreHyphen);
     687    auto text = this->text();
    691688    TextRun textRun = createTextRun(text);
    692689    lineFont().adjustSelectionRectForText(textRun, selectionRect, startOffset, endOffset);
     690
     691    // FIXME: Support painting combined text.
    693692    context.fillRect(snapRectToDevicePixelsWithWritingDirection(selectionRect, renderer().document().deviceScaleFactor(), textRun.ltr()), color);
    694693}
    … …  
    782781        unsigned endPosition = clampedOffset(subrange.endOffset);
    783782
    784         if (m_truncation != cNoTruncation)
    785             endPosition = std::min(endPosition, static_cast<unsigned>(m_truncation));
    786 
    787783        // Calculate start & width
    788         // FIXME: Adjust text run for combined text.
    789         bool ignoreCombinedText = true;
    790784        int deltaY = renderer().style().isFlippedLinesWritingMode() ? selectionBottom() - logicalBottom() : logicalTop() - selectionTop();
    791785        int selHeight = selectionHeight();
    792786        FloatPoint startPoint(boxOrigin.x(), boxOrigin.y() - deltaY);
    793         auto text = this->text(ignoreCombinedText);
     787        auto text = this->text();
    794788        TextRun run = createTextRun(text);
    795789
    796790        LayoutRect selectionRect = LayoutRect(startPoint, FloatSize(0, selHeight));
    797         lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition >= len() ? run.length() : endPosition);
     791        lineFont().adjustSelectionRectForText(run, selectionRect, startPosition, endPosition);
    798792        IntRect markerRect = enclosingIntRect(selectionRect);
    799793        start = markerRect.x() - startPoint.x();
    … …  
    839833        underlineOffset = baseline + 2;
    840834    }
     835    // FIXME: Support painting combined text.
    841836    context.drawLineForDocumentMarker(FloatPoint(boxOrigin.x() + start, boxOrigin.y() + underlineOffset), width, lineStyleForSubrangeType(subrange.type));
    842837}
Note: See TracChangeset for help on using the changeset viewer.