Commit 52ea0c9f authored by allan.jensen@digia.com's avatar allan.jensen@digia.com
Browse files

Font’s fast code path doesn’t handle partial runs correctly when kerning or ligatures are enabled

https://bugs.webkit.org/show_bug.cgi?id=100050

Reviewed by Antti Koivisto.

Source/WebCore:

Always let WidthIterator iterate over an entire TextRun to avoid problems
with pixel rounding or shaping on partial runs.

This fix is necessary for Qt because the complex font-path can not disable
shaping, leading to the complex path painting slighly different from the
fast path, which messes up selection painting.

No change in functionality, no new tests.

* platform/graphics/Font.cpp:
(WebCore::Font::drawText):
(WebCore::Font::drawEmphasisMarks):
(WebCore::Font::selectionRectForText):
(WebCore::Font::offsetForPosition):
* platform/graphics/FontFastPath.cpp:
(WebCore::Font::getGlyphsAndAdvancesForSimpleText):
(WebCore::Font::selectionRectForSimpleText):
(WebCore::Font::offsetForPositionForSimpleText):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::add):
(GlyphBuffer):
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal):
* platform/graphics/WidthIterator.h:
(WidthIterator): Removed now unused advanceOneCharacter method.

LayoutTests:

* fast/text/resources/PTS55F-webfont.ttf: Added.
* fast/text/partial-textruns-expected.html: Added.
* fast/text/partial-textruns.html: Added.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@154384 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 1bf1cb62
2013-08-21 Allan Sandfeld Jensen <allan.jensen@digia.com>
Font’s fast code path doesn’t handle partial runs correctly when kerning or ligatures are enabled
https://bugs.webkit.org/show_bug.cgi?id=100050
Reviewed by Antti Koivisto.
* fast/text/resources/PTS55F-webfont.ttf: Added.
* fast/text/partial-textruns-expected.html: Added.
* fast/text/partial-textruns.html: Added.
2013-08-20 Antonio Gomes <a1.gomes@sisa.samsung.com>
 
Harden RenderBox::canBeScrolledAndHasScrollableArea logic
<html>
<head>
<style>
@font-face {
font-family: PTSans;
src: url('resources/PTS55F-webfont.ttf') format("truetype");
font-weight: normal;
font-style: normal;
}
#sandbox { position: absolute; width: 400px; }
article {
width: 400px;
font-family: PTSans;
font-size: 24px;
-webkit-font-kerning: normal;
}
</style>
</head>
<body>
<div id=sandbox>
<article>
This test is meant <span id=selection>to test if font metrics change when partially rendered. To test select multiple lines with the begin</span> or end line being only partially selected. The test succedes if none of the letters in the partially selected lines shift compared to when they are not selected or the line fully selected.
</article>
</div>
<script>
var selection = window.getSelection();
if (selection.rangeCount > 0)
selection.removeAllRanges();
var selectionNode = document.getElementById("selection");
var range = document.createRange();
range.selectNode(selectionNode);
selection.addRange(range);
</script>
</body>
</html>
\ No newline at end of file
<html>
<head>
<style>
@font-face {
font-family: PTSans;
src: url('resources/PTS55F-webfont.ttf') format("truetype");
font-weight: normal;
font-style: normal;
}
#sandbox { position: absolute; width: 400px; }
article {
width: 400px;
font-family: PTSans;
font-size: 24px;
-webkit-font-kerning: normal;
}
</style>
</head>
<body>
<div id=sandbox>
<article id=article>This test is meant to test if font metrics change when partially rendered. To test select multiple lines with the begin or end line being only partially selected. The test succedes if none of the letters in the partially selected lines shift compared to when they are not selected or the line fully selected.</article>
</div>
<script>
var selection = window.getSelection();
if (selection.rangeCount > 0)
selection.removeAllRanges();
var article = document.getElementById("article");
var range = document.createRange();
range.setStart(article.firstChild, 19);
range.setEnd(article.firstChild, 119);
selection.addRange(range);
</script>
</body>
</html>
\ No newline at end of file
2013-08-21 Allan Sandfeld Jensen <allan.jensen@digia.com>
Font’s fast code path doesn’t handle partial runs correctly when kerning or ligatures are enabled
https://bugs.webkit.org/show_bug.cgi?id=100050
Reviewed by Antti Koivisto.
Always let WidthIterator iterate over an entire TextRun to avoid problems
with pixel rounding or shaping on partial runs.
This fix is necessary for Qt because the complex font-path can not disable
shaping, leading to the complex path painting slighly different from the
fast path, which messes up selection painting.
No change in functionality, no new tests.
* platform/graphics/Font.cpp:
(WebCore::Font::drawText):
(WebCore::Font::drawEmphasisMarks):
(WebCore::Font::selectionRectForText):
(WebCore::Font::offsetForPosition):
* platform/graphics/FontFastPath.cpp:
(WebCore::Font::getGlyphsAndAdvancesForSimpleText):
(WebCore::Font::selectionRectForSimpleText):
(WebCore::Font::offsetForPositionForSimpleText):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::add):
(GlyphBuffer):
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::advanceInternal):
* platform/graphics/WidthIterator.h:
(WidthIterator): Removed now unused advanceOneCharacter method.
2013-08-20 Antonio Gomes <a1.gomes@sisa.samsung.com>
 
Harden RenderBox::canBeScrolledAndHasScrollableArea logic
......@@ -264,12 +264,7 @@ void Font::drawText(GraphicsContext* context, const TextRun& run, const FloatPoi
to = (to == -1 ? run.length() : to);
CodePath codePathToUse = codePath(run);
// FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()) && !run.renderingContext())
codePathToUse = Complex;
if (codePathToUse != Complex)
if (codePath(run) != Complex)
return drawSimpleText(context, run, point, from, to);
return drawComplexText(context, run, point, from, to);
......@@ -283,12 +278,7 @@ void Font::drawEmphasisMarks(GraphicsContext* context, const TextRun& run, const
if (to < 0)
to = run.length();
CodePath codePathToUse = codePath(run);
// FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()) && !run.renderingContext())
codePathToUse = Complex;
if (codePathToUse != Complex)
if (codePath(run) != Complex)
drawEmphasisMarksForSimpleText(context, run, mark, point, from, to);
else
drawEmphasisMarksForComplexText(context, run, mark, point, from, to);
......@@ -360,12 +350,7 @@ FloatRect Font::selectionRectForText(const TextRun& run, const FloatPoint& point
{
to = (to == -1 ? run.length() : to);
CodePath codePathToUse = codePath(run);
// FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
if (codePathToUse != Complex && typesettingFeatures() && (from || to != run.length()) && !run.renderingContext())
codePathToUse = Complex;
if (codePathToUse != Complex)
if (codePath(run) != Complex)
return selectionRectForSimpleText(run, point, h, from, to);
return selectionRectForComplexText(run, point, h, from, to);
......@@ -373,8 +358,7 @@ FloatRect Font::selectionRectForText(const TextRun& run, const FloatPoint& point
int Font::offsetForPosition(const TextRun& run, float x, bool includePartialGlyphs) const
{
// FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
if (codePath(run) != Complex && !typesettingFeatures())
if (codePath(run) != Complex)
return offsetForPositionForSimpleText(run, x, includePartialGlyphs);
return offsetForPositionForComplexText(run, x, includePartialGlyphs);
......
......@@ -129,32 +129,34 @@ int Font::emphasisMarkHeight(const AtomicString& mark) const
float Font::getGlyphsAndAdvancesForSimpleText(const TextRun& run, int from, int to, GlyphBuffer& glyphBuffer, ForTextEmphasisOrNot forTextEmphasis) const
{
float initialAdvance;
WidthIterator it(this, run, 0, false, forTextEmphasis);
// FIXME: Using separate glyph buffers for the prefix and the suffix is incorrect when kerning or
// ligatures are enabled.
GlyphBuffer localGlyphBuffer;
it.advance(from, &localGlyphBuffer);
float beforeWidth = it.m_runWidthSoFar;
it.advance(to, &glyphBuffer);
it.advance(run.length(), &localGlyphBuffer);
if (glyphBuffer.isEmpty())
if (localGlyphBuffer.isEmpty())
return 0;
float afterWidth = it.m_runWidthSoFar;
float totalWidth = it.m_runWidthSoFar;
float beforeWidth = 0;
int glyphPos = 0;
for (; glyphPos < localGlyphBuffer.size() && it.m_characterIndex[glyphPos] < from; ++glyphPos)
beforeWidth += localGlyphBuffer.advanceAt(glyphPos).width();
int glyphFrom = glyphPos;
if (run.rtl()) {
float finalRoundingWidth = it.m_finalRoundingWidth;
it.advance(run.length(), &localGlyphBuffer);
initialAdvance = finalRoundingWidth + it.m_runWidthSoFar - afterWidth;
} else
initialAdvance = beforeWidth;
float afterWidth = totalWidth;
glyphPos = localGlyphBuffer.size() - 1;
for (; glyphPos >= glyphFrom && it.m_characterIndex[glyphPos] >= to; --glyphPos)
afterWidth -= localGlyphBuffer.advanceAt(glyphPos).width();
int glyphTo = glyphPos + 1;
glyphBuffer.add(&localGlyphBuffer, glyphFrom, glyphTo - glyphFrom);
if (run.rtl())
if (run.rtl()) {
glyphBuffer.reverse(0, glyphBuffer.size());
return totalWidth - afterWidth;
}
return initialAdvance;
return beforeWidth;
}
void Font::drawSimpleText(GraphicsContext* context, const TextRun& run, const FloatPoint& point, int from, int to) const
......@@ -296,15 +298,22 @@ FloatRect Font::selectionRectForSimpleText(const TextRun& run, const FloatPoint&
{
GlyphBuffer glyphBuffer;
WidthIterator it(this, run);
it.advance(from, &glyphBuffer);
float beforeWidth = it.m_runWidthSoFar;
it.advance(to, &glyphBuffer);
float afterWidth = it.m_runWidthSoFar;
it.advance(run.length(), &glyphBuffer);
float totalWidth = it.m_runWidthSoFar;
float beforeWidth = 0;
int glyphPos = 0;
for (; glyphPos < glyphBuffer.size() && it.m_characterIndex[glyphPos] < from; ++glyphPos)
beforeWidth += glyphBuffer.advanceAt(glyphPos).width();
int glyphFrom = glyphPos;
float afterWidth = totalWidth;
glyphPos = glyphBuffer.size() - 1;
for (; glyphPos >= glyphFrom && it.m_characterIndex[glyphPos] >= to; --glyphPos)
afterWidth -= glyphBuffer.advanceAt(glyphPos).width();
// Using roundf() rather than ceilf() for the right edge as a compromise to ensure correct caret positioning.
if (run.rtl()) {
it.advance(run.length(), &glyphBuffer);
float totalWidth = it.m_runWidthSoFar;
return FloatRect(floorf(point.x() + totalWidth - afterWidth), point.y(), roundf(point.x() + totalWidth - beforeWidth) - floorf(point.x() + totalWidth - afterWidth), h);
}
......@@ -313,45 +322,50 @@ FloatRect Font::selectionRectForSimpleText(const TextRun& run, const FloatPoint&
int Font::offsetForPositionForSimpleText(const TextRun& run, float x, bool includePartialGlyphs) const
{
float delta = x;
GlyphBuffer glyphBuffer;
WidthIterator it(this, run);
GlyphBuffer localGlyphBuffer;
unsigned offset;
it.advance(run.length(), &glyphBuffer);
int characterOffset = 0;
if (run.rtl()) {
delta -= floatWidthForSimpleText(run);
while (1) {
offset = it.m_currentCharacter;
float w;
if (!it.advanceOneCharacter(w, localGlyphBuffer))
float currentX = it.m_runWidthSoFar;
for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
if (glyphPosition == glyphBuffer.size()) {
characterOffset = run.length();
break;
delta += w;
}
characterOffset = it.m_characterIndex[glyphPosition];
float glyphWidth = glyphBuffer.advanceAt(glyphPosition).width();
if (includePartialGlyphs) {
if (delta - w / 2 >= 0)
if (currentX - glyphWidth / 2.0f <= x)
break;
} else {
if (delta >= 0)
if (currentX - glyphWidth <= x)
break;
}
currentX -= glyphWidth;
}
} else {
while (1) {
offset = it.m_currentCharacter;
float w;
if (!it.advanceOneCharacter(w, localGlyphBuffer))
float currentX = 0;
for (int glyphPosition = 0; glyphPosition <= glyphBuffer.size(); ++glyphPosition) {
if (glyphPosition == glyphBuffer.size()) {
characterOffset = run.length();
break;
delta -= w;
}
characterOffset = it.m_characterIndex[glyphPosition];
float glyphWidth = glyphBuffer.advanceAt(glyphPosition).width();
if (includePartialGlyphs) {
if (delta + w / 2 <= 0)
if (currentX + glyphWidth / 2.0f >= x)
break;
} else {
if (delta <= 0)
if (currentX + glyphWidth >= x)
break;
}
currentX += glyphWidth;
}
}
return offset;
return characterOffset;
}
}
} // namespace WebCore
......@@ -139,6 +139,16 @@ public:
#endif
}
void add(const GlyphBuffer* glyphBuffer, int from, int len)
{
m_glyphs.append(glyphBuffer->glyphs(from), len);
m_advances.append(glyphBuffer->advances(from), len);
m_fontData.append(glyphBuffer->m_fontData.data() + from, len);
#if PLATFORM(WIN)
m_offsets.append(glyphBuffer->m_offsets.data() + from, len);
#endif
}
void add(Glyph glyph, const SimpleFontData* font, float width, const FloatSize* offset = 0)
{
m_fontData.append(font);
......
......@@ -162,7 +162,8 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
CharactersTreatedAsSpace charactersTreatedAsSpace;
while (textIterator.consume(character, clusterLength)) {
unsigned advanceLength = clusterLength;
const GlyphData& glyphData = glyphDataForCharacter(character, rtl, textIterator.currentCharacter(), advanceLength);
int currentCharacterIndex = textIterator.currentCharacter();
const GlyphData& glyphData = glyphDataForCharacter(character, rtl, currentCharacterIndex, advanceLength);
Glyph glyph = glyphData.glyph;
const SimpleFontData* fontData = glyphData.fontData;
......@@ -230,6 +231,7 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
glyphBuffer->add(fontData->zeroWidthSpaceGlyph(), fontData, m_expansionPerOpportunity);
else
glyphBuffer->add(fontData->spaceGlyph(), fontData, expansionAtThisOpportunity);
m_characterIndex.append(currentCharacterIndex);
} else
glyphBuffer->expandLastAdvance(expansionAtThisOpportunity);
}
......@@ -296,8 +298,10 @@ inline unsigned WidthIterator::advanceInternal(TextIterator& textIterator, Glyph
widthSinceLastRounding += width;
}
if (glyphBuffer)
if (glyphBuffer) {
glyphBuffer->add(glyph, fontData, (rtl ? oldWidth + lastRoundingWidth : width));
m_characterIndex.append(currentCharacterIndex);
}
lastRoundingWidth = width - oldWidth;
......@@ -337,15 +341,4 @@ unsigned WidthIterator::advance(int offset, GlyphBuffer* glyphBuffer)
return advanceInternal(textIterator, glyphBuffer);
}
bool WidthIterator::advanceOneCharacter(float& width, GlyphBuffer& glyphBuffer)
{
int oldSize = glyphBuffer.size();
advance(m_currentCharacter + 1, &glyphBuffer);
float w = 0;
for (int i = oldSize; i < glyphBuffer.size(); ++i)
w += glyphBuffer.advanceAt(i).width();
width = w;
return glyphBuffer.size() > oldSize;
}
}
......@@ -43,7 +43,6 @@ public:
WidthIterator(const Font*, const TextRun&, HashSet<const SimpleFontData*>* fallbackFonts = 0, bool accountForGlyphBounds = false, bool forTextEmphasis = false);
unsigned advance(int to, GlyphBuffer*);
bool advanceOneCharacter(float& width, GlyphBuffer&);
float maxGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_maxGlyphBoundingBoxY; }
float minGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_minGlyphBoundingBoxY; }
......@@ -83,6 +82,7 @@ public:
float m_expansionPerOpportunity;
bool m_isAfterExpansion;
float m_finalRoundingWidth;
Vector<int> m_characterIndex;
#if ENABLE(SVG_FONTS)
String m_lastGlyphName;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment