Fixing src rectangle clipping with drawImage

Source/core/html/canvas/CanvasRenderingContext2D.cpp

Index: Source/core/html/canvas/CanvasRenderingContext2D.cpp
diff --git a/Source/core/html/canvas/CanvasRenderingContext2D.cpp b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
index b2701331d1dffccff104b5be49378965be011792..93c790ef4b9550f6012d73398297e4f147165099 100644
--- a/Source/core/html/canvas/CanvasRenderingContext2D.cpp
+++ b/Source/core/html/canvas/CanvasRenderingContext2D.cpp
@@ -1191,6 +1191,25 @@ static inline FloatRect normalizeRect(const FloatRect& rect)
         max(rect.height(), -rect.height()));
 }
 
+static inline void clipRectsToImageRect(const FloatRect& imageRect, FloatRect& srcRect, FloatRect& dstRect)

[Stephen White, 2013/07/22 22:02:06]

Just personal style, but when it comes to arguments, I prefer non-const pointers
over non-const references. It makes it easier to see what may be modified at the
call site. (Code style doesn't say either way, though, so I won't stop you.)

[Justin Novosad, 2013/07/23 17:38:34]

Done.

+{
+    if (imageRect.contains(srcRect))
+        return;
+
+    // Compute the src to dst transform
+    FloatSize scale(dstRect.size().width() / srcRect.size().width(), dstRect.size().height() / srcRect.size().height());
+    FloatPoint scaledSrcLocation = srcRect.location();
+    scaledSrcLocation.scale(scale.width(), scale.height());
+    FloatSize offset = dstRect.location() - scaledSrcLocation;
+
+    srcRect.intersect(imageRect);
+
+    // To clip the destination rectangle in the same proportion, transform the clipped src rect
+    dstRect = srcRect;
+    dstRect.scale(scale.width(), scale.height());
+    dstRect.move(offset);
+}
+
 void CanvasRenderingContext2D::drawImageInternal(Image* image, const FloatRect& srcRect, const FloatRect& dstRect, const CompositeOperator& op, const BlendMode& blendMode)
 {
     if (!image)
@@ -1272,9 +1291,11 @@ void CanvasRenderingContext2D::drawImage(ImageBitmap* bitmap,
         ec = IndexSizeError;
         return;
     }
-    if (!imageRect.contains(normalizedSrcRect))
+    if (!imageRect.intersects(normalizedSrcRect))
         return;
 
+    clipRectsToImageRect(imageRect, normalizedSrcRect, actualDstRect);
+
     Image* imageForRendering = bitmap->bitmapImage();
 
     drawImageInternal(imageForRendering, normalizedSrcRect, actualDstRect, state().m_globalComposite, state().m_globalBlend);
@@ -1340,9 +1361,11 @@ void CanvasRenderingContext2D::drawImage(HTMLImageElement* image, const FloatRec
         ec = IndexSizeError;
         return;
     }
-    if (!imageRect.contains(normalizedSrcRect))
+    if (!imageRect.intersects(normalizedSrcRect))
         return;
 
+    clipRectsToImageRect(imageRect, normalizedSrcRect, normalizedDstRect);
+
     CachedImage* cachedImage = image->cachedImage();
     if (!cachedImage)
         return;
@@ -1400,9 +1423,14 @@ void CanvasRenderingContext2D::drawImage(HTMLCanvasElement* sourceCanvas, const
 
     ec = 0;
 
-    if (!srcCanvasRect.contains(normalizeRect(srcRect)) || !dstRect.width() || !dstRect.height())
+    FloatRect normalizedSrcRect = normalizeRect(srcRect);
+    FloatRect normalizedDstRect = normalizeRect(dstRect);
+
+    if (!srcCanvasRect.intersects(normalizedSrcRect) || !normalizedDstRect.width() || !normalizedDstRect.height())
         return;
 
+    clipRectsToImageRect(srcCanvasRect, normalizedSrcRect, normalizedDstRect);
+
     GraphicsContext* c = drawingContext();
     if (!c)
         return;
@@ -1423,19 +1451,19 @@ void CanvasRenderingContext2D::drawImage(HTMLCanvasElement* sourceCanvas, const
     if (!isAccelerated() || !sourceContext || !sourceContext->isAccelerated() || !sourceContext->is2d())
         sourceCanvas->makeRenderingResultsAvailable();
 
-    if (rectContainsCanvas(dstRect)) {
-        c->drawImageBuffer(buffer, dstRect, srcRect, state().m_globalComposite, state().m_globalBlend);
+    if (rectContainsCanvas(normalizedDstRect)) {
+        c->drawImageBuffer(buffer, normalizedDstRect, normalizedSrcRect, state().m_globalComposite, state().m_globalBlend);
         didDrawEntireCanvas();
     } else if (isFullCanvasCompositeMode(state().m_globalComposite)) {
-        fullCanvasCompositedDrawImage(buffer, dstRect, srcRect, state().m_globalComposite);
+        fullCanvasCompositedDrawImage(buffer, normalizedDstRect, normalizedSrcRect, state().m_globalComposite);
         didDrawEntireCanvas();
     } else if (state().m_globalComposite == CompositeCopy) {
         clearCanvas();
-        c->drawImageBuffer(buffer, dstRect, srcRect, state().m_globalComposite, state().m_globalBlend);
+        c->drawImageBuffer(buffer, normalizedDstRect, normalizedSrcRect, state().m_globalComposite, state().m_globalBlend);
         didDrawEntireCanvas();
     } else {
-        c->drawImageBuffer(buffer, dstRect, srcRect, state().m_globalComposite, state().m_globalBlend);
-        didDraw(dstRect);
+        c->drawImageBuffer(buffer, normalizedDstRect, normalizedSrcRect, state().m_globalComposite, state().m_globalBlend);
+        didDraw(normalizedDstRect);
     }
 }
 
@@ -1486,9 +1514,14 @@ void CanvasRenderingContext2D::drawImage(HTMLVideoElement* video, const FloatRec
         return;
     }
 
-    if (!videoRect.contains(normalizeRect(srcRect)) || !dstRect.width() || !dstRect.height())
+    FloatRect normalizedSrcRect = normalizeRect(srcRect);
+    FloatRect normalizedDstRect = normalizeRect(dstRect);
+
+    if (!videoRect.intersects(normalizeRect(normalizedSrcRect)) || !normalizedDstRect.width() || !normalizedDstRect.height())

[aandrey, 2013/07/23 09:25:40]

remove redundant normalizeRect() call?

[Justin Novosad, 2013/07/23 17:38:34]

Done.

         return;
 
+    clipRectsToImageRect(videoRect, normalizedSrcRect, normalizedDstRect);
+
     GraphicsContext* c = drawingContext();
     if (!c)
         return;
@@ -1498,10 +1531,10 @@ void CanvasRenderingContext2D::drawImage(HTMLVideoElement* video, const FloatRec
     checkOrigin(video);
 
     GraphicsContextStateSaver stateSaver(*c);
-    c->clip(dstRect);
-    c->translate(dstRect.x(), dstRect.y());
-    c->scale(FloatSize(dstRect.width() / srcRect.width(), dstRect.height() / srcRect.height()));
-    c->translate(-srcRect.x(), -srcRect.y());
+    c->clip(normalizedDstRect);
+    c->translate(normalizedDstRect.x(), normalizedDstRect.y());
+    c->scale(FloatSize(normalizedDstRect.width() / normalizedSrcRect.width(), normalizedDstRect.height() / normalizedSrcRect.height()));
+    c->translate(-normalizedSrcRect.x(), -normalizedSrcRect.y());
     video->paintCurrentFrameInContext(c, IntRect(IntPoint(), size(video)));
     stateSaver.restore();
     didDraw(dstRect);