Skip to content
Permalink
Browse files
[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.


Canonical link: https://commits.webkit.org/115222@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@129175 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
bashi committed Sep 20, 2012
1 parent 7f4691e commit 7ec03c931b1fa64dbe69203571a2fc2ae85a3ab1
Showing 7 changed files with 129 additions and 30 deletions.
@@ -1,3 +1,15 @@
2012-09-20 Kenichi Ishibashi <bashi@chromium.org>

[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 <dpranke@chromium.org>

make Skip, WontFix be the only expectations on a line
@@ -0,0 +1,22 @@
<!DOCTYPE html>
<html>
<body>

<!-- To calculate width of composed glyph -->
<div style="font-size:500%">
<span id="reference">&#x5e9;&#x5b0;</span>
</div>

<p>U+05e9 U+05b0 of following text should be selected.</p>
<div style="font-size:500%">
<span id="target">&#x5e1;&#x5b0;&#x5e9;&#x5b0;</span>
</div>

<script>
var target = document.getElementById("target");
var text = target.firstChild;
window.getSelection().setBaseAndExtent(text, 2, text, 4);
target.focus();
</script>
</body>
</html>
@@ -0,0 +1,27 @@
<!DOCTYPE html>
<html>
<body>

<!-- To calculate width of composed glyph -->
<div style="font-size:500%">
<span id="reference">&#x5e9;&#x5b0;</span>
</div>

<p>U+05e9 U+05b0 of following text should be selected.</p>
<div style="font-size:500%">
<span id="target">&#x5e1;&#x5b0;&#x5e9;&#x5b0;</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>
@@ -3558,4 +3558,6 @@ webkit.org/b/96962 fast/js/cross-frame-really-bad-time-with-__proto__.html [ Fai

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 ]

@@ -1,3 +1,30 @@
2012-09-20 Kenichi Ishibashi <bashi@chromium.org>

[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 <tony@chromium.org>

Replace RenderListBox::updateLogicalHeight with RenderListBox::computeLogicalHeight
@@ -86,30 +86,11 @@ void HarfBuzzShaper::HarfBuzzRun::applyShapeResult(hb_buffer_t* harfbuzzBuffer)
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 @@ int HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition(float targetX)
{
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 @@ int HarfBuzzShaper::HarfBuzzRun::characterIndexForXPosition(float targetX)
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 @@ static void normalizeCharacters(const UChar* source, UChar* destination, int len
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))
@@ -94,7 +94,6 @@ class HarfBuzzShaper : public HarfBuzzShaperBase {
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;

0 comments on commit 7ec03c9

Please sign in to comment.