From 3888bf4b3a64b91eac832b6550bf1ab130a1f69c Mon Sep 17 00:00:00 2001 From: "roc+%cs.cmu.edu" Date: Fri, 7 Mar 2008 08:34:14 +0000 Subject: [PATCH] Bug 403181. Various fixes to image rendering. Most importantly, we track the desired subimage of a CSS background image and ensure we don't sample outside it. r=vlad,sr=dbaron --- gfx/cairo/README | 1 + gfx/cairo/libpixman/src/pixman-compose.c | 4 +- .../pixman-fbFetchTransformed-backout.patch | 0 gfx/public/nsIImage.h | 10 +- gfx/src/thebes/nsThebesImage.cpp | 110 ++++++++++++------ gfx/src/thebes/nsThebesImage.h | 1 + gfx/thebes/public/gfxRect.h | 8 +- gfx/thebes/src/gfxRect.cpp | 15 +++ layout/base/nsCSSRendering.cpp | 9 +- layout/base/nsLayoutUtils.cpp | 11 +- layout/base/nsLayoutUtils.h | 3 +- layout/reftests/bugs/412093-1-ref.html | 1 - layout/reftests/bugs/412093-1.html | 1 - layout/reftests/bugs/reftest.list | 3 +- widget/src/xpwidgets/nsBaseDragService.cpp | 2 +- 15 files changed, 130 insertions(+), 49 deletions(-) create mode 100644 gfx/cairo/pixman-fbFetchTransformed-backout.patch diff --git a/gfx/cairo/README b/gfx/cairo/README index 9d99d2862bc..438df4ec952 100644 --- a/gfx/cairo/README +++ b/gfx/cairo/README @@ -31,3 +31,4 @@ buggy-repeat.patch: Unconditionally turn on buggy-repeat handling to bandaid bug endian.patch: include cairo-platform.h for endian macros +pixman-fbFetchTransformed-backout.patch: back out bad pixman commit, see http://lists.cairographics.org/archives/cairo/2008-March/013294.html diff --git a/gfx/cairo/libpixman/src/pixman-compose.c b/gfx/cairo/libpixman/src/pixman-compose.c index f56885f5adb..7cb3d251c2b 100644 --- a/gfx/cairo/libpixman/src/pixman-compose.c +++ b/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 */ diff --git a/gfx/cairo/pixman-fbFetchTransformed-backout.patch b/gfx/cairo/pixman-fbFetchTransformed-backout.patch new file mode 100644 index 00000000000..e69de29bb2d diff --git a/gfx/public/nsIImage.h b/gfx/public/nsIImage.h index d02e31ad5ae..ff549e456b7 100644 --- a/gfx/public/nsIImage.h +++ b/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; /** diff --git a/gfx/src/thebes/nsThebesImage.cpp b/gfx/src/thebes/nsThebesImage.cpp index c5fea7c6efb..b857dc70a36 100644 --- a/gfx/src/thebes/nsThebesImage.cpp +++ b/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 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 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 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) diff --git a/gfx/src/thebes/nsThebesImage.h b/gfx/src/thebes/nsThebesImage.h index 975884bc46f..b0cfb4a2e77 100644 --- a/gfx/src/thebes/nsThebesImage.h +++ b/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, diff --git a/gfx/thebes/public/gfxRect.h b/gfx/thebes/public/gfxRect.h index 1971bf7238f..f20386b8f24 100644 --- a/gfx/thebes/public/gfxRect.h +++ b/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); } diff --git a/gfx/thebes/src/gfxRect.cpp b/gfx/thebes/src/gfxRect.cpp index 88da6a31200..c151b5fcb38 100644 --- a/gfx/thebes/src/gfxRect.cpp +++ b/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. */ diff --git a/layout/base/nsCSSRendering.cpp b/layout/base/nsCSSRendering.cpp index 2073851a45f..2af7b47c7b5 100644 --- a/layout/base/nsCSSRendering.cpp +++ b/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); } diff --git a/layout/base/nsLayoutUtils.cpp b/layout/base/nsLayoutUtils.cpp index d271fede78f..c91e51fe2ed 100644 --- a/layout/base/nsLayoutUtils.cpp +++ b/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 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 diff --git a/layout/base/nsLayoutUtils.h b/layout/base/nsLayoutUtils.h index 51c0afa30f7..f2fce61227b 100644 --- a/layout/base/nsLayoutUtils.h +++ b/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 diff --git a/layout/reftests/bugs/412093-1-ref.html b/layout/reftests/bugs/412093-1-ref.html index 52b204100a3..799b3372386 100644 --- a/layout/reftests/bugs/412093-1-ref.html +++ b/layout/reftests/bugs/412093-1-ref.html @@ -1,4 +1,3 @@ - diff --git a/layout/reftests/bugs/412093-1.html b/layout/reftests/bugs/412093-1.html index 082161717c1..9d1ca10ca2e 100644 --- a/layout/reftests/bugs/412093-1.html +++ b/layout/reftests/bugs/412093-1.html @@ -1,4 +1,3 @@ - diff --git a/layout/reftests/bugs/reftest.list b/layout/reftests/bugs/reftest.list index 362e870f291..20a78601e42 100644 --- a/layout/reftests/bugs/reftest.list +++ b/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 @@ -647,6 +647,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 diff --git a/widget/src/xpwidgets/nsBaseDragService.cpp b/widget/src/xpwidgets/nsBaseDragService.cpp index fe428ecd88b..11dd6357ac7 100644 --- a/widget/src/xpwidgets/nsBaseDragService.cpp +++ b/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