⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 292026 in webkit


Ignore:
Timestamp:
Mar 29, 2022, 2:43:07 AM (5 years ago)
Author:
Said Abou-Hallawa
Message:

REGRESSION(r291771): [ iOS ] Text sometimes draw with incorrect color
​https://bugs.webkit.org/show_bug.cgi?id=238466
rdar://90941790

Reviewed by Simon Fraser.

r291771 uncovers this bug: TextBoxPainter::paintForeground() records the
glyphs to a DisplayList before settings the destination GraphicsContext.

The fix is to apply all the changes to the GraphicsContext before calling
TextPainter::setGlyphDisplayListIfNeeded().

Delete TextPainter::paint() because it is not used.

Initialize TextPainter with a reference to FontCascade.

  • rendering/TextBoxPainter.cpp:

(WebCore::TextBoxPainter::paintForeground):

  • rendering/TextPainter.cpp:

(WebCore::TextPainter::TextPainter):
(WebCore::TextPainter::paintTextAndEmphasisMarksIfNeeded):
(WebCore::TextPainter::paintRange):
(WebCore::TextPainter::paint): Deleted.

  • rendering/TextPainter.h:

(WebCore::TextPainter::setShadowColorFilter):
(WebCore::TextPainter::setGlyphDisplayListIfNeeded):
(WebCore::TextPainter::setFont): Deleted.

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebCore/ChangeLog

    r292025 r292026  
     12022-03-29  Said Abou-Hallawa  <said@apple.com>
     2
     3        REGRESSION(r291771): [ iOS ] Text sometimes draw with incorrect color
     4        https://bugs.webkit.org/show_bug.cgi?id=238466
     5        rdar://90941790
     6
     7        Reviewed by Simon Fraser.
     8
     9        r291771 uncovers this bug: TextBoxPainter::paintForeground() records the
     10        glyphs to a DisplayList before settings the destination GraphicsContext.
     11
     12        The fix is to apply all the changes to the GraphicsContext before calling
     13        TextPainter::setGlyphDisplayListIfNeeded().
     14
     15        Delete TextPainter::paint() because it is not used.
     16
     17        Initialize TextPainter with a reference to FontCascade.
     18
     19        * rendering/TextBoxPainter.cpp:
     20        (WebCore::TextBoxPainter::paintForeground):
     21        * rendering/TextPainter.cpp:
     22        (WebCore::TextPainter::TextPainter):
     23        (WebCore::TextPainter::paintTextAndEmphasisMarksIfNeeded):
     24        (WebCore::TextPainter::paintRange):
     25        (WebCore::TextPainter::paint): Deleted.
     26        * rendering/TextPainter.h:
     27        (WebCore::TextPainter::setShadowColorFilter):
     28        (WebCore::TextPainter::setGlyphDisplayListIfNeeded):
     29        (WebCore::TextPainter::setFont): Deleted.
     30
    1312022-03-29  Zan Dobersek  <zdobersek@igalia.com>
    232

  • trunk/Source/WebCore/rendering/TextBoxPainter.cpp

    r291552 r292026  
    325325        emphasisMarkOffset = *m_emphasisMarkExistsAndIsAbove ? -font.metricsOfPrimaryFont().ascent() - font.emphasisMarkDescent(emphasisMark) : font.metricsOfPrimaryFont().descent() + font.emphasisMarkAscent(emphasisMark);
    326326
    327     TextPainter textPainter { context };
    328     textPainter.setFont(font);
     327    TextPainter textPainter { context, font };
    329328    textPainter.setStyle(markedText.style.textStyles);
    330329    textPainter.setIsHorizontal(textBox().isHorizontal());
    … …  
    338337        textPainter.setShadow(debugShadow);
    339338
     339    GraphicsContextStateSaver stateSaver(context, markedText.style.textStyles.strokeWidth > 0 || markedText.type == MarkedText::DraggedContent);
     340    if (markedText.type == MarkedText::DraggedContent)
     341        context.setAlpha(markedText.style.alpha);
     342    updateGraphicsContext(context, markedText.style.textStyles);
     343
    340344    if (auto* legacyInlineBox = textBox().legacyInlineBox())
    341         textPainter.setGlyphDisplayListIfNeeded(*legacyInlineBox, m_paintInfo, font, context, m_paintTextRun);
     345        textPainter.setGlyphDisplayListIfNeeded(*legacyInlineBox, m_paintInfo, m_paintTextRun);
    342346#if ENABLE(LAYOUT_FORMATTING_CONTEXT)
    343347    else
    344         textPainter.setGlyphDisplayListIfNeeded(*textBox().inlineBox(), m_paintInfo, font, context, m_paintTextRun);
     348        textPainter.setGlyphDisplayListIfNeeded(*textBox().inlineBox(), m_paintInfo, m_paintTextRun);
    345349#endif
    346350
    347     GraphicsContextStateSaver stateSaver { context, false };
    348     if (markedText.type == MarkedText::DraggedContent) {
    349         stateSaver.save();
    350         context.setAlpha(markedText.style.alpha);
    351     }
    352351    // TextPainter wants the box rectangle and text origin of the entire line box.
    353352    textPainter.paintRange(m_paintTextRun, m_paintRect, textOriginFromPaintRect(m_paintRect), markedText.startOffset, markedText.endOffset);

  • trunk/Source/WebCore/rendering/TextPainter.cpp

    r288942 r292026  
    9898}
    9999
    100 TextPainter::TextPainter(GraphicsContext& context)
     100TextPainter::TextPainter(GraphicsContext& context, const FontCascade& font)
    101101    : m_context(context)
     102    , m_font(font)
    102103{
    103104}
    … …  
    160161    if (paintStyle.paintOrder == PaintOrder::Normal) {
    161162        // FIXME: Truncate right-to-left text correctly.
    162         paintTextWithShadows(shadow, shadowColorFilter, *m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, paintStyle.strokeWidth > 0);
     163        paintTextWithShadows(shadow, shadowColorFilter, m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, paintStyle.strokeWidth > 0);
    163164    } else {
    164165        auto textDrawingMode = m_context.textDrawingMode();
    … …  
    172173                textDrawingModeWithoutStroke.remove(TextDrawingMode::Stroke);
    173174                m_context.setTextDrawingMode(textDrawingModeWithoutStroke);
    174                 paintTextWithShadows(shadowToUse, shadowColorFilter, *m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, false);
     175                paintTextWithShadows(shadowToUse, shadowColorFilter, m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, false);
    175176                shadowToUse = nullptr;
    176177                m_context.setTextDrawingMode(textDrawingMode);
    … …  
    181182                textDrawingModeWithoutFill.remove(TextDrawingMode::Fill);
    182183                m_context.setTextDrawingMode(textDrawingModeWithoutFill);
    183                 paintTextWithShadows(shadowToUse, shadowColorFilter, *m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, paintStyle.strokeWidth > 0);
     184                paintTextWithShadows(shadowToUse, shadowColorFilter, m_font, textRun, boxRect, textOrigin, startOffset, endOffset, nullAtom(), 0, paintStyle.strokeWidth > 0);
    184185                shadowToUse = nullptr;
    185186                m_context.setTextDrawingMode(textDrawingMode);
    … …  
    199200    static NeverDestroyed<TextRun> objectReplacementCharacterTextRun(StringView(&objectReplacementCharacter, 1));
    200201    const TextRun& emphasisMarkTextRun = m_combinedText ? objectReplacementCharacterTextRun.get() : textRun;
    201     FloatPoint emphasisMarkTextOrigin = m_combinedText ? FloatPoint(boxOrigin.x() + boxRect.width() / 2, boxOrigin.y() + m_font->metricsOfPrimaryFont().ascent()) : textOrigin;
     202    FloatPoint emphasisMarkTextOrigin = m_combinedText ? FloatPoint(boxOrigin.x() + boxRect.width() / 2, boxOrigin.y() + m_font.metricsOfPrimaryFont().ascent()) : textOrigin;
    202203    if (m_combinedText)
    203204        m_context.concatCTM(rotation(boxRect, Clockwise));
    204205
    205206    // FIXME: Truncate right-to-left text correctly.
    206     paintTextWithShadows(shadow, shadowColorFilter, m_combinedText ? m_combinedText->originalFont() : *m_font, emphasisMarkTextRun, boxRect, emphasisMarkTextOrigin, startOffset, endOffset,
     207    paintTextWithShadows(shadow, shadowColorFilter, m_combinedText ? m_combinedText->originalFont() : m_font, emphasisMarkTextRun, boxRect, emphasisMarkTextOrigin, startOffset, endOffset,
    207208        m_emphasisMark, m_emphasisMarkOffset, paintStyle.strokeWidth > 0);
    208209
    … …  
    211212}
    212213
    213 void TextPainter::paint(const TextRun& textRun, const FloatRect& boxRect, const FloatPoint& textOrigin)
    214 {
    215     paintRange(textRun, boxRect, textOrigin, 0, textRun.length());
    216 }
    217 
    218214void TextPainter::paintRange(const TextRun& textRun, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned start, unsigned end)
    219215{
    220     ASSERT(m_font);
    221216    ASSERT(start < end);
    222 
    223     GraphicsContextStateSaver stateSaver(m_context, m_style.strokeWidth > 0);
    224     updateGraphicsContext(m_context, m_style);
    225217    paintTextAndEmphasisMarksIfNeeded(textRun, boxRect, textOrigin, start, end, m_style, m_shadow, m_shadowColorFilter);
    226218}

  • trunk/Source/WebCore/rendering/TextPainter.h

    r246490 r292026  
    5050class TextPainter {
    5151public:
    52     TextPainter(GraphicsContext&);
     52    TextPainter(GraphicsContext&, const FontCascade&);
    5353
    5454    void setStyle(const TextPaintStyle& textPaintStyle) { m_style = textPaintStyle; }
    5555    void setShadow(const ShadowData* shadow) { m_shadow = shadow; }
    5656    void setShadowColorFilter(const FilterOperations* colorFilter) { m_shadowColorFilter = colorFilter; }
    57     void setFont(const FontCascade& font) { m_font = &font; }
    5857    void setIsHorizontal(bool isHorizontal) { m_textBoxIsHorizontal = isHorizontal; }
    5958    void setEmphasisMark(const AtomString& mark, float offset, const RenderCombineText*);
    6059
    61     void paint(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin);
    6260    void paintRange(const TextRun&, const FloatRect& boxRect, const FloatPoint& textOrigin, unsigned start, unsigned end);
    6361
    6462    template<typename LayoutRun>
    65     void setGlyphDisplayListIfNeeded(const LayoutRun& run, const PaintInfo& paintInfo, const FontCascade& font, GraphicsContext& context, const TextRun& textRun)
     63    void setGlyphDisplayListIfNeeded(const LayoutRun& run, const PaintInfo& paintInfo, const TextRun& textRun)
    6664    {
    6765        if (!TextPainter::shouldUseGlyphDisplayList(paintInfo))
    6866            TextPainter::removeGlyphDisplayList(run);
    6967        else
    70             m_glyphDisplayList = GlyphDisplayListCache<LayoutRun>::singleton().get(run, font, context, textRun);
     68            m_glyphDisplayList = GlyphDisplayListCache<LayoutRun>::singleton().get(run, m_font, m_context, textRun);
    7169    }
    7270
    … …  
    8684
    8785    GraphicsContext& m_context;
    88     const FontCascade* m_font { nullptr };
     86    const FontCascade& m_font;
    8987    TextPaintStyle m_style;
    9088    AtomString m_emphasisMark;
Note: See TracChangeset for help on using the changeset viewer.