b=366548, increased GDI usage leading to repainting problems, r/a=stuart

gfx/cairo/cairo/src/cairo-win32-surface.c

@@ -358,11 +358,7 @@ _cairo_win32_surface_create_for_dc (HDC             original_dc,
     surface->clip_rect.width = width;
     surface->clip_rect.height = height;
 
-    surface->saved_clip = CreateRectRgn (0, 0, 0, 0);
+    surface->saved_clip = NULL;
-    if (GetClipRgn (surface->dc, surface->saved_clip) == 0) {
-	DeleteObject(surface->saved_clip);
-	surface->saved_clip = NULL;
-    }
 
     surface->extents = surface->clip_rect;
 
@@ -409,13 +405,9 @@ _cairo_win32_surface_create_similar_internal (void	    *abstract_src,
 	/* otherwise, create a ddb */
 	HBITMAP ddb = CreateCompatibleBitmap (src->dc, width, height);
 	HDC ddb_dc = CreateCompatibleDC (src->dc);
-	HRGN crgn = CreateRectRgn (0, 0, width, height);
 	HBITMAP saved_dc_bitmap;
 
 	saved_dc_bitmap = SelectObject (ddb_dc, ddb);
-	SelectClipRgn (ddb_dc, crgn);
-
-	DeleteObject (crgn);
 
 	new_surf = (cairo_win32_surface_t*) cairo_win32_surface_create (ddb_dc);
 	new_surf->bitmap = ddb;
@@ -486,16 +478,33 @@ _cairo_win32_surface_finish (void *abstract_surface)
     if (surface->image)
 	cairo_surface_destroy (surface->image);
 
-    if (surface->saved_clip)
-	DeleteObject (surface->saved_clip);
-
     /* If we created the Bitmap and DC, destroy them */
     if (surface->bitmap) {
 	SelectObject (surface->dc, surface->saved_dc_bitmap);
 	DeleteObject (surface->bitmap);
 	DeleteDC (surface->dc);
+    } else {
+	/* otherwise, restore the old clip region on the DC */
+	SelectClipRgn (surface->dc, surface->saved_clip);
+
+	if (surface->saved_clip == NULL) {
+	    /* We never had a clip region, so just restore the clip
+	     * to the bounds. */
+	    if (surface->clip_rect.width != 0 &&
+		surface->clip_rect.height != 0)
+	    {
+		IntersectClipRect (surface->dc,
+				   surface->clip_rect.x,
+				   surface->clip_rect.y,
+				   surface->clip_rect.x + surface->clip_rect.width,
+				   surface->clip_rect.y + surface->clip_rect.height);
+	    }
+	}
     }
 
+    if (surface->saved_clip)
+	DeleteObject (surface->saved_clip);
+
     return CAIRO_STATUS_SUCCESS;
 }
 
@@ -1463,54 +1472,72 @@ _cairo_win32_surface_set_clip_region (void           *abstract_surface,
 	if (_cairo_region_get_boxes (region, &num_boxes, &boxes) != CAIRO_STATUS_SUCCESS)
 	    return CAIRO_STATUS_NO_MEMORY;
 
-	data_size = sizeof (RGNDATAHEADER) + num_boxes * sizeof (RECT);
+	if (num_boxes == 1 && 
-	data = malloc (data_size);
+	    boxes[0].p1.x == 0 &&
-	if (!data) {
+	    boxes[0].p1.y == 0 &&
-	    _cairo_region_boxes_fini (region, boxes);
+	    boxes[0].p2.x == surface->extents.width &&
-	    return CAIRO_STATUS_NO_MEMORY;
+	    boxes[0].p2.y == surface->extents.height)
-	}
+	{
-	rects = (RECT *)data->Buffer;
+	    gdi_region = NULL;
 
-	data->rdh.dwSize = sizeof (RGNDATAHEADER);
+	    SelectClipRgn (surface->dc, NULL);
-	data->rdh.iType = RDH_RECTANGLES;
+	    IntersectClipRect (surface->dc,
-	data->rdh.nCount = num_boxes;
+			       boxes[0].p1.x,
-	data->rdh.nRgnSize = num_boxes * sizeof (RECT);
+			       boxes[0].p1.y,
-	data->rdh.rcBound.left = extents.x;
+			       boxes[0].p2.x,
-	data->rdh.rcBound.top = extents.y;
+			       boxes[0].p2.y);
-	data->rdh.rcBound.right = extents.x + extents.width;
+	} else {
-	data->rdh.rcBound.bottom = extents.y + extents.height;
+	    data_size = sizeof (RGNDATAHEADER) + num_boxes * sizeof (RECT);
+	    data = malloc (data_size);
+	    if (!data) {
+		_cairo_region_boxes_fini (region, boxes);
+		return CAIRO_STATUS_NO_MEMORY;
+	    }
+	    rects = (RECT *)data->Buffer;
 
-	for (i = 0; i < num_boxes; i++) {
+	    data->rdh.dwSize = sizeof (RGNDATAHEADER);
-	    rects[i].left = boxes[i].p1.x;
+	    data->rdh.iType = RDH_RECTANGLES;
-	    rects[i].top = boxes[i].p1.y;
+	    data->rdh.nCount = num_boxes;
-	    rects[i].right = boxes[i].p2.x;
+	    data->rdh.nRgnSize = num_boxes * sizeof (RECT);
-	    rects[i].bottom = boxes[i].p2.y;
+	    data->rdh.rcBound.left = extents.x;
+	    data->rdh.rcBound.top = extents.y;
+	    data->rdh.rcBound.right = extents.x + extents.width;
+	    data->rdh.rcBound.bottom = extents.y + extents.height;
+
+	    for (i = 0; i < num_boxes; i++) {
+		rects[i].left = boxes[i].p1.x;
+		rects[i].top = boxes[i].p1.y;
+		rects[i].right = boxes[i].p2.x;
+		rects[i].bottom = boxes[i].p2.y;
+	    }
+
+	    gdi_region = ExtCreateRegion (NULL, data_size, data);
+	    free (data);
+
+	    if (!gdi_region)
+		return CAIRO_STATUS_NO_MEMORY;
+
+	    /* Combine the new region with the original clip */
+	    if (surface->saved_clip) {
+		if (CombineRgn (gdi_region, gdi_region, surface->saved_clip, RGN_AND) == ERROR)
+		    goto FAIL;
+	    }
+
+	    if (SelectClipRgn (surface->dc, gdi_region) == ERROR)
+		goto FAIL;
+
+	    DeleteObject (gdi_region);
 	}
 
 	_cairo_region_boxes_fini (region, boxes);
 
-	gdi_region = ExtCreateRegion (NULL, data_size, data);
-	free (data);
-
-	if (!gdi_region)
-	    return CAIRO_STATUS_NO_MEMORY;
-
-	/* Combine the new region with the original clip */
-
-	if (surface->saved_clip) {
-	    if (CombineRgn (gdi_region, gdi_region, surface->saved_clip, RGN_AND) == ERROR)
-		goto FAIL;
-	}
-
-	if (SelectClipRgn (surface->dc, gdi_region) == ERROR)
-	    goto FAIL;
-
-	DeleteObject (gdi_region);
 	return CAIRO_STATUS_SUCCESS;
 
     FAIL:
 	status = _cairo_win32_print_gdi_error ("_cairo_win32_surface_set_clip_region");
-	DeleteObject (gdi_region);
+	if (gdi_region)
+	    DeleteObject (gdi_region);
+
 	return status;
     }
 }
@@ -1691,10 +1718,12 @@ cairo_win32_surface_create (HDC hdc)
     RECT rect;
     int depth;
     cairo_format_t format;
+    int clipBoxType;
 
     /* Try to figure out the drawing bounds for the Device context
      */
-    if (GetClipBox (hdc, &rect) == ERROR) {
+    clipBoxType = GetClipBox (hdc, &rect);
+    if (clipBoxType == ERROR) {
 	_cairo_win32_print_gdi_error ("cairo_win32_surface_create");
 	/* XXX: Can we make a more reasonable guess at the error cause here? */
 	_cairo_error (CAIRO_STATUS_NO_MEMORY);
@@ -1743,16 +1772,15 @@ cairo_win32_surface_create (HDC hdc)
     surface->clip_rect.width = (uint16_t) (rect.right - rect.left);
     surface->clip_rect.height = (uint16_t) (rect.bottom - rect.top);
 
-    if (surface->clip_rect.width == 0 ||
+    if (clipBoxType == COMPLEXREGION) {
-	surface->clip_rect.height == 0)
-    {
-	surface->saved_clip = NULL;
-    } else {
 	surface->saved_clip = CreateRectRgn (0, 0, 0, 0);
 	if (GetClipRgn (hdc, surface->saved_clip) == 0) {
+	    /* this should never happen */
 	    DeleteObject(surface->saved_clip);
 	    surface->saved_clip = NULL;
 	}
+    } else {
+	surface->saved_clip = NULL;
     }
 
     surface->extents = surface->clip_rect;
@@ -1812,11 +1840,10 @@ cairo_win32_surface_create_with_ddb (HDC hdc,
     cairo_win32_surface_t *new_surf;
     HBITMAP ddb;
     HDC screen_dc, ddb_dc;
-    HRGN crgn;
     HBITMAP saved_dc_bitmap;
 
     if (format != CAIRO_FORMAT_RGB24)
-	return NULL;
+	return NIL_SURFACE;
 /* XXX handle these eventually
 	format != CAIRO_FORMAT_A8 ||
 	format != CAIRO_FORMAT_A1)
@@ -1829,20 +1856,34 @@ cairo_win32_surface_create_with_ddb (HDC hdc,
 	screen_dc = NULL;
     }
 
-    ddb = CreateCompatibleBitmap (hdc, width, height);
     ddb_dc = CreateCompatibleDC (hdc);
+    if (ddb_dc == NULL) {
+	_cairo_win32_print_gdi_error("CreateCompatibleDC");
+	new_surf = NIL_SURFACE;
+	goto FINISH;
+    }
+
+    ddb = CreateCompatibleBitmap (hdc, width, height);
+    if (ddb == NULL) {
+	DeleteDC (ddb_dc);
+
+	/* Note that if an app actually does hit this out of memory
+	 * condition, it's going to have lots of other issues, as
+	 * video memory is probably exhausted.
+	 */
+	_cairo_win32_print_gdi_error("CreateCompatibleBitmap");
+	new_surf = NIL_SURFACE;
+	goto FINISH;
+    }
 
     saved_dc_bitmap = SelectObject (ddb_dc, ddb);
 
-    crgn = CreateRectRgn (0, 0, width, height);
-    SelectClipRgn (ddb_dc, crgn);
-    DeleteObject (crgn);
-
     new_surf = (cairo_win32_surface_t*) cairo_win32_surface_create (ddb_dc);
     new_surf->bitmap = ddb;
     new_surf->saved_dc_bitmap = saved_dc_bitmap;
     new_surf->is_dib = FALSE;
 
+FINISH:
     if (screen_dc)
 	ReleaseDC (NULL, screen_dc);
 

gfx/src/thebes/nsThebesImage.cpp

@@ -48,6 +48,15 @@
 
 static PRBool gDisableOptimize = PR_FALSE;
 
+#ifdef XP_WIN
+static PRUint32 gTotalDDBs = 0;
+static PRUint32 gTotalDDBSize = 0;
+// only use up a maximum of 64MB in DDBs
+#define kMaxDDBSize (64*1024*1024)
+// and don't let anything in that's bigger than 4MB
+#define kMaxSingleDDBSize (4*1024*1024)
+#endif
+
 NS_IMPL_ISUPPORTS1(nsThebesImage, nsIImage)
 
 nsThebesImage::nsThebesImage()
@@ -66,6 +75,10 @@ nsThebesImage::nsThebesImage()
         }
         hasCheckedOptimize = PR_TRUE;
     }
+
+#ifdef XP_WIN
+    mIsDDBSurface = PR_FALSE;
+#endif
 }
 
 nsresult
@@ -127,6 +140,12 @@ nsThebesImage::Init(PRInt32 aWidth, PRInt32 aHeight, PRInt32 aDepth, nsMaskRequi
 
 nsThebesImage::~nsThebesImage()
 {
+#ifdef XP_WIN
+    if (mIsDDBSurface) {
+        gTotalDDBs--;
+        gTotalDDBSize -= mWidth*mHeight*4;
+    }
+#endif
 }
 
 PRInt32
@@ -241,12 +260,31 @@ nsThebesImage::Optimize(nsIDeviceContext* aContext)
     // as we can.
     if (mWinSurface) {
         // Don't do DDBs for large images; see bug 359147
-        // We use 1024 as a reasonable sized maximum; the real fix
+        // Note that we bother with DDBs at all because they are much faster
-        // will be to make sure we don't ever make a DDB that's bigger
+        // on some systems; on others there isn't much of a speed difference
-        // than the primary screen size (rule of thumb).
+        // between DIBs and DDBs.
-        if (mWidth <= 1024 && mHeight <= 1024) {
+        //
+        // Originally this just limited to 1024x1024; but that still
+        // had us hitting overall total memory usage limits (which was
+        // around 220MB on my intel shared memory system with 2GB RAM
+        // and 16-128mb in use by the video card, so I can't make
+        // heads or tails out of this limit).
+        //
+        // So instead, we clamp the max size to 64MB (this limit shuld
+        // be made dynamic based on.. something.. as soon a we figure
+        // out that something) and also limit each individual image to
+        // be less than 4MB to keep very large images out of DDBs.
+
+        // assume (almost -- we don't quadword-align) worst-case size
+        PRUint32 ddbSize = mWidth * mHeight * 4;
+        if (ddbSize <= kMaxSingleDDBSize &&
+            ddbSize + gTotalDDBSize <= kMaxDDBSize)
+        {
             nsRefPtr<gfxWindowsSurface> wsurf = mWinSurface->OptimizeToDDB(nsnull, gfxIntSize(mWidth, mHeight), mFormat);
             if (wsurf) {
+                gTotalDDBs++;
+                gTotalDDBSize += ddbSize;
+                mIsDDBSurface = PR_TRUE;
                 mOptSurface = wsurf;
             }
         }

gfx/src/thebes/nsThebesImage.h

@@ -142,6 +142,9 @@ protected:
     nsRect mDecoded;
     PRPackedBool mImageComplete;
     PRPackedBool mSinglePixel;
+#ifdef XP_WIN
+    PRPackedBool mIsDDBSurface;
+#endif
 
     gfxRGBA mSinglePixelColor;
 

gfx/thebes/src/gfxASurface.cpp

@@ -63,8 +63,6 @@ static cairo_user_data_key_t gfxasurface_pointer_key;
 nsrefcnt
 gfxASurface::AddRef(void)
 {
-    NS_PRECONDITION(mSurface != nsnull, "gfxASurface::AddRef without mSurface");
-
     if (mSurfaceValid) {
         if (mFloatingRefs) {
             // eat a floating ref
@@ -84,8 +82,6 @@ gfxASurface::AddRef(void)
 nsrefcnt
 gfxASurface::Release(void)
 {
-    NS_PRECONDITION(mSurface != nsnull, "gfxASurface::Release without mSurface");
-
     if (mSurfaceValid) {
         NS_ASSERTION(mFloatingRefs == 0, "gfxASurface::Release with floating refs still hanging around!");
 

gfx/thebes/src/gfxWindowsSurface.cpp

@@ -75,7 +75,10 @@ gfxWindowsSurface::gfxWindowsSurface(const gfxIntSize& size, gfxImageFormat imag
                                                                 size.width, size.height);
     Init(surf);
 
-    mDC = cairo_win32_surface_get_dc(CairoSurface());
+    if (CairoStatus() == 0)
+        mDC = cairo_win32_surface_get_dc(CairoSurface());
+    else
+        mDC = nsnull;
 }
 
 gfxWindowsSurface::gfxWindowsSurface(HDC dc, const gfxIntSize& size, gfxImageFormat imageFormat) :
@@ -88,14 +91,20 @@ gfxWindowsSurface::gfxWindowsSurface(HDC dc, const gfxIntSize& size, gfxImageFor
                                                                 size.width, size.height);
     Init(surf);
 
-    mDC = cairo_win32_surface_get_dc(CairoSurface());
+    if (CairoStatus() == 0)
+        mDC = cairo_win32_surface_get_dc(CairoSurface());
+    else
+        mDC = nsnull;
 }
 
 
 gfxWindowsSurface::gfxWindowsSurface(cairo_surface_t *csurf) :
     mOwnsDC(PR_FALSE), mForPrinting(PR_FALSE), mWnd(nsnull)
 {
-    mDC = cairo_win32_surface_get_dc(csurf);
+    if (cairo_surface_status(csurf) == 0)
+        mDC = cairo_win32_surface_get_dc(csurf);
+    else
+        mDC = nsnull;
 
     if (cairo_surface_get_type(csurf) == CAIRO_SURFACE_TYPE_WIN32_PRINTING)
         mForPrinting = PR_TRUE;
@@ -135,14 +144,14 @@ gfxWindowsSurface::OptimizeToDDB(HDC dc, const gfxIntSize& size, gfxImageFormat
     if (mForPrinting)
         return nsnull;
 
-    gfxImageFormat realFormat = format;
+    if (format != ImageFormatRGB24)
-
-    if (realFormat != ImageFormatRGB24)
         return nsnull;
 
-    nsRefPtr<gfxWindowsSurface> wsurf = new gfxWindowsSurface(dc, size, realFormat);
+    nsRefPtr<gfxWindowsSurface> wsurf = new gfxWindowsSurface(dc, size, format);
+    if (wsurf->CairoStatus() != 0)
+        return nsnull;
 
-    nsRefPtr<gfxContext> tmpCtx(new gfxContext(wsurf));
+    nsRefPtr<gfxContext> tmpCtx = new gfxContext(wsurf);
     tmpCtx->SetOperator(gfxContext::OPERATOR_SOURCE);
     tmpCtx->SetSource(this);
     tmpCtx->Paint();