[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
commit-queue at webkit.org
commit-queue at webkit.org
Wed Dec 22 15:46:51 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 56fb7495af99f3e577a2813a414e2f307bbd59c9
Author: commit-queue at webkit.org <commit-queue at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Fri Nov 12 06:04:05 2010 +0000
2010-11-11 James Simonsen <simonjam at chromium.org>
Reviewed by Tony Chang.
[chromium] Support letter spacing and fix whitespace wrapping on linux
When lines wrapped on RTL text, the whitespace was inserted before
the first character on the new line instead of at the end of the
previous line. This has been fixed.
The missing cluster information needed for letter spacing was hidden in
harfbuzz's attributes struct.
https://bugs.webkit.org/show_bug.cgi?id=49405
* platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum: Rebaselined.
* platform/chromium-linux/fast/text/atsui-spacing-features-expected.png: Ditto.
* platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt: Ditto.
2010-11-11 James Simonsen <simonjam at chromium.org>
Reviewed by Tony Chang.
[chromium] Support letter spacing and fix whitespace wrapping on linux
When lines wrapped on RTL text, the whitespace was inserted before
the first character on the new line instead of at the end of the
previous line. This has been fixed.
The missing cluster information needed for letter spacing was hidden in
harfbuzz's attributes struct.
https://bugs.webkit.org/show_bug.cgi?id=49405
* platform/graphics/chromium/FontLinux.cpp:
(WebCore::TextRunWalker::letterSpacing): Added.
(WebCore::TextRunWalker::isWordBreak): No need for isRTL.
(WebCore::TextRunWalker::setPadding): Ditto.
(WebCore::TextRunWalker::setGlyphXPositions): Support letter spacing. No whitespace before RTL text on new line.
(WebCore::glyphIndexForXPositionInScriptRun): Support letter spacing.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71887 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index dbaa356..46c8af2 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,22 @@
+2010-11-11 James Simonsen <simonjam at chromium.org>
+
+ Reviewed by Tony Chang.
+
+ [chromium] Support letter spacing and fix whitespace wrapping on linux
+
+ When lines wrapped on RTL text, the whitespace was inserted before
+ the first character on the new line instead of at the end of the
+ previous line. This has been fixed.
+
+ The missing cluster information needed for letter spacing was hidden in
+ harfbuzz's attributes struct.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49405
+
+ * platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum: Rebaselined.
+ * platform/chromium-linux/fast/text/atsui-spacing-features-expected.png: Ditto.
+ * platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt: Ditto.
+
2010-11-11 MORITA Hajime <morrita at google.com>
Reviewed by Kent Tamura.
diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
index 017faca..06e37d4 100644
--- a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
+++ b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.checksum
@@ -1 +1 @@
-fa49df9d2633b76626f829c18252b2c6
\ No newline at end of file
+e5c501032720feb9db7488334e694f3c
\ No newline at end of file
diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png
index 99dbbc8..6a2e641 100644
Binary files a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png and b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.png differ
diff --git a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
index eacea52..2e74faf 100644
--- a/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
+++ b/LayoutTests/platform/chromium-linux/fast/text/atsui-spacing-features-expected.txt
@@ -1,8 +1,8 @@
layer at (0,0) size 800x600
RenderView at (0,0) size 800x600
-layer at (0,0) size 800x340
- RenderBlock {HTML} at (0,0) size 800x340
- RenderBody {BODY} at (8,16) size 784x316
+layer at (0,0) size 800x378
+ RenderBlock {HTML} at (0,0) size 800x378
+ RenderBody {BODY} at (8,16) size 784x354
RenderBlock {P} at (0,0) size 784x40
RenderText {#text} at (0,0) size 167x19
text run at (0,0) width 167: "Test for regressions against "
@@ -19,8 +19,8 @@ layer at (0,0) size 800x340
RenderText {#text} at (0,0) size 483x19
text run at (0,0) width 483: "Each green box should be identical to the blue box it follows, except for accents."
RenderBlock {HR} at (0,84) size 784x2 [border: (1px inset #000000)]
- RenderTable {TABLE} at (0,94) size 620x222
- RenderTableSection {TBODY} at (0,0) size 620x222
+ RenderTable {TABLE} at (0,94) size 620x260
+ RenderTableSection {TBODY} at (0,0) size 620x260
RenderTableRow {TR} at (0,2) size 620x22
RenderTableCell {TD} at (2,2) size 204x22 [r=0 c=0 rs=1 cs=1]
RenderText {#text} at (59,1) size 85x19
@@ -31,7 +31,7 @@ layer at (0,0) size 800x340
RenderTableCell {TD} at (414,2) size 204x22 [r=0 c=2 rs=1 cs=1]
RenderText {#text} at (67,1) size 69x19
text run at (67,1) width 69: "Justification"
- RenderTableRow {TR} at (0,26) size 620x194
+ RenderTableRow {TR} at (0,26) size 620x232
RenderTableCell {TD} at (2,26) size 204x174 [r=1 c=0 rs=1 cs=1]
RenderBlock {DIV} at (1,1) size 202x172
RenderBlock {DIV} at (0,0) size 202x38 [border: (1px solid #0000FF)]
@@ -49,18 +49,20 @@ layer at (0,0) size 800x340
text run at (1,1) width 162: "Lorem ipsum dolor sit"
text run at (1,21) width 196: "amet, consectetuer adipiscing"
text run at (1,41) width 21: "elit."
- RenderTableCell {TD} at (208,26) size 204x194 [r=1 c=1 rs=1 cs=1]
- RenderBlock {DIV} at (1,1) size 202x192
- RenderBlock {DIV} at (0,0) size 202x38 [border: (1px solid #0000FF)]
- RenderText {#text} at (16,1) size 185x37
- text run at (16,1) width 185 RTL: "\x{5D9}\x{5B0}\x{5D4}\x{5B4}\x{5D9}, \x{5D0}\x{5B8}\x{5D7}\x{5B4}\x{5D9}, \x{5DC}\x{5B0}\x{5DA}\x{5B8} \x{5E1}\x{5B5}\x{5E4}\x{5B6}\x{5E8} \x{5E9}\x{5C1}\x{5B0}\x{5DC}\x{5B7}\x{5D7}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5} \x{5D5}\x{5BC}\x{5DE}\x{5B4}\x{5DE}\x{5B0}\x{5DB}\x{5BC}\x{5B6}\x{5E8}\x{5B6}\x{5EA}"
- text run at (97,19) width 104 RTL: "\x{5E6}\x{5B0}\x{5DE}\x{5B4}\x{5D9}\x{5EA}\x{5D5}\x{5BC}\x{5EA} \x{5DC}\x{5B8}\x{5DA}\x{5B0} \x{5DE}\x{5B0}\x{5DB}\x{5B7}\x{5E8}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}."
- RenderBlock {DIV} at (0,43) size 202x62 [border: (1px solid #0000FF)]
- RenderText {#text} at (1,1) size 130x59
- text run at (1,1) width 115: "Lore\x{300}m ipsum dolor"
- text run at (1,21) width 130: "sit ame\x{300}t, consectetue\x{300}r"
- text run at (1,41) width 84: "adipiscing e\x{300}lit."
- RenderBlock {DIV} at (0,110) size 202x82 [border: (1px solid #008000)]
+ RenderTableCell {TD} at (208,26) size 204x232 [r=1 c=1 rs=1 cs=1]
+ RenderBlock {DIV} at (1,1) size 202x230
+ RenderBlock {DIV} at (0,0) size 202x56 [border: (1px solid #0000FF)]
+ RenderText {#text} at (28,1) size 189x55
+ text run at (28,1) width 173 RTL: "\x{5D9}\x{5B0}\x{5D4}\x{5B4}\x{5D9}, \x{5D0}\x{5B8}\x{5D7}\x{5B4}\x{5D9}, \x{5DC}\x{5B0}\x{5DA}\x{5B8} \x{5E1}\x{5B5}\x{5E4}\x{5B6}\x{5E8}"
+ text run at (48,19) width 153 RTL: "\x{5E9}\x{5C1}\x{5B0}\x{5DC}\x{5B7}\x{5D7}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5} \x{5D5}\x{5BC}\x{5DE}\x{5B4}\x{5DE}\x{5B0}\x{5DB}\x{5BC}\x{5B6}\x{5E8}\x{5B6}\x{5EA}"
+ text run at (12,37) width 189 RTL: "\x{5E6}\x{5B0}\x{5DE}\x{5B4}\x{5D9}\x{5EA}\x{5D5}\x{5BC}\x{5EA} \x{5DC}\x{5B8}\x{5DA}\x{5B0} \x{5DE}\x{5B0}\x{5DB}\x{5B7}\x{5E8}\x{5B0}\x{5EA}\x{5BC}\x{5B4}\x{5D9}\x{5D5}."
+ RenderBlock {DIV} at (0,61) size 202x82 [border: (1px solid #0000FF)]
+ RenderText {#text} at (1,1) size 200x79
+ text run at (1,1) width 200: "Lore\x{300}m ipsum dolor"
+ text run at (1,21) width 95: "sit ame\x{300}t,"
+ text run at (1,41) width 136: "consectetue\x{300}r"
+ text run at (1,61) width 164: "adipiscing e\x{300}lit."
+ RenderBlock {DIV} at (0,148) size 202x82 [border: (1px solid #008000)]
RenderText {#text} at (1,1) size 200x79
text run at (1,1) width 200: "Lorem ipsum dolor"
text run at (1,21) width 95: "sit amet,"
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index a6359a0..622f6a9 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,25 @@
+2010-11-11 James Simonsen <simonjam at chromium.org>
+
+ Reviewed by Tony Chang.
+
+ [chromium] Support letter spacing and fix whitespace wrapping on linux
+
+ When lines wrapped on RTL text, the whitespace was inserted before
+ the first character on the new line instead of at the end of the
+ previous line. This has been fixed.
+
+ The missing cluster information needed for letter spacing was hidden in
+ harfbuzz's attributes struct.
+
+ https://bugs.webkit.org/show_bug.cgi?id=49405
+
+ * platform/graphics/chromium/FontLinux.cpp:
+ (WebCore::TextRunWalker::letterSpacing): Added.
+ (WebCore::TextRunWalker::isWordBreak): No need for isRTL.
+ (WebCore::TextRunWalker::setPadding): Ditto.
+ (WebCore::TextRunWalker::setGlyphXPositions): Support letter spacing. No whitespace before RTL text on new line.
+ (WebCore::glyphIndexForXPositionInScriptRun): Support letter spacing.
+
2010-11-11 Kavita Kanetkar <kkanetkar at chromium.org>
Reviewed by Dumitru Daniliuc.
diff --git a/WebCore/platform/graphics/chromium/FontLinux.cpp b/WebCore/platform/graphics/chromium/FontLinux.cpp
index e73747f..5b2fb0b 100644
--- a/WebCore/platform/graphics/chromium/FontLinux.cpp
+++ b/WebCore/platform/graphics/chromium/FontLinux.cpp
@@ -164,7 +164,8 @@ public:
TextRunWalker(const TextRun&, unsigned, const Font*);
~TextRunWalker();
- bool isWordBreak(unsigned, bool);
+ bool isWordBreak(unsigned);
+ unsigned determineWordBreakSpacing(unsigned);
// setPadding sets a number of pixels to be distributed across the TextRun.
// WebKit uses this to justify text.
void setPadding(int);
@@ -182,10 +183,8 @@ public:
// setLetterSpacingAdjustment sets an additional number of pixels that is
// added to the advance after each output cluster. This matches the behaviour
// of WidthIterator::advance.
- //
- // (NOTE: currently does nothing because I don't know how to get the
- // cluster information from Harfbuzz.)
void setLetterSpacingAdjustment(int letterSpacingAdjustment) { m_letterSpacing = letterSpacingAdjustment; }
+ int letterSpacing() const { return m_letterSpacing; }
// Set the x offset for the next script run. This affects the values in
// |xPositions|
@@ -301,11 +300,32 @@ TextRunWalker::~TextRunWalker()
delete[] m_item.log_clusters;
}
-bool TextRunWalker::isWordBreak(unsigned index, bool isRTL)
+bool TextRunWalker::isWordBreak(unsigned index)
{
- if (!isRTL)
- return index && isCodepointSpace(m_item.string[index]) && !isCodepointSpace(m_item.string[index - 1]);
- return index != m_item.stringLength - 1 && isCodepointSpace(m_item.string[index]) && !isCodepointSpace(m_item.string[index + 1]);
+ return index && isCodepointSpace(m_item.string[index]) && !isCodepointSpace(m_item.string[index - 1]);
+}
+
+unsigned TextRunWalker::determineWordBreakSpacing(unsigned logClustersIndex)
+{
+ unsigned wordBreakSpacing = 0;
+ // The first half of the conjunction works around the case where
+ // output glyphs aren't associated with any codepoints by the
+ // clusters log.
+ if (logClustersIndex < m_item.item.length
+ && isWordBreak(m_item.item.pos + logClustersIndex)) {
+ wordBreakSpacing = m_wordSpacingAdjustment;
+
+ if (m_padding > 0) {
+ unsigned toPad = roundf(m_padPerWordBreak + m_padError);
+ m_padError += m_padPerWordBreak - toPad;
+
+ if (m_padding < toPad)
+ toPad = m_padding;
+ m_padding -= toPad;
+ wordBreakSpacing += toPad;
+ }
+ }
+ return wordBreakSpacing;
}
// setPadding sets a number of pixels to be distributed across the TextRun.
@@ -323,7 +343,7 @@ void TextRunWalker::setPadding(int padding)
bool isRTL = m_iterateBackwards;
for (unsigned i = 0; i < m_item.stringLength; i++) {
- if (isWordBreak(i, isRTL))
+ if (isWordBreak(i))
numWordBreaks++;
}
@@ -490,51 +510,49 @@ void TextRunWalker::setGlyphXPositions(bool isRTL)
// RTL) codepoint of the current glyph. Each time we advance a glyph,
// we skip over all the codepoints that contributed to the current
// glyph.
- unsigned logClustersIndex = isRTL ? m_item.num_glyphs - 1 : 0;
+ int logClustersIndex = 0;
+
+ if (isRTL) {
+ logClustersIndex = m_item.num_glyphs - 1;
- for (unsigned iter = 0; iter < m_item.num_glyphs; ++iter) {
// Glyphs are stored in logical order, but for layout purposes we
// always go left to right.
- int i = isRTL ? m_item.num_glyphs - iter - 1 : iter;
-
- m_glyphs16[i] = m_item.glyphs[i];
- double offsetX = truncateFixedPointToInteger(m_item.offsets[i].x);
- m_xPositions[i] = m_offsetX + position + offsetX;
-
- double advance = truncateFixedPointToInteger(m_item.advances[i]);
- // The first half of the conjuction works around the case where
- // output glyphs aren't associated with any codepoints by the
- // clusters log.
- if (logClustersIndex < m_item.item.length
- && isWordBreak(m_item.item.pos + logClustersIndex, isRTL)) {
- advance += m_wordSpacingAdjustment;
-
- if (m_padding > 0) {
- unsigned toPad = roundf(m_padPerWordBreak + m_padError);
- m_padError += m_padPerWordBreak - toPad;
-
- if (m_padding < toPad)
- toPad = m_padding;
- m_padding -= toPad;
- advance += toPad;
- }
- }
+ for (int i = m_item.num_glyphs - 1; i >= 0; --i) {
+ // Whitespace must be laid out in logical order, so when inserting
+ // spaces in RTL (but iterating in LTR order) we must insert spaces
+ // _before_ the next glyph.
+ if (i + 1 >= m_item.num_glyphs || m_item.attributes[i + 1].clusterStart)
+ position += m_letterSpacing;
+
+ position += determineWordBreakSpacing(logClustersIndex);
- // We would like to add m_letterSpacing after each cluster, but I
- // don't know where the cluster information is. This is typically
- // fine for Roman languages, but breaks more complex languages
- // terribly.
- // advance += m_letterSpacing;
+ m_glyphs16[i] = m_item.glyphs[i];
+ double offsetX = truncateFixedPointToInteger(m_item.offsets[i].x);
+ m_xPositions[i] = m_offsetX + position + offsetX;
- if (isRTL) {
while (logClustersIndex > 0 && logClusters()[logClustersIndex] == i)
logClustersIndex--;
- } else {
+
+ position += truncateFixedPointToInteger(m_item.advances[i]);
+ }
+ } else {
+ for (int i = 0; i < m_item.num_glyphs; ++i) {
+ m_glyphs16[i] = m_item.glyphs[i];
+ double offsetX = truncateFixedPointToInteger(m_item.offsets[i].x);
+ m_xPositions[i] = m_offsetX + position + offsetX;
+
+ double advance = truncateFixedPointToInteger(m_item.advances[i]);
+
+ advance += determineWordBreakSpacing(logClustersIndex);
+
+ if (m_item.attributes[i].clusterStart)
+ advance += m_letterSpacing;
+
while (logClustersIndex < m_item.item.length && logClusters()[logClustersIndex] == i)
logClustersIndex++;
- }
- position += advance;
+ position += advance;
+ }
}
m_pixelWidth = position;
@@ -675,16 +693,19 @@ float Font::floatWidthForComplexText(const TextRun& run, HashSet<const SimpleFon
static int glyphIndexForXPositionInScriptRun(const TextRunWalker& walker, int x)
{
const HB_Fixed* advances = walker.advances();
+ int letterSpacing = walker.letterSpacing();
int glyphIndex;
if (walker.rtl()) {
for (glyphIndex = walker.length() - 1; glyphIndex >= 0; --glyphIndex) {
- if (x < truncateFixedPointToInteger(advances[glyphIndex]))
+ // When iterating LTR over RTL text, we must include the whitespace
+ // _before_ the glyph, so no + 1 here.
+ if (x < (walker.length() - glyphIndex) * letterSpacing + truncateFixedPointToInteger(advances[glyphIndex]))
break;
x -= truncateFixedPointToInteger(advances[glyphIndex]);
}
} else {
for (glyphIndex = 0; static_cast<unsigned>(glyphIndex) < walker.length(); ++glyphIndex) {
- if (x < truncateFixedPointToInteger(advances[glyphIndex]))
+ if (x < (glyphIndex * letterSpacing + truncateFixedPointToInteger(advances[glyphIndex])))
break;
x -= truncateFixedPointToInteger(advances[glyphIndex]);
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list