Bug 489389. Limit size of temporary surface for tiling based on current clip extents. r=vlad
authorRobert O'Callahan <robert@ocallahan.org>
Fri, 08 May 2009 15:19:41 +1200
changeset 28115 26c4c46fcaac14ede31283854e2bb830c7dece99
parent 28114 c9db288cd5cca9ff3b7a58e57c763b95909be2ba
child 28116 7db881363a22822bbadcbe1f5c400069ebbd40ac
push id6896
push userrocallahan@mozilla.com
push dateFri, 08 May 2009 03:22:56 +0000
treeherdermozilla-central@c97e93f23f89 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersvlad
bugs489389
milestone1.9.2a1pre
Bug 489389. Limit size of temporary surface for tiling based on current clip extents. r=vlad
gfx/src/thebes/nsThebesImage.cpp
gfx/thebes/public/gfxContext.h
--- a/gfx/src/thebes/nsThebesImage.cpp
+++ b/gfx/src/thebes/nsThebesImage.cpp
@@ -615,37 +615,56 @@ nsThebesImage::Draw(gfxContext*        a
         if (doTile) {
             pattern->SetExtend(gfxPattern::EXTEND_REPEAT);
         }
     } else {
         if (doTile || !subimage.Contains(imageRect)) {
             // EXTEND_PAD won't help us here; we have to create a temporary
             // surface to hold the subimage of pixels we're allowed to
             // sample
-            gfxRect needed = subimage.Intersect(sourceRect);
+
+            gfxRect userSpaceClipExtents = aContext->GetClipExtents();
+            // This isn't optimal --- if aContext has a rotation then
+            // GetClipExtents will have to do a bounding-box computation,
+            // and TransformBounds might too, so we could get a better
+            // result if we computed image space clip extents in one go
+            // --- but it doesn't really matter and this is easier to
+            // understand.
+            gfxRect imageSpaceClipExtents =
+              userSpaceToImageSpace.TransformBounds(userSpaceClipExtents);
+            // Inflate by one pixel because bilinear filtering will sample
+            // at most one pixel beyond the computed image pixel coordinate.
+            imageSpaceClipExtents.Outset(1.0);
+
+            gfxRect needed =
+              imageSpaceClipExtents.Intersect(sourceRect).Intersect(subimage);
             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) {
-                gfxContext tmpCtx(temp);
-                tmpCtx.SetOperator(gfxContext::OPERATOR_SOURCE);
-                nsRefPtr<gfxPattern> tmpPattern = new gfxPattern(surface);
-                if (tmpPattern) {
-                    tmpPattern->SetExtend(gfxPattern::EXTEND_REPEAT);
-                    tmpPattern->SetMatrix(gfxMatrix().Translate(needed.pos));
-                    tmpCtx.SetPattern(tmpPattern);
-                    tmpCtx.Paint();
-                    tmpPattern = new gfxPattern(temp);
+            // if 'needed' is empty, nothing will be drawn since aFill
+            // must be entirely outside the clip region, so it doesn't
+            // matter what we do here, but we should avoid trying to
+            // create a zero-size surface.
+            if (!needed.IsEmpty()) {
+                gfxIntSize size(PRInt32(needed.Width()), PRInt32(needed.Height()));
+                nsRefPtr<gfxASurface> temp =
+                    gfxPlatform::GetPlatform()->CreateOffscreenSurface(size, format);
+                if (temp && temp->CairoStatus() == 0) {
+                    gfxContext tmpCtx(temp);
+                    tmpCtx.SetOperator(gfxContext::OPERATOR_SOURCE);
+                    nsRefPtr<gfxPattern> tmpPattern = new gfxPattern(surface);
                     if (tmpPattern) {
-                        pattern.swap(tmpPattern);
-                        pattern->SetMatrix(
-                            gfxMatrix(userSpaceToImageSpace).Multiply(gfxMatrix().Translate(-needed.pos)));
+                        tmpPattern->SetExtend(gfxPattern::EXTEND_REPEAT);
+                        tmpPattern->SetMatrix(gfxMatrix().Translate(needed.pos));
+                        tmpCtx.SetPattern(tmpPattern);
+                        tmpCtx.Paint();
+                        tmpPattern = new gfxPattern(temp);
+                        if (tmpPattern) {
+                            pattern.swap(tmpPattern);
+                            pattern->SetMatrix(
+                                gfxMatrix(userSpaceToImageSpace).Multiply(gfxMatrix().Translate(-needed.pos)));
+                        }
                     }
                 }
             }
         }
   
         // In theory we can handle this using cairo's EXTEND_PAD,
         // but implementation limitations mean we have to consult
         // the surface type.
--- a/gfx/thebes/public/gfxContext.h
+++ b/gfx/thebes/public/gfxContext.h
@@ -573,17 +573,18 @@ public:
 
     /**
      * This will ensure that the surface actually has its clip set.
      * Useful if you are doing native drawing.
      */
     void UpdateSurfaceClip();
 
     /**
-     * This will return the current bounds of the clip region.
+     * This will return the current bounds of the clip region in user
+     * space.
      */
     gfxRect GetClipExtents();
 
     /**
      * Groups
      */
     void PushGroup(gfxASurface::gfxContentType content = gfxASurface::CONTENT_COLOR);
     already_AddRefed<gfxPattern> PopGroup();