Changeset 145338 in webkit


Ignore:
Timestamp:
Mar 10, 2013 8:30:57 PM (11 years ago)
Author:
glenn@skynav.com
Message:

Line breaking opportunities at the end of a text node are missed
​https://bugs.webkit.org/show_bug.cgi?id=17427

Reviewed by Darin Adler.

Source/WebCore:

When initializing context for determining next break position,
reuse last two characters from previous text node(s) within block.
This additional state is stored in the current LazyLineBreakIterator
as an optimization to prevent having to add two new parameters to
isBreakable().

At present, this fixes only the ASCII shortcut code path, but
does not yet handle the non-ASCII path. Since the ASCII path is
the most performant critical, the handling of this latter path
will be addressed by webkit.org/b/105692.

Additionally test for case where last two characters context
is derived from distinct nodes, possibly with intervening empty
inline node(s).

Test: fast/text/line-break-between-text-nodes.html

  • platform/text/TextBreakIterator.h:

(WebCore::LazyLineBreakIterator::LazyLineBreakIterator):
(WebCore::LazyLineBreakIterator::lastCharacter):
(WebCore::LazyLineBreakIterator::secondToLastCharacter):
(WebCore::LazyLineBreakIterator::setLastTwoCharacters):
(WebCore::LazyLineBreakIterator::resetLastTwoCharacters):
(WebCore::LazyLineBreakIterator::updateLastTwoCharacters):
(LazyLineBreakIterator):
Add state variables to retain last two characters of previous text node(s)
for reuse when initializing nextBreakPosition<>() context.

  • rendering/RenderBlockLineLayout.cpp:

(WebCore::RenderBlock::layoutRunsAndFloatsInRange):
(WebCore::RenderBlock::LineBreaker::nextSegmentBreak):
Record and reset retained last two characters of previous text node(s) as
appropriate.

  • rendering/break_lines.cpp:

(WebCore::nextBreakablePosition):
Use state variables holding retained last two characters of previous text node(s)
for when initializing nextBreakPosition<>() context.

LayoutTests:

  • fast/text/line-break-between-text-nodes-expected.html: Added.
  • fast/text/line-break-between-text-nodes.html: Added.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r145323 r145338  
     12013-03-10  Glenn Adams  <glenn@skynav.com>
     2
     3        Line breaking opportunities at the end of a text node are missed
     4        https://bugs.webkit.org/show_bug.cgi?id=17427
     5
     6        Reviewed by Darin Adler.
     7
     8        * fast/text/line-break-between-text-nodes-expected.html: Added.
     9        * fast/text/line-break-between-text-nodes.html: Added.
     10
    1112013-03-09  Sheriff Bot  <webkit.review.bot@gmail.com>
    212

  • trunk/Source/WebCore/ChangeLog

    r145336 r145338  
     12013-03-10  Glenn Adams  <glenn@skynav.com>
     2
     3        Line breaking opportunities at the end of a text node are missed
     4        https://bugs.webkit.org/show_bug.cgi?id=17427
     5
     6        Reviewed by Darin Adler.
     7
     8        When initializing context for determining next break position,
     9        reuse last two characters from previous text node(s) within block.
     10        This additional state is stored in the current LazyLineBreakIterator
     11        as an optimization to prevent having to add two new parameters to
     12        isBreakable().
     13
     14        At present, this fixes only the ASCII shortcut code path, but
     15        does not yet handle the non-ASCII path. Since the ASCII path is
     16        the most performant critical, the handling of this latter path
     17        will be addressed by webkit.org/b/105692.
     18
     19        Additionally test for case where last two characters context
     20        is derived from distinct nodes, possibly with intervening empty
     21        inline node(s).
     22
     23        Test: fast/text/line-break-between-text-nodes.html
     24
     25        * platform/text/TextBreakIterator.h:
     26        (WebCore::LazyLineBreakIterator::LazyLineBreakIterator):
     27        (WebCore::LazyLineBreakIterator::lastCharacter):
     28        (WebCore::LazyLineBreakIterator::secondToLastCharacter):
     29        (WebCore::LazyLineBreakIterator::setLastTwoCharacters):
     30        (WebCore::LazyLineBreakIterator::resetLastTwoCharacters):
     31        (WebCore::LazyLineBreakIterator::updateLastTwoCharacters):
     32        (LazyLineBreakIterator):
     33        Add state variables to retain last two characters of previous text node(s)
     34        for reuse when initializing nextBreakPosition<>() context.
     35        * rendering/RenderBlockLineLayout.cpp:
     36        (WebCore::RenderBlock::layoutRunsAndFloatsInRange):
     37        (WebCore::RenderBlock::LineBreaker::nextSegmentBreak):
     38        Record and reset retained last two characters of previous text node(s) as
     39        appropriate.
     40        * rendering/break_lines.cpp:
     41        (WebCore::nextBreakablePosition):
     42        Use state variables holding retained last two characters of previous text node(s)
     43        for when initializing nextBreakPosition<>() context.
     44
    1452013-03-10  Darin Adler  <darin@apple.com>
    246

  • trunk/Source/WebCore/platform/text/TextBreakIterator.h

    r144073 r145338  
    6060    LazyLineBreakIterator()
    6161        : m_iterator(0)
     62        , m_lastCharacter(0)
     63        , m_secondToLastCharacter(0)
    6264    {
    6365    }
    … …  
    6769        , m_locale(locale)
    6870        , m_iterator(0)
     71        , m_lastCharacter(0)
     72        , m_secondToLastCharacter(0)
    6973    {
    7074    }
    … …  
    7781
    7882    String string() const { return m_string; }
     83
     84    UChar lastCharacter() const { return m_lastCharacter; }
     85    UChar secondToLastCharacter() const { return m_secondToLastCharacter; }
     86    void setLastTwoCharacters(UChar last, UChar secondToLast)
     87    {
     88        m_lastCharacter = last;
     89        m_secondToLastCharacter = secondToLast;
     90    }
     91    void updateLastTwoCharacters(UChar last)
     92    {
     93        m_secondToLastCharacter = m_lastCharacter;
     94        m_lastCharacter = last;
     95    }
     96    void resetLastTwoCharacters()
     97    {
     98        m_lastCharacter = 0;
     99        m_secondToLastCharacter = 0;
     100    }
    79101
    80102    TextBreakIterator* get()
    … …  
    103125    AtomicString m_locale;
    104126    TextBreakIterator* m_iterator;
     127    UChar m_lastCharacter;
     128    UChar m_secondToLastCharacter;
    105129};
    106130

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

    r145215 r145338  
    16071607        WordMeasurements wordMeasurements;
    16081608        end = lineBreaker.nextLineBreak(resolver, layoutState.lineInfo(), renderTextInfo, lastFloatFromPreviousLine, consecutiveHyphenatedLines, wordMeasurements);
     1609        renderTextInfo.m_lineBreakIterator.resetLastTwoCharacters();
    16091610        if (resolver.position().atEnd()) {
    16101611            // FIXME: We shouldn't be creating any runs in nextLineBreak to begin with!
    … …  
    27222723                m_positionedObjects.append(box);
    27232724            width.addUncommittedWidth(inlineLogicalWidth(current.m_obj));
     2725            // Reset prior line break context characters.
     2726            renderTextInfo.m_lineBreakIterator.resetLastTwoCharacters();
    27242727        } else if (current.m_obj->isFloating()) {
    27252728            RenderBox* floatBox = toRenderBox(current.m_obj);
    … …  
    27372740            } else
    27382741                floatsFitOnLine = false;
     2742            // Update prior line break context characters, using U+FFFD (OBJECT REPLACEMENT CHARACTER) for floating element.
     2743            renderTextInfo.m_lineBreakIterator.updateLastTwoCharacters(replacementCharacter);
    27392744        } else if (current.m_obj->isRenderInline()) {
    27402745            // Right now, we should only encounter empty inlines here.
    … …  
    28052810            if (current.m_obj->isRubyRun())
    28062811                width.applyOverhang(toRenderRubyRun(current.m_obj), last, next);
     2812            // Update prior line break context characters, using U+FFFD (OBJECT REPLACEMENT CHARACTER) for replaced element.
     2813            renderTextInfo.m_lineBreakIterator.updateLastTwoCharacters(replacementCharacter);
    28072814        } else if (current.m_obj->isText()) {
    28082815            if (!current.m_pos)
    … …  
    28672874            float wordTrailingSpaceWidth = (f.typesettingFeatures() & Kerning) && !textLayout ? f.width(constructTextRun(t, f, &space, 1, style)) + wordSpacing : 0;
    28682875
     2876            UChar lastCharacterInNode = renderTextInfo.m_lineBreakIterator.lastCharacter();
     2877            UChar secondToLastCharacterInNode = renderTextInfo.m_lineBreakIterator.secondToLastCharacter();
    28692878            for (; current.m_pos < t->textLength(); current.fastIncrementInTextNode()) {
    28702879                bool previousCharacterIsSpace = currentCharacterIsSpace;
    … …  
    29092918                        } else {
    29102919                            // Just keep ignoring these spaces.
    2911                             continue;
     2920                            goto nextCharacter;
    29122921                        }
    29132922                    }
    … …  
    30703079
    30713080                atStart = false;
    3072             }
     3081            nextCharacter:
     3082                secondToLastCharacterInNode = lastCharacterInNode;
     3083                lastCharacterInNode = c;
     3084            }
     3085
     3086            renderTextInfo.m_lineBreakIterator.setLastTwoCharacters(lastCharacterInNode, secondToLastCharacterInNode);
    30733087
    30743088            wordMeasurements.grow(wordMeasurements.size() + 1);

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

    r144073 r145338  
    154154    int nextBreak = -1;
    155155
    156     CharacterType lastLastCh = pos > 1 ? str[pos - 2] : 0;
    157     CharacterType lastCh = pos > 0 ? str[pos - 1] : 0;
     156    CharacterType lastLastCh = pos > 1 ? str[pos - 2] : static_cast<CharacterType>(lazyBreakIterator.secondToLastCharacter());
     157    CharacterType lastCh = pos > 0 ? str[pos - 1] : static_cast<CharacterType>(lazyBreakIterator.lastCharacter());
    158158    for (int i = pos; i < len; i++) {
    159159        CharacterType ch = str[i];
Note: See TracChangeset for help on using the changeset viewer.