Bug 403181. Pass the desired source rectangle for background images down into nsThebesImage and ensure we don't sample outside it. Back out a bad pixman patch that caused regressions, and adjust image encoding tests so they don't depend on the regression. Also switchs some reftests to quirks mode so that the 1-appunit width of <br>s doesn't bite us. r=vlad,sr=dbaron

gfx/cairo/libpixman/src/pixman-compose.c

@@ -4192,8 +4192,8 @@ fbFetchTransformed(bits_image_t * pict, int x, int y, int width, uint32_t *buffe
     stride = pict->rowstride;
 
     /* reference point is the center of the pixel */
-    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2 - 1;
-    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2 - 1;
+    v.vector[0] = pixman_int_to_fixed(x) + pixman_fixed_1 / 2;
+    v.vector[1] = pixman_int_to_fixed(y) + pixman_fixed_1 / 2;
     v.vector[2] = pixman_fixed_1;
 
     /* when using convolution filters one might get here without a transform */

gfx/public/nsIImage.h

@@ -72,10 +72,10 @@ typedef enum {
 #define  nsImageUpdateFlags_kBitsChanged     0x2
 
 // IID for the nsIImage interface
-// fd31e1f2-bd46-47f1-b8b6-b94ce954f9ce
+// 96d9d7ce-e575-4265-8507-35555112a430
 #define NS_IIMAGE_IID \
-{ 0xfd31e1f2, 0xbd46, 0x47f1, \
-  { 0xb8, 0xb6, 0xb9, 0x4c, 0xe9, 0x54, 0xf9, 0xce } }
+{ 0x96d9d7ce, 0xe575, 0x4265, \
+  { 0x85, 0x07, 0x35, 0x55, 0x51, 0x12, 0xa4, 0x30 } }
 
 // Interface to Images
 class nsIImage : public nsISupports
@@ -190,10 +190,14 @@ public:
   /**
    * BitBlit the nsIImage to a device, the source and dest can be scaled
    * @param aSourceRect  source rectangle, in image pixels
+   * @param aSubimageRect the subimage that we're extracting the contents from.
+   * It must contain aSourceRect. Pixels outside this rectangle must not
+   * be sampled.
    * @param aDestRect  destination rectangle, in device pixels
    */
   NS_IMETHOD Draw(nsIRenderingContext &aContext,
                   const gfxRect &aSourceRect,
+                  const gfxRect &aSubimageRect,
                   const gfxRect &aDestRect) = 0;
 
   /**

gfx/src/thebes/nsThebesImage.cpp

@@ -412,6 +412,7 @@ nsThebesImage::UnlockImagePixels(PRBool aMaskPixels)
 NS_IMETHODIMP
 nsThebesImage::Draw(nsIRenderingContext &aContext,
                     const gfxRect &aSourceRect,
+                    const gfxRect &aSubimageRect,
                     const gfxRect &aDestRect)
 {
     if (NS_UNLIKELY(aDestRect.IsEmpty())) {
@@ -452,21 +453,24 @@ nsThebesImage::Draw(nsIRenderingContext &aContext,
     gfxFloat yscale = aDestRect.size.height / aSourceRect.size.height;
 
     gfxRect srcRect(aSourceRect);
+    gfxRect subimageRect(aSubimageRect);
     gfxRect destRect(aDestRect);
 
     if (!GetIsImageComplete()) {
-      srcRect = srcRect.Intersect(gfxRect(mDecoded.x, mDecoded.y,
-                                          mDecoded.width, mDecoded.height));
+        gfxRect decoded = gfxRect(mDecoded.x, mDecoded.y,
+                                  mDecoded.width, mDecoded.height);
+        srcRect = srcRect.Intersect(decoded);
+        subimageRect = subimageRect.Intersect(decoded);
 
-      // This happens when mDecoded.width or height is zero. bug 368427.
-      if (NS_UNLIKELY(srcRect.size.width == 0 || srcRect.size.height == 0))
-          return NS_OK;
+        // This happens when mDecoded.width or height is zero. bug 368427.
+        if (NS_UNLIKELY(srcRect.size.width == 0 || srcRect.size.height == 0))
+            return NS_OK;
 
-      destRect.pos.x += (srcRect.pos.x - aSourceRect.pos.x)*xscale;
-      destRect.pos.y += (srcRect.pos.y - aSourceRect.pos.y)*yscale;
+        destRect.pos.x += (srcRect.pos.x - aSourceRect.pos.x)*xscale;
+        destRect.pos.y += (srcRect.pos.y - aSourceRect.pos.y)*yscale;
 
-      destRect.size.width  = srcRect.size.width * xscale;
-      destRect.size.height = srcRect.size.height * yscale;
+        destRect.size.width  = srcRect.size.width * xscale;
+        destRect.size.height = srcRect.size.height * yscale;
     }
 
     // if either rectangle is empty now (possibly after the image complete check)
@@ -477,7 +481,39 @@ nsThebesImage::Draw(nsIRenderingContext &aContext,
     if (!AllowedImageSize(destRect.size.width + 1, destRect.size.height + 1))
         return NS_ERROR_FAILURE;
 
+    // Expand the subimageRect to place its edges on integer coordinates.
+    // Basically, if we're allowed to sample part of a pixel we can
+    // sample the whole pixel.
+    subimageRect.RoundOut();
+
     nsRefPtr<gfxPattern> pat;
+    PRBool ctxHasNonTranslation = ctx->CurrentMatrix().HasNonTranslation();
+    if ((xscale == 1.0 && yscale == 1.0 && !ctxHasNonTranslation) ||
+        subimageRect == gfxRect(0, 0, mWidth, mHeight))
+    {
+        // No need to worry about sampling outside the subimage rectangle,
+        // so no need for a temporary
+        // XXX should we also check for situations where the source rect
+        // is well inside the subimage so we can't sample outside?
+        pat = new gfxPattern(ThebesSurface());
+    } else {
+        // Because of the RoundOut above, the subimageRect has
+        // integer width and height.
+        gfxIntSize size(PRInt32(subimageRect.Width()),
+                        PRInt32(subimageRect.Height()));
+        nsRefPtr<gfxASurface> temp =
+            gfxPlatform::GetPlatform()->CreateOffscreenSurface(size, mFormat);
+        if (!temp || temp->CairoStatus() != 0)
+            return NS_ERROR_FAILURE;
+
+        gfxContext tempctx(temp);
+        tempctx.SetSource(ThebesSurface(), -subimageRect.pos);
+        tempctx.SetOperator(gfxContext::OPERATOR_SOURCE);
+        tempctx.Paint();
+
+        pat = new gfxPattern(temp);
+        srcRect.MoveBy(-subimageRect.pos);
+    }
 
     /* See bug 364968 to understand the necessity of this goop; we basically
      * have to pre-downscale any image that would fall outside of a scaled 16-bit
@@ -500,13 +536,12 @@ nsThebesImage::Draw(nsIRenderingContext &aContext,
 
         gfxContext tempctx(temp);
 
-        gfxPattern srcpat(ThebesSurface());
         gfxMatrix mat;
         mat.Translate(srcRect.pos);
         mat.Scale(1.0 / xscale, 1.0 / yscale);
-        srcpat.SetMatrix(mat);
+        pat->SetMatrix(mat);
 
-        tempctx.SetPattern(&srcpat);
+        tempctx.SetPattern(pat);
         tempctx.SetOperator(gfxContext::OPERATOR_SOURCE);
         tempctx.NewPath();
         tempctx.Rectangle(gfxRect(0.0, 0.0, dim.width, dim.height));
@@ -523,10 +558,6 @@ nsThebesImage::Draw(nsIRenderingContext &aContext,
         yscale = 1.0;
     }
 
-    if (!pat) {
-        pat = new gfxPattern(ThebesSurface());
-    }
-
     gfxMatrix mat;
     mat.Translate(srcRect.pos);
     mat.Scale(1.0/xscale, 1.0/yscale);
@@ -538,25 +569,36 @@ nsThebesImage::Draw(nsIRenderingContext &aContext,
 
     pat->SetMatrix(mat);
 
-#if !defined(XP_MACOSX) && !defined(XP_WIN) && !defined(XP_OS2)
-    // See bug 324698.  This is a workaround.
-    //
-    // Set the filter to CAIRO_FILTER_FAST if we're scaling up -- otherwise,
-    // pixman's sampling will sample transparency for the outside edges and we'll
-    // get blurry edges.  CAIRO_EXTEND_PAD would also work here, if
-    // available
-    //
-    // This effectively disables smooth upscaling for images.
-    if (xscale > 1.0 || yscale > 1.0)
-        pat->SetFilter(0);
-#endif
+    nsRefPtr<gfxASurface> target = ctx->CurrentSurface();
+    switch (target->GetType()) {
+    case gfxASurface::SurfaceTypeXlib:
+    case gfxASurface::SurfaceTypeXcb:
+        // See bug 324698.  This is a workaround for EXTEND_PAD not being
+        // implemented correctly on linux in the X server.
+        //
+        // Set the filter to CAIRO_FILTER_FAST if we're scaling up -- otherwise,
+        // pixman's sampling will sample transparency for the outside edges and we'll
+        // get blurry edges.  CAIRO_EXTEND_PAD would also work here, if
+        // available
+        //
+        // This effectively disables smooth upscaling for images.
+        if (xscale > 1.0 || yscale > 1.0 || ctxHasNonTranslation)
+            pat->SetFilter(0);
+        break;
 
-#if defined(XP_WIN) || defined(XP_OS2)
-    // turn on EXTEND_PAD only for win32, and only when scaling;
-    // it's not implemented correctly on linux in the X server.
-    if (xscale != 1.0 || yscale != 1.0)
-        pat->SetExtend(gfxPattern::EXTEND_PAD);
-#endif
+    case gfxASurface::SurfaceTypeQuartz:
+    case gfxASurface::SurfaceTypeQuartzImage:
+        // Do nothing, Mac seems to be OK. Really?
+        break;
+
+    default:
+        // turn on EXTEND_PAD.
+        // This is what we really want for all surface types, if the
+        // implementation was universally good.
+        if (xscale != 1.0 || yscale != 1.0 || ctxHasNonTranslation)
+            pat->SetExtend(gfxPattern::EXTEND_PAD);
+        break;
+    }
 
     gfxContext::GraphicsOperator op = ctx->CurrentOperator();
     if (op == gfxContext::OPERATOR_OVER && mFormat == gfxASurface::ImageFormatRGB24)

gfx/src/thebes/nsThebesImage.h

@@ -77,6 +77,7 @@ public:
 
     NS_IMETHOD Draw(nsIRenderingContext &aContext,
                     const gfxRect &aSourceRect,
+                    const gfxRect &aSubimageRect,
                     const gfxRect &aDestRect);
 
     nsresult ThebesDrawTile(gfxContext *thebesContext,

gfx/thebes/public/gfxRect.h

@@ -115,7 +115,9 @@ struct THEBES_API gfxRect {
         Outset(sides[0], sides[1], sides[2], sides[3]);
     }
 
-    // Round the rectangle to integer coordinates.
+    // Round the rectangle edges to integer coordinates, such that the rounded
+    // rectangle has the same set of pixel centers as the original rectangle.
+    // Edges at offset 0.5 round up.
     // Suitable for most places where integral device coordinates
     // are needed, but note that any translation should be applied first to
     // avoid pixel rounding errors.
@@ -125,6 +127,10 @@ struct THEBES_API gfxRect {
     // If you need similar method which is using NS_round(), you should create
     // new |RoundAwayFromZero()| method.
     void Round();
+    
+    // Snap the rectangle edges to integer coordinates, such that the
+    // resulting rectangle contains the original rectangle.
+    void RoundOut();
 
     // grabbing specific points
     gfxPoint TopLeft() const { return gfxPoint(pos); }

gfx/thebes/src/gfxRect.cpp

@@ -89,6 +89,21 @@ gfxRect::Round()
     size.height = y1 - y0;
 }
 
+void
+gfxRect::RoundOut()
+{
+    gfxFloat x0 = NS_floor(X());
+    gfxFloat y0 = NS_floor(Y());
+    gfxFloat x1 = NS_ceil(XMost());
+    gfxFloat y1 = NS_ceil(YMost());
+
+    pos.x = x0;
+    pos.y = y0;
+
+    size.width = x1 - x0;
+    size.height = y1 - y0;
+}
+
 /* Clamp r to CAIRO_COORD_MIN .. CAIRO_COORD_MAX
  * these are to be device coordinates.
  */

layout/base/nsCSSRendering.cpp

@@ -3907,7 +3907,14 @@ nsCSSRendering::PaintBackgroundWithSC(nsPresContext* aPresContext,
     if (sourceRect.XMost() <= tileWidth && sourceRect.YMost() <= tileHeight) {
       // The entire drawRect is contained inside a single tile; just
       // draw the corresponding part of the image once.
-      nsLayoutUtils::DrawImage(&aRenderingContext, image, absTileRect, drawRect);
+      // Pass in the subimage rectangle that we expect to be sampled.
+      // This is the tile rectangle, clipped to the bgClipArea, and then
+      // passed in relative to the image top-left.
+      nsRect destRect; // The rectangle we would draw ignoring dirty-rect
+      destRect.IntersectRect(absTileRect, bgClipArea);
+      nsRect subimageRect = destRect - aBorderArea.TopLeft() - tileRect.TopLeft();
+      nsLayoutUtils::DrawImage(&aRenderingContext, image,
+              destRect, drawRect, &subimageRect);
     } else {
       aRenderingContext.DrawTile(image, absTileRect.x, absTileRect.y, &drawRect);
     }

layout/base/nsLayoutUtils.cpp

@@ -2314,6 +2314,7 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
     pxSrc.size.width = gfxFloat(w);
     pxSrc.size.height = gfxFloat(h);
   }
+  gfxRect pxSubimage = pxSrc;
 
   nsCOMPtr<nsIDeviceContext> dc;
   aRenderingContext->GetDeviceContext(*getter_AddRefs(dc));
@@ -2375,7 +2376,9 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
   imgFrame->GetRect(pxImgFrameRect);
 
   if (pxImgFrameRect.x > 0) {
-    pxSrc.pos.x -= gfxFloat(pxImgFrameRect.x);
+    gfxFloat fx(pxImgFrameRect.x);
+    pxSubimage.pos.x -= fx;
+    pxSrc.pos.x -= fx;
 
     gfxFloat scaled_x = pxSrc.pos.x;
     if (pxDirty.size.width != pxSrc.size.width) {
@@ -2396,7 +2399,9 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
   }
 
   if (pxImgFrameRect.y > 0) {
-    pxSrc.pos.y -= gfxFloat(pxImgFrameRect.y);
+    gfxFloat fy(pxImgFrameRect.y);
+    pxSubimage.pos.y -= fy;
+    pxSrc.pos.y -= fy;
 
     gfxFloat scaled_y = pxSrc.pos.y;
     if (pxDirty.size.height != pxSrc.size.height) {
@@ -2416,7 +2421,7 @@ nsLayoutUtils::DrawImage(nsIRenderingContext* aRenderingContext,
     return NS_OK;
   }
 
-  return img->Draw(*aRenderingContext, pxSrc, pxDirty);
+  return img->Draw(*aRenderingContext, pxSrc, pxSubimage, pxDirty);
 }
 
 void

layout/base/nsLayoutUtils.h

@@ -715,7 +715,8 @@ public:
    *   @param aDestRect         Where to draw the image (app units).
    *   @param aDirtyRect        Draw only within this region (rounded to the
    *                            nearest pixel); the intersection of
-   *                            invalidation and clipping.
+   *                            invalidation and clipping (this is the
+   *                            destination clip)
    *   @param aSourceRect       If null, draw the entire image so it fits in
    *                            aDestRect.  If non-null, the subregion of the
    *                            image that should be drawn (in app units, such

layout/reftests/bugs/412093-1-ref.html

@@ -1,4 +1,3 @@
-<!DOCTYPE html>
 <html>
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">

layout/reftests/bugs/412093-1.html

@@ -1,4 +1,3 @@
-<!DOCTYPE html>
 <html>
 <head>
   <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">

layout/reftests/bugs/reftest.list

@@ -420,7 +420,7 @@ skip-if(MOZ_WIDGET_TOOLKIT=="windows") == 347496-1.xhtml 347496-1-ref.xhtml # Bu
 == 367612-1f.html 367612-1-ref.html
 != 367612-1g.html 367612-1-ref.html
 == 368020-1.html 368020-1-ref.html
-random-if(MOZ_WIDGET_TOOLKIT=="gtk2") == 368020-2.html 368020-2-ref.html # bug 368157 (gtk)
+random == 368020-2.html 368020-2-ref.html # bug 368157 (gtk), bug 403181
 fails == 368020-3.html 368020-3-ref.html # bug 368085
 fails == 368020-4.html 368020-4-ref.html # bug 368085
 random-if(MOZ_WIDGET_TOOLKIT=="gtk2") == 368020-5.html 368020-5-ref.html # bug 388591
@@ -651,6 +651,7 @@ skip-if(MOZ_WIDGET_TOOLKIT!="windows") == 391045.html 391045-ref.html # windows-
 == 403129-3.html 403129-3-ref.html
 == 403129-4.html 403129-4-ref.html
 random == 403134-1.html 403134-1-ref.html # bug 405377
+== 403181-1.xml 403181-1-ref.xml
 == 403249-1a.html 403249-1-ref.html
 == 403249-1b.html 403249-1-ref.html
 == 403249-2a.html 403249-2-ref.html

modules/libpr0n/test/unit/test_imgtools.js

@@ -296,7 +296,7 @@ encodedBytes = streamToArray(istream);
 refName = "image3ico32x32.png";
 refFile = do_get_file(TESTDIR + refName);
 istream = getFileInputStream(refFile);
-do_check_eq(istream.available(), 2372);
+do_check_eq(istream.available(), 2281);
 referenceBytes = streamToArray(istream);
 
 // compare the encoder's output to the reference file.

widget/src/xpwidgets/nsBaseDragService.cpp

@@ -566,7 +566,7 @@ nsBaseDragService::DrawDragForImage(nsPresContext* aPresContext,
 
   gfxRect inRect = gfxRect(srcRect.x, srcRect.y, srcRect.width, srcRect.height);
   gfxRect outRect = gfxRect(destRect.x, destRect.y, destRect.width, destRect.height);
-  return img->Draw(*rc, inRect, outRect);
+  return img->Draw(*rc, inRect, inRect, outRect);
 }
 
 void