Changeset 63485 in webkit


Ignore:
Timestamp:
Jul 15, 2010 4:14:04 PM (14 years ago)
Author:
commit-queue@webkit.org
Message:

2010-07-15 Alex Nicolaou <anicolao@chromium.org>

Reviewed by Eric Seidel.

Convolution computation causes bad alpha channel values
https://bugs.webkit.org/show_bug.cgi?id=42273

Fixed by clamping colour channel values to the alpha value so that
r <= a, g <= a, and b <= a after the convolution is applied. See
the bug for why I believe the SVG specification needs to be updated.
Test must be drawn to crash. 100x100 green rectangle is used to
indicate pass to minimize the chance of regression.

  • platform/mac/svg/custom/convolution-crash-expected.checksum: Added.
  • platform/mac/svg/custom/convolution-crash-expected.txt: Added.
  • svg/custom/convolution-crash.svg: Added.

2010-07-15 Alex Nicolaou <anicolao@chromium.org>

Reviewed by Eric Seidel.

Convolution computation causes bad alpha channel values
https://bugs.webkit.org/show_bug.cgi?id=42273

Fixed by clamping colour channel values to the alpha value so that
r <= a, g <= a, and b <= a after the convolution is applied. See
the bug for why I believe the SVG specification needs to be updated.
Test must be drawn to crash. 100x100 green rectangle is used to
indicate pass to minimize the chance of regression.

Test: svg/custom/convolution-crash.svg

  • platform/graphics/skia/SkiaUtils.cpp: (WebCore::SkPMColorToColor):
  • svg/graphics/filters/SVGFEConvolveMatrix.cpp: (WebCore::clampRGBAValue): (WebCore::setDestinationPixels): (WebCore::FEConvolveMatrix::fastSetInteriorPixels): (WebCore::FEConvolveMatrix::fastSetOuterPixels):
Location:
trunk
Files:
4 added
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/LayoutTests/ChangeLog

    r63479 r63485  
     12010-07-15  Alex Nicolaou  <anicolao@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Convolution computation causes bad alpha channel values
     6        https://bugs.webkit.org/show_bug.cgi?id=42273
     7
     8        Fixed by clamping colour channel values to the alpha value so that
     9        r <= a, g <= a, and b <= a after the convolution is applied. See
     10        the bug for why I believe the SVG specification needs to be updated.
     11        Test must be drawn to crash. 100x100 green rectangle is used to
     12        indicate pass to minimize the chance of regression.
     13
     14        * platform/mac/svg/custom/convolution-crash-expected.checksum: Added.
     15        * platform/mac/svg/custom/convolution-crash-expected.txt: Added.
     16        * svg/custom/convolution-crash.svg: Added.
     17
    1182010-07-15  Dumitru Daniliuc  <dumi@chromium.org>
    219
  • trunk/WebCore/ChangeLog

    r63479 r63485  
     12010-07-15  Alex Nicolaou  <anicolao@chromium.org>
     2
     3        Reviewed by Eric Seidel.
     4
     5        Convolution computation causes bad alpha channel values
     6        https://bugs.webkit.org/show_bug.cgi?id=42273
     7
     8        Fixed by clamping colour channel values to the alpha value so that
     9        r <= a, g <= a, and b <= a after the convolution is applied. See
     10        the bug for why I believe the SVG specification needs to be updated.
     11        Test must be drawn to crash. 100x100 green rectangle is used to
     12        indicate pass to minimize the chance of regression.
     13
     14        Test: svg/custom/convolution-crash.svg
     15
     16        * platform/graphics/skia/SkiaUtils.cpp:
     17        (WebCore::SkPMColorToColor):
     18        * svg/graphics/filters/SVGFEConvolveMatrix.cpp:
     19        (WebCore::clampRGBAValue):
     20        (WebCore::setDestinationPixels):
     21        (WebCore::FEConvolveMatrix::fastSetInteriorPixels):
     22        (WebCore::FEConvolveMatrix::fastSetOuterPixels):
     23
    1242010-07-15  Dumitru Daniliuc  <dumi@chromium.org>
    225
  • trunk/WebCore/platform/graphics/skia/SkiaUtils.cpp

    r62553 r63485  
    8383SkColor SkPMColorToColor(SkPMColor pm)
    8484{
    85     if (0 == pm)
     85    if (!pm)
    8686        return 0;
     87    unsigned a = SkGetPackedA32(pm);
     88    if (!a) {
     89        // A zero alpha value when there are non-zero R, G, or B channels is an
     90        // invalid premultiplied color (since all channels should have been
     91        // multiplied by 0 if a=0).
     92        SkASSERT(false);
     93        // In production, return 0 to protect against division by zero.
     94        return 0;
     95    }
    8796   
    88     unsigned a = SkGetPackedA32(pm);
    8997    uint32_t scale = (255 << 16) / a;
    9098   
  • trunk/WebCore/svg/graphics/filters/SVGFEConvolveMatrix.cpp

    r62243 r63485  
    195195*/
    196196
    197 static ALWAYS_INLINE unsigned char clampRGBAValue(float rgba)
    198 {
    199     if (rgba <= 0)
     197static ALWAYS_INLINE unsigned char clampRGBAValue(float channel, unsigned char max = 255)
     198{
     199    if (channel <= 0)
    200200        return 0;
    201     if (rgba >= 255)
    202         return 255;
    203     return rgba;
     201    if (channel >= max)
     202        return max;
     203    return channel;
     204}
     205
     206template<bool preserveAlphaValues>
     207ALWAYS_INLINE void setDestinationPixels(CanvasPixelArray* image, int& pixel, float* totals, float divisor, float bias, CanvasPixelArray* src)
     208{
     209    int numTotals = preserveAlphaValues ? 3 : 4;
     210    for (int i = 0; i < numTotals; ++i)
     211        totals[i] = totals[i] / divisor + bias;
     212
     213    unsigned char maxAlpha;
     214    if (!preserveAlphaValues)
     215        maxAlpha = clampRGBAValue(totals[3]);
     216    else
     217        maxAlpha = 255;
     218
     219    for (int i = 0; i < 3; ++i)
     220        image->set(pixel++, clampRGBAValue(totals[0], maxAlpha));
     221
     222    if (!preserveAlphaValues)
     223        image->set(pixel++, maxAlpha);
     224    else {
     225        image->set(pixel, src->get(pixel));
     226        ++pixel;
     227    }
    204228}
    205229
     
    245269            }
    246270
    247             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[0] / m_divisor + paintingData.bias));
    248             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[1] / m_divisor + paintingData.bias));
    249             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[2] / m_divisor + paintingData.bias));
    250             if (!preserveAlphaValues)
    251                 paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[3] / m_divisor + paintingData.bias));
    252             else {
    253                 paintingData.dstPixelArray->set(pixel, paintingData.srcPixelArray->get(pixel));
    254                 ++pixel;
    255             }
     271            setDestinationPixels<preserveAlphaValues>(paintingData.dstPixelArray, pixel, totals, m_divisor, paintingData.bias, paintingData.srcPixelArray);
    256272            startKernelPixel += 4;
    257273        }
     
    338354            }
    339355
    340             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[0] / m_divisor + paintingData.bias));
    341             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[1] / m_divisor + paintingData.bias));
    342             paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[2] / m_divisor + paintingData.bias));
    343             if (!preserveAlphaValues)
    344                 paintingData.dstPixelArray->set(pixel++, clampRGBAValue(totals[3] / m_divisor + paintingData.bias));
    345             else {
    346                 paintingData.dstPixelArray->set(pixel, paintingData.srcPixelArray->get(pixel));
    347                 ++pixel;
    348             }
     356            setDestinationPixels<preserveAlphaValues>(paintingData.dstPixelArray, pixel, totals, m_divisor, paintingData.bias, paintingData.srcPixelArray);
    349357            ++startKernelPixelX;
    350358        }
Note: See TracChangeset for help on using the changeset viewer.