[win32] Better tracking of initial clip

There were a few corner cases that the win32 surface was failing
at when there was an initial clip set; the win32-printing surface
had more serious problems when painting meta surface patterns.
This cleans up the initial DC clip tracking for both surfaces.

3 files changed, 197 insertions, 116 deletions

diff --git a/src/cairo-win32-printing-surface.c b/src/cairo-win32-printing-surface.c
index ee08d4af..cc74bcf7 100644
--- a/src/cairo-win32-printing-surface.c
+++ b/src/cairo-win32-printing-surface.c
@@ -460,11 +460,14 @@ _cairo_win32_printing_surface_paint_meta_pattern (cairo_win32_surface_t   *surfa
 	    EndPath (surface->dc);
 	    SelectClipPath (surface->dc, RGN_AND);
 
+	    SaveDC (surface->dc); /* Allow clip path to be reset during replay */
 	    status = _cairo_meta_surface_replay (meta_surface, &surface->base);
+
+	    /* Restore both the clip save and our earlier path SaveDC */
+	    RestoreDC (surface->dc, -2);
+
 	    if (status)
 		return status;
-
-	    RestoreDC (surface->dc, -1);
 	}
     }
 
@@ -970,9 +973,8 @@ _cairo_win32_printing_surface_show_page (void *abstract_surface)
 {
     cairo_win32_surface_t *surface = abstract_surface;
 
-    if (surface->clip_saved_dc != 0)
-	RestoreDC (surface->dc, surface->clip_saved_dc);
-    RestoreDC (surface->dc, -1);
+    /* Undo both SaveDC's that we did in start_page */
+    RestoreDC (surface->dc, -2);
 
     return CAIRO_STATUS_SUCCESS;
 }
@@ -991,10 +993,9 @@ _cairo_win32_printing_surface_intersect_clip_path (void		      *abstract_surface
 	return CAIRO_STATUS_SUCCESS;
 
     if (path == NULL) {
-	if (surface->clip_saved_dc != 0) {
-	    RestoreDC (surface->dc, surface->clip_saved_dc);
-	    surface->clip_saved_dc = 0;
-	}
+	RestoreDC (surface->dc, -1);
+	SaveDC (surface->dc);
+
 	return CAIRO_STATUS_SUCCESS;
     }
 
@@ -1013,8 +1014,6 @@ _cairo_win32_printing_surface_intersect_clip_path (void		      *abstract_surface
 	ASSERT_NOT_REACHED;
     }
 
-    if (surface->clip_saved_dc == 0)
-	surface->clip_saved_dc = SaveDC (surface->dc);
     SelectClipPath (surface->dc, RGN_AND);
 
     return status;
@@ -1417,7 +1416,8 @@ _cairo_win32_printing_surface_start_page (void *abstract_surface)
     cairo_win32_surface_t *surface = abstract_surface;
     XFORM xform;
 
-    SaveDC (surface->dc);
+    SaveDC (surface->dc); /* Save application context first, before doing MWT */
+
     SetGraphicsMode (surface->dc, GM_ADVANCED);
     GetWorldTransform(surface->dc, &xform);
     surface->ctm.xx = xform.eM11;
@@ -1430,6 +1430,8 @@ _cairo_win32_printing_surface_start_page (void *abstract_surface)
     if (!ModifyWorldTransform (surface->dc, NULL, MWT_IDENTITY))
 	return _cairo_win32_print_gdi_error ("_cairo_win32_printing_surface_start_page:ModifyWorldTransform");
 
+    SaveDC (surface->dc); /* Then save Cairo's known-good clip state, so the clip path can be reset */
+
     return CAIRO_STATUS_SUCCESS;
 }
 
@@ -1462,21 +1464,18 @@ cairo_surface_t *
 cairo_win32_printing_surface_create (HDC hdc)
 {
     cairo_win32_surface_t *surface;
-    RECT rect;
     int xr, yr;
-
-    /* Try to figure out the drawing bounds for the Device context
-     */
-    if (GetClipBox (hdc, &rect) == ERROR) {
-	_cairo_win32_print_gdi_error ("cairo_win32_surface_create");
-	/* XXX: Can we make a more reasonable guess at the error cause here? */
-	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
-    }
+    RECT rect;
 
     surface = malloc (sizeof (cairo_win32_surface_t));
     if (surface == NULL)
 	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
 
+    if (_cairo_win32_save_initial_clip (hdc, surface) != CAIRO_STATUS_SUCCESS) {
+	free (surface);
+	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
+    }
+
     surface->image = NULL;
     surface->format = CAIRO_FORMAT_RGB24;
     surface->content = CAIRO_CONTENT_COLOR_ALPHA;
@@ -1488,28 +1487,14 @@ cairo_win32_printing_surface_create (HDC hdc)
     surface->brush = NULL;
     surface->old_brush = NULL;
 
-    surface->clip_rect.x = (int16_t) rect.left;
-    surface->clip_rect.y = (int16_t) rect.top;
-    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 ||
-	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) {
-	    DeleteObject(surface->saved_clip);
-	    surface->saved_clip = NULL;
-	}
-    }
-
-    surface->extents = surface->clip_rect;
+    GetClipBox(hdc, &rect);
+    surface->extents.x = rect.left;
+    surface->extents.y = rect.top;
+    surface->extents.width = rect.right - rect.left;
+    surface->extents.height = rect.bottom - rect.top;
 
     surface->flags = _cairo_win32_flags_for_dc (surface->dc);
     surface->flags |= CAIRO_WIN32_SURFACE_FOR_PRINTING;
-    surface->clip_saved_dc = 0;
 
     _cairo_win32_printing_surface_init_ps_mode (surface);
     _cairo_surface_init (&surface->base, &cairo_win32_printing_surface_backend,
@@ -1521,8 +1506,8 @@ cairo_win32_printing_surface_create (HDC hdc)
 
     return _cairo_paginated_surface_create (&surface->base,
                                             CAIRO_CONTENT_COLOR_ALPHA,
-                                            rect.right - rect.left,
-                                            rect.bottom - rect.top,
+					    surface->extents.width,
+					    surface->extents.height,
                                             &cairo_win32_surface_paginated_backend);
 }
 
diff --git a/src/cairo-win32-private.h b/src/cairo-win32-private.h
index 6d30da8e..704ec4ed 100644
--- a/src/cairo-win32-private.h
+++ b/src/cairo-win32-private.h
@@ -71,12 +71,17 @@ typedef struct _cairo_win32_surface {
 
     cairo_surface_t *image;
 
-    cairo_rectangle_int_t clip_rect;
-
-    HRGN saved_clip;
-
     cairo_rectangle_int_t extents;
 
+    /* Initial clip bits
+     * We need these kept around so that we maintain
+     * whatever clip was set on the original DC at creation
+     * time when cairo is asked to reset the surface clip.
+     */
+    cairo_rectangle_int_t clip_rect;
+    HRGN initial_clip_rgn;
+    cairo_bool_t had_simple_clip;
+
     /* Surface DC flags */
     uint32_t flags;
 
@@ -86,7 +91,6 @@ typedef struct _cairo_win32_surface {
     cairo_bool_t path_empty;
     cairo_bool_t has_ctm;
     cairo_matrix_t ctm;
-    int clip_saved_dc;
     HBRUSH brush, old_brush;
 } cairo_win32_surface_t;
 
@@ -168,4 +172,13 @@ _cairo_matrix_to_win32_xform (const cairo_matrix_t *m,
     xform->eDy = (FLOAT) m->y0;
 }
 
+cairo_int_status_t
+_cairo_win32_save_initial_clip (HDC dc, cairo_win32_surface_t *surface);
+
+cairo_int_status_t
+_cairo_win32_restore_initial_clip (cairo_win32_surface_t *surface);
+
+void
+_cairo_win32_debug_dump_hrgn (HRGN rgn, char *header);
+
 #endif /* CAIRO_WIN32_PRIVATE_H */
diff --git a/src/cairo-win32-surface.c b/src/cairo-win32-surface.c
index 881532b6..69c64069 100644
--- a/src/cairo-win32-surface.c
+++ b/src/cairo-win32-surface.c
@@ -354,7 +354,8 @@ _cairo_win32_surface_create_for_dc (HDC             original_dc,
     surface->clip_rect.width = width;
     surface->clip_rect.height = height;
 
-    surface->saved_clip = NULL;
+    surface->initial_clip_rgn = NULL;
+    surface->had_simple_clip = FALSE;
 
     surface->extents = surface->clip_rect;
 
@@ -473,26 +474,11 @@ _cairo_win32_surface_finish (void *abstract_surface)
 	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);
-	    }
-	}
+	_cairo_win32_restore_initial_clip (surface);
     }
 
-    if (surface->saved_clip)
-	DeleteObject (surface->saved_clip);
+    if (surface->initial_clip_rgn)
+	DeleteObject (surface->initial_clip_rgn);
 
     return CAIRO_STATUS_SUCCESS;
 }
@@ -565,8 +551,8 @@ _cairo_win32_surface_acquire_source_image (void                    *abstract_sur
     }
 
     status = _cairo_win32_surface_get_subimage (abstract_surface, 0, 0,
-						surface->clip_rect.width,
-						surface->clip_rect.height, &local);
+						surface->extents.width,
+						surface->extents.height, &local);
     if (status)
 	return status;
 
@@ -605,8 +591,8 @@ _cairo_win32_surface_acquire_dest_image (void                    *abstract_surfa
 
 	image_rect->x = 0;
 	image_rect->y = 0;
-	image_rect->width = surface->clip_rect.width;
-	image_rect->height = surface->clip_rect.height;
+	image_rect->width = surface->extents.width;
+	image_rect->height = surface->extents.height;
 
 	*image_out = (cairo_image_surface_t *)surface->image;
 	*image_extra = NULL;
@@ -1440,13 +1426,11 @@ _cairo_win32_surface_set_clip_region (void           *abstract_surface,
      * save the original clip when first setting a clip on surface.
      */
 
-    if (region == NULL) {
-	/* Clear any clip set by cairo, return to the original */
-	if (SelectClipRgn (surface->dc, surface->saved_clip) == ERROR)
-	    return _cairo_win32_print_gdi_error ("_cairo_win32_surface_set_clip_region (reset)");
+    /* Clear any clip set by cairo, return to the original first */
+    status = _cairo_win32_restore_initial_clip (surface);
 
-	status = CAIRO_STATUS_SUCCESS;
-    } else {
+    /* Then combine any new region with it */
+    if (region) {
 	cairo_rectangle_int_t extents;
 	cairo_box_int_t *boxes;
 	int num_boxes;
@@ -1480,6 +1464,13 @@ _cairo_win32_surface_set_clip_region (void           *abstract_surface,
 
 	    _cairo_region_boxes_fini (region, boxes);
 	} else {
+	    /* XXX see notes in _cairo_win32_save_initial_clip --
+	     * this code will interact badly with a HDC which had an initial
+	     * world transform -- we should probably manually transform the
+	     * region rects, because SelectClipRgn takes device units, not
+	     * logical units (unlike IntersectClipRect).
+	     */
+
 	    data_size = sizeof (RGNDATAHEADER) + num_boxes * sizeof (RECT);
 	    data = malloc (data_size);
 	    if (!data) {
@@ -1512,17 +1503,9 @@ _cairo_win32_surface_set_clip_region (void           *abstract_surface,
 	    if (!gdi_region)
 		return _cairo_error (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)
-		    status = _cairo_win32_print_gdi_error ("_cairo_win32_surface_set_clip_region");
-	    }
-
-	    /* Then select the new clip region into our surface if everything went ok */
-	    if (status == CAIRO_STATUS_SUCCESS) {
-		if (SelectClipRgn (surface->dc, gdi_region) == ERROR)
-		    status = _cairo_win32_print_gdi_error ("_cairo_win32_surface_set_clip_region");
-	    }
+	    /* AND the new region into our DC */
+	    if (ExtSelectClipRgn (surface->dc, gdi_region, RGN_AND) == ERROR)
+		status = _cairo_win32_print_gdi_error ("_cairo_win32_surface_set_clip_region");
 
 	    DeleteObject (gdi_region);
 	}
@@ -1704,19 +1687,10 @@ cairo_surface_t *
 cairo_win32_surface_create (HDC hdc)
 {
     cairo_win32_surface_t *surface;
-    RECT rect;
+
     int depth;
     cairo_format_t format;
-    int clipBoxType;
-
-    /* Try to figure out the drawing bounds for the Device context
-     */
-    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? */
-	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
-    }
+    RECT rect;
 
     if (GetDeviceCaps(hdc, TECHNOLOGY) == DT_RASDISPLAY) {
 	depth = GetDeviceCaps(hdc, BITSPIXEL);
@@ -1742,6 +1716,11 @@ cairo_win32_surface_create (HDC hdc)
     if (surface == NULL)
 	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
 
+    if (_cairo_win32_save_initial_clip (hdc, surface) != CAIRO_STATUS_SUCCESS) {
+	free (surface);
+	return _cairo_surface_create_in_error (_cairo_error (CAIRO_STATUS_NO_MEMORY));
+    }
+
     surface->image = NULL;
     surface->format = format;
 
@@ -1752,26 +1731,13 @@ cairo_win32_surface_create (HDC hdc)
     surface->brush = NULL;
     surface->old_brush = NULL;
 
-    surface->clip_rect.x = (int16_t) rect.left;
-    surface->clip_rect.y = (int16_t) rect.top;
-    surface->clip_rect.width = (uint16_t) (rect.right - rect.left);
-    surface->clip_rect.height = (uint16_t) (rect.bottom - rect.top);
-
-    if (clipBoxType == COMPLEXREGION) {
-	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;
+    GetClipBox(hdc, &rect);
+    surface->extents.x = rect.left;
+    surface->extents.y = rect.top;
+    surface->extents.width = rect.right - rect.left;
+    surface->extents.height = rect.bottom - rect.top;
 
     surface->flags = _cairo_win32_flags_for_dc (surface->dc);
-    surface->clip_saved_dc = 0;
 
     _cairo_surface_init (&surface->base, &cairo_win32_surface_backend,
 			 _cairo_content_from_format (format));
@@ -2041,3 +2007,120 @@ DllMain (HINSTANCE hinstDLL,
 
 #endif
 
+cairo_int_status_t
+_cairo_win32_save_initial_clip (HDC hdc, cairo_win32_surface_t *surface)
+{
+    RECT rect;
+    int clipBoxType;
+    int gm;
+    XFORM saved_xform;
+
+    /* GetClipBox/GetClipRgn and friends interact badly with a world transform
+     * set.  GetClipBox returns values in logical (transformed) coordinates;
+     * it's unclear what GetClipRgn returns, because the region is empty in the
+     * case of a SIMPLEREGION clip, but I assume device (untransformed) coordinates.
+     * Similarily, IntersectClipRect works in logical units, whereas SelectClipRgn
+     * works in device units.
+     *
+     * So, avoid the whole mess and get rid of the world transform
+     * while we store our initial data and when we restore initial coordinates.
+     *
+     * XXX we may need to modify x/y by the ViewportOrg or WindowOrg
+     * here in GM_COMPATIBLE; unclear.
+     */
+    gm = GetGraphicsMode (hdc);
+    if (gm == GM_ADVANCED) {
+	GetWorldTransform (hdc, &saved_xform);
+	ModifyWorldTransform (hdc, NULL, MWT_IDENTITY);
+    }
+
+    clipBoxType = GetClipBox (hdc, &rect);
+    if (clipBoxType == ERROR) {
+	_cairo_win32_print_gdi_error ("cairo_win32_surface_create");
+	SetGraphicsMode (hdc, gm);
+	/* XXX: Can we make a more reasonable guess at the error cause here? */
+	return _cairo_error (CAIRO_STATUS_NO_MEMORY);
+    }
+
+    surface->clip_rect.x = rect.left;
+    surface->clip_rect.y = rect.top;
+    surface->clip_rect.width = rect.right - rect.left;
+    surface->clip_rect.height = rect.bottom - rect.top;
+
+    surface->initial_clip_rgn = NULL;
+    surface->had_simple_clip = FALSE;
+
+    if (clipBoxType == COMPLEXREGION) {
+	surface->initial_clip_rgn = CreateRectRgn (0, 0, 0, 0);
+	if (GetClipRgn (hdc, surface->initial_clip_rgn) == -1) {
+	    /* this should never happen */
+	    DeleteObject(surface->initial_clip_rgn);
+	    surface->initial_clip_rgn = NULL;
+	}
+    } else if (clipBoxType == SIMPLEREGION) {
+	surface->had_simple_clip = TRUE;
+    }
+
+    if (gm == GM_ADVANCED)
+	SetWorldTransform (hdc, &saved_xform);
+
+    return CAIRO_STATUS_SUCCESS;
+}
+
+cairo_int_status_t
+_cairo_win32_restore_initial_clip (cairo_win32_surface_t *surface)
+{
+    cairo_int_status_t status = CAIRO_STATUS_SUCCESS;
+
+    XFORM saved_xform;
+    int gm = GetGraphicsMode (surface->dc);
+    if (gm == GM_ADVANCED) {
+	GetWorldTransform (surface->dc, &saved_xform);
+	ModifyWorldTransform (surface->dc, NULL, MWT_IDENTITY);
+    }
+
+    /* initial_clip_rgn will either be a real region or NULL (which means reset to no clip region) */
+    SelectClipRgn (surface->dc, surface->initial_clip_rgn);
+
+    if (surface->had_simple_clip) {
+	/* then if we had a simple clip, intersect */
+	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 (gm == GM_ADVANCED)
+	SetWorldTransform (surface->dc, &saved_xform);
+
+    return status;
+}
+
+void
+_cairo_win32_debug_dump_hrgn (HRGN rgn, char *header)
+{
+    RGNDATA *rd;
+    int z;
+
+    if (header)
+	fprintf (stderr, "%s\n", header);
+
+    if (rgn == NULL) {
+	fprintf (stderr, " NULL\n");
+    }
+
+    z = GetRegionData(rgn, 0, NULL);
+    rd = (RGNDATA*) malloc(z);
+    z = GetRegionData(rgn, z, rd);
+
+    fprintf (stderr, " %d rects, bounds: %d %d %d %d\n", rd->rdh.nCount, rd->rdh.rcBound.left, rd->rdh.rcBound.top, rd->rdh.rcBound.right - rd->rdh.rcBound.left, rd->rdh.rcBound.bottom - rd->rdh.rcBound.top);
+
+    for (z = 0; z < rd->rdh.nCount; z++) {
+	RECT r = ((RECT*)rd->Buffer)[z];
+	fprintf (stderr, " [%d]: [%d %d %d %d]\n", z, r.left, r.top, r.right - r.left, r.bottom - r.top);
+    }
+
+    free(rd);
+    fflush (stderr);
+}