From 421ea5a76efe87bf066b03d57f450cbcbdf61416 Mon Sep 17 00:00:00 2001 From: "roc+@cs.cmu.edu" Date: Mon, 31 Mar 2008 02:40:53 -0700 Subject: [PATCH] Bug 421885. Make tiled image drawing sample only the correct subimage by manually padding if necessary. r=vlad --- gfx/public/nsIRenderingContext.h | 7 +- gfx/public/nsRect.h | 4 + gfx/src/nsRect.cpp | 11 ++ gfx/src/thebes/nsThebesImage.cpp | 115 ++++++++++++++++-- gfx/src/thebes/nsThebesImage.h | 1 + gfx/src/thebes/nsThebesRenderingContext.cpp | 19 ++- gfx/src/thebes/nsThebesRenderingContext.h | 2 +- gfx/thebes/public/gfxPoint.h | 5 + layout/base/nsCSSRendering.cpp | 18 +-- layout/reftests/bugs/reftest.list | 1 + .../reftests/bugs/square-left-right-32x32.png | Bin 0 -> 158 bytes .../reftests/bugs/square-top-bottom-32x32.png | Bin 0 -> 161 bytes .../xul/base/src/tree/src/nsTreeBodyFrame.cpp | 4 +- 13 files changed, 161 insertions(+), 26 deletions(-) create mode 100644 layout/reftests/bugs/square-left-right-32x32.png create mode 100644 layout/reftests/bugs/square-top-bottom-32x32.png diff --git a/gfx/public/nsIRenderingContext.h b/gfx/public/nsIRenderingContext.h index faf5a9197bfd..39e6a9279e6b 100644 --- a/gfx/public/nsIRenderingContext.h +++ b/gfx/public/nsIRenderingContext.h @@ -598,11 +598,14 @@ public: * @param aXImageStart x location where the origin (0,0) of the image starts * @param aYImageStart y location where the origin (0,0) of the image starts * @param aTargetRect area to draw to + * @param aSubimageRect the subimage (in tile space) which we expect to + * sample from; may be null to indicate that the whole image is + * OK to sample from */ NS_IMETHOD DrawTile(imgIContainer *aImage, nscoord aXImageStart, nscoord aYImageStart, - const nsRect * aTargetRect) = 0; - + const nsRect * aTargetRect, + const nsIntRect * aSubimageRect) = 0; /** * Get cluster details for a chunk of text. diff --git a/gfx/public/nsRect.h b/gfx/public/nsRect.h index 469afa68da1e..cf96e10f9278 100644 --- a/gfx/public/nsRect.h +++ b/gfx/public/nsRect.h @@ -177,6 +177,10 @@ struct NS_GFX nsRect { // Scale by aScale, converting coordinates to integers so that the result // is the smallest integer-coordinate rectangle containing the unrounded result nsRect& ScaleRoundOut(float aScale); + // Scale by the inverse of aScale, converting coordinates to integers so that the result + // is the smallest integer-coordinate rectangle containing the unrounded result. + // More accurate than ScaleRoundOut(1.0/aScale). + nsRect& ScaleRoundOutInverse(float aScale); // Scale by aScale, converting coordinates to integers so that the result // is the larges integer-coordinate rectangle contained in the unrounded result nsRect& ScaleRoundIn(float aScale); diff --git a/gfx/src/nsRect.cpp b/gfx/src/nsRect.cpp index 01e650620404..e7ba395672ca 100644 --- a/gfx/src/nsRect.cpp +++ b/gfx/src/nsRect.cpp @@ -185,6 +185,17 @@ nsRect& nsRect::ScaleRoundOut(float aScale) return *this; } +nsRect& nsRect::ScaleRoundOutInverse(float aScale) +{ + nscoord right = NSToCoordCeil(float(XMost()) / aScale); + nscoord bottom = NSToCoordCeil(float(YMost()) / aScale); + x = NSToCoordFloor(float(x) / aScale); + y = NSToCoordFloor(float(y) / aScale); + width = (right - x); + height = (bottom - y); + return *this; +} + // scale the rect but round to largest contained rect nsRect& nsRect::ScaleRoundIn(float aScale) { diff --git a/gfx/src/thebes/nsThebesImage.cpp b/gfx/src/thebes/nsThebesImage.cpp index b857dc70a36e..a32dd0b85e52 100644 --- a/gfx/src/thebes/nsThebesImage.cpp +++ b/gfx/src/thebes/nsThebesImage.cpp @@ -620,6 +620,7 @@ nsThebesImage::ThebesDrawTile(gfxContext *thebesContext, nsIDeviceContext* dx, const gfxPoint& offset, const gfxRect& targetRect, + const nsIntRect& aSubimageRect, const PRInt32 xPadding, const PRInt32 yPadding) { @@ -634,8 +635,9 @@ nsThebesImage::ThebesDrawTile(gfxContext *thebesContext, PRBool doSnap = !(thebesContext->CurrentMatrix().HasNonTranslation()); PRBool hasPadding = ((xPadding != 0) || (yPadding != 0)); - - nsRefPtr tmpSurfaceGrip; + gfxImageSurface::gfxImageFormat format = mFormat; + + gfxPoint tmpOffset = offset; if (mSinglePixel && !hasPadding) { thebesContext->SetColor(mSinglePixelColor); @@ -653,14 +655,13 @@ nsThebesImage::ThebesDrawTile(gfxContext *thebesContext, if (!AllowedImageSize(width, height)) return NS_ERROR_FAILURE; - surface = new gfxImageSurface(gfxIntSize(width, height), - gfxASurface::ImageFormatARGB32); + format = gfxASurface::ImageFormatARGB32; + surface = gfxPlatform::GetPlatform()->CreateOffscreenSurface( + gfxIntSize(width, height), format); if (!surface || surface->CairoStatus()) { return NS_ERROR_OUT_OF_MEMORY; } - tmpSurfaceGrip = surface; - gfxContext tmpContext(surface); if (mSinglePixel) { tmpContext.SetColor(mSinglePixelColor); @@ -675,17 +676,105 @@ nsThebesImage::ThebesDrawTile(gfxContext *thebesContext, height = mHeight; surface = ThebesSurface(); } - - gfxMatrix patMat; - gfxPoint p0; - - p0.x = - floor(offset.x + 0.5); - p0.y = - floor(offset.y + 0.5); + // Scale factor to account for CSS pixels; note that the offset (and // therefore p0) is in device pixels, while the width and height are in // CSS pixels. gfxFloat scale = gfxFloat(dx->AppUnitsPerDevPixel()) / gfxFloat(nsIDeviceContext::AppUnitsPerCSSPixel()); + + if ((aSubimageRect.width < width || aSubimageRect.height < height) && + (thebesContext->CurrentMatrix().HasNonTranslation() || scale != 1.0)) { + // Some of the source image should not be drawn, and we're going + // to be doing more than just translation, so we might accidentally + // sample the non-drawn pixels. Avoid that by creating a + // temporary image representing the portion that will be drawn, + // with built-in padding since we can't use EXTEND_PAD and + // EXTEND_REPEAT at the same time for different axes. + PRInt32 padX = aSubimageRect.width < width ? 1 : 0; + PRInt32 padY = aSubimageRect.height < height ? 1 : 0; + PRInt32 tileWidth = PR_MIN(aSubimageRect.width, width); + PRInt32 tileHeight = PR_MIN(aSubimageRect.height, height); + + // This tmpSurface will contain a snapshot of the repeated + // tile image at (aSubimageRect.x, aSubimageRect.y, + // tileWidth, tileHeight), with padX padding added to the left + // and right sides and padY padding added to the top and bottom + // sides. + nsRefPtr tmpSurface; + tmpSurface = gfxPlatform::GetPlatform()->CreateOffscreenSurface( + gfxIntSize(tileWidth + 2*padX, tileHeight + 2*padY), format); + if (!tmpSurface || tmpSurface->CairoStatus()) { + return NS_ERROR_OUT_OF_MEMORY; + } + + gfxContext tmpContext(tmpSurface); + tmpContext.SetOperator(gfxContext::OPERATOR_SOURCE); + gfxPattern pat(surface); + pat.SetExtend(gfxPattern::EXTEND_REPEAT); + + // Copy the needed portion of the source image to the temporary + // surface. We also copy over horizontal and/or vertical padding + // strips one pixel wide, plus the corner pixels if necessary. + // So in the most general case the temporary surface ends up + // looking like + // P P P ... P P P + // P X X ... X X P + // P X X ... X X P + // ............... + // P X X ... X X P + // P X X ... X X P + // P P P ... P P P + // Where each P pixel has the color of its nearest source X + // pixel. We implement this as a loop over all nine possible + // areas, [padding, body, padding] x [padding, body, padding]. + // Note that we will not need padding on both axes unless + // we are painting just a single tile, in which case this + // will hardly ever get called since nsCSSRendering converts + // the single-tile case to nsLayoutUtils::DrawImage. But this + // could be called on other paths (XUL trees?) and it's simpler + // and clearer to do it the general way. + PRInt32 destY = 0; + for (PRInt32 y = -1; y <= 1; ++y) { + PRInt32 stripHeight = y == 0 ? tileHeight : padY; + if (stripHeight == 0) + continue; + PRInt32 srcY = y == 1 ? aSubimageRect.YMost() - padY : aSubimageRect.y; + + PRInt32 destX = 0; + for (PRInt32 x = -1; x <= 1; ++x) { + PRInt32 stripWidth = x == 0 ? tileWidth : padX; + if (stripWidth == 0) + continue; + PRInt32 srcX = x == 1 ? aSubimageRect.XMost() - padX : aSubimageRect.x; + + gfxMatrix patMat; + patMat.Translate(gfxPoint(srcX - destX, srcY - destY)); + pat.SetMatrix(patMat); + tmpContext.SetPattern(&pat); + tmpContext.Rectangle(gfxRect(destX, destY, stripWidth, stripHeight)); + tmpContext.Fill(); + tmpContext.NewPath(); + + destX += stripWidth; + } + destY += stripHeight; + } + + // tmpOffset was the top-left of the old tile image. Make it + // the top-left of the new tile image. Note that tmpOffset is + // in destination coordinate space so we have to scale our + // CSS pixels. + tmpOffset += gfxPoint(aSubimageRect.x - padX, aSubimageRect.y - padY)/scale; + + surface = tmpSurface; + } + + gfxMatrix patMat; + gfxPoint p0; + + p0.x = - floor(tmpOffset.x + 0.5); + p0.y = - floor(tmpOffset.y + 0.5); patMat.Scale(scale, scale); patMat.Translate(p0); @@ -705,7 +794,7 @@ nsThebesImage::ThebesDrawTile(gfxContext *thebesContext, } gfxContext::GraphicsOperator op = thebesContext->CurrentOperator(); - if (op == gfxContext::OPERATOR_OVER && mFormat == gfxASurface::ImageFormatRGB24) + if (op == gfxContext::OPERATOR_OVER && format == gfxASurface::ImageFormatRGB24) thebesContext->SetOperator(gfxContext::OPERATOR_SOURCE); thebesContext->NewPath(); diff --git a/gfx/src/thebes/nsThebesImage.h b/gfx/src/thebes/nsThebesImage.h index b4646466128a..811facd1f481 100644 --- a/gfx/src/thebes/nsThebesImage.h +++ b/gfx/src/thebes/nsThebesImage.h @@ -84,6 +84,7 @@ public: nsIDeviceContext* dx, const gfxPoint& aOffset, const gfxRect& aTileRect, + const nsIntRect& aSubimageRect, const PRInt32 aXPadding, const PRInt32 aYPadding); diff --git a/gfx/src/thebes/nsThebesRenderingContext.cpp b/gfx/src/thebes/nsThebesRenderingContext.cpp index d3b9c583be82..e54d1eed4aef 100644 --- a/gfx/src/thebes/nsThebesRenderingContext.cpp +++ b/gfx/src/thebes/nsThebesRenderingContext.cpp @@ -778,7 +778,8 @@ nsThebesRenderingContext::PopFilter() NS_IMETHODIMP nsThebesRenderingContext::DrawTile(imgIContainer *aImage, nscoord twXOffset, nscoord twYOffset, - const nsRect *twTargetRect) + const nsRect *twTargetRect, + const nsIntRect *subimageRect) { PR_LOG(gThebesGFXLog, PR_LOG_DEBUG, ("## %p nsTRC::DrawTile %p %f %f [%f,%f,%f,%f]\n", this, aImage, FROM_TWIPS(twXOffset), FROM_TWIPS(twYOffset), @@ -813,18 +814,34 @@ nsThebesRenderingContext::DrawTile(imgIContainer *aImage, PRInt32 xPadding = 0; PRInt32 yPadding = 0; + nsIntRect tmpSubimageRect; + if (subimageRect) { + tmpSubimageRect = *subimageRect; + } else { + tmpSubimageRect = nsIntRect(0, 0, containerWidth, containerHeight); + } + if (imgFrameRect.width != containerWidth || imgFrameRect.height != containerHeight) { xPadding = containerWidth - imgFrameRect.width; yPadding = containerHeight - imgFrameRect.height; + // XXXroc shouldn't we be adding to 'phase' here? it's tbe origin + // at which the image origin should be drawn, and ThebesDrawTile + // just draws the origin of its "frame" there, so we should be + // adding imgFrameRect.x/y. so that the imgFrame draws in the + // right place. phase.x -= imgFrameRect.x; phase.y -= imgFrameRect.y; + + tmpSubimageRect.x -= imgFrameRect.x; + tmpSubimageRect.y -= imgFrameRect.y; } return thebesImage->ThebesDrawTile (mThebes, mDeviceContext, phase, GFX_RECT_FROM_TWIPS_RECT(*twTargetRect), + tmpSubimageRect, xPadding, yPadding); } diff --git a/gfx/src/thebes/nsThebesRenderingContext.h b/gfx/src/thebes/nsThebesRenderingContext.h index e7337caa678f..9d192668f0c1 100644 --- a/gfx/src/thebes/nsThebesRenderingContext.h +++ b/gfx/src/thebes/nsThebesRenderingContext.h @@ -185,7 +185,7 @@ public: NS_IMETHOD SetTranslation(nscoord aX, nscoord aY); NS_IMETHOD DrawTile(imgIContainer *aImage, nscoord aXOffset, nscoord aYOffset, - const nsRect * aTargetRect); + const nsRect * aTargetRect, const nsIntRect * aSubimageRect); NS_IMETHOD SetRightToLeftText(PRBool aIsRTL); NS_IMETHOD GetRightToLeftText(PRBool* aIsRTL); virtual void SetTextRunRTL(PRBool aIsRTL); diff --git a/gfx/thebes/public/gfxPoint.h b/gfx/thebes/public/gfxPoint.h index 68ea3062f925..bd76ed074228 100644 --- a/gfx/thebes/public/gfxPoint.h +++ b/gfx/thebes/public/gfxPoint.h @@ -120,6 +120,11 @@ struct THEBES_API gfxPoint { int operator!=(const gfxPoint& p) const { return ((x != p.x) || (y != p.y)); } + const gfxPoint& operator+=(const gfxPoint& p) { + x += p.x; + y += p.y; + return *this; + } gfxPoint operator+(const gfxPoint& p) const { return gfxPoint(x + p.x, y + p.y); } diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 2af7b47c7b56..64fc82869e40 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/layout/base/nsCSSRendering.cpp @@ -3904,19 +3904,23 @@ nsCSSRendering::PaintBackgroundWithSC(nsPresContext* aPresContext, "Bogus y coord for draw rect"); // Figure out whether we can get away with not tiling at all. nsRect sourceRect = drawRect - absTileRect.TopLeft(); + // Compute 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(); 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. - // 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); + // Note that the subimage is in tile space so it may cover + // multiple tiles of the image. + subimageRect.ScaleRoundOutInverse(nsIDeviceContext::AppUnitsPerCSSPixel()); + aRenderingContext.DrawTile(image, absTileRect.x, absTileRect.y, + &drawRect, &subimageRect); } } diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index 930afef3a2df..ca702f72e1df 100644 --- a/layout/reftests/bugs/reftest.list +++ b/layout/reftests/bugs/reftest.list @@ -773,6 +773,7 @@ fails == 413027-3.html 413027-3-ref.html == 421069-ref.html 421069-ref2.html == 421234-1.html 421234-1-ref.html == 421419-1.html 421419-1-ref.html +== 421885-1.xml 421885-1-ref.xml == 421955-1.html 421955-1-ref.html == 422394-1.html 422394-1-ref.html == 423130-1.html 423130-1-ref.html diff --git a/layout/reftests/bugs/square-left-right-32x32.png b/layout/reftests/bugs/square-left-right-32x32.png new file mode 100644 index 0000000000000000000000000000000000000000..d088179e88073ee2ce65e6384e8c02a71de6b311 GIT binary patch literal 158 zcmeAS@N?(olHy`uVBq!ia0vp^3LwnE1SJ1Ryj={W*pj^6U4S$Y{B+)352QE?JR*x3 z7`TN&n2}-D90{Nxdx@v7EBg&*DF$}7H}g+f1BJ9bT^vI^j=w!?$jD&8!@MBnpY3+F vYy3PHRE#G}=f?MOvM@C|CgTe~DWM4f2vsFJ literal 0 HcmV?d00001 diff --git a/layout/reftests/bugs/square-top-bottom-32x32.png b/layout/reftests/bugs/square-top-bottom-32x32.png new file mode 100644 index 0000000000000000000000000000000000000000..3de5214edd1c8c3389d6cf02d54e7956b8ce741b GIT binary patch literal 161 zcmeAS@N?(olHy`uVBq!ia0vp^3LwnE1SJ1Ryj={W*pj^6U4S$Y{B+)352QE?JR*x3 z7`TN&n2}-D90{Nxdx@v7EBg&*DF${HrirV?fkJwoE{-7{$KRfJ6^BbrL0(LN6`Q4ksY-xHv>K{nN)78&qol`;+0E(a}A^-pY literal 0 HcmV?d00001 diff --git a/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp b/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp index 5e8df62c8d98..3d30e58b3b3d 100644 --- a/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp +++ b/layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp @@ -3657,7 +3657,7 @@ nsTreeBodyFrame::PaintProgressMeter(PRInt32 aRowIndex, nsCOMPtr image; GetImage(aRowIndex, aColumn, PR_TRUE, meterContext, useImageRegion, getter_AddRefs(image)); if (image) - aRenderingContext.DrawTile(image, 0, 0, &meterRect); + aRenderingContext.DrawTile(image, 0, 0, &meterRect, nsnull); else aRenderingContext.FillRect(meterRect); } @@ -3669,7 +3669,7 @@ nsTreeBodyFrame::PaintProgressMeter(PRInt32 aRowIndex, nsCOMPtr image; GetImage(aRowIndex, aColumn, PR_TRUE, meterContext, useImageRegion, getter_AddRefs(image)); if (image) - aRenderingContext.DrawTile(image, 0, 0, &meterRect); + aRenderingContext.DrawTile(image, 0, 0, &meterRect, nsnull); } }