Skip to content
Permalink
Browse files
[FreeType] Simple and complex paths are not applied consistently
https://bugs.webkit.org/show_bug.cgi?id=177601

Reviewed by Michael Catanzaro.

Due to bug #100050, when rendering text, the complex path is forced in case kerning or shaping is enabled and
only part of the run is going to be rendered. This happens in the GTK+ port when selecting text (except when
selecting the whole run, of course). The text is initially rendered using the simple path as returned by
FontCascade::codePath() and then the selection is rendered using the complex path, overriding what
FontCascade::codePath() returned in that case. This doesn't happen in mac, because the selection is rendered
differently, so FontCascade::drawText always renders the full run (simple path) when selecting text. Selecting
text is the most noticeable inconsistency, but it's not the only one. Similar exceptions are applied when
calculating the text width, or getting the offset of a given position. The rendered text is the simple one, but
the calculations are performed using the complex path, so depending on the kerning and ligatures we might end up
with wrong results. If the text has been rendered using the simple path, the selections and all other
calculations should be performed with the simple path too. This patch moves the condition to force complex text
to FontCascade::codePath(), and only for non Freetype ports. This ensures that all callers to
FontCascade::codePath() will get a consistent result.

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::drawText const): Use the mode returned by codePath().
(WebCore::FontCascade::drawEmphasisMarks const): Ditto.
(WebCore::FontCascade::adjustSelectionRectForText const): Use the mode returned by codePath().
(WebCore::FontCascade::offsetForPosition const): Ditto.
(WebCore::FontCascade::codePath const): Force complex text for partial runs for ports not enabling advance text
rendering mode by default.
* platform/graphics/FontCascade.h: Add to and from optional parameters to codePath().

Canonical link: https://commits.webkit.org/195180@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@224223 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
carlosgcampos committed Oct 31, 2017
1 parent 1e99c66 commit 16c87e91c709eed25781d4d3749a0edfeac5fab5
Showing 3 changed files with 45 additions and 25 deletions.
@@ -1,3 +1,33 @@
2017-10-31 Carlos Garcia Campos <cgarcia@igalia.com>

[FreeType] Simple and complex paths are not applied consistently
https://bugs.webkit.org/show_bug.cgi?id=177601

Reviewed by Michael Catanzaro.

Due to bug #100050, when rendering text, the complex path is forced in case kerning or shaping is enabled and
only part of the run is going to be rendered. This happens in the GTK+ port when selecting text (except when
selecting the whole run, of course). The text is initially rendered using the simple path as returned by
FontCascade::codePath() and then the selection is rendered using the complex path, overriding what
FontCascade::codePath() returned in that case. This doesn't happen in mac, because the selection is rendered
differently, so FontCascade::drawText always renders the full run (simple path) when selecting text. Selecting
text is the most noticeable inconsistency, but it's not the only one. Similar exceptions are applied when
calculating the text width, or getting the offset of a given position. The rendered text is the simple one, but
the calculations are performed using the complex path, so depending on the kerning and ligatures we might end up
with wrong results. If the text has been rendered using the simple path, the selections and all other
calculations should be performed with the simple path too. This patch moves the condition to force complex text
to FontCascade::codePath(), and only for non Freetype ports. This ensures that all callers to
FontCascade::codePath() will get a consistent result.

* platform/graphics/FontCascade.cpp:
(WebCore::FontCascade::drawText const): Use the mode returned by codePath().
(WebCore::FontCascade::drawEmphasisMarks const): Ditto.
(WebCore::FontCascade::adjustSelectionRectForText const): Use the mode returned by codePath().
(WebCore::FontCascade::offsetForPosition const): Ditto.
(WebCore::FontCascade::codePath const): Force complex text for partial runs for ports not enabling advance text
rendering mode by default.
* platform/graphics/FontCascade.h: Add to and from optional parameters to codePath().

2017-10-30 Chris Dumez <cdumez@apple.com>

Fire updatefound event after resolving the registration promise
@@ -282,14 +282,8 @@ float FontCascade::glyphBufferForTextRun(CodePath codePathToUse, const TextRun&
float FontCascade::drawText(GraphicsContext& context, const TextRun& run, const FloatPoint& point, unsigned from, std::optional<unsigned> to, CustomFontNotReadyAction customFontNotReadyAction) const
{
unsigned destination = to.value_or(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 && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
codePathToUse = Complex;

GlyphBuffer glyphBuffer;
float startX = point.x() + glyphBufferForTextRun(codePathToUse, run, from, destination, glyphBuffer);
float startX = point.x() + glyphBufferForTextRun(codePath(run, from, to), run, from, destination, glyphBuffer);
// We couldn't generate any glyphs for the run. Give up.
if (glyphBuffer.isEmpty())
return 0;
@@ -305,13 +299,7 @@ void FontCascade::drawEmphasisMarks(GraphicsContext& context, const TextRun& run
return;

unsigned destination = to.value_or(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 && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
codePathToUse = Complex;

if (codePathToUse != Complex)
if (codePath(run, from, to) != Complex)
drawEmphasisMarksForSimpleText(context, run, mark, point, from, destination);
else
drawEmphasisMarksForComplexText(context, run, mark, point, from, destination);
@@ -511,22 +499,15 @@ bool FontCascade::fastAverageCharWidthIfAvailable(float& width) const
void FontCascade::adjustSelectionRectForText(const TextRun& run, LayoutRect& selectionRect, unsigned from, std::optional<unsigned> to) const
{
unsigned destination = to.value_or(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 && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
codePathToUse = Complex;

if (codePathToUse != Complex)
if (codePath(run, from, to) != Complex)
return adjustSelectionRectForSimpleText(run, selectionRect, from, destination);

return adjustSelectionRectForComplexText(run, selectionRect, from, destination);
}

int FontCascade::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 && (!(enableKerning() || requiresShaping())))
if (codePath(run, x) != Complex)
return offsetForPositionForSimpleText(run, x, includePartialGlyphs);

return offsetForPositionForComplexText(run, x, includePartialGlyphs);
@@ -577,11 +558,20 @@ FontCascade::CodePath FontCascade::codePath()
return s_codePath;
}

FontCascade::CodePath FontCascade::codePath(const TextRun& run) const
FontCascade::CodePath FontCascade::codePath(const TextRun& run, std::optional<unsigned> from, std::optional<unsigned> to) const
{
if (s_codePath != Auto)
return s_codePath;

#if !USE(FREETYPE)
// FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
if ((enableKerning() || requiresShaping()) && (from.value_or(0) || to.value_or(run.length()) != run.length()))
return Complex;
#else
UNUSED_PARAM(from);
UNUSED_PARAM(to);
#endif

#if PLATFORM(COCOA) || USE(FREETYPE)
// Because Font::applyTransforms() doesn't know which features to enable/disable in the simple code path, it can't properly handle feature or variant settings.
// FIXME: https://bugs.webkit.org/show_bug.cgi?id=150791: @font-face features should also cause this to be complex.
@@ -189,7 +189,7 @@ class FontCascade {
WEBCORE_EXPORT static bool shouldUseSmoothing();

enum CodePath { Auto, Simple, Complex, SimpleWithGlyphOverflow };
CodePath codePath(const TextRun&) const;
CodePath codePath(const TextRun&, std::optional<unsigned> from = std::nullopt, std::optional<unsigned> to = std::nullopt) const;
static CodePath characterRangeCodePath(const LChar*, unsigned) { return Simple; }
static CodePath characterRangeCodePath(const UChar*, unsigned len);

0 comments on commit 16c87e9

Please sign in to comment.