Changeset 268561 in webkit


Ignore:
Timestamp:
Oct 15, 2020 4:51:06 PM (3 years ago)
Author:
mmaxfield@apple.com
Message:

[Cocoa] Don't change the text matrix multiple times inside FontCascade::drawGlyphs()
https://bugs.webkit.org/show_bug.cgi?id=217749

Reviewed by Simon Fraser.

FontCascade::drawGlyphs() may actually call CTFontDrawGlyphs() 4 times:
2 for synthetic bold, times 2 for emulated shadows that we draw ourselves.
Previously, each of these calls can have a different text matrix, because
showGlyphsWithAdvances() called CGContextSetTextPosition(), which affects
the text matrix.

This makes https://bugs.webkit.org/show_bug.cgi?id=217506 difficult, because
we don't actually track the text matrix inside the GraphicsContext. In that
patch, we call FontCascade::drawGlyphs() and intercept the resulting CGContext
calls. However, if we detect a modified text matrix inside those CGContext
calls, we have no way of knowing whether it was modified by WebKit or
internally inside Core Text. We need to know this, though, because the
modifications Core Text makes have to be preserved across to the GPU process,
but the modifications WebKit makes need to not be preserved (since the GPU
process will make them naturally).

Tracking the text matrix in the CGContext (a la the CTM) is unnecessary; all
we need to know is whether WebKit changed it or Core Text changed it. Therefore,
this patch migrates all the text matrix computation into standalone functions
that can be called from GPU Process infrastructure, so we can know what WebKit
does there (and also what Core Text does, by omission).

This only works, though, if the text matrix is identical for all the calls to
CTFontDrawGlyphs() inside FontCascade::drawGlyphs(). This patch enforces this
by emulating CGContextSetTextPosition() ourselves by adding a constant to
all the glyph positions. Performance is not a conern because we already iterate
over all the glyphs inside showGlyphsWithAdvances(), so we're already paying
the cost of the loop.

No new tests because there is no behavior change.

  • platform/graphics/FontCascade.h:

(WebCore::FontCascade::syntheticObliqueAngle): Deleted.

  • platform/graphics/cocoa/FontCascadeCocoa.mm:

(WebCore::showLetterpressedGlyphsWithAdvances):

  • platform/graphics/coretext/FontCascadeCoreText.cpp:

(WebCore::rotateLeftTransform):
(WebCore::fillVectorWithHorizontalGlyphPositions):
(WebCore::fillVectorWithVerticalGlyphPositions):
(WebCore::showGlyphsWithAdvances):
(WebCore::computeOverallTextMatrix):
(WebCore::computeVerticalTextMatrix):
(WebCore::FontCascade::drawGlyphs):

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r268560 r268561  
     12020-10-15  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        [Cocoa] Don't change the text matrix multiple times inside FontCascade::drawGlyphs()
     4        https://bugs.webkit.org/show_bug.cgi?id=217749
     5
     6        Reviewed by Simon Fraser.
     7
     8        FontCascade::drawGlyphs() may actually call CTFontDrawGlyphs() 4 times:
     9        2 for synthetic bold, times 2 for emulated shadows that we draw ourselves.
     10        Previously, each of these calls can have a different text matrix, because
     11        showGlyphsWithAdvances() called CGContextSetTextPosition(), which affects
     12        the text matrix.
     13
     14        This makes https://bugs.webkit.org/show_bug.cgi?id=217506 difficult, because
     15        we don't actually track the text matrix inside the GraphicsContext. In that
     16        patch, we call FontCascade::drawGlyphs() and intercept the resulting CGContext
     17        calls. However, if we detect a modified text matrix inside those CGContext
     18        calls, we have no way of knowing whether it was modified by WebKit or
     19        internally inside Core Text. We need to know this, though, because the
     20        modifications Core Text makes have to be preserved across to the GPU process,
     21        but the modifications WebKit makes need to not be preserved (since the GPU
     22        process will make them naturally).
     23
     24        Tracking the text matrix in the CGContext (a la the CTM) is unnecessary; all
     25        we need to know is whether WebKit changed it or Core Text changed it. Therefore,
     26        this patch migrates all the text matrix computation into standalone functions
     27        that can be called from GPU Process infrastructure, so we can know what WebKit
     28        does there (and also what Core Text does, by omission).
     29
     30        This only works, though, if the text matrix is identical for all the calls to
     31        CTFontDrawGlyphs() inside FontCascade::drawGlyphs(). This patch enforces this
     32        by emulating CGContextSetTextPosition() ourselves by adding a constant to
     33        all the glyph positions. Performance is not a conern because we already iterate
     34        over all the glyphs inside showGlyphsWithAdvances(), so we're already paying
     35        the cost of the loop.
     36
     37        No new tests because there is no behavior change.
     38
     39        * platform/graphics/FontCascade.h:
     40        (WebCore::FontCascade::syntheticObliqueAngle): Deleted.
     41        * platform/graphics/cocoa/FontCascadeCocoa.mm:
     42        (WebCore::showLetterpressedGlyphsWithAdvances):
     43        * platform/graphics/coretext/FontCascadeCoreText.cpp:
     44        (WebCore::rotateLeftTransform):
     45        (WebCore::fillVectorWithHorizontalGlyphPositions):
     46        (WebCore::fillVectorWithVerticalGlyphPositions):
     47        (WebCore::showGlyphsWithAdvances):
     48        (WebCore::computeOverallTextMatrix):
     49        (WebCore::computeVerticalTextMatrix):
     50        (WebCore::FontCascade::drawGlyphs):
     51
    1522020-10-15  Chris Dumez  <cdumez@apple.com>
    253
  • trunk/Source/WebCore/platform/graphics/FontCascade.h

    r266936 r268561  
    8484#if USE(CORE_TEXT)
    8585void showLetterpressedGlyphsWithAdvances(const FloatPoint&, const Font&, GraphicsContext&, const CGGlyph*, const CGSize* advances, unsigned count);
    86 void fillVectorWithHorizontalGlyphPositions(Vector<CGPoint, 256>& positions, CGContextRef, const CGSize* advances, unsigned count);
     86void fillVectorWithHorizontalGlyphPositions(Vector<CGPoint, 256>& positions, CGContextRef, const CGSize* advances, unsigned count, const FloatPoint&);
     87AffineTransform computeOverallTextMatrix(const Font&);
     88AffineTransform computeVerticalTextMatrix(const Font&, const AffineTransform& previousTextMatrix);
    8789#endif
    8890
     
    194196
    195197    bool primaryFontIsSystemFont() const;
     198
     199    static int syntheticObliqueAngle() { return 14; }
    196200
    197201    std::unique_ptr<DisplayList::DisplayList> displayListForTextRun(GraphicsContext&, const TextRun&, unsigned from = 0, Optional<unsigned> to = { }, CustomFontNotReadyAction = CustomFontNotReadyAction::DoNotPaintIfFontNotReady) const;
     
    308312    }
    309313
    310     static int syntheticObliqueAngle() { return 14; }
    311 
    312314#if PLATFORM(WIN) && USE(CG)
    313315    static double s_fontSmoothingContrast;
  • trunk/Source/WebCore/platform/graphics/cocoa/FontCascadeCocoa.mm

    r266936 r268561  
    6363    CGContextRef context = coreContext.platformContext();
    6464
    65     CGContextSetTextPosition(context, point.x(), point.y());
    6665    Vector<CGPoint, 256> positions(count);
    67     fillVectorWithHorizontalGlyphPositions(positions, context, advances, count);
     66    fillVectorWithHorizontalGlyphPositions(positions, context, advances, count, point);
    6867
    6968    CTFontRef ctFont = platformData.ctFont();
  • trunk/Source/WebCore/platform/graphics/coretext/FontCascadeCoreText.cpp

    r267116 r268561  
    6262}
    6363
    64 void fillVectorWithHorizontalGlyphPositions(Vector<CGPoint, 256>& positions, CGContextRef context, const CGSize* advances, unsigned count)
     64static const AffineTransform& rotateLeftTransform()
     65{
     66    static AffineTransform result(0, -1, 1, 0, 0, 0);
     67    return result;
     68}
     69
     70void fillVectorWithHorizontalGlyphPositions(Vector<CGPoint, 256>& positions, CGContextRef context, const CGSize* advances, unsigned count, const FloatPoint& point)
    6571{
    6672    CGAffineTransform matrix = CGAffineTransformInvert(CGContextGetTextMatrix(context));
    67     positions[0] = CGPointZero;
     73    positions[0] = CGPointApplyAffineTransform(point, matrix);
    6874    for (unsigned i = 1; i < count; ++i) {
    6975        CGSize advance = CGSizeApplyAffineTransform(advances[i - 1], matrix);
    7076        positions[i].x = positions[i - 1].x + advance.width;
    7177        positions[i].y = positions[i - 1].y + advance.height;
     78    }
     79}
     80
     81static void fillVectorWithVerticalGlyphPositions(Vector<CGPoint, 256>& positions, CGContextRef context, const CGSize* translations, const CGSize* advances, unsigned count, const FloatPoint& point, float ascentDelta)
     82{
     83    CGAffineTransform transform = CGAffineTransformInvert(CGContextGetTextMatrix(context));
     84
     85    auto position = CGPointMake(point.x(), point.y() + ascentDelta);
     86    for (unsigned i = 0; i < count; ++i) {
     87        CGSize translation = CGSizeApplyAffineTransform(translations[i], rotateLeftTransform());
     88        positions[i] = CGPointApplyAffineTransform(CGPointMake(position.x - translation.width, position.y + translation.height), transform);
     89        position.x += advances[i].width;
     90        position.y += advances[i].height;
    7291    }
    7392}
     
    83102}
    84103
    85 static void showGlyphsWithAdvances(const FloatPoint& point, const Font& font, CGContextRef context, const CGGlyph* glyphs, const CGSize* advances, unsigned count)
     104static void showGlyphsWithAdvances(const FloatPoint& point, const Font& font, CGContextRef context, const CGGlyph* glyphs, const CGSize* advances, unsigned count, const AffineTransform& textMatrix)
    86105{
    87106    if (!count)
    88107        return;
    89 
    90     CGContextSetTextPosition(context, point.x(), point.y());
    91108
    92109    const FontPlatformData& platformData = font.platformData();
    93110    Vector<CGPoint, 256> positions(count);
    94111    if (platformData.orientation() == FontOrientation::Vertical) {
    95         CGAffineTransform rotateLeftTransform = CGAffineTransformMake(0, -1, 1, 0, 0, 0);
    96         CGAffineTransform textMatrix = CGContextGetTextMatrix(context);
    97         CGAffineTransform runMatrix = CGAffineTransformConcat(textMatrix, rotateLeftTransform);
    98         ScopedTextMatrix savedMatrix(runMatrix, context);
     112        ScopedTextMatrix savedMatrix(computeVerticalTextMatrix(font, textMatrix), context);
    99113
    100114        Vector<CGSize, 256> translations(count);
    101115        CTFontGetVerticalTranslationsForGlyphs(platformData.ctFont(), glyphs, translations.data(), count);
    102116
    103         CGAffineTransform transform = CGAffineTransformInvert(CGContextGetTextMatrix(context));
    104 
    105         CGPoint position = FloatPoint(point.x(), point.y() + font.fontMetrics().floatAscent(IdeographicBaseline) - font.fontMetrics().floatAscent());
    106         for (unsigned i = 0; i < count; ++i) {
    107             CGSize translation = CGSizeApplyAffineTransform(translations[i], rotateLeftTransform);
    108             positions[i] = CGPointApplyAffineTransform(CGPointMake(position.x - translation.width, position.y + translation.height), transform);
    109             position.x += advances[i].width;
    110             position.y += advances[i].height;
    111         }
     117        auto ascentDelta = font.fontMetrics().floatAscent(IdeographicBaseline) - font.fontMetrics().floatAscent();
     118        fillVectorWithVerticalGlyphPositions(positions, context, translations.data(), advances, count, point, ascentDelta);
    112119        CTFontDrawGlyphs(platformData.ctFont(), glyphs, positions.data(), count, context);
    113120    } else {
    114         fillVectorWithHorizontalGlyphPositions(positions, context, advances, count);
     121        fillVectorWithHorizontalGlyphPositions(positions, context, advances, count, point);
    115122        CTFontDrawGlyphs(platformData.ctFont(), glyphs, positions.data(), count, context);
    116123    }
     
    131138}
    132139
     140AffineTransform computeOverallTextMatrix(const Font& font)
     141{
     142    auto& platformData = font.platformData();
     143    AffineTransform result;
     144    if (!platformData.isColorBitmapFont())
     145        result = CTFontGetMatrix(platformData.font());
     146    result.setB(-result.b());
     147    result.setD(-result.d());
     148    if (platformData.syntheticOblique()) {
     149        float obliqueSkew = tanf(FontCascade::syntheticObliqueAngle() * piFloat / 180);
     150        if (platformData.orientation() == FontOrientation::Vertical) {
     151            if (font.isTextOrientationFallback())
     152                result = AffineTransform(1, obliqueSkew, 0, 1, 0, 0) * result;
     153            else
     154                result = AffineTransform(1, -obliqueSkew, 0, 1, 0, 0) * result;
     155        } else
     156            result = AffineTransform(1, 0, -obliqueSkew, 1, 0, 0);
     157    }
     158
     159    // We're emulating the behavior of CGContextSetTextPosition() by adding constant amounts to each glyph's position
     160    // (see fillVectorWithHorizontalGlyphPositions() and fillVectorWithVerticalGlyphPositions()).
     161    // CGContextSetTextPosition() has the side effect of clobbering the E and F fields of the text matrix,
     162    // so we do that explicitly here.
     163    result.setE(0);
     164    result.setF(0);
     165    return result;
     166}
     167
     168AffineTransform computeVerticalTextMatrix(const Font& font, const AffineTransform& previousTextMatrix)
     169{
     170    ASSERT_UNUSED(font, font.platformData().orientation() == FontOrientation::Vertical);
     171    // The translation here ("e" and "f" fields) are irrelevant, because
     172    // this matrix is inverted in fillVectorWithVerticalGlyphPositions to place the glyphs in the CTM's coordinate system.
     173    // All we're trying to do here is rotate the text matrix so glyphs appear visually upright.
     174    // We have to include the previous text matrix because it includes things like synthetic oblique.
     175    return rotateLeftTransform() * previousTextMatrix;
     176}
     177
    133178void FontCascade::drawGlyphs(GraphicsContext& context, const Font& font, const GlyphBuffer& glyphBuffer, unsigned from, unsigned numGlyphs, const FloatPoint& anchorPoint, FontSmoothingMode smoothingMode)
    134179{
    135     const FontPlatformData& platformData = font.platformData();
     180    const auto& platformData = font.platformData();
    136181    if (!platformData.size())
    137182        return;
     
    176221    FloatPoint point = anchorPoint;
    177222
    178     CGAffineTransform matrix = CGAffineTransformIdentity;
    179     if (!platformData.isColorBitmapFont())
    180         matrix = CTFontGetMatrix(platformData.font());
    181     matrix.b = -matrix.b;
    182     matrix.d = -matrix.d;
    183     if (platformData.syntheticOblique()) {
    184         static float obliqueSkew = tanf(syntheticObliqueAngle() * piFloat / 180);
    185         if (platformData.orientation() == FontOrientation::Vertical) {
    186             if (font.isTextOrientationFallback())
    187                 matrix = CGAffineTransformConcat(matrix, CGAffineTransformMake(1, obliqueSkew, 0, 1, 0, 0));
    188             else
    189                 matrix = CGAffineTransformConcat(matrix, CGAffineTransformMake(1, -obliqueSkew, 0, 1, 0, 0));
    190         } else
    191             matrix = CGAffineTransformConcat(matrix, CGAffineTransformMake(1, 0, -obliqueSkew, 1, 0, 0));
    192     }
    193     ScopedTextMatrix restorer(matrix, cgContext);
     223    auto textMatrix = computeOverallTextMatrix(font);
     224    ScopedTextMatrix restorer(textMatrix, cgContext);
    194225
    195226    setCGFontRenderingMode(context);
     
    222253        // If shadows are ignoring transforms, then we haven't applied the Y coordinate flip yet, so down is negative.
    223254        float shadowTextY = point.y() + shadowOffset.height() * (context.shadowsIgnoreTransforms() ? -1 : 1);
    224         showGlyphsWithAdvances(FloatPoint(shadowTextX, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs);
     255        showGlyphsWithAdvances(FloatPoint(shadowTextX, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs, textMatrix);
    225256        if (syntheticBoldOffset)
    226             showGlyphsWithAdvances(FloatPoint(shadowTextX + syntheticBoldOffset, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs);
     257            showGlyphsWithAdvances(FloatPoint(shadowTextX + syntheticBoldOffset, shadowTextY), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs, textMatrix);
    227258        context.setFillColor(fillColor);
    228259    }
     
    234265#endif
    235266    {
    236         showGlyphsWithAdvances(point, font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs);
     267        showGlyphsWithAdvances(point, font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs, textMatrix);
    237268    }
    238269
    239270    if (syntheticBoldOffset)
    240         showGlyphsWithAdvances(FloatPoint(point.x() + syntheticBoldOffset, point.y()), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs);
     271        showGlyphsWithAdvances(FloatPoint(point.x() + syntheticBoldOffset, point.y()), font, cgContext, glyphBuffer.glyphs(from), glyphBuffer.advances(from), numGlyphs, textMatrix);
    241272
    242273    if (hasSimpleShadow)
Note: See TracChangeset for help on using the changeset viewer.