- Revision
- 265488
- Author
- [email protected]
- Date
- 2020-08-10 22:17:04 -0700 (Mon, 10 Aug 2020)
Log Message
Spacing of Chinese characters is inconsistent in macOS 11/Safari 14 beta
https://bugs.webkit.org/show_bug.cgi?id=214769
Reviewed by Darin Adler.
Source/WebCore:
This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=206208..
In the general case, text shaping is Turing-complete. In order for it to work properly,
we need to feed the text shaping virtual machine the correct inputs so that it can
produce correct outputs. The input to text shaping is supposed to be the raw glyph
advances straight from CTFontGetAdvancesForGlyphs().
Previously, we were applying letter-spacing, word-spacing, justification, and tab stops
before shaping. Most fonts don't care about this and still produce the correct results.
However, Ping Fang on macOS Big Sur and iOS 14 _does_ care about this, and its shaping
rules operate incorrectly when fed these pre-expanded glyph widths.
The solution is to apply this extra spacing after shaping occurs. However, because
shaping is Turing-complete, it is free to add or remove glyphs willy-nilly. This means
we need some way of tracing back which character in the input string correspond to which
output glyphs, so we know which glyphs to add additional spacing to. This is what
https://bugs.webkit.org/show_bug.cgi?id=215059 does: It switches from using
CTFontTransformGlyphsWithLanguage() to CTFontShapeGlyphs(), which outputs this
glyph-character tracing info. Then, once we have this tracing info, we can apply spacing
properly after shaping has completed. That's what this patch does.
Tests: fast/text/letter-spacing-shaping.html
fast/text/tab-letter-space.html
* platform/graphics/FontCascade.cpp: Clients of WidthIterator::advance() need to call
WidthIterator::finalize() (see below).
(WebCore::FontCascade::widthOfTextRange const):
(WebCore::FontCascade::layoutSimpleText const):
(WebCore::FontCascade::floatWidthForSimpleText const):
(WebCore::FontCascade::adjustSelectionRectForSimpleText const):
(WebCore::FontCascade::offsetForPositionForSimpleText const):
* platform/graphics/GlyphBuffer.h:
(WebCore::GlyphBuffer::expandInitialAdvance):
(WebCore::GlyphBuffer::expandAdvance):
* platform/graphics/WidthIterator.cpp:
(WebCore::WidthIterator::WidthIterator):
(WebCore::WidthIterator::hasExtraSpacing const):
(WebCore::WidthIterator::advanceInternal): Delete the code that used to apply spacing
before shaping. Instead, it gets moved to applyExtraSpacingAfterShaping().
(WebCore::WidthIterator::calculateAdditionalWidth const): This is a refactoring of
the additional space calculation code. This is a const function, and has no side-effects.
It outputs 4 floats:
- How much space needs to be added to the left of the current character
- How much space needs to be added to the right of the current character
- How much space needs to be added to the left of the current character due to justification
- How much space needs to be added to the right of the current character due to justification
We need these last two values because one of the outputs of WidthIterator::advance() is
a bool which represents whether or not we are ending on a justification space, so that the
next WidthIterator that gets created for the next thing on the line can forbid/require a
leading justification appropriately.
(WebCore::WidthIterator::applyAdditionalWidth): Given the 4 values calculated in
calculateAdditionalWidth(), apply them. We're operating in logical order, so this function has
to do some translation from visual order to logical order (hence all the m_run.ltr() calls).
If we're trying to add size to the left of the first character in LTR, we can't directly do
that, because advances can only add space to the right side of a character, and there is no
character to the left of the first character to expand. Therefore, we do this by increasing
the initial advance of the GlyphBuffer, which is a special advance created just to solve this
problem. In RTL, the problem is a bit more complicated - we don't know whether advance() will
be called again, thereby delivering a glyph which we _can_ expand, so we instead store what
would have been expanded inside m_leftoverJustificationWidth, and wait for the next call to
advance(). If none comes, clients have to call finalize() instead, which will apply
m_leftoverJustificationWidth to the GlyphBuffer's initial advance.
(WebCore::WidthIterator::applyExtraSpacingAfterShaping):
(WebCore::WidthIterator::finalize):
(WebCore::WidthIterator::advance):
* platform/graphics/WidthIterator.h:
* rendering/svg/SVGTextMetricsBuilder.cpp:
(WebCore::SVGTextMetricsBuilder::measureTextRenderer):
LayoutTests:
* fast/text/letter-spacing-shaping-expected.html: Added.
* fast/text/letter-spacing-shaping.html: Added.
* fast/text/tab-letter-space-expected.html: Added.
* fast/text/tab-letter-space.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (265487 => 265488)
--- trunk/LayoutTests/ChangeLog 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/LayoutTests/ChangeLog 2020-08-11 05:17:04 UTC (rev 265488)
@@ -1,5 +1,17 @@
2020-08-10 Myles C. Maxfield <[email protected]>
+ Spacing of Chinese characters is inconsistent in macOS 11/Safari 14 beta
+ https://bugs.webkit.org/show_bug.cgi?id=214769
+
+ Reviewed by Darin Adler.
+
+ * fast/text/letter-spacing-shaping-expected.html: Added.
+ * fast/text/letter-spacing-shaping.html: Added.
+ * fast/text/tab-letter-space-expected.html: Added.
+ * fast/text/tab-letter-space.html: Added.
+
+2020-08-10 Myles C. Maxfield <[email protected]>
+
[Cocoa] Migrate from CTFontTransformGlyphsWithLanguage() to CTFontShapeGlyphs()
https://bugs.webkit.org/show_bug.cgi?id=215059
Added: trunk/LayoutTests/fast/text/letter-spacing-shaping-expected.html (0 => 265488)
--- trunk/LayoutTests/fast/text/letter-spacing-shaping-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/letter-spacing-shaping-expected.html 2020-08-11 05:17:04 UTC (rev 265488)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html lang="zh-hk">
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+This test makes sure that letter-spacing is applied after text shaping.
+The test passes if the two characters below have some space between them.
+<div style="font: 48px 'PingFang TC'; letter-spacing: 24px;"><span>情</span><span>升</span></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/letter-spacing-shaping.html (0 => 265488)
--- trunk/LayoutTests/fast/text/letter-spacing-shaping.html (rev 0)
+++ trunk/LayoutTests/fast/text/letter-spacing-shaping.html 2020-08-11 05:17:04 UTC (rev 265488)
@@ -0,0 +1,11 @@
+<!DOCTYPE html>
+<html lang="zh-hk">
+<head>
+<meta charset="utf-8">
+</head>
+<body>
+This test makes sure that letter-spacing is applied after text shaping.
+The test passes if the two characters below have some space between them.
+<div style="font: 48px 'PingFang TC'; letter-spacing: 24px;">情升</div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/tab-letter-space-expected.html (0 => 265488)
--- trunk/LayoutTests/fast/text/tab-letter-space-expected.html (rev 0)
+++ trunk/LayoutTests/fast/text/tab-letter-space-expected.html 2020-08-11 05:17:04 UTC (rev 265488)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html lang="zh-hk">
+<head>
+<meta charset="utf-8">
+<style>
+pre {
+ margin: 0px;
+}
+</style>
+</head>
+<body>
+This test makes sure that tab characters get the appropriate amount of letter-spacing.
+The test passes if the left edge of the "c" below is 158 pixels from the left edge of the "a" below.
+<pre style="position: relative;"><pre style="position: absolute; left: 0px;">a</pre><pre style="position: absolute; left: 158px;">c</pre></pre>
+</body>
+</html>
Added: trunk/LayoutTests/fast/text/tab-letter-space.html (0 => 265488)
--- trunk/LayoutTests/fast/text/tab-letter-space.html (rev 0)
+++ trunk/LayoutTests/fast/text/tab-letter-space.html 2020-08-11 05:17:04 UTC (rev 265488)
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html lang="zh-hk">
+<head>
+<meta charset="utf-8">
+<style>
+pre {
+ margin: 0px;
+}
+</style>
+</head>
+<body>
+This test makes sure that tab characters get the appropriate amount of letter-spacing.
+The test passes if the left edge of the "c" below is 158 pixels from the left edge of the "a" below.
+<pre style="letter-spacing: 30px; tab-size: 128px;">a	c</pre>
+</body>
+</html>
Modified: trunk/LayoutTests/platform/win/fast/text/basic/013-expected.txt (265487 => 265488)
--- trunk/LayoutTests/platform/win/fast/text/basic/013-expected.txt 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/LayoutTests/platform/win/fast/text/basic/013-expected.txt 2020-08-11 05:17:04 UTC (rev 265488)
@@ -42,8 +42,8 @@
RenderInline {B} at (0,0) size 50x18
RenderText {#text} at (131,0) size 50x18
text run at (131,0) width 50: "bold"
- RenderText {#text} at (180,0) size 101x18
- text run at (180,0) width 101: " element."
+ RenderText {#text} at (180,0) size 102x18
+ text run at (180,0) width 102: " element."
RenderBlock {P} at (0,34) size 784x18
RenderText {#text} at (0,0) size 132x18
text run at (0,0) width 132: "Text inside "
Modified: trunk/Source/WebCore/ChangeLog (265487 => 265488)
--- trunk/Source/WebCore/ChangeLog 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/ChangeLog 2020-08-11 05:17:04 UTC (rev 265488)
@@ -1,5 +1,81 @@
2020-08-10 Myles C. Maxfield <[email protected]>
+ Spacing of Chinese characters is inconsistent in macOS 11/Safari 14 beta
+ https://bugs.webkit.org/show_bug.cgi?id=214769
+
+ Reviewed by Darin Adler.
+
+ This is in preparation for https://bugs.webkit.org/show_bug.cgi?id=206208..
+
+ In the general case, text shaping is Turing-complete. In order for it to work properly,
+ we need to feed the text shaping virtual machine the correct inputs so that it can
+ produce correct outputs. The input to text shaping is supposed to be the raw glyph
+ advances straight from CTFontGetAdvancesForGlyphs().
+
+ Previously, we were applying letter-spacing, word-spacing, justification, and tab stops
+ before shaping. Most fonts don't care about this and still produce the correct results.
+ However, Ping Fang on macOS Big Sur and iOS 14 _does_ care about this, and its shaping
+ rules operate incorrectly when fed these pre-expanded glyph widths.
+
+ The solution is to apply this extra spacing after shaping occurs. However, because
+ shaping is Turing-complete, it is free to add or remove glyphs willy-nilly. This means
+ we need some way of tracing back which character in the input string correspond to which
+ output glyphs, so we know which glyphs to add additional spacing to. This is what
+ https://bugs.webkit.org/show_bug.cgi?id=215059 does: It switches from using
+ CTFontTransformGlyphsWithLanguage() to CTFontShapeGlyphs(), which outputs this
+ glyph-character tracing info. Then, once we have this tracing info, we can apply spacing
+ properly after shaping has completed. That's what this patch does.
+
+ Tests: fast/text/letter-spacing-shaping.html
+ fast/text/tab-letter-space.html
+
+ * platform/graphics/FontCascade.cpp: Clients of WidthIterator::advance() need to call
+ WidthIterator::finalize() (see below).
+ (WebCore::FontCascade::widthOfTextRange const):
+ (WebCore::FontCascade::layoutSimpleText const):
+ (WebCore::FontCascade::floatWidthForSimpleText const):
+ (WebCore::FontCascade::adjustSelectionRectForSimpleText const):
+ (WebCore::FontCascade::offsetForPositionForSimpleText const):
+ * platform/graphics/GlyphBuffer.h:
+ (WebCore::GlyphBuffer::expandInitialAdvance):
+ (WebCore::GlyphBuffer::expandAdvance):
+ * platform/graphics/WidthIterator.cpp:
+ (WebCore::WidthIterator::WidthIterator):
+ (WebCore::WidthIterator::hasExtraSpacing const):
+ (WebCore::WidthIterator::advanceInternal): Delete the code that used to apply spacing
+ before shaping. Instead, it gets moved to applyExtraSpacingAfterShaping().
+ (WebCore::WidthIterator::calculateAdditionalWidth const): This is a refactoring of
+ the additional space calculation code. This is a const function, and has no side-effects.
+ It outputs 4 floats:
+ - How much space needs to be added to the left of the current character
+ - How much space needs to be added to the right of the current character
+ - How much space needs to be added to the left of the current character due to justification
+ - How much space needs to be added to the right of the current character due to justification
+ We need these last two values because one of the outputs of WidthIterator::advance() is
+ a bool which represents whether or not we are ending on a justification space, so that the
+ next WidthIterator that gets created for the next thing on the line can forbid/require a
+ leading justification appropriately.
+ (WebCore::WidthIterator::applyAdditionalWidth): Given the 4 values calculated in
+ calculateAdditionalWidth(), apply them. We're operating in logical order, so this function has
+ to do some translation from visual order to logical order (hence all the m_run.ltr() calls).
+ If we're trying to add size to the left of the first character in LTR, we can't directly do
+ that, because advances can only add space to the right side of a character, and there is no
+ character to the left of the first character to expand. Therefore, we do this by increasing
+ the initial advance of the GlyphBuffer, which is a special advance created just to solve this
+ problem. In RTL, the problem is a bit more complicated - we don't know whether advance() will
+ be called again, thereby delivering a glyph which we _can_ expand, so we instead store what
+ would have been expanded inside m_leftoverJustificationWidth, and wait for the next call to
+ advance(). If none comes, clients have to call finalize() instead, which will apply
+ m_leftoverJustificationWidth to the GlyphBuffer's initial advance.
+ (WebCore::WidthIterator::applyExtraSpacingAfterShaping):
+ (WebCore::WidthIterator::finalize):
+ (WebCore::WidthIterator::advance):
+ * platform/graphics/WidthIterator.h:
+ * rendering/svg/SVGTextMetricsBuilder.cpp:
+ (WebCore::SVGTextMetricsBuilder::measureTextRenderer):
+
+2020-08-10 Myles C. Maxfield <[email protected]>
+
[Cocoa] Migrate from CTFontTransformGlyphsWithLanguage() to CTFontShapeGlyphs()
https://bugs.webkit.org/show_bug.cgi?id=215059
Modified: trunk/Source/WebCore/platform/graphics/FontCascade.cpp (265487 => 265488)
--- trunk/Source/WebCore/platform/graphics/FontCascade.cpp 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/platform/graphics/FontCascade.cpp 2020-08-11 05:17:04 UTC (rev 265488)
@@ -380,6 +380,7 @@
offsetAfterRange = simpleIterator.runWidthSoFar();
simpleIterator.advance(run.length(), glyphBuffer);
totalWidth = simpleIterator.runWidthSoFar();
+ simpleIterator.finalize(glyphBuffer);
}
if (outWidthBeforeRange)
@@ -1390,11 +1391,13 @@
float initialAdvance = 0;
if (run.rtl()) {
it.advance(run.length(), localGlyphBuffer);
+ it.finalize(localGlyphBuffer);
initialAdvance = it.runWidthSoFar() - afterWidth;
- } else
+ } else {
+ it.finalize(localGlyphBuffer);
initialAdvance = beforeWidth;
- // FIXME: Deal with the GlyphBuffer's current initialAdvance.
- glyphBuffer.setInitialAdvance(FloatSize(initialAdvance, 0));
+ }
+ glyphBuffer.expandInitialAdvance(initialAdvance);
// The glyph buffer is currently in logical order,
// but we need to return the results in visual order.
@@ -1527,6 +1530,7 @@
WidthIterator it(*this, run, fallbackFonts, glyphOverflow);
GlyphBuffer glyphBuffer;
it.advance(run.length(), glyphBuffer);
+ it.finalize(glyphBuffer);
if (glyphOverflow) {
glyphOverflow->top = std::max<int>(glyphOverflow->top, ceilf(-it.minGlyphBoundingBoxY()) - (glyphOverflow->computeBounds ? 0 : fontMetrics().ascent()));
@@ -1561,10 +1565,13 @@
if (run.rtl()) {
it.advance(run.length(), glyphBuffer);
+ it.finalize(glyphBuffer);
float totalWidth = it.runWidthSoFar();
selectionRect.move(totalWidth - afterWidth, 0);
- } else
+ } else {
+ it.finalize(glyphBuffer);
selectionRect.move(beforeWidth, 0);
+ }
selectionRect.setWidth(LayoutUnit::fromFloatCeil(afterWidth - beforeWidth));
}
@@ -1623,6 +1630,7 @@
}
}
+ it.finalize(localGlyphBuffer);
return offset;
}
Modified: trunk/Source/WebCore/platform/graphics/GlyphBuffer.h (265487 => 265488)
--- trunk/Source/WebCore/platform/graphics/GlyphBuffer.h 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/platform/graphics/GlyphBuffer.h 2020-08-11 05:17:04 UTC (rev 265488)
@@ -29,6 +29,7 @@
#pragma once
+#include "FloatPoint.h"
#include "FloatSize.h"
#include "Glyph.h"
#include <climits>
@@ -202,6 +203,7 @@
void setInitialAdvance(GlyphBufferAdvance initialAdvance) { m_initialAdvance = initialAdvance; }
const GlyphBufferAdvance& initialAdvance() const { return m_initialAdvance; }
+ void expandInitialAdvance(float width) { m_initialAdvance.setWidth(m_initialAdvance.width() + width); }
static constexpr GlyphBufferStringOffset noOffset = std::numeric_limits<GlyphBufferStringOffset>::max();
void add(Glyph glyph, const Font& font, float width, GlyphBufferStringOffset offsetInString = noOffset)
@@ -254,6 +256,13 @@
lastAdvance.setWidth(lastAdvance.width() + width);
}
+ void expandAdvance(unsigned index, float width)
+ {
+ ASSERT(index < size());
+ auto& lastAdvance = m_advances[index];
+ lastAdvance.setWidth(lastAdvance.width() + width);
+ }
+
void expandLastAdvance(GlyphBufferAdvance expansion)
{
ASSERT(!isEmpty());
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.cpp (265487 => 265488)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.cpp 2020-08-11 05:17:04 UTC (rev 265488)
@@ -46,6 +46,8 @@
, m_requiresShaping(font.requiresShaping())
, m_forTextEmphasis(forTextEmphasis)
{
+ // FIXME: Should we clamp m_expansion so it can never be negative?
+
if (!m_expansion)
m_expansionPerOpportunity = 0;
else {
@@ -182,19 +184,17 @@
}
}
+bool WidthIterator::hasExtraSpacing() const
+{
+ return (m_font.letterSpacing() || m_font.wordSpacing() || m_expansion) && !m_run.spacingDisabled();
+}
+
template <typename TextIterator>
inline void WidthIterator::advanceInternal(TextIterator& textIterator, GlyphBuffer& glyphBuffer)
{
// The core logic here needs to match FontCascade::widthForSimpleText()
bool rtl = m_run.rtl();
- bool hasExtraSpacing = (m_font.letterSpacing() || m_font.wordSpacing() || m_expansion) && !m_run.spacingDisabled();
- bool runForcesLeftExpansion = (m_run.expansionBehavior() & LeftExpansionMask) == ForceLeftExpansion;
- bool runForcesRightExpansion = (m_run.expansionBehavior() & RightExpansionMask) == ForceRightExpansion;
- bool runForbidsLeftExpansion = (m_run.expansionBehavior() & LeftExpansionMask) == ForbidLeftExpansion;
- bool runForbidsRightExpansion = (m_run.expansionBehavior() & RightExpansionMask) == ForbidRightExpansion;
- float leftoverJustificationWidth = 0;
-
FloatRect bounds;
const Font& primaryFont = m_font.primaryFont();
@@ -208,7 +208,11 @@
float widthOfCurrentFontRange = 0;
// We are iterating in string order, not glyph order. Compare this to ComplexTextController::adjustGlyphsAndAdvances()
while (textIterator.consume(character, clusterLength)) {
+ m_containsTabs |= character == tabCharacter;
+ currentCharacterIndex = textIterator.currentIndex();
unsigned advanceLength = clusterLength;
+ if (currentCharacterIndex + advanceLength == m_run.length())
+ m_lastCharacterIndex = currentCharacterIndex;
bool characterMustDrawSomething = !isDefaultIgnorableCodePoint(character);
#if USE(FREETYPE)
// Freetype based ports only override the characters with Default_Ignorable unicode property when the font
@@ -229,17 +233,8 @@
const Font* font = glyphData.font ? glyphData.font : &m_font.primaryFont();
ASSERT(font);
- // Now that we have a glyph and font data, get its width.
- float width;
- if (character == '\t' && m_run.allowTabs())
- width = m_font.tabWidth(*font, m_run.tabSize(), m_run.xPos() + m_runWidthSoFar);
- else {
- width = font->widthForGlyph(glyph);
+ float width = font->widthForGlyph(glyph);
- // SVG uses horizontalGlyphStretch(), when textLength is used to stretch/squeeze text.
- width *= m_run.horizontalGlyphStretch();
- }
-
if (font != lastFontData) {
commitCurrentFontRange(glyphBuffer, lastGlyphCount, currentCharacterIndex, *lastFontData, primaryFont, character, widthOfCurrentFontRange, charactersTreatedAsSpace);
lastGlyphCount = glyphBuffer.size();
@@ -248,71 +243,6 @@
} else
widthOfCurrentFontRange += width;
- if (hasExtraSpacing) {
- // Account for letter-spacing.
- if (width) {
- width += m_font.letterSpacing();
- width += leftoverJustificationWidth;
- leftoverJustificationWidth = 0;
- }
-
- static bool expandAroundIdeographs = FontCascade::canExpandAroundIdeographsInComplexText();
- bool treatAsSpace = FontCascade::treatAsSpace(character);
- bool currentIsLastCharacter = currentCharacterIndex + advanceLength == static_cast<size_t>(m_run.length());
- bool forceLeftExpansion = false; // On the left, regardless of m_run.ltr()
- bool forceRightExpansion = false; // On the right, regardless of m_run.ltr()
- bool forbidLeftExpansion = false;
- bool forbidRightExpansion = false;
- if (runForcesLeftExpansion)
- forceLeftExpansion = m_run.ltr() ? !currentCharacterIndex : currentIsLastCharacter;
- if (runForcesRightExpansion)
- forceRightExpansion = m_run.ltr() ? currentIsLastCharacter : !currentCharacterIndex;
- if (runForbidsLeftExpansion)
- forbidLeftExpansion = m_run.ltr() ? !currentCharacterIndex : currentIsLastCharacter;
- if (runForbidsRightExpansion)
- forbidRightExpansion = m_run.ltr() ? currentIsLastCharacter : !currentCharacterIndex;
- bool ideograph = (expandAroundIdeographs && FontCascade::isCJKIdeographOrSymbol(character));
- if (treatAsSpace || ideograph || forceLeftExpansion || forceRightExpansion) {
- // Distribute the run's total expansion evenly over all expansion opportunities in the run.
- if (m_expansion) {
- auto [expandLeft, expandRight] = expansionLocation(ideograph, treatAsSpace, m_run.ltr(), m_isAfterExpansion, forbidLeftExpansion, forbidRightExpansion, forceLeftExpansion, forceRightExpansion);
- if (expandLeft) {
- if (m_run.ltr()) {
- // Increase previous width
- m_expansion -= m_expansionPerOpportunity;
- m_runWidthSoFar += m_expansionPerOpportunity;
- if (glyphBuffer.isEmpty()) {
- if (m_forTextEmphasis)
- glyphBuffer.add(font->zeroWidthSpaceGlyph(), *font, m_expansionPerOpportunity, currentCharacterIndex);
- else
- glyphBuffer.add(font->spaceGlyph(), *font, m_expansionPerOpportunity, currentCharacterIndex);
- lastGlyphCount = glyphBuffer.size();
- m_currentCharacterIndex = currentCharacterIndex;
- } else
- glyphBuffer.expandLastAdvance(m_expansionPerOpportunity);
- } else {
- // Increase next width
- leftoverJustificationWidth += m_expansionPerOpportunity;
- m_isAfterExpansion = true;
- }
- }
- if (expandRight) {
- m_expansion -= m_expansionPerOpportunity;
- width += m_expansionPerOpportunity;
- if (m_run.ltr())
- m_isAfterExpansion = true;
- }
- } else
- m_isAfterExpansion = false;
-
- // Account for word spacing.
- // We apply additional space between "words" by adding width to the space character.
- if (treatAsSpace && (character != '\t' || !m_run.allowTabs()) && (currentCharacterIndex || character == noBreakSpace) && m_font.wordSpacing())
- width += m_font.wordSpacing();
- } else
- m_isAfterExpansion = false;
- }
-
auto transformsType = shouldApplyFontTransforms(glyphBuffer, lastGlyphCount, currentCharacterIndex);
if (transformsType != TransformsType::None && FontCascade::treatAsSpace(character)) {
charactersTreatedAsSpace.constructAndAppend(
@@ -347,17 +277,184 @@
}
commitCurrentFontRange(glyphBuffer, lastGlyphCount, currentCharacterIndex, *lastFontData, primaryFont, character, widthOfCurrentFontRange, charactersTreatedAsSpace);
+}
- if (leftoverJustificationWidth) {
- if (m_forTextEmphasis)
- glyphBuffer.add(lastFontData->zeroWidthSpaceGlyph(), *lastFontData, leftoverJustificationWidth, m_run.length() - 1);
+auto WidthIterator::calculateAdditionalWidth(GlyphBuffer& glyphBuffer, GlyphBufferStringOffset currentCharacterIndex, unsigned leadingGlyphIndex, unsigned trailingGlyphIndex, float position) const -> AdditionalWidth
+{
+ float leftAdditionalWidth = 0;
+ float rightAdditionalWidth = 0;
+ float leftExpansionAdditionalWidth = 0;
+ float rightExpansionAdditionalWidth = 0;
+
+ auto character = m_run[currentCharacterIndex];
+
+ if (character == tabCharacter && m_run.allowTabs()) {
+ auto& font = glyphBuffer.fontAt(trailingGlyphIndex);
+ auto newWidth = m_font.tabWidth(font, m_run.tabSize(), position);
+ auto currentWidth = glyphBuffer.advanceAt(trailingGlyphIndex).width();
+ rightAdditionalWidth += newWidth - currentWidth;
+ }
+
+ if (hasExtraSpacing()) {
+ bool treatAsSpace = FontCascade::treatAsSpace(character);
+
+ // This is a heuristic to determine if the character is non-visible. Non-visible characters don't get letter-spacing.
+ float baseWidth = 0;
+ for (unsigned i = leadingGlyphIndex; i <= trailingGlyphIndex; ++i)
+ baseWidth += glyphBuffer.advanceAt(i).width();
+ if (baseWidth)
+ rightAdditionalWidth += m_font.letterSpacing();
+
+ if (treatAsSpace && (character != tabCharacter || !m_run.allowTabs()) && (currentCharacterIndex || character == noBreakSpace) && m_font.wordSpacing())
+ rightAdditionalWidth += m_font.wordSpacing();
+
+ if (m_expansion > 0) {
+ bool currentIsLastCharacter = m_lastCharacterIndex && currentCharacterIndex == static_cast<GlyphBufferStringOffset>(*m_lastCharacterIndex);
+
+ bool isLeftmostCharacter = !currentCharacterIndex;
+ bool isRightmostCharacter = currentIsLastCharacter;
+ if (!m_run.ltr())
+ std::swap(isLeftmostCharacter, isRightmostCharacter);
+
+ bool forceLeftExpansion = isLeftmostCharacter && (m_run.expansionBehavior() & LeftExpansionMask) == ForceLeftExpansion;
+ bool forceRightExpansion = isRightmostCharacter && (m_run.expansionBehavior() & RightExpansionMask) == ForceRightExpansion;
+ bool forbidLeftExpansion = isLeftmostCharacter && (m_run.expansionBehavior() & LeftExpansionMask) == ForbidLeftExpansion;
+ bool forbidRightExpansion = isRightmostCharacter && (m_run.expansionBehavior() & RightExpansionMask) == ForbidRightExpansion;
+
+ static const bool expandAroundIdeographs = FontCascade::canExpandAroundIdeographsInComplexText();
+ bool isIdeograph = expandAroundIdeographs && FontCascade::isCJKIdeographOrSymbol(character);
+
+ if (treatAsSpace || isIdeograph || forceLeftExpansion || forceRightExpansion) {
+ auto [expandLeft, expandRight] = expansionLocation(isIdeograph, treatAsSpace, m_run.ltr(), m_isAfterExpansion, forbidLeftExpansion, forbidRightExpansion, forceLeftExpansion, forceRightExpansion);
+
+ if (expandLeft)
+ leftExpansionAdditionalWidth += m_expansionPerOpportunity;
+ if (expandRight)
+ rightExpansionAdditionalWidth += m_expansionPerOpportunity;
+ }
+ }
+ }
+
+ return { leftAdditionalWidth, rightAdditionalWidth, leftExpansionAdditionalWidth, rightExpansionAdditionalWidth };
+}
+
+struct GlyphIndexRange {
+ // This means the character got expanded to glyphs inside the GlyphBuffer at indices [leadingGlyphIndex, trailingGlyphIndex].
+ unsigned leadingGlyphIndex;
+ unsigned trailingGlyphIndex;
+};
+
+void WidthIterator::applyAdditionalWidth(GlyphBuffer& glyphBuffer, GlyphIndexRange glyphIndexRange, float leftAdditionalWidth, float rightAdditionalWidth, float leftExpansionAdditionalWidth, float rightExpansionAdditionalWidth)
+{
+ m_expansion -= leftExpansionAdditionalWidth + rightExpansionAdditionalWidth;
+
+ leftAdditionalWidth += leftExpansionAdditionalWidth;
+ rightAdditionalWidth += rightExpansionAdditionalWidth;
+
+ m_runWidthSoFar += leftAdditionalWidth;
+ m_runWidthSoFar += rightAdditionalWidth;
+
+ if (leftAdditionalWidth) {
+ if (m_run.ltr()) {
+ // Left additional width in LTR means the previous (leading) glyph's right side gets expanded.
+ auto leadingGlyphIndex = glyphIndexRange.leadingGlyphIndex;
+ if (leadingGlyphIndex)
+ glyphBuffer.expandAdvance(leadingGlyphIndex - 1, leftAdditionalWidth);
+ else
+ glyphBuffer.expandInitialAdvance(leftAdditionalWidth);
+ } else {
+ // Left additional width in RTL means the next (trailing) glyph's right side gets expanded.
+ auto trailingGlyphIndex = glyphIndexRange.trailingGlyphIndex;
+ if (trailingGlyphIndex + 1 < glyphBuffer.size())
+ glyphBuffer.expandAdvance(trailingGlyphIndex + 1, leftAdditionalWidth);
+ else {
+ m_leftoverJustificationWidth = leftAdditionalWidth;
+ // We can't actually add in this width just yet.
+ // Add it in when the client calls advance() again or finalize().
+ m_runWidthSoFar -= m_leftoverJustificationWidth;
+ }
+ }
+ }
+
+ if (rightAdditionalWidth) {
+ // Right additional width means the current glyph's right side gets expanded. This is true for both LTR and RTL.
+ glyphBuffer.expandAdvance(glyphIndexRange.trailingGlyphIndex, rightAdditionalWidth);
+ }
+}
+
+void WidthIterator::applyExtraSpacingAfterShaping(GlyphBuffer& glyphBuffer, unsigned characterStartIndex, unsigned glyphBufferStartIndex, unsigned characterDestinationIndex, float startingRunWidth)
+{
+ Vector<Optional<GlyphIndexRange>> characterIndexToGlyphIndexRange(m_run.length(), WTF::nullopt);
+ Vector<float> advanceWidths(m_run.length(), 0);
+ for (unsigned i = glyphBufferStartIndex; i < glyphBuffer.size(); ++i) {
+ auto stringOffset = glyphBuffer.stringOffsetAt(i);
+ advanceWidths[stringOffset] += glyphBuffer.advanceAt(i).width();
+ auto& glyphIndexRange = characterIndexToGlyphIndexRange[stringOffset];
+ if (glyphIndexRange)
+ glyphIndexRange->trailingGlyphIndex = i;
else
- glyphBuffer.add(lastFontData->spaceGlyph(), *lastFontData, leftoverJustificationWidth, m_run.length() - 1);
+ glyphIndexRange = {{i, i}};
}
+
+ // SVG can stretch advances
+ if (m_run.horizontalGlyphStretch() != 1) {
+ for (unsigned i = glyphBufferStartIndex; i < glyphBuffer.size(); ++i) {
+ // All characters' advances get stretched, except apparently tab characters...
+ // This doesn't make much sense, because even tab characters get letter-spacing...
+ auto stringOffset = glyphBuffer.stringOffsetAt(i);
+ auto character = m_run[stringOffset];
+ if (character == tabCharacter)
+ continue;
+
+ auto currentAdvance = glyphBuffer.advanceAt(i).width();
+ auto newAdvance = currentAdvance * m_run.horizontalGlyphStretch();
+ glyphBuffer.expandAdvance(i, newAdvance - currentAdvance);
+ }
+ }
+
+ float position = m_run.xPos() + startingRunWidth;
+ for (auto i = characterStartIndex; i < characterDestinationIndex; ++i) {
+ auto& glyphIndexRange = characterIndexToGlyphIndexRange[i];
+ if (!glyphIndexRange)
+ continue;
+
+ auto width = calculateAdditionalWidth(glyphBuffer, i, glyphIndexRange->leadingGlyphIndex, glyphIndexRange->trailingGlyphIndex, position);
+ applyAdditionalWidth(glyphBuffer, glyphIndexRange.value(), width.left, width.right, width.leftExpansion, width.rightExpansion);
+
+ m_isAfterExpansion = (m_run.ltr() && width.rightExpansion) || (!m_run.ltr() && width.leftExpansion);
+
+ // This isn't quite perfect, because we may come across a tab character in between two glyphs which both report to correspond to a previous character.
+ // But, the fundamental concept of tabs isn't really compatible with complex text shaping, so this is probably okay.
+ // We can probably just do the best we can here.
+ // The only alternative, to calculate this position in glyph-space rather than character-space,
+ // is O(n^2) because we're iterating across the string here, rather than glyphs, so we can't keep the calculation up-to-date,
+ // which means calculateAdditionalWidth() would have to calculate the result from scratch whenever it's needed.
+ // And we can't do some sort of prefix-sum thing because applyAdditionalWidth() would modify the values,
+ // so updating the data structure each turn of this loop would also end up being O(n^2).
+ // Unfortunately, strings with tabs are more likely to be long data-table kind of strings, which means O(n^2) is not acceptable.
+ // Also, even if we did the O(n^2) thing, there would still be cases that wouldn't be perfect
+ // (because the fundamental concept of tabs isn't really compatible with complex text shaping),
+ // so let's choose the fast-wrong approach here instead of the slow-wrong approach.
+ position += advanceWidths[i]
+ + width.left
+ + width.right
+ + width.leftExpansion
+ + width.rightExpansion;
+ }
}
+void WidthIterator::finalize(GlyphBuffer& buffer)
+{
+ ASSERT(m_run.rtl() || !m_leftoverJustificationWidth);
+ // In LTR this does nothing. In RTL, this adds left width by moving the whole run to the right.
+ buffer.expandInitialAdvance(m_leftoverJustificationWidth);
+ m_runWidthSoFar += m_leftoverJustificationWidth;
+ m_leftoverJustificationWidth = 0;
+}
+
void WidthIterator::advance(unsigned offset, GlyphBuffer& glyphBuffer)
{
+ m_containsTabs = false;
unsigned length = m_run.length();
if (offset > length)
@@ -366,14 +463,28 @@
if (m_currentCharacterIndex >= offset)
return;
+ unsigned characterStartIndex = m_currentCharacterIndex;
+ unsigned glyphBufferStartIndex = glyphBuffer.size();
+ float startingRunWidth = m_runWidthSoFar;
+
if (m_run.is8Bit()) {
Latin1TextIterator textIterator(m_run.data8(m_currentCharacterIndex), m_currentCharacterIndex, offset, length);
advanceInternal(textIterator, glyphBuffer);
- return;
+ } else {
+ SurrogatePairAwareTextIterator textIterator(m_run.data16(m_currentCharacterIndex), m_currentCharacterIndex, offset, length);
+ advanceInternal(textIterator, glyphBuffer);
}
- SurrogatePairAwareTextIterator textIterator(m_run.data16(m_currentCharacterIndex), m_currentCharacterIndex, offset, length);
- advanceInternal(textIterator, glyphBuffer);
+ // In general, we have to apply spacing after shaping, because shaping requires its input to be unperturbed (see https://bugs.webkit.org/show_bug.cgi?id=215052).
+ // So, if there's extra spacing to add, do it here after shaping occurs.
+ if (glyphBufferStartIndex < glyphBuffer.size()) {
+ glyphBuffer.expandAdvance(glyphBufferStartIndex, m_leftoverJustificationWidth);
+ m_runWidthSoFar += m_leftoverJustificationWidth;
+ m_leftoverJustificationWidth = 0;
+ }
+
+ if (hasExtraSpacing() || m_containsTabs || m_run.horizontalGlyphStretch() != 1)
+ applyExtraSpacingAfterShaping(glyphBuffer, characterStartIndex, glyphBufferStartIndex, offset, startingRunWidth);
}
bool WidthIterator::advanceOneCharacter(float& width, GlyphBuffer& glyphBuffer)
Modified: trunk/Source/WebCore/platform/graphics/WidthIterator.h (265487 => 265488)
--- trunk/Source/WebCore/platform/graphics/WidthIterator.h 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/platform/graphics/WidthIterator.h 2020-08-11 05:17:04 UTC (rev 265488)
@@ -22,6 +22,7 @@
#ifndef WidthIterator_h
#define WidthIterator_h
+#include "GlyphBuffer.h"
#include <unicode/umachine.h>
#include <wtf/HashSet.h>
#include <wtf/Vector.h>
@@ -29,10 +30,10 @@
namespace WebCore {
class FontCascade;
-class GlyphBuffer;
class Font;
class TextRun;
struct GlyphData;
+struct GlyphIndexRange;
struct OriginalAdvancesForCharacterTreatedAsSpace;
using CharactersTreatedAsSpace = Vector<OriginalAdvancesForCharacterTreatedAsSpace, 64>;
@@ -44,6 +45,7 @@
void advance(unsigned to, GlyphBuffer&);
bool advanceOneCharacter(float& width, GlyphBuffer&);
+ void finalize(GlyphBuffer&);
float maxGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_maxGlyphBoundingBoxY; }
float minGlyphBoundingBoxY() const { ASSERT(m_accountForGlyphBounds); return m_minGlyphBoundingBoxY; }
@@ -64,11 +66,24 @@
float applyFontTransforms(GlyphBuffer&, unsigned lastGlyphCount, unsigned currentCharacterIndex, const Font&, bool force, CharactersTreatedAsSpace&);
void commitCurrentFontRange(GlyphBuffer&, unsigned lastGlyphCount, unsigned currentCharacterIndex, const Font&, const Font& primaryFont, UChar32 character, float widthOfCurrentFontRange, CharactersTreatedAsSpace&);
+ bool hasExtraSpacing() const;
+ void applyExtraSpacingAfterShaping(GlyphBuffer&, unsigned characterStartIndex, unsigned glyphBufferStartIndex, unsigned characterDestinationIndex, float startingRunWidth);
+ struct AdditionalWidth {
+ float left;
+ float right;
+ float leftExpansion;
+ float rightExpansion;
+ };
+ AdditionalWidth calculateAdditionalWidth(GlyphBuffer&, GlyphBufferStringOffset currentCharacterIndex, unsigned leadingGlyphIndex, unsigned trailingGlyphIndex, float position) const;
+ void applyAdditionalWidth(GlyphBuffer&, GlyphIndexRange, float leftAdditionalWidth, float rightAdditionalWidth, float leftExpansionAdditionalWidth, float rightExpansionAdditionalWidth);
+
const FontCascade& m_font;
const TextRun& m_run;
HashSet<const Font*>* m_fallbackFonts { nullptr };
+ Optional<unsigned> m_lastCharacterIndex;
unsigned m_currentCharacterIndex { 0 };
+ float m_leftoverJustificationWidth { 0 };
float m_runWidthSoFar { 0 };
float m_expansion { 0 };
float m_expansionPerOpportunity { 0 };
@@ -76,6 +91,7 @@
float m_minGlyphBoundingBoxY { std::numeric_limits<float>::max() };
float m_firstGlyphOverflow { 0 };
float m_lastGlyphOverflow { 0 };
+ bool m_containsTabs { false };
bool m_isAfterExpansion { false };
bool m_accountForGlyphBounds { false };
bool m_enableKerning { false };
Modified: trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp (265487 => 265488)
--- trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp 2020-08-11 04:23:51 UTC (rev 265487)
+++ trunk/Source/WebCore/rendering/svg/SVGTextMetricsBuilder.cpp 2020-08-11 05:17:04 UTC (rev 265488)
@@ -165,6 +165,11 @@
data->lastCharacter = currentCharacter;
}
+ if (m_simpleWidthIterator) {
+ GlyphBuffer glyphBuffer;
+ m_simpleWidthIterator->finalize(glyphBuffer);
+ }
+
if (!data->allCharactersMap)
return;
