- Revision
- 129175
- Author
- [email protected]
- Date
- 2012-09-20 16:16:16 -0700 (Thu, 20 Sep 2012)
Log Message
[Chromium] Improve glyph selection of HarfBuzzShaper
https://bugs.webkit.org/show_bug.cgi?id=97164
Reviewed by Tony Chang.
Source/WebCore:
Take into account clusters for selection.
Test: fast/text/international/hebrew-selection.html
* platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:
(WebCore::HarfBuzzShaper::HarfBuzzRun::applyShapeResult): Removed m_logClusters.
m_logCluster is no longer used.
(WebCore::HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition):
- If targetX is in the left side of the first cluster, return the leftmost character index.
- If targetX is in the right side of the last cluster, return the rightmost character index.
- If targetX is between the right side of the cluster N and the left side of the cluster N+1, then:
- return N+1 for LTR.
- return N for RTL.
(WebCore::HarfBuzzShaper::HarfBuzzRun::xPositionForOffset):
Find the cluster of index in question, then:
- return the left side boundary of the cluster for LTR.
- return the right side boundary of the cluster for RTL.
* platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:
(HarfBuzzRun):
LayoutTests:
Added a test for complex text selection.
* fast/text/international/hebrew-selection-expected.html: Added.
* fast/text/international/hebrew-selection.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (129174 => 129175)
--- trunk/LayoutTests/ChangeLog 2012-09-20 23:11:16 UTC (rev 129174)
+++ trunk/LayoutTests/ChangeLog 2012-09-20 23:16:16 UTC (rev 129175)
@@ -1,3 +1,15 @@
+2012-09-20 Kenichi Ishibashi <[email protected]>
+
+ [Chromium] Improve glyph selection of HarfBuzzShaper
+ https://bugs.webkit.org/show_bug.cgi?id=97164
+
+ Reviewed by Tony Chang.
+
+ Added a test for complex text selection.
+
+ * fast/text/international/hebrew-selection-expected.html: Added.
+ * fast/text/international/hebrew-selection.html: Added.
+
2012-09-20 Dirk Pranke <[email protected]>
make Skip, WontFix be the only expectations on a line
Added: trunk/LayoutTests/fast/text/international/hebrew-selection-expected.html (0 => 129175)
--- trunk/LayoutTests/fast/text/international/hebrew-selection-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/international/hebrew-selection-expected.html 2012-09-20 23:16:16 UTC (rev 129175)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+<html>
+<body>
+
+<!-- To calculate width of composed glyph -->
+<div style="font-size:500%">
+<span id="reference">שְ</span>
+</div>
+
+<p>U+05e9 U+05b0 of following text should be selected.</p>
+<div style="font-size:500%">
+<span id="target">סְשְ</span>
+</div>
+
+<script>
+var target = document.getElementById("target");
+var text = target.firstChild;
+window.getSelection().setBaseAndExtent(text, 2, text, 4);
+target.focus();
+</script>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/international/hebrew-selection.html (0 => 129175)
--- trunk/LayoutTests/fast/text/international/hebrew-selection.html (rev 0)
+++ trunk/LayoutTests/fast/text/international/hebrew-selection.html 2012-09-20 23:16:16 UTC (rev 129175)
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<body>
+
+<!-- To calculate width of composed glyph -->
+<div style="font-size:500%">
+<span id="reference">שְ</span>
+</div>
+
+<p>U+05e9 U+05b0 of following text should be selected.</p>
+<div style="font-size:500%">
+<span id="target">סְשְ</span>
+</div>
+
+<script>
+var width = document.getElementById("reference").offsetWidth;
+var target = document.getElementById("target");
+
+if (window.eventSender) {
+ window.eventSender.mouseMoveTo(target.offsetLeft + 5, target.offsetTop + 5);
+ window.eventSender.mouseDown();
+ window.eventSender.mouseMoveTo(target.offsetLeft + 5 + width / 2, target.offsetTop + 5);
+ window.eventSender.mouseUp();
+}
+</script>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/chromium/TestExpectations (129174 => 129175)
--- trunk/LayoutTests/platform/chromium/TestExpectations 2012-09-20 23:11:16 UTC (rev 129174)
+++ trunk/LayoutTests/platform/chromium/TestExpectations 2012-09-20 23:16:16 UTC (rev 129175)
@@ -3558,4 +3558,6 @@
webkit.org/b/97179 http/tests/css/link-css-disabled-value-with-slow-loading-sheet.html [ Failure Pass ]
+# The test should pass after harfbuzz transition.
+webkit.org/b/97164 [ Linux ] fast/text/international/hebrew-selection.html [ Failure ]
Modified: trunk/Source/WebCore/ChangeLog (129174 => 129175)
--- trunk/Source/WebCore/ChangeLog 2012-09-20 23:11:16 UTC (rev 129174)
+++ trunk/Source/WebCore/ChangeLog 2012-09-20 23:16:16 UTC (rev 129175)
@@ -1,3 +1,30 @@
+2012-09-20 Kenichi Ishibashi <[email protected]>
+
+ [Chromium] Improve glyph selection of HarfBuzzShaper
+ https://bugs.webkit.org/show_bug.cgi?id=97164
+
+ Reviewed by Tony Chang.
+
+ Take into account clusters for selection.
+
+ Test: fast/text/international/hebrew-selection.html
+
+ * platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp:
+ (WebCore::HarfBuzzShaper::HarfBuzzRun::applyShapeResult): Removed m_logClusters.
+ m_logCluster is no longer used.
+ (WebCore::HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition):
+ - If targetX is in the left side of the first cluster, return the leftmost character index.
+ - If targetX is in the right side of the last cluster, return the rightmost character index.
+ - If targetX is between the right side of the cluster N and the left side of the cluster N+1, then:
+ - return N+1 for LTR.
+ - return N for RTL.
+ (WebCore::HarfBuzzShaper::HarfBuzzRun::xPositionForOffset):
+ Find the cluster of index in question, then:
+ - return the left side boundary of the cluster for LTR.
+ - return the right side boundary of the cluster for RTL.
+ * platform/graphics/harfbuzz/ng/HarfBuzzShaper.h:
+ (HarfBuzzRun):
+
2012-09-20 Tony Chang <[email protected]>
Replace RenderListBox::updateLogicalHeight with RenderListBox::computeLogicalHeight
Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp (129174 => 129175)
--- trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp 2012-09-20 23:11:16 UTC (rev 129174)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.cpp 2012-09-20 23:16:16 UTC (rev 129175)
@@ -86,30 +86,11 @@
m_glyphs.resize(m_numGlyphs);
m_advances.resize(m_numGlyphs);
m_glyphToCharacterIndexes.resize(m_numGlyphs);
- m_logClusters.resize(m_numCharacters);
m_offsets.resize(m_numGlyphs);
hb_glyph_info_t* infos = hb_buffer_get_glyph_infos(harfbuzzBuffer, 0);
for (unsigned i = 0; i < m_numGlyphs; ++i)
m_glyphToCharacterIndexes[i] = infos[i].cluster;
-
- // Fill logical clusters
- unsigned index = 0;
- while (index < m_numGlyphs) {
- unsigned nextIndex = index + 1;
- while (nextIndex < m_numGlyphs && infos[index].cluster == infos[nextIndex].cluster)
- ++nextIndex;
- if (rtl()) {
- int nextCluster = nextIndex < m_numGlyphs ? infos[nextIndex].cluster : -1;
- for (int j = infos[index].cluster; j > nextCluster; --j)
- m_logClusters[j] = index;
- } else {
- unsigned nextCluster = nextIndex < m_numGlyphs ? infos[nextIndex].cluster : m_numCharacters;
- for (unsigned j = infos[index].cluster; j < nextCluster; ++j)
- m_logClusters[j] = index;
- }
- index = nextIndex;
- }
}
void HarfBuzzShaper::HarfBuzzRun::setGlyphAndPositions(unsigned index, uint16_t glyphId, float advance, float offsetX, float offsetY)
@@ -123,14 +104,30 @@
{
ASSERT(targetX <= m_width);
float currentX = 0;
- float prevAdvance = 0;
- for (unsigned i = 0; i < m_numGlyphs; ++i) {
- float currentAdvance = m_advances[i] / 2.0;
+ float currentAdvance = m_advances[0];
+ unsigned glyphIndex = 0;
+
+ // Sum up advances that belong to a character.
+ while (glyphIndex < m_numGlyphs - 1 && m_glyphToCharacterIndexes[glyphIndex] == m_glyphToCharacterIndexes[glyphIndex + 1])
+ currentAdvance += m_advances[++glyphIndex];
+ currentAdvance = currentAdvance / 2.0;
+ if (targetX <= currentAdvance)
+ return rtl() ? m_numCharacters : 0;
+
+ ++glyphIndex;
+ while (glyphIndex < m_numGlyphs) {
+ unsigned prevCharacterIndex = m_glyphToCharacterIndexes[glyphIndex - 1];
+ float prevAdvance = currentAdvance;
+ currentAdvance = m_advances[glyphIndex];
+ while (glyphIndex < m_numGlyphs - 1 && m_glyphToCharacterIndexes[glyphIndex] == m_glyphToCharacterIndexes[glyphIndex + 1])
+ currentAdvance += m_advances[++glyphIndex];
+ currentAdvance = currentAdvance / 2.0;
float nextX = currentX + prevAdvance + currentAdvance;
if (currentX <= targetX && targetX <= nextX)
- return m_glyphToCharacterIndexes[i] + (rtl() ? 1 : 0);
+ return rtl() ? prevCharacterIndex : m_glyphToCharacterIndexes[glyphIndex];
currentX = nextX;
prevAdvance = currentAdvance;
+ ++glyphIndex;
}
return rtl() ? 0 : m_numCharacters;
@@ -139,13 +136,26 @@
float HarfBuzzShaper::HarfBuzzRun::xPositionForOffset(unsigned offset)
{
ASSERT(offset < m_numCharacters);
- unsigned glyphIndex = m_logClusters[offset];
- ASSERT(glyphIndex <= m_numGlyphs);
+ unsigned glyphIndex = 0;
float position = 0;
- for (unsigned i = 0; i < glyphIndex; ++i)
- position += m_advances[i];
- if (rtl())
+ if (rtl()) {
+ while (glyphIndex < m_numGlyphs && m_glyphToCharacterIndexes[glyphIndex] > offset) {
+ position += m_advances[glyphIndex];
+ ++glyphIndex;
+ }
+ // For RTL, we need to return the right side boundary of the character.
+ // Add advance of glyphs which are part of the character.
+ while (glyphIndex < m_numGlyphs - 1 && m_glyphToCharacterIndexes[glyphIndex] == m_glyphToCharacterIndexes[glyphIndex + 1]) {
+ position += m_advances[glyphIndex];
+ ++glyphIndex;
+ }
position += m_advances[glyphIndex];
+ } else {
+ while (glyphIndex < m_numGlyphs && m_glyphToCharacterIndexes[glyphIndex] < offset) {
+ position += m_advances[glyphIndex];
+ ++glyphIndex;
+ }
+ }
return position;
}
@@ -157,7 +167,7 @@
UChar32 character;
int nextPosition = position;
U16_NEXT(source, nextPosition, length, character);
- // Don't normalize tabs as they are not treated as spaces for word-end
+ // Don't normalize tabs as they are not treated as spaces for word-end.
if (Font::treatAsSpace(character) && character != '\t')
character = ' ';
else if (Font::treatAsZeroWidthSpaceInComplexScript(character))
Modified: trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.h (129174 => 129175)
--- trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.h 2012-09-20 23:11:16 UTC (rev 129174)
+++ trunk/Source/WebCore/platform/graphics/harfbuzz/ng/HarfBuzzShaper.h 2012-09-20 23:16:16 UTC (rev 129175)
@@ -94,7 +94,6 @@
TextDirection m_direction;
Vector<uint16_t, 256> m_glyphs;
Vector<float, 256> m_advances;
- Vector<uint16_t, 256> m_logClusters;
Vector<uint16_t, 256> m_glyphToCharacterIndexes;
Vector<FloatPoint, 256> m_offsets;
float m_width;
