[webkit-reviews] review canceled: [Bug 38390] [Qt] Failed on http://philip.html5.org/tests/canvas/suite/tests/2d.drawImage.negativesource.html : [Attachment 59691] patch2

[bugzilla-daemon at webkit.org]

Andreas Kling <andreas.kling at nokia.com> has canceled qi
<qi.2.zhang at nokia.com>'s request for review:
Bug 38390: [Qt] Failed on
http://philip.html5.org/tests/canvas/suite/tests/2d.drawImage.negativesource.ht
ml
https://bugs.webkit.org/show_bug.cgi?id=38390

Attachment 59691: patch2
https://bugs.webkit.org/attachment.cgi?id=59691&action=review

------- Additional Comments from Andreas Kling <andreas.kling at nokia.com>
> Index: WebCore/ChangeLog
> ===================================================================
> --- WebCore/ChangeLog (revision 61783)
> +++ WebCore/ChangeLog (working copy)
> @@ -1,3 +1,17 @@
> +2010-06-24  Qi Zhang  <qi.2.zhang at nokia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Qt]  Failed on
http://philip.html5.org/tests/canvas/suite/tests/2d.drawImage.negativesource.ht
ml
> +
> +	   Support negative width and height in canvas image draw
> +
> +	   * platform/graphics/FloatRect.h:
> +	   * platform/graphics/qt/FloatRectQt.cpp:
> +	   (WebCore::FloatRect::normalized):
> +	   * platform/graphics/qt/ImageQt.cpp:
> +	   (WebCore::BitmapImage::draw):
> +
>  2010-06-24  Nate Chapin  <japhet at chromium.org>
>  
>	   Reviewed by Adam Barth.
> Index: WebCore/platform/graphics/FloatRect.h
> ===================================================================
> --- WebCore/platform/graphics/FloatRect.h	(revision 61759)
> +++ WebCore/platform/graphics/FloatRect.h	(working copy)
> @@ -143,6 +143,7 @@ public:
>  #if PLATFORM(QT)
>      FloatRect(const QRectF&);
>      operator QRectF() const;
> +    FloatRect normalized() const;
>  #endif
>  
>  #if PLATFORM(WX) && USE(WXGC)
> Index: WebCore/platform/graphics/qt/FloatRectQt.cpp
> ===================================================================
> --- WebCore/platform/graphics/qt/FloatRectQt.cpp	(revision 61759)
> +++ WebCore/platform/graphics/qt/FloatRectQt.cpp	(working copy)
> @@ -44,6 +44,21 @@ FloatRect::operator QRectF() const
>      return QRectF(x(), y(), width(), height());
>  }
>  
> +FloatRect FloatRect::normalized() const
> +{
> +    FloatRect normalizedRect = *this;
> +
> +    if (width() < 0) {
> +	   normalizedRect.setX(x() + width());
> +	   normalizedRect.setWidth(-width());
> +    }
> +    if (height() < 0) {
> +	   normalizedRect.setY(y() + height());
> +	   normalizedRect.setHeight(-height());
> +    }
> +    return normalizedRect;
> +}
> +
>  }
>  
>  // vim: ts=4 sw=4 et
> Index: WebCore/platform/graphics/qt/ImageQt.cpp
> ===================================================================
> --- WebCore/platform/graphics/qt/ImageQt.cpp	(revision 61759)
> +++ WebCore/platform/graphics/qt/ImageQt.cpp	(working copy)
> @@ -164,6 +164,9 @@ void BitmapImage::invalidatePlatformData
>  void BitmapImage::draw(GraphicsContext* ctxt, const FloatRect& dst,
>			  const FloatRect& src, ColorSpace styleColorSpace,
CompositeOperator op)
>  {
> +    FloatRect normalizedDst = dst.normalized();
> +    FloatRect normalizedSrc = src.normalized();
> +
>      startAnimation();
>  
>      QPixmap* image = nativeImageForCurrentFrame();
> @@ -171,7 +174,7 @@ void BitmapImage::draw(GraphicsContext* 
>	   return;
>  
>      if (mayFillWithSolidColor()) {
> -	   fillWithSolidColor(ctxt, dst, solidColor(), styleColorSpace, op);
> +	   fillWithSolidColor(ctxt, normalizedDst, solidColor(),
styleColorSpace, op);
>	   return;
>      }
>  
> @@ -191,22 +194,22 @@ void BitmapImage::draw(GraphicsContext* 
>      float shadowBlur;
>      Color shadowColor;
>      if (ctxt->getShadow(shadowSize, shadowBlur, shadowColor)) {
> -	   FloatRect shadowImageRect(dst);
> +	   FloatRect shadowImageRect(normalizedDst);
>	   shadowImageRect.move(shadowSize.width(), shadowSize.height());
>  
> -	   QImage shadowImage(QSize(static_cast<int>(src.width()),
static_cast<int>(src.height())), QImage::Format_ARGB32_Premultiplied);
> +	   QImage shadowImage(QSize(static_cast<int>(normalizedSrc.width()),
static_cast<int>(normalizedSrc.height())),
QImage::Format_ARGB32_Premultiplied);
>	   QPainter p(&shadowImage);
>	   p.setCompositionMode(QPainter::CompositionMode_Source);
>	   p.fillRect(shadowImage.rect(), shadowColor);
>	   p.setCompositionMode(QPainter::CompositionMode_DestinationIn);
> -	   p.drawPixmap(dst, *image, src);
> +	   p.drawPixmap(normalizedDst, *image, normalizedSrc);
>	   p.end();
> -	   painter->drawImage(shadowImageRect, shadowImage, src);
> +	   painter->drawImage(shadowImageRect, shadowImage, normalizedSrc);
>      }
>  
>      // Test using example site at
>      // http://www.meyerweb.com/eric/css/edge/complexspiral/demo.html
> -    painter->drawPixmap(dst, *image, src);
> +    painter->drawPixmap(normalizedDst, *image, normalizedSrc);
>  
>      painter->setCompositionMode(lastCompositionMode);
>  
> Index: LayoutTests/ChangeLog
> ===================================================================
> --- LayoutTests/ChangeLog	(revision 61783)
> +++ LayoutTests/ChangeLog	(working copy)
> @@ -1,3 +1,16 @@
> +2010-06-24  Qi Zhang  <qi.2.zhang at nokia.com>
> +
> +	   Reviewed by NOBODY (OOPS!).
> +
> +	   [Qt]  Failed on
http://philip.html5.org/tests/canvas/suite/tests/2d.drawImage.negativesource.ht
ml
> +	   
> +	   Remove the following test case from Skipped:
> +	   canvas/philip/tests/2d.drawImage.negativedest.html
> +	   canvas/philip/tests/2d.drawImage.negativedir.html
> +	   canvas/philip/tests/2d.drawImage.negativesource.html
> +
> +	   * platform/qt/Skipped:
> +
>  2010-06-24  Martin Robinson	<mrobinson at igalia.com>
>  
>	   Unreviewed.
> Index: LayoutTests/platform/qt/Skipped
> ===================================================================
> --- LayoutTests/platform/qt/Skipped	(revision 61759)
> +++ LayoutTests/platform/qt/Skipped	(working copy)
> @@ -5247,9 +5247,6 @@ canvas/philip/tests/2d.composite.uncover
>  canvas/philip/tests/2d.composite.uncovered.pattern.source-out.html
>  canvas/philip/tests/2d.drawImage.broken.html
>  canvas/philip/tests/2d.drawImage.incomplete.html
> -canvas/philip/tests/2d.drawImage.negativedest.html
> -canvas/philip/tests/2d.drawImage.negativedir.html
> -canvas/philip/tests/2d.drawImage.negativesource.html
>  canvas/philip/tests/2d.drawImage.null.html
>  canvas/philip/tests/2d.drawImage.wrongtype.html
>  canvas/philip/tests/2d.drawImage.zerocanvas.html

WebCore/platform/graphics/FloatRect.h:146
 +	FloatRect normalized() const;
IMO this should be a cross-platform method and replace normalizeRect(FloatRect)
in CanvasRenderingContext2D.cpp

Rest looks good to me.