Changeset 93354 in webkit


Ignore:
Timestamp:
Aug 18, 2011 3:03:11 PM (13 years ago)
Author:
commit-queue@webkit.org
Message:

Source/WebCore: Unwarranted DOM Exception when when canvas2D drawImage is called with src
rect out of bounds
​https://bugs.webkit.org/show_bug.cgi?id=65709

Patch by Justin Novosad <​junov@chromium.org> on 2011-08-18
Reviewed by Stephen White.

Test: fast/canvas/drawImage-clipped-source.html

  • html/canvas/CanvasRenderingContext2D.cpp:

(WebCore::CanvasRenderingContext2D::drawImage):
Removed the unnecessary dom exceptions for out of bounds source rectangles
The overloads that receive video and image elements as source images
now use the normalized versions of the source rectangle, which
GraphicsContext (and its various platform flavors) can handle correctly.
The normalized rectangle is the equivalent rectangle with width and height
greater than 0. The canvas version of this method, which had better layout
test coverage, was already correctly using the normalized rectangle. The
newly added layout test verifies correct behavior with negative
source rectangle dimensions.

  • platform/graphics/cg/GraphicsContextCG.cpp:

(WebCore::GraphicsContext::drawNativeImage):
Fixed algorithm that adjusts the destination rectangle to match the clipping
applied to the source rect. The case of scaled filtered images with source
rectangles that overlap the edge of the image was not being handled
correctly. This use case was previously unsupported and used to trigger
a DOM exception.

LayoutTests: Unwarranted DOM Exception when when canvas2D drawImage is called with src
rect out of bounds
​https://bugs.webkit.org/show_bug.cgi?id=65709

Patch by Justin Novosad <​junov@chromium.org> on 2011-08-18
Reviewed by Stephen White.

  • fast/canvas/drawImage-clipped-source-expected.txt: Added.
  • fast/canvas/drawImage-clipped-source.html: Added.

New test that verifies the clipping behavior when source rectangles
are partially out of the bounds of the source image

  • fast/canvas/drawImage-clipped-source.js: Added.

(patternTest.this.testPixel):
(patternTest.this.testRedSquare):
(patternTest.this.testPattern):
(patternTest.this.testAggregatePattern):
(patternTest):
(drawTestPattern):
(executeTest):

  • fast/canvas/drawImage-with-invalid-args-expected.txt:
  • fast/canvas/drawImage-with-invalid-args.html:

This test covers (among other things) cases where the source rectangle is
_completely_ outside the bounds of the source image. It was modified to no
longer expect DOM exceptions

  • platform/chromium/test_expectations.txt:

Out-dated test canvas/philip/tests/2d.drawImage.outsidesource.html
is now expected to fail

  • platform/mac/Skipped:

Skipping canvas/philip/tests/2d.drawImage.outsidesource.html

Location:
trunk
Files:
3 added
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r93353 r93354  
     12011-08-18  Justin Novosad  <junov@chromium.org>
     2
     3        Unwarranted DOM Exception when when canvas2D drawImage is called with src
     4        rect out of bounds
     5        https://bugs.webkit.org/show_bug.cgi?id=65709
     6
     7        Reviewed by Stephen White.
     8
     9        * fast/canvas/drawImage-clipped-source-expected.txt: Added.
     10        * fast/canvas/drawImage-clipped-source.html: Added.
     11        New test that verifies the clipping behavior when source rectangles
     12        are partially out of the bounds of the source image
     13        * fast/canvas/drawImage-clipped-source.js: Added.
     14        (patternTest.this.testPixel):
     15        (patternTest.this.testRedSquare):
     16        (patternTest.this.testPattern):
     17        (patternTest.this.testAggregatePattern):
     18        (patternTest):
     19        (drawTestPattern):
     20        (executeTest):
     21        * fast/canvas/drawImage-with-invalid-args-expected.txt:
     22        * fast/canvas/drawImage-with-invalid-args.html:
     23        This test covers (among other things) cases where the source rectangle is
     24        _completely_ outside the bounds of the source image.  It was modified to no
     25        longer expect DOM exceptions
     26        * platform/chromium/test_expectations.txt:
     27        Out-dated test canvas/philip/tests/2d.drawImage.outsidesource.html
     28        is now expected to fail
     29        * platform/mac/Skipped:
     30        Skipping canvas/philip/tests/2d.drawImage.outsidesource.html
     31
    1322011-08-18  Alexey Proskuryakov  <ap@apple.com>
    233

  • trunk/LayoutTests/fast/canvas/drawImage-with-invalid-args-expected.txt

    r36043 r93354  
    1616PASS: null image, got exception as expected
    1717PASS: null image, got exception as expected
    18 PASS: imageRect does not contain sourceRect on the left side, got exception as expected
    19 PASS: imageRect does not contain sourceRect on the right side, got exception as expected
    20 PASS: imageRect does not contain sourceRect on top, got exception as expected
    21 PASS: imageRect does not contain sourceRect on bottom, got exception as expected
    22 PASS: sourceRect is bigger than imageSource on every side, got exception as expected
    23 PASS: negative size of source, imageRect does not contain sourceRect on the left side, got exception as expected
    24 PASS: negative size of source, imageRect does not contain sourceRect on the right side, got exception as expected
    25 PASS: negative size of source, imageRect does not contain sourceRect on top, got exception as expected
    26 PASS: negative size of source, imageRect does not contain sourceRect on bottom, got exception as expected
    27 PASS: negative size of source, imageRect does not contain sourceRect on every side, got exception as expected
     18PASS: imageRect does not contain sourceRect on the left side
     19PASS: imageRect does not contain sourceRect on the right side
     20PASS: imageRect does not contain sourceRect on top
     21PASS: imageRect does not contain sourceRect on bottom
     22PASS: sourceRect is bigger than imageSource on every side
     23PASS: negative size of source, imageRect does not contain sourceRect on the left side
     24PASS: negative size of source, imageRect does not contain sourceRect on the right side
     25PASS: negative size of source, imageRect does not contain sourceRect on top
     26PASS: negative size of source, imageRect does not contain sourceRect on bottom
     27PASS: negative size of source, imageRect does not contain sourceRect on every side
    2828Test complete.
    2929

  • trunk/LayoutTests/fast/canvas/drawImage-with-invalid-args.html

    r36043 r93354  
    115115        try{
    116116            ctx.drawImage(myImage, -10, 0, 52, 64, 0, 0, 20, 20);
     117            print("PASS: imageRect does not contain sourceRect on the left side");
     118        } catch (e) {
    117119            print("FAIL");
    118         } catch (e) {
    119             print("PASS: imageRect does not contain sourceRect on the left side, got exception as expected");
    120120        }
    121121        try{
    122122            ctx.drawImage(myImage, 10, 0, 52, 64, 0, 0, 20, 20);
     123            print("PASS: imageRect does not contain sourceRect on the right side");
     124        } catch (e) {
    123125            print("FAIL");
    124         } catch (e) {
    125             print("PASS: imageRect does not contain sourceRect on the right side, got exception as expected");
    126126        }
    127127        try{
    128128            ctx.drawImage(myImage, 0, -10, 52, 64, 0, 0, 20, 20);
     129            print("PASS: imageRect does not contain sourceRect on top");
     130        } catch (e) {
    129131            print("FAIL");
    130         } catch (e) {
    131             print("PASS: imageRect does not contain sourceRect on top, got exception as expected");
    132132        }
    133133        try{
    134134            ctx.drawImage(myImage, 0, 10, 52, 64, 0, 0, 20, 20);
     135            print("PASS: imageRect does not contain sourceRect on bottom");
     136        } catch (e) {
    135137            print("FAIL");
    136         } catch (e) {
    137             print("PASS: imageRect does not contain sourceRect on bottom, got exception as expected");
    138138        }
    139139        try{
    140140            ctx.drawImage(myImage, -10, -10, 72, 84, 0, 0, 20, 20);
     141            print("PASS: sourceRect is bigger than imageSource on every side");
     142        } catch (e) {
    141143            print("FAIL");
    142         } catch (e) {
    143             print("PASS: sourceRect is bigger than imageSource on every side, got exception as expected");
    144144        }
    145145        try{
    146146            ctx.drawImage(myImage, 42, 64, -52, -64, 0, 0, 20, 20);
     147            print("PASS: negative size of source, imageRect does not contain sourceRect on the left side");
     148        } catch (e) {
    147149            print("FAIL");
    148         } catch (e) {
    149             print("PASS: negative size of source, imageRect does not contain sourceRect on the left side, got exception as expected");
    150150        }
    151151        try{
    152152            ctx.drawImage(myImage, 62, 64, -52, -64, 0, 0, 20, 20);
     153            print("PASS: negative size of source, imageRect does not contain sourceRect on the right side");
     154        } catch (e) {
    153155            print("FAIL");
    154         } catch (e) {
    155             print("PASS: negative size of source, imageRect does not contain sourceRect on the right side, got exception as expected");
    156156        }
    157157        try{
    158158            ctx.drawImage(myImage, 52, 54, -52, -64, 0, 0, 20, 20);
     159            print("PASS: negative size of source, imageRect does not contain sourceRect on top");
     160        } catch (e) {
    159161            print("FAIL");
    160         } catch (e) {
    161             print("PASS: negative size of source, imageRect does not contain sourceRect on top, got exception as expected");
    162162        }
    163163        try{
    164164            ctx.drawImage(myImage, 52, 74, -52, -64, 0, 0, 20, 20);
     165            print("PASS: negative size of source, imageRect does not contain sourceRect on bottom");
     166        } catch (e) {
    165167            print("FAIL");
    166         } catch (e) {
    167             print("PASS: negative size of source, imageRect does not contain sourceRect on bottom, got exception as expected");
    168168        }
    169169        try{
    170170            ctx.drawImage(myImage, 62, 74, -72, -84, 0, 0, 20, 20);
     171            print("PASS: negative size of source, imageRect does not contain sourceRect on every side");
     172        } catch (e) {
    171173            print("FAIL");
    172         } catch (e) {
    173             print("PASS: negative size of source, imageRect does not contain sourceRect on every side, got exception as expected");
    174174        }
    175175        ctx.fillStyle = 'green';

  • trunk/LayoutTests/platform/chromium/test_expectations.txt

    r93325 r93354  
    222222// We don't let anyone set status in the browser.
    223223WONTFIX : plugins/set-status.html = TEXT
     224
     225// This test is out of date with respect to the latest version of the HTML5 spec
     226// Test had to be decommissioned to fix https://bugs.webkit.org/show_bug.cgi?id=65709
     227// We can re-enable the test when it gets changed upstream to become spec-compliant
     228WONTFIX : canvas/philip/tests/2d.drawImage.outsidesource.html = TEXT
    224229
    225230// We don't care about dashboard compatibility mode.

  • trunk/LayoutTests/platform/mac/Skipped

    r93187 r93354  
    167167canvas/philip/tests/2d.imageData.object.wrap.html
    168168
     169# This canvas test is skipped because it is out of date with respect to
     170# the current spec, and the fix for https://bugs.webkit.org/show_bug.cgi?id=65709
     171# which complies with the current spec, makes this test fail by no longer throwing
     172# exceptions that were previously expected
     173canvas/philip/tests/2d.drawImage.outsidesource.html
     174
    169175# IndexedDB is not yet enabled.
    170176storage/indexeddb

  • trunk/Source/WebCore/ChangeLog

    r93350 r93354  
     12011-08-18  Justin Novosad  <junov@chromium.org>
     2
     3        Unwarranted DOM Exception when when canvas2D drawImage is called with src
     4        rect out of bounds
     5        https://bugs.webkit.org/show_bug.cgi?id=65709
     6
     7        Reviewed by Stephen White.
     8
     9        Test: fast/canvas/drawImage-clipped-source.html
     10
     11        * html/canvas/CanvasRenderingContext2D.cpp:
     12        (WebCore::CanvasRenderingContext2D::drawImage):
     13        Removed the unnecessary dom exceptions for out of bounds source rectangles
     14        The overloads that receive video and image elements as source images
     15        now use the normalized versions of the source rectangle, which
     16        GraphicsContext (and its various platform flavors) can handle correctly.
     17        The normalized rectangle is the equivalent rectangle with width and height
     18        greater than 0.  The canvas version of this method, which had better layout
     19        test coverage, was already correctly using the normalized rectangle. The
     20        newly added layout test verifies correct behavior with negative
     21        source rectangle dimensions.
     22        * platform/graphics/cg/GraphicsContextCG.cpp:
     23        (WebCore::GraphicsContext::drawNativeImage):
     24        Fixed algorithm that adjusts the destination rectangle to match the clipping
     25        applied to the source rect. The case of scaled filtered images with source
     26        rectangles that overlap the edge of the image was not being handled
     27        correctly. This use case was previously unsupported and used to trigger
     28        a DOM exception.
     29
    1302011-08-18  Sheriff Bot  <webkit.review.bot@gmail.com>
    231

  • trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp

    r93279 r93354  
    12701270
    12711271    FloatRect imageRect = FloatRect(FloatPoint(), size(image));
    1272     if (!imageRect.contains(normalizedSrcRect) || !srcRect.width() || !srcRect.height()) {
     1272    if (!srcRect.width() || !srcRect.height()) {
    12731273        ec = INDEX_SIZE_ERR;
    12741274        return;
    12751275    }
     1276    if (!imageRect.intersects(normalizedSrcRect))
     1277        return;
    12761278
    12771279    GraphicsContext* c = drawingContext();
    … …  
    13321334    }
    13331335
    1334     if (!srcCanvasRect.contains(normalizeRect(srcRect)) || !srcRect.width() || !srcRect.height()) {
     1336    if (!srcRect.width() || !srcRect.height()) {
    13351337        ec = INDEX_SIZE_ERR;
    13361338        return;
    … …  
    13391341    ec = 0;
    13401342
    1341     if (!dstRect.width() || !dstRect.height())
     1343    FloatRect normalizedSrcRect = normalizeRect(srcRect);
     1344    if (!srcCanvasRect.intersects(normalizedSrcRect) || !dstRect.width() || !dstRect.height())
    13421345        return;
    13431346
    … …  
    13661369#endif
    13671370
    1368     c->drawImageBuffer(buffer, ColorSpaceDeviceRGB, dstRect, srcRect, state().m_globalComposite);
     1371    c->drawImageBuffer(buffer, ColorSpaceDeviceRGB, normalizeRect(dstRect), normalizedSrcRect, state().m_globalComposite);
    13691372    didDraw(dstRect);
    13701373}
    … …  
    14131416
    14141417    FloatRect videoRect = FloatRect(FloatPoint(), size(video));
    1415     if (!videoRect.contains(normalizeRect(srcRect)) || !srcRect.width() || !srcRect.height()) {
     1418    if (!srcRect.width() || !srcRect.height()) {
    14161419        ec = INDEX_SIZE_ERR;
    14171420        return;
    14181421    }
    14191422
    1420     if (!dstRect.width() || !dstRect.height())
     1423    FloatRect normalizedSrcRect = normalizeRect(srcRect);
     1424    FloatRect normalizedDstRect = normalizeRect(dstRect);
     1425    if (!videoRect.intersects(normalizedSrcRect) || !dstRect.width() || !dstRect.height())
    14211426        return;
    14221427
    … …  
    14301435
    14311436    GraphicsContextStateSaver stateSaver(*c);
    1432     c->clip(dstRect);
    1433     c->translate(dstRect.x(), dstRect.y());
    1434     c->scale(FloatSize(dstRect.width() / srcRect.width(), dstRect.height() / srcRect.height()));
    1435     c->translate(-srcRect.x(), -srcRect.y());
     1437    c->clip(normalizedDstRect);
     1438    c->translate(normalizedDstRect.x(), normalizedDstRect.y());
     1439    c->scale(FloatSize(normalizedDstRect.width() / normalizedSrcRect.width(), normalizedDstRect.height() / normalizedSrcRect.height()));
     1440    c->translate(-normalizedSrcRect.x(), -normalizedSrcRect.y());
    14361441    video->paintCurrentFrameInContext(c, IntRect(IntPoint(), size(video)));
    14371442    stateSaver.restore();
    1438     didDraw(dstRect);
     1443    didDraw(normalizedDstRect);
    14391444}
    14401445#endif

  • trunk/Source/WebCore/platform/graphics/cg/GraphicsContextCG.cpp

    r91496 r93354  
    181181        if (shouldUseSubimage) {
    182182            FloatRect subimageRect = srcRect;
    183             float leftPadding = srcRect.x() - floorf(srcRect.x());
    184             float topPadding = srcRect.y() - floorf(srcRect.y());
     183            float leftPadding = (srcRect.x() >= 0) ? (srcRect.x() - floorf(srcRect.x())) : srcRect.x();
     184            float topPadding = (srcRect.y() >= 0) ? (srcRect.y() - floorf(srcRect.y())) : srcRect.y();
    185185
    186186            subimageRect.move(-leftPadding, -topPadding);
    187187            adjustedDestRect.move(-leftPadding / xScale, -topPadding / yScale);
    188188
    189             subimageRect.setWidth(ceilf(subimageRect.width() + leftPadding));
     189            subimageRect.setWidth(min(ceilf(subimageRect.width() + leftPadding), CGImageGetWidth(image.get()) - subimageRect.x()));
    190190            adjustedDestRect.setWidth(subimageRect.width() / xScale);
    191191
    192             subimageRect.setHeight(ceilf(subimageRect.height() + topPadding));
     192            subimageRect.setHeight(min(ceilf(subimageRect.height() + topPadding), currHeight - subimageRect.y() ));
    193193            adjustedDestRect.setHeight(subimageRect.height() / yScale);
    194194
     195            ASSERT(CGRectContainsRect(CGRectMake(0, 0, CGImageGetWidth(image.get()), currHeight), subimageRect));
    195196            image.adoptCF(CGImageCreateWithImageInRect(image.get(), subimageRect));
    196             if (currHeight < srcRect.maxY()) {
    197                 ASSERT(CGImageGetHeight(image.get()) == currHeight - CGRectIntegral(srcRect).origin.y);
    198                 adjustedDestRect.setHeight(CGImageGetHeight(image.get()) / yScale);
    199             }
    200197        } else {
    201198            adjustedDestRect.setLocation(FloatPoint(destRect.x() - srcRect.x() / xScale, destRect.y() - srcRect.y() / yScale));
Note: See TracChangeset for help on using the changeset viewer.