Changeset 224223 in webkit


Ignore:
Timestamp:
Oct 31, 2017 12:10:50 AM (6 years ago)
Author:
Carlos Garcia Campos
Message:

[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().
Location:
trunk/Source/WebCore
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r224218 r224223  
     12017-10-31  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [FreeType] Simple and complex paths are not applied consistently
     4        https://bugs.webkit.org/show_bug.cgi?id=177601
     5
     6        Reviewed by Michael Catanzaro.
     7
     8        Due to bug #100050, when rendering text, the complex path is forced in case kerning or shaping is enabled and
     9        only part of the run is going to be rendered. This happens in the GTK+ port when selecting text (except when
     10        selecting the whole run, of course). The text is initially rendered using the simple path as returned by
     11        FontCascade::codePath() and then the selection is rendered using the complex path, overriding what
     12        FontCascade::codePath() returned in that case. This doesn't happen in mac, because the selection is rendered
     13        differently, so FontCascade::drawText always renders the full run (simple path) when selecting text. Selecting
     14        text is the most noticeable inconsistency, but it's not the only one. Similar exceptions are applied when
     15        calculating the text width, or getting the offset of a given position. The rendered text is the simple one, but
     16        the calculations are performed using the complex path, so depending on the kerning and ligatures we might end up
     17        with wrong results. If the text has been rendered using the simple path, the selections and all other
     18        calculations should be performed with the simple path too. This patch moves the condition to force complex text
     19        to FontCascade::codePath(), and only for non Freetype ports. This ensures that all callers to
     20        FontCascade::codePath() will get a consistent result.
     21
     22        * platform/graphics/FontCascade.cpp:
     23        (WebCore::FontCascade::drawText const): Use the mode returned by codePath().
     24        (WebCore::FontCascade::drawEmphasisMarks const): Ditto.
     25        (WebCore::FontCascade::adjustSelectionRectForText const): Use the mode returned by codePath().
     26        (WebCore::FontCascade::offsetForPosition const): Ditto.
     27        (WebCore::FontCascade::codePath const): Force complex text for partial runs for ports not enabling advance text
     28        rendering mode by default.
     29        * platform/graphics/FontCascade.h: Add to and from optional parameters to codePath().
     30
    1312017-10-30  Chris Dumez  <cdumez@apple.com>
    232

  • trunk/Source/WebCore/platform/graphics/FontCascade.cpp

    r223476 r224223  
    283283{
    284284    unsigned destination = to.value_or(run.length());
    285 
    286     CodePath codePathToUse = codePath(run);
    287     // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
    288     if (codePathToUse != Complex && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
    289         codePathToUse = Complex;
    290 
    291285    GlyphBuffer glyphBuffer;
    292     float startX = point.x() + glyphBufferForTextRun(codePathToUse, run, from, destination, glyphBuffer);
     286    float startX = point.x() + glyphBufferForTextRun(codePath(run, from, to), run, from, destination, glyphBuffer);
    293287    // We couldn't generate any glyphs for the run. Give up.
    294288    if (glyphBuffer.isEmpty())
    … …  
    306300
    307301    unsigned destination = to.value_or(run.length());
    308 
    309     CodePath codePathToUse = codePath(run);
    310     // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
    311     if (codePathToUse != Complex && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
    312         codePathToUse = Complex;
    313 
    314     if (codePathToUse != Complex)
     302    if (codePath(run, from, to) != Complex)
    315303        drawEmphasisMarksForSimpleText(context, run, mark, point, from, destination);
    316304    else
    … …  
    512500{
    513501    unsigned destination = to.value_or(run.length());
    514 
    515     CodePath codePathToUse = codePath(run);
    516     // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
    517     if (codePathToUse != Complex && (enableKerning() || requiresShaping()) && (from || destination != run.length()))
    518         codePathToUse = Complex;
    519 
    520     if (codePathToUse != Complex)
     502    if (codePath(run, from, to) != Complex)
    521503        return adjustSelectionRectForSimpleText(run, selectionRect, from, destination);
    522504
    … …  
    526508int FontCascade::offsetForPosition(const TextRun& run, float x, bool includePartialGlyphs) const
    527509{
    528     // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
    529     if (codePath(run) != Complex && (!(enableKerning() || requiresShaping())))
     510    if (codePath(run, x) != Complex)
    530511        return offsetForPositionForSimpleText(run, x, includePartialGlyphs);
    531512
    … …  
    578559}
    579560
    580 FontCascade::CodePath FontCascade::codePath(const TextRun& run) const
     561FontCascade::CodePath FontCascade::codePath(const TextRun& run, std::optional<unsigned> from, std::optional<unsigned> to) const
    581562{
    582563    if (s_codePath != Auto)
    583564        return s_codePath;
     565
     566#if !USE(FREETYPE)
     567    // FIXME: Use the fast code path once it handles partial runs with kerning and ligatures. See http://webkit.org/b/100050
     568    if ((enableKerning() || requiresShaping()) && (from.value_or(0) || to.value_or(run.length()) != run.length()))
     569        return Complex;
     570#else
     571    UNUSED_PARAM(from);
     572    UNUSED_PARAM(to);
     573#endif
    584574
    585575#if PLATFORM(COCOA) || USE(FREETYPE)

  • trunk/Source/WebCore/platform/graphics/FontCascade.h

    r223728 r224223  
    190190
    191191    enum CodePath { Auto, Simple, Complex, SimpleWithGlyphOverflow };
    192     CodePath codePath(const TextRun&) const;
     192    CodePath codePath(const TextRun&, std::optional<unsigned> from = std::nullopt, std::optional<unsigned> to = std::nullopt) const;
    193193    static CodePath characterRangeCodePath(const LChar*, unsigned) { return Simple; }
    194194    static CodePath characterRangeCodePath(const UChar*, unsigned len);
Note: See TracChangeset for help on using the changeset viewer.