Changeset 173418 in webkit


Ignore:
Timestamp:
Sep 8, 2014 7:56:24 PM (10 years ago)
Author:
mmaxfield@apple.com
Message:

REGRESSION (r172153): Text drawn with wrong color when second text shadow has zero offset and blur (breaks buttons at aws.amazon.com)
​https://bugs.webkit.org/show_bug.cgi?id=136612

Reviewed by Darin Adler.

Source/WebCore:

r172153 was fundamentally broken, and would restore graphics contexts that had never been saved. This patch
reimplements r172153 by using "continue" to skip loop iterations instead of changing the internal logic of
the loop.

In addition, I have refactored InlineTextBox::applyShadowToGraphicsContext() to take an extra boolean
reference as an out parameter in order to make it obvious it if saved a graphics context that needs
to be restored. This should make it less likely to make these kinds of mistakes in the future.

Test: fast/text/empty-shadow-with-color.html

  • rendering/InlineTextBox.cpp:

(WebCore::InlineTextBox::applyShadowToGraphicsContext): Add bool reference out param.

  • rendering/InlineTextBox.h: Ditto.
  • rendering/TextPainter.cpp:

(WebCore::isEmptyShadow): Change logic to not skip loop iterations on a null shadow.
(WebCore::paintTextWithShadows): Use continue to skip loop iterations for empty shadows. In addition,
use the out param in InlineTextBox::applyShadowToGraphicsContext().

  • rendering/svg/SVGInlineTextBox.cpp:

(WebCore::SVGInlineTextBox::paintTextWithShadows): Update for new sigurature of
InlineTextBox::applyShadowToGraphicsContext().

LayoutTests:

Make sure that text is drawn with correct color when second text shadow has zero offset and blur

  • fast/text/empty-shadow-with-color-expected.html: Added.
  • fast/text/empty-shadow-with-color.html: Added.
Location:
trunk
Files:
2 added
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r173416 r173418  
     12014-09-08  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION (r172153): Text drawn with wrong color when second text shadow has zero offset and blur (breaks buttons at aws.amazon.com)
     4        https://bugs.webkit.org/show_bug.cgi?id=136612
     5
     6        Reviewed by Darin Adler.
     7
     8        Make sure that text is drawn with correct color when second text shadow has zero offset and blur
     9
     10        * fast/text/empty-shadow-with-color-expected.html: Added.
     11        * fast/text/empty-shadow-with-color.html: Added.
     12
    1132014-09-08  Roger Fong  <roger_fong@apple.com>
    214

  • trunk/Source/WebCore/ChangeLog

    r173410 r173418  
     12014-09-08  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        REGRESSION (r172153): Text drawn with wrong color when second text shadow has zero offset and blur (breaks buttons at aws.amazon.com)
     4        https://bugs.webkit.org/show_bug.cgi?id=136612
     5
     6        Reviewed by Darin Adler.
     7
     8        r172153 was fundamentally broken, and would restore graphics contexts that had never been saved. This patch
     9        reimplements r172153 by using "continue" to skip loop iterations instead of changing the internal logic of
     10        the loop.
     11
     12        In addition, I have refactored InlineTextBox::applyShadowToGraphicsContext() to take an extra boolean
     13        reference as an out parameter in order to make it obvious it if saved a graphics context that needs
     14        to be restored. This should make it less likely to make these kinds of mistakes in the future.
     15
     16        Test: fast/text/empty-shadow-with-color.html
     17
     18        * rendering/InlineTextBox.cpp:
     19        (WebCore::InlineTextBox::applyShadowToGraphicsContext): Add bool reference out param.
     20        * rendering/InlineTextBox.h: Ditto.
     21        * rendering/TextPainter.cpp:
     22        (WebCore::isEmptyShadow): Change logic to not skip loop iterations on a null shadow.
     23        (WebCore::paintTextWithShadows): Use continue to skip loop iterations for empty shadows. In addition,
     24        use the out param in InlineTextBox::applyShadowToGraphicsContext().
     25        * rendering/svg/SVGInlineTextBox.cpp:
     26        (WebCore::SVGInlineTextBox::paintTextWithShadows): Update for new sigurature of
     27        InlineTextBox::applyShadowToGraphicsContext().
     28
    1292014-09-08  Commit Queue  <commit-queue@webkit.org>
    230

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

    r173114 r173418  
    425425}
    426426
    427 FloatSize InlineTextBox::applyShadowToGraphicsContext(GraphicsContext* context, const ShadowData* shadow, const FloatRect& textRect, bool stroked, bool opaque, bool horizontal)
     427FloatSize InlineTextBox::applyShadowToGraphicsContext(GraphicsContext* context, const ShadowData* shadow, const FloatRect& textRect, bool stroked, bool opaque, bool horizontal, bool& didSaveContext)
    428428{
    429429    if (!shadow)
    … …  
    446446        extraOffset = FloatSize(0, 2 * textRect.height() + std::max(0.0f, shadowOffset.height()) + shadowRadius);
    447447        shadowOffset -= extraOffset;
     448        didSaveContext = true;
    448449    }
    449450

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

    r173114 r173418  
    156156
    157157    // Needs to be public, so the static paintTextWithShadows() function can use it.
    158     static FloatSize applyShadowToGraphicsContext(GraphicsContext*, const ShadowData*, const FloatRect& textRect, bool stroked, bool opaque, bool horizontal);
     158    static FloatSize applyShadowToGraphicsContext(GraphicsContext*, const ShadowData*, const FloatRect& textRect, bool stroked, bool opaque, bool horizontal, bool& didSaveContext);
    159159
    160160protected:

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

    r173241 r173418  
    6464{
    6565    if (!shadow)
    66         return true;
     66        return false;
    6767    return shadow->location() == IntPoint() && !shadow->radius();
    6868}
    … …  
    7979
    8080    do {
     81        if (isEmptyShadow(shadow)) {
     82            shadow = shadow->next();
     83            continue;
     84        }
     85
    8186        IntSize extraOffset;
    82         bool shadowIsEmpty = isEmptyShadow(shadow);
    83         if (!shadowIsEmpty)
    84             extraOffset = roundedIntSize(InlineTextBox::applyShadowToGraphicsContext(context, shadow, boxRect, stroked, opaque, horizontal));
     87        bool didSaveContext = false;
     88        if (shadow)
     89            extraOffset = roundedIntSize(InlineTextBox::applyShadowToGraphicsContext(context, shadow, boxRect, stroked, opaque, horizontal, didSaveContext));
    8590        else if (!opaque)
    8691            context->setFillColor(fillColor, fillColorSpace);
    … …  
    98103            break;
    99104
    100         if (shadow->next() || stroked || !opaque)
     105        if (didSaveContext)
    101106            context->restore();
    102         else if (!shadowIsEmpty)
     107        else
    103108            context->clearShadow();
    104109

  • trunk/Source/WebCore/rendering/svg/SVGInlineTextBox.cpp

    r173047 r173418  
    596596
    597597        FloatSize extraOffset;
     598        bool didSaveContext = false;
    598599        if (shadow)
    599             extraOffset = applyShadowToGraphicsContext(context, shadow, shadowRect, false /* stroked */, true /* opaque */, true /* horizontal */);
     600            extraOffset = applyShadowToGraphicsContext(context, shadow, shadowRect, false /* stroked */, true /* opaque */, true /* horizontal */, didSaveContext);
    600601
    601602        context->save();
    … …  
    607608
    608609        if (shadow) {
    609             if (shadow->next())
     610            if (didSaveContext)
    610611                context->restore();
    611612            else
Note: See TracChangeset for help on using the changeset viewer.