inconsistency in drawImage with target rect negative dimensions.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Jan 2013 20:25:06 +0000 (20:25 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 16 Jan 2013 20:25:06 +0000 (20:25 +0000)
https://bugs.webkit.org/show_bug.cgi?id=100026

Patch by Arnaud Renevier <a.renevier@sisa.samsung.com> on 2013-01-16
Reviewed by Dean Jackson.

PerformanceTests:

Create a drawImage performance test. There is no significative change
in performance: 27144.6851528 runs/s without the patch; 27153.517612
runs/s with the patch. Test is currently skipped.

* Canvas/drawimage.html: Added.
* Skipped:

Source/WebCore:

Remove -1, -1 special case in drawImage and drawImageBuffer. Replace
all -1 -1 arguments calls to with the correct rectangle dimensions.

Remove FloatRect(0, 0, -1, -1) default argument for srcRect, and
instead, add new overloaded functions to create a FloatRect from image
size.

Replace -1 -1 arguments calls in FEComposite::platformApplySoftware
with correct rectangle dimensions.

Replace ImageGStreamer rect method (which may return -1 -1 rectangle)
with cropRect method, and make caller check for rectangle emptiness.

* fast/canvas/drawImage-with-negative-source-destination-expected.txt:
* fast/canvas/drawImage-with-negative-source-destination.js:

* platform/graphics/GraphicsContext.cpp:
(WebCore::GraphicsContext::drawImage):
(WebCore):
(WebCore::GraphicsContext::drawImageBuffer):
* platform/graphics/GraphicsContext.h:
(GraphicsContext):
* platform/graphics/filters/FEComposite.cpp:
(WebCore::FEComposite::platformApplySoftware):
* platform/graphics/gstreamer/ImageGStreamer.h:
(WebCore::ImageGStreamer::rect):

LayoutTests:

Add a drawImage check for a destination rectangle with -1px
width/height. When drawing to (1, 1, -1, -1) rectangle, first
(top-left) pixel should have been and been the only one drawn into.

* fast/canvas/drawImage-with-negative-source-destination-expected.txt:
* fast/canvas/drawImage-with-negative-source-destination.js:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@139911 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/canvas/drawImage-with-negative-source-destination-expected.txt
LayoutTests/fast/canvas/drawImage-with-negative-source-destination.js
PerformanceTests/Canvas/drawimage.html [new file with mode: 0644]
PerformanceTests/ChangeLog
PerformanceTests/Skipped
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsContext.cpp
Source/WebCore/platform/graphics/GraphicsContext.h
Source/WebCore/platform/graphics/filters/FEComposite.cpp
Source/WebCore/platform/graphics/gstreamer/ImageGStreamer.h

index 9e5eafa..ac8f92c 100644 (file)
@@ -1,3 +1,17 @@
+2013-01-16  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        inconsistency in drawImage with target rect negative dimensions.
+        https://bugs.webkit.org/show_bug.cgi?id=100026
+
+        Reviewed by Dean Jackson.
+
+        Add a drawImage check for a destination rectangle with -1px
+        width/height. When drawing to (1, 1, -1, -1) rectangle, first
+        (top-left) pixel should have been and been the only one drawn into.
+
+        * fast/canvas/drawImage-with-negative-source-destination-expected.txt:
+        * fast/canvas/drawImage-with-negative-source-destination.js:
+
 2013-01-16  W. James MacLean  <wjmaclean@chromium.org>
 
         LinkHighlight should use touch adjustment to match active state on GestureTapDown.
index 738568b..f41ac92 100644 (file)
@@ -6,6 +6,12 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 PASS imgdata[4] is 0
 PASS imgdata[5] is 255
 PASS imgdata[6] is 0
+PASS imgdata[0] is 255
+PASS imgdata[1] is 0
+PASS imgdata[2] is 0
+PASS imgdata[4] is 0
+PASS imgdata[5] is 0
+PASS imgdata[6] is 0
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 0a8a09c..671fd73 100644 (file)
@@ -19,3 +19,16 @@ var imgdata = imageData.data;
 shouldBe("imgdata[4]", "0");
 shouldBe("imgdata[5]", "255");
 shouldBe("imgdata[6]", "0");
+
+// test width or height -1
+canvas.fillStyle = '#000';
+canvas.fillRect(0, 0, 1, 2);
+canvas.drawImage(canvas2, 0, 0, 1, 1, 1, 1, -1, -1);
+var imageData = canvas.getImageData(0, 0, 1, 2);
+var imgdata = imageData.data;
+shouldBe("imgdata[0]", "255");
+shouldBe("imgdata[1]", "0");
+shouldBe("imgdata[2]", "0");
+shouldBe("imgdata[4]", "0");
+shouldBe("imgdata[5]", "0");
+shouldBe("imgdata[6]", "0");
diff --git a/PerformanceTests/Canvas/drawimage.html b/PerformanceTests/Canvas/drawimage.html
new file mode 100644 (file)
index 0000000..c9d4492
--- /dev/null
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<html>
+<body>
+<script src="../resources/runner.js"></script>
+<script>
+
+var source = document.createElement("canvas");
+source.width = 300;
+source.height = 150;
+source.getContext("2d").fillStyle = 'green';
+source.getContext("2d").fillRect(0, 0, source.width, source.height);
+
+var target = document.createElement("canvas");
+target.width = source.width;
+target.height = source.height;
+var context = target.getContext("2d")
+
+PerfTestRunner.measureRunsPerSecond({run: function() {
+    context.drawImage(source, 0, 0);   
+}});
+</script>
+</body>
+</html>
index 1881875..11ba648 100644 (file)
@@ -1,3 +1,17 @@
+2013-01-16  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        inconsistency in drawImage with target rect negative dimensions.
+        https://bugs.webkit.org/show_bug.cgi?id=100026
+
+        Reviewed by Dean Jackson.
+
+        Create a drawImage performance test. There is no significative change
+        in performance: 27144.6851528 runs/s without the patch; 27153.517612
+        runs/s with the patch. Test is currently skipped.
+
+        * Canvas/drawimage.html: Added.
+        * Skipped:
+
 2013-01-15  Dominic Cooney  <dominicc@chromium.org>
 
         Allow zero values as results from the runFunction.
index 509234a..5fd4149 100644 (file)
@@ -62,3 +62,5 @@ SVG/SvgNestedUse.html
 
 # Bug 102646 - ShadowDOM is not enabled in some ports.
 ShadowDOM
+
+Canvas/drawimage.html
index dbd89f9..20a6599 100644 (file)
@@ -1,3 +1,37 @@
+2013-01-16  Arnaud Renevier  <a.renevier@sisa.samsung.com>
+
+        inconsistency in drawImage with target rect negative dimensions.
+        https://bugs.webkit.org/show_bug.cgi?id=100026
+
+        Reviewed by Dean Jackson.
+
+        Remove -1, -1 special case in drawImage and drawImageBuffer. Replace
+        all -1 -1 arguments calls to with the correct rectangle dimensions.
+
+        Remove FloatRect(0, 0, -1, -1) default argument for srcRect, and
+        instead, add new overloaded functions to create a FloatRect from image
+        size.
+
+        Replace -1 -1 arguments calls in FEComposite::platformApplySoftware
+        with correct rectangle dimensions.
+
+        Replace ImageGStreamer rect method (which may return -1 -1 rectangle)
+        with cropRect method, and make caller check for rectangle emptiness.
+
+        * fast/canvas/drawImage-with-negative-source-destination-expected.txt:
+        * fast/canvas/drawImage-with-negative-source-destination.js:
+
+        * platform/graphics/GraphicsContext.cpp:
+        (WebCore::GraphicsContext::drawImage):
+        (WebCore):
+        (WebCore::GraphicsContext::drawImageBuffer):
+        * platform/graphics/GraphicsContext.h:
+        (GraphicsContext):
+        * platform/graphics/filters/FEComposite.cpp:
+        (WebCore::FEComposite::platformApplySoftware):
+        * platform/graphics/gstreamer/ImageGStreamer.h:
+        (WebCore::ImageGStreamer::rect):
+
 2013-01-16  Simon Fraser  <simon.fraser@apple.com>
 
         Allow PaintInfo to carry all PaintBehavior flags
index d4d5c40..69ad4fd 100644 (file)
@@ -448,12 +448,16 @@ void GraphicsContext::drawHighlightForText(const Font& font, const TextRun& run,
 
 void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const IntPoint& p, CompositeOperator op, RespectImageOrientationEnum shouldRespectImageOrientation)
 {
-    drawImage(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, shouldRespectImageOrientation);
+    if (!image)
+        return;
+    drawImage(image, styleColorSpace, p, IntRect(IntPoint(), image->size()), op, shouldRespectImageOrientation);
 }
 
 void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const IntRect& r, CompositeOperator op, RespectImageOrientationEnum shouldRespectImageOrientation, bool useLowQualityScale)
 {
-    drawImage(image, styleColorSpace, r, IntRect(0, 0, -1, -1), op, shouldRespectImageOrientation, useLowQualityScale);
+    if (!image)
+        return;
+    drawImage(image, styleColorSpace, r, IntRect(IntPoint(), image->size()), op, shouldRespectImageOrientation, useLowQualityScale);
 }
 
 void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const IntPoint& dest, const IntRect& srcRect, CompositeOperator op, RespectImageOrientationEnum shouldRespectImageOrientation)
@@ -471,25 +475,17 @@ void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const
     drawImage(image, styleColorSpace, FloatRect(dest), src, op, BlendModeNormal, shouldRespectImageOrientation, useLowQualityScale);
 }
 
+void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const FloatRect& dest)
+{
+    if (!image)
+        return;
+    drawImage(image, styleColorSpace, dest, FloatRect(IntRect(IntPoint(), image->size())));
+}
+
 void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const FloatRect& dest, const FloatRect& src, CompositeOperator op, BlendMode blendMode, RespectImageOrientationEnum shouldRespectImageOrientation, bool useLowQualityScale)
 {    if (paintingDisabled() || !image)
         return;
 
-    float tsw = src.width();
-    float tsh = src.height();
-    float tw = dest.width();
-    float th = dest.height();
-
-    if (tsw == -1)
-        tsw = image->width();
-    if (tsh == -1)
-        tsh = image->height();
-
-    if (tw == -1)
-        tw = image->width();
-    if (th == -1)
-        th = image->height();
-
     InterpolationQuality previousInterpolationQuality = InterpolationDefault;
 
     if (useLowQualityScale) {
@@ -502,7 +498,7 @@ void GraphicsContext::drawImage(Image* image, ColorSpace styleColorSpace, const
 #endif
     }
 
-    image->draw(this, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), styleColorSpace, op, blendMode, shouldRespectImageOrientation);
+    image->draw(this, dest, src, styleColorSpace, op, blendMode, shouldRespectImageOrientation);
 
     if (useLowQualityScale)
         setImageInterpolationQuality(previousInterpolationQuality);
@@ -545,12 +541,16 @@ void GraphicsContext::drawTiledImage(Image* image, ColorSpace styleColorSpace, c
 
 void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorSpace, const IntPoint& p, CompositeOperator op, BlendMode blendMode)
 {
-    drawImageBuffer(image, styleColorSpace, p, IntRect(0, 0, -1, -1), op, blendMode);
+    if (!image)
+        return;
+    drawImageBuffer(image, styleColorSpace, p, IntRect(IntPoint(), image->logicalSize()), op, blendMode);
 }
 
 void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorSpace, const IntRect& r, CompositeOperator op, BlendMode blendMode, bool useLowQualityScale)
 {
-    drawImageBuffer(image, styleColorSpace, r, IntRect(0, 0, -1, -1), op, blendMode, useLowQualityScale);
+    if (!image)
+        return;
+    drawImageBuffer(image, styleColorSpace, r, IntRect(IntPoint(), image->logicalSize()), op, blendMode, useLowQualityScale);
 }
 
 void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorSpace, const IntPoint& dest, const IntRect& srcRect, CompositeOperator op, BlendMode blendMode)
@@ -563,26 +563,18 @@ void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorS
     drawImageBuffer(image, styleColorSpace, FloatRect(dest), srcRect, op, blendMode, useLowQualityScale);
 }
 
+void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorSpace, const FloatRect& dest)
+{
+    if (!image)
+        return;
+    drawImageBuffer(image, styleColorSpace, dest, FloatRect(IntRect(IntPoint(), image->logicalSize())));
+}
+
 void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorSpace, const FloatRect& dest, const FloatRect& src, CompositeOperator op, BlendMode blendMode, bool useLowQualityScale)
 {
     if (paintingDisabled() || !image)
         return;
 
-    float tsw = src.width();
-    float tsh = src.height();
-    float tw = dest.width();
-    float th = dest.height();
-
-    if (tsw == -1)
-        tsw = image->logicalSize().width();
-    if (tsh == -1)
-        tsh = image->logicalSize().height();
-
-    if (tw == -1)
-        tw = image->logicalSize().width();
-    if (th == -1)
-        th = image->logicalSize().height();
-
     if (useLowQualityScale) {
         InterpolationQuality previousInterpolationQuality = imageInterpolationQuality();
 #if PLATFORM(CHROMIUM)
@@ -591,10 +583,10 @@ void GraphicsContext::drawImageBuffer(ImageBuffer* image, ColorSpace styleColorS
         // FIXME (49002): Should be InterpolationLow
         setImageInterpolationQuality(InterpolationNone);
 #endif
-        image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, blendMode, useLowQualityScale);
+        image->draw(this, styleColorSpace, dest, src, op, blendMode, useLowQualityScale);
         setImageInterpolationQuality(previousInterpolationQuality);
     } else
-        image->draw(this, styleColorSpace, FloatRect(dest.location(), FloatSize(tw, th)), FloatRect(src.location(), FloatSize(tsw, tsh)), op, blendMode, useLowQualityScale);
+        image->draw(this, styleColorSpace, dest, src, op, blendMode, useLowQualityScale);
 }
 
 #if !PLATFORM(QT)
index 8e12e09..9d389f6 100644 (file)
@@ -322,8 +322,8 @@ namespace WebCore {
         void drawImage(Image*, ColorSpace styleColorSpace, const IntRect&, CompositeOperator = CompositeSourceOver, RespectImageOrientationEnum = DoNotRespectImageOrientation, bool useLowQualityScale = false);
         void drawImage(Image*, ColorSpace styleColorSpace, const IntPoint& destPoint, const IntRect& srcRect, CompositeOperator = CompositeSourceOver, RespectImageOrientationEnum = DoNotRespectImageOrientation);
         void drawImage(Image*, ColorSpace styleColorSpace, const IntRect& destRect, const IntRect& srcRect, CompositeOperator = CompositeSourceOver, RespectImageOrientationEnum = DoNotRespectImageOrientation, bool useLowQualityScale = false);
-        void drawImage(Image*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1),
-                       CompositeOperator = CompositeSourceOver, RespectImageOrientationEnum = DoNotRespectImageOrientation, bool useLowQualityScale = false);
+        void drawImage(Image*, ColorSpace styleColorSpace, const FloatRect& destRect);
+        void drawImage(Image*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator = CompositeSourceOver, RespectImageOrientationEnum = DoNotRespectImageOrientation, bool useLowQualityScale = false);
         void drawImage(Image*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator, BlendMode, RespectImageOrientationEnum = DoNotRespectImageOrientation, bool useLowQualityScale = false);
         
         void drawTiledImage(Image*, ColorSpace styleColorSpace, const IntRect& destRect, const IntPoint& srcPoint, const IntSize& tileSize,
@@ -336,7 +336,8 @@ namespace WebCore {
         void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const IntRect&, CompositeOperator = CompositeSourceOver, BlendMode = BlendModeNormal, bool useLowQualityScale = false);
         void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const IntPoint& destPoint, const IntRect& srcRect, CompositeOperator = CompositeSourceOver, BlendMode = BlendModeNormal);
         void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const IntRect& destRect, const IntRect& srcRect, CompositeOperator = CompositeSourceOver, BlendMode = BlendModeNormal, bool useLowQualityScale = false);
-        void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect = FloatRect(0, 0, -1, -1), CompositeOperator = CompositeSourceOver, BlendMode = BlendModeNormal, bool useLowQualityScale = false);
+        void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const FloatRect& destRect);
+        void drawImageBuffer(ImageBuffer*, ColorSpace styleColorSpace, const FloatRect& destRect, const FloatRect& srcRect, CompositeOperator = CompositeSourceOver, BlendMode = BlendModeNormal, bool useLowQualityScale = false);
 
         void setImageInterpolationQuality(InterpolationQuality);
         InterpolationQuality imageInterpolationQuality() const;
index 9d566ed..27e73aa 100644 (file)
@@ -274,11 +274,15 @@ void FEComposite::platformApplySoftware()
         return;
     GraphicsContext* filterContext = resultImage->context();
 
-    FloatRect srcRect = FloatRect(0, 0, -1, -1);
+    ImageBuffer* imageBuffer = in->asImageBuffer();
+    ImageBuffer* imageBuffer2 = in2->asImageBuffer();
+    ASSERT(imageBuffer);
+    ASSERT(imageBuffer2);
+
     switch (m_type) {
     case FECOMPOSITE_OPERATOR_OVER:
-        filterContext->drawImageBuffer(in2->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
-        filterContext->drawImageBuffer(in->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()));
+        filterContext->drawImageBuffer(imageBuffer2, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
+        filterContext->drawImageBuffer(imageBuffer, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()));
         break;
     case FECOMPOSITE_OPERATOR_IN: {
         // Applies only to the intersected region.
@@ -292,21 +296,21 @@ void FEComposite::platformApplySoftware()
                                     destinationRect.y() - in->absolutePaintRect().y()), destinationRect.size());
         IntRect source2Rect(IntPoint(destinationRect.x() - in2->absolutePaintRect().x(),
                                      destinationRect.y() - in2->absolutePaintRect().y()), destinationRect.size());
-        filterContext->drawImageBuffer(in2->asImageBuffer(), ColorSpaceDeviceRGB, destinationPoint, source2Rect);
-        filterContext->drawImageBuffer(in->asImageBuffer(), ColorSpaceDeviceRGB, destinationPoint, sourceRect, CompositeSourceIn);
+        filterContext->drawImageBuffer(imageBuffer2, ColorSpaceDeviceRGB, destinationPoint, source2Rect);
+        filterContext->drawImageBuffer(imageBuffer, ColorSpaceDeviceRGB, destinationPoint, sourceRect, CompositeSourceIn);
         break;
     }
     case FECOMPOSITE_OPERATOR_OUT:
-        filterContext->drawImageBuffer(in->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()));
-        filterContext->drawImageBuffer(in2->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()), srcRect, CompositeDestinationOut);
+        filterContext->drawImageBuffer(imageBuffer, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()));
+        filterContext->drawImageBuffer(imageBuffer2, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()), IntRect(IntPoint(), imageBuffer2->logicalSize()), CompositeDestinationOut);
         break;
     case FECOMPOSITE_OPERATOR_ATOP:
-        filterContext->drawImageBuffer(in2->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
-        filterContext->drawImageBuffer(in->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()), srcRect, CompositeSourceAtop);
+        filterContext->drawImageBuffer(imageBuffer2, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
+        filterContext->drawImageBuffer(imageBuffer, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()), IntRect(IntPoint(), imageBuffer->logicalSize()), CompositeSourceAtop);
         break;
     case FECOMPOSITE_OPERATOR_XOR:
-        filterContext->drawImageBuffer(in2->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
-        filterContext->drawImageBuffer(in->asImageBuffer(), ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()), srcRect, CompositeXOR);
+        filterContext->drawImageBuffer(imageBuffer2, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in2->absolutePaintRect()));
+        filterContext->drawImageBuffer(imageBuffer, ColorSpaceDeviceRGB, drawingRegionOfInputImage(in->absolutePaintRect()), IntRect(IntPoint(), imageBuffer->logicalSize()), CompositeXOR);
         break;
     default:
         break;
index 40f3552..8b46cd6 100644 (file)
@@ -51,9 +51,8 @@ class ImageGStreamer : public RefCounted<ImageGStreamer> {
         {
             if (!m_cropRect.isEmpty())
                 return FloatRect(m_cropRect);
-
-            // Default rectangle used by GraphicsContext::drawImage().
-            return FloatRect(0, 0, -1, -1);
+            ASSERT(m_image);
+            return FloatRect(0, 0, m_image->size().width(), m_image->size().height());
         }
 
     private: