- Revision
- 145338
- Author
- gl...@skynav.com
- Date
- 2013-03-10 20:30:57 -0700 (Sun, 10 Mar 2013)
Log 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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (145337 => 145338)
--- trunk/LayoutTests/ChangeLog 2013-03-11 01:21:36 UTC (rev 145337)
+++ trunk/LayoutTests/ChangeLog 2013-03-11 03:30:57 UTC (rev 145338)
@@ -1,3 +1,13 @@
+2013-03-10 Glenn Adams <gl...@skynav.com>
+
+ 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.
+
+ * fast/text/line-break-between-text-nodes-expected.html: Added.
+ * fast/text/line-break-between-text-nodes.html: Added.
+
2013-03-09 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r145299.
Added: trunk/LayoutTests/fast/text/line-break-between-text-nodes-expected.html (0 => 145338)
--- trunk/LayoutTests/fast/text/line-break-between-text-nodes-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/line-break-between-text-nodes-expected.html 2013-03-11 03:30:57 UTC (rev 145338)
@@ -0,0 +1,15 @@
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?dolor
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?dolor
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?dolor
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?dolor
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?dolor
+</div>
Added: trunk/LayoutTests/fast/text/line-break-between-text-nodes.html (0 => 145338)
--- trunk/LayoutTests/fast/text/line-break-between-text-nodes.html (rev 0)
+++ trunk/LayoutTests/fast/text/line-break-between-text-nodes.html 2013-03-11 03:30:57 UTC (rev 145338)
@@ -0,0 +1,15 @@
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum?<span>dolor</span>
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsum<span>?</span><span>dolor</span>
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsu<span>m</span><span>?</span><span>dolor</span>
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsu<span>m</span><span>?</span><span></span><span>dolor</span>
+</div>
+<div id="target" style="width: 100px; background: yellow;">
+Lorem ipsu<span>m</span><span></span><span>?</span><span></span><span>dolor</span>
+</div>
Modified: trunk/Source/WebCore/ChangeLog (145337 => 145338)
--- trunk/Source/WebCore/ChangeLog 2013-03-11 01:21:36 UTC (rev 145337)
+++ trunk/Source/WebCore/ChangeLog 2013-03-11 03:30:57 UTC (rev 145338)
@@ -1,3 +1,47 @@
+2013-03-10 Glenn Adams <gl...@skynav.com>
+
+ 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.
+
+ 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.
+
2013-03-10 Darin Adler <da...@apple.com>
NetworkStorageSession leaks its CFURLStorageSessionRef
Modified: trunk/Source/WebCore/platform/text/TextBreakIterator.h (145337 => 145338)
--- trunk/Source/WebCore/platform/text/TextBreakIterator.h 2013-03-11 01:21:36 UTC (rev 145337)
+++ trunk/Source/WebCore/platform/text/TextBreakIterator.h 2013-03-11 03:30:57 UTC (rev 145338)
@@ -59,6 +59,8 @@
public:
LazyLineBreakIterator()
: m_iterator(0)
+ , m_lastCharacter(0)
+ , m_secondToLastCharacter(0)
{
}
@@ -66,6 +68,8 @@
: m_string(string)
, m_locale(locale)
, m_iterator(0)
+ , m_lastCharacter(0)
+ , m_secondToLastCharacter(0)
{
}
@@ -77,6 +81,24 @@
String string() const { return m_string; }
+ UChar lastCharacter() const { return m_lastCharacter; }
+ UChar secondToLastCharacter() const { return m_secondToLastCharacter; }
+ void setLastTwoCharacters(UChar last, UChar secondToLast)
+ {
+ m_lastCharacter = last;
+ m_secondToLastCharacter = secondToLast;
+ }
+ void updateLastTwoCharacters(UChar last)
+ {
+ m_secondToLastCharacter = m_lastCharacter;
+ m_lastCharacter = last;
+ }
+ void resetLastTwoCharacters()
+ {
+ m_lastCharacter = 0;
+ m_secondToLastCharacter = 0;
+ }
+
TextBreakIterator* get()
{
if (!m_iterator) {
@@ -102,6 +124,8 @@
String m_string;
AtomicString m_locale;
TextBreakIterator* m_iterator;
+ UChar m_lastCharacter;
+ UChar m_secondToLastCharacter;
};
// Iterates over "extended grapheme clusters", as defined in UAX #29.
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (145337 => 145338)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2013-03-11 01:21:36 UTC (rev 145337)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2013-03-11 03:30:57 UTC (rev 145338)
@@ -1606,6 +1606,7 @@
#endif
WordMeasurements wordMeasurements;
end = lineBreaker.nextLineBreak(resolver, layoutState.lineInfo(), renderTextInfo, lastFloatFromPreviousLine, consecutiveHyphenatedLines, wordMeasurements);
+ renderTextInfo.m_lineBreakIterator.resetLastTwoCharacters();
if (resolver.position().atEnd()) {
// FIXME: We shouldn't be creating any runs in nextLineBreak to begin with!
// Once BidiRunList is separated from BidiResolver this will not be needed.
@@ -2721,6 +2722,8 @@
} else
m_positionedObjects.append(box);
width.addUncommittedWidth(inlineLogicalWidth(current.m_obj));
+ // Reset prior line break context characters.
+ renderTextInfo.m_lineBreakIterator.resetLastTwoCharacters();
} else if (current.m_obj->isFloating()) {
RenderBox* floatBox = toRenderBox(current.m_obj);
FloatingObject* f = m_block->insertFloatingObject(floatBox);
@@ -2736,6 +2739,8 @@
}
} else
floatsFitOnLine = false;
+ // Update prior line break context characters, using U+FFFD (OBJECT REPLACEMENT CHARACTER) for floating element.
+ renderTextInfo.m_lineBreakIterator.updateLastTwoCharacters(replacementCharacter);
} else if (current.m_obj->isRenderInline()) {
// Right now, we should only encounter empty inlines here.
ASSERT(isEmptyInline(current.m_obj));
@@ -2804,6 +2809,8 @@
width.addUncommittedWidth(replacedLogicalWidth);
if (current.m_obj->isRubyRun())
width.applyOverhang(toRenderRubyRun(current.m_obj), last, next);
+ // Update prior line break context characters, using U+FFFD (OBJECT REPLACEMENT CHARACTER) for replaced element.
+ renderTextInfo.m_lineBreakIterator.updateLastTwoCharacters(replacementCharacter);
} else if (current.m_obj->isText()) {
if (!current.m_pos)
appliedStartWidth = false;
@@ -2866,6 +2873,8 @@
// words with their trailing space, then subtract its width.
float wordTrailingSpaceWidth = (f.typesettingFeatures() & Kerning) && !textLayout ? f.width(constructTextRun(t, f, &space, 1, style)) + wordSpacing : 0;
+ UChar lastCharacterInNode = renderTextInfo.m_lineBreakIterator.lastCharacter();
+ UChar secondToLastCharacterInNode = renderTextInfo.m_lineBreakIterator.secondToLastCharacter();
for (; current.m_pos < t->textLength(); current.fastIncrementInTextNode()) {
bool previousCharacterIsSpace = currentCharacterIsSpace;
bool previousCharacterIsWS = currentCharacterIsWS;
@@ -2908,7 +2917,7 @@
stoppedIgnoringSpaces = true;
} else {
// Just keep ignoring these spaces.
- continue;
+ goto nextCharacter;
}
}
@@ -3069,8 +3078,13 @@
trailingObjects.clear();
atStart = false;
+ nextCharacter:
+ secondToLastCharacterInNode = lastCharacterInNode;
+ lastCharacterInNode = c;
}
+ renderTextInfo.m_lineBreakIterator.setLastTwoCharacters(lastCharacterInNode, secondToLastCharacterInNode);
+
wordMeasurements.grow(wordMeasurements.size() + 1);
WordMeasurement& wordMeasurement = wordMeasurements.last();
wordMeasurement.renderer = t;
Modified: trunk/Source/WebCore/rendering/break_lines.cpp (145337 => 145338)
--- trunk/Source/WebCore/rendering/break_lines.cpp 2013-03-11 01:21:36 UTC (rev 145337)
+++ trunk/Source/WebCore/rendering/break_lines.cpp 2013-03-11 03:30:57 UTC (rev 145338)
@@ -153,8 +153,8 @@
int len = static_cast<int>(length);
int nextBreak = -1;
- CharacterType lastLastCh = pos > 1 ? str[pos - 2] : 0;
- CharacterType lastCh = pos > 0 ? str[pos - 1] : 0;
+ CharacterType lastLastCh = pos > 1 ? str[pos - 2] : static_cast<CharacterType>(lazyBreakIterator.secondToLastCharacter());
+ CharacterType lastCh = pos > 0 ? str[pos - 1] : static_cast<CharacterType>(lazyBreakIterator.lastCharacter());
for (int i = pos; i < len; i++) {
CharacterType ch = str[i];
