Bug 467283. Ignore dirty rect when doing any image resampling --- it will lead to artifacts. r+sr=dbaron,r=vlad

gfx/src/thebes/nsThebesImage.cpp

@@ -477,6 +477,9 @@ nsThebesImage::Draw(gfxContext*        aContext,
     nsRefPtr<gfxASurface> surface;
     gfxImageSurface::gfxImageFormat format;
 
+    NS_ASSERTION(!sourceRect.Intersect(subimage).IsEmpty(),
+                 "We must be allowed to sample *some* source pixels!");
+
     PRBool doTile = !imageRect.Contains(sourceRect);
     if (doPadding || doPartialDecode) {
         gfxRect available = gfxRect(mDecoded.x, mDecoded.y, mDecoded.width, mDecoded.height) +
@@ -600,6 +603,8 @@ nsThebesImage::Draw(gfxContext*        aContext,
             gfxRect needed = subimage.Intersect(sourceRect);
             needed.RoundOut();
             gfxIntSize size(PRInt32(needed.Width()), PRInt32(needed.Height()));
+            NS_ASSERTION(size.width > 0 && size.height > 0,
+                         "We must have some needed pixels, otherwise we don't know what to sample");
             nsRefPtr<gfxASurface> temp =
                 gfxPlatform::GetPlatform()->CreateOffscreenSurface(size, format);
             if (temp && temp->CairoStatus() == 0) {

gfx/thebes/public/gfxContext.h

@@ -740,4 +740,32 @@ private:
     nsRefPtr<gfxPath> mPath;
 };
 
+/**
+ * Sentry helper class for functions with multiple return points that need to
+ * back up the current matrix of a context and have it automatically restored
+ * before they return.
+ */
+class THEBES_API gfxContextMatrixAutoSaveRestore
+{
+public:
+    gfxContextMatrixAutoSaveRestore(gfxContext *aContext) :
+        mContext(aContext), mMatrix(aContext->CurrentMatrix())
+    {
+    }
+
+    ~gfxContextMatrixAutoSaveRestore()
+    {
+        mContext->SetMatrix(mMatrix);
+    }
+
+    const gfxMatrix& Matrix()
+    {
+        return mMatrix;
+    }
+
+private:
+    gfxContext *mContext;
+    gfxMatrix   mMatrix;
+};
+
 #endif /* GFX_CONTEXT_H */

layout/base/nsLayoutUtils.cpp

@@ -2773,11 +2773,10 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
                        aAnchor.y/appUnitsPerDevPixel);
   gfxPoint imageSpaceAnchorPoint =
     MapToFloatImagePixels(imageSize, aDest, aAnchor);
-  gfxMatrix currentMatrix = ctx->CurrentMatrix();
+  gfxContextMatrixAutoSaveRestore saveMatrix(ctx);
 
-  gfxRect finalFillRect = fill;
   if (didSnap) {
-    NS_ASSERTION(!currentMatrix.HasNonAxisAlignedTransform(),
+    NS_ASSERTION(!saveMatrix.Matrix().HasNonAxisAlignedTransform(),
                  "How did we snap, then?");
     imageSpaceAnchorPoint.Round();
     anchorPoint = imageSpaceAnchorPoint;
@@ -2786,42 +2785,48 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
                          aDest.width/appUnitsPerDevPixel,
                          aDest.height/appUnitsPerDevPixel);
     anchorPoint = MapToFloatUserPixels(imageSize, devPixelDest, anchorPoint);
-    anchorPoint = currentMatrix.Transform(anchorPoint);
+    anchorPoint = saveMatrix.Matrix().Transform(anchorPoint);
     anchorPoint.Round();
 
     // This form of Transform is safe to call since non-axis-aligned
     // transforms wouldn't be snapped.
-    dirty = currentMatrix.Transform(dirty);
-    dirty.RoundOut();
-    finalFillRect = fill.Intersect(dirty);
-    if (finalFillRect.IsEmpty())
-      return NS_OK;
+    dirty = saveMatrix.Matrix().Transform(dirty);
 
     ctx->IdentityMatrix();
   }
-  // If we're not snapping, then we ignore the dirty rect. It's hard
-  // to correctly use it with arbitrary transforms --- it really *has*
-  // to be aligned perfectly with pixel boundaries or the choice of
-  // dirty rect will affect the values of rendered pixels.
 
   gfxFloat scaleX = imageSize.width*appUnitsPerDevPixel/aDest.width;
   gfxFloat scaleY = imageSize.height*appUnitsPerDevPixel/aDest.height;
   if (didSnap) {
     // ctx now has the identity matrix, so we need to adjust our
     // scales to match
-    scaleX /= currentMatrix.xx;
-    scaleY /= currentMatrix.yy;
+    scaleX /= saveMatrix.Matrix().xx;
+    scaleY /= saveMatrix.Matrix().yy;
   }
   gfxFloat translateX = imageSpaceAnchorPoint.x - anchorPoint.x*scaleX;
   gfxFloat translateY = imageSpaceAnchorPoint.y - anchorPoint.y*scaleY;
   gfxMatrix transform(scaleX, 0, 0, scaleY, translateX, translateY);
 
+  gfxRect finalFillRect = fill;
+  // If the user-space-to-image-space transform is not a straight
+  // translation by integers, then filtering will occur, and
+  // restricting the fill rect to the dirty rect would change the values
+  // computed for edge pixels, which we can't allow.
+  // Also, if didSnap is false then rounding out 'dirty' might not
+  // produce pixel-aligned coordinates, which would also break the values
+  // computed for edge pixels.
+  if (didSnap && !transform.HasNonIntegerTranslation()) {
+    dirty.RoundOut();
+    finalFillRect = fill.Intersect(dirty);
+  }
+  if (finalFillRect.IsEmpty())
+    return NS_OK;
+
   nsIntRect innerRect;
   imgFrame->GetRect(innerRect);
   nsIntMargin padding(innerRect.x, innerRect.y,
     imageSize.width - innerRect.XMost(), imageSize.height - innerRect.YMost());
   img->Draw(ctx, transform, finalFillRect, padding, intSubimage);
-  ctx->SetMatrix(currentMatrix);
   return NS_OK;
 }