Bug 698901: Drop pixmaps/knockout group upon error when ending group.

Previously the call to fz_convert_pixmap() threw causing a destination
pixmap leak. This illustrated a bigger issue with the error handling
so now all types of pixmaps are dropped and care is taken to also end
the knockout group, should there be any.

1 files changed, 61 insertions, 45 deletions

diff --git a/source/fitz/draw-device.c b/source/fitz/draw-device.c
index 0696b3bd..43dbb52d 100644
--- a/source/fitz/draw-device.c
+++ b/source/fitz/draw-device.c
@@ -2382,6 +2382,7 @@ fz_draw_end_group(fz_context *ctx, fz_device *devp)
 
 	state = &dev->stack[--dev->top];
 	STACK_POPPED("group");
+
 	alpha = state[1].alpha;
 	blendmode = state[1].blendmode & FZ_BLEND_MODEMASK;
 	isolated = state[1].blendmode & FZ_BLEND_ISOLATED;
@@ -2406,61 +2407,76 @@ fz_draw_end_group(fz_context *ctx, fz_device *devp)
 	if (state[1].blendmode & FZ_BLEND_KNOCKOUT)
 		printf(" (knockout)");
 #endif
-	if (state[0].dest->colorspace != state[1].dest->colorspace)
+
+	fz_try(ctx)
 	{
-		fz_pixmap *converted = fz_convert_pixmap(ctx, state[1].dest, state[0].dest->colorspace, NULL, dev->default_cs, fz_default_color_params(ctx), 1);
-		fz_drop_pixmap(ctx, state[1].dest);
-		state[1].dest = converted;
-	}
+		if (state[0].dest->colorspace != state[1].dest->colorspace)
+		{
+			fz_pixmap *converted = fz_convert_pixmap(ctx, state[1].dest, state[0].dest->colorspace, NULL, dev->default_cs, fz_default_color_params(ctx), 1);
+			fz_drop_pixmap(ctx, state[1].dest);
+			state[1].dest = converted;
+		}
 
-	if ((blendmode == 0) && (state[0].shape == state[1].shape) && (state[0].group_alpha == state[1].group_alpha))
-		fz_paint_pixmap(state[0].dest, state[1].dest, alpha * 255);
-	else
-		fz_blend_pixmap(ctx, state[0].dest, state[1].dest, alpha * 255, blendmode, isolated, state[1].group_alpha);
+		if ((blendmode == 0) && (state[0].shape == state[1].shape) && (state[0].group_alpha == state[1].group_alpha))
+			fz_paint_pixmap(state[0].dest, state[1].dest, alpha * 255);
+		else
+			fz_blend_pixmap(ctx, state[0].dest, state[1].dest, alpha * 255, blendmode, isolated, state[1].group_alpha);
 
-	if (state[0].shape != state[1].shape)
-	{
-		/* The 'D' on page 7 of Altona_Technical_v20_x4.pdf goes wrong if this
-		 * isn't alpha * 255, as the blend back fails to take account of alpha. */
-		if (state[0].shape)
+		if (state[0].shape != state[1].shape)
+		{
+			/* The 'D' on page 7 of Altona_Technical_v20_x4.pdf goes wrong if this
+			 * isn't alpha * 255, as the blend back fails to take account of alpha. */
+			if (state[0].shape)
+			{
+				if (state[1].shape)
+					fz_paint_pixmap(state[0].shape, state[1].shape, alpha * 255);
+				else
+					fz_paint_pixmap_alpha(state[0].shape, state[1].dest, alpha * 255);
+			}
+		}
+		assert(state[0].group_alpha == NULL || state[0].group_alpha != state[1].group_alpha);
+		if (state[0].group_alpha && state[0].group_alpha != state[1].group_alpha)
 		{
-			if (state[1].shape)
-				fz_paint_pixmap(state[0].shape, state[1].shape, alpha * 255);
+			/* The 'D' on page 7 of Altona_Technical_v20_x4.pdf uses an isolated group,
+			 * and goes wrong if this is 255 * alpha, as an alpha effectively gets
+			 * applied twice. CATX5233 page 7 uses a non-isolated group, and goes wrong
+			 * if alpha isn't applied here. */
+			if (state[1].group_alpha)
+				fz_paint_pixmap(state[0].group_alpha, state[1].group_alpha, isolated ? 255 : alpha * 255);
 			else
-				fz_paint_pixmap_alpha(state[0].shape, state[1].dest, alpha * 255);
+				fz_paint_pixmap_alpha(state[0].group_alpha, state[1].dest, isolated ? 255 : alpha * 255);
 		}
-		fz_drop_pixmap(ctx, state[1].shape);
+
+		assert(state[0].dest != state[1].dest);
+
+#ifdef DUMP_GROUP_BLENDS
+		fz_dump_blend(ctx, " to get ", state[0].dest);
+		if (state[0].shape)
+			fz_dump_blend(ctx, "/S=", state[0].shape);
+		if (state[0].group_alpha)
+			fz_dump_blend(ctx, "/GA=", state[0].group_alpha);
+		printf("\n");
+#endif
 	}
-	assert(state[0].group_alpha == NULL || state[0].group_alpha != state[1].group_alpha);
-	if (state[0].group_alpha && state[0].group_alpha != state[1].group_alpha)
+	fz_always(ctx)
 	{
-		/* The 'D' on page 7 of Altona_Technical_v20_x4.pdf uses an isolated group,
-		 * and goes wrong if this is 255 * alpha, as an alpha effectively gets
-		 * applied twice. CATX5233 page 7 uses a non-isolated group, and goes wrong
-		 * if alpha isn't applied here. */
-		if (state[1].group_alpha)
-			fz_paint_pixmap(state[0].group_alpha, state[1].group_alpha, isolated ? 255 : alpha * 255);
-		else
-			fz_paint_pixmap_alpha(state[0].group_alpha, state[1].dest, isolated ? 255 : alpha * 255);
+		if (state[0].shape != state[1].shape)
+			fz_drop_pixmap(ctx, state[1].shape);
+		fz_drop_pixmap(ctx, state[1].group_alpha);
+		/* The following test should not be required, but just occasionally
+		 * errors can cause the stack to get out of sync, and this might save
+		 * our bacon. */
+		if (state[0].dest != state[1].dest)
+			fz_drop_pixmap(ctx, state[1].dest);
+
+		if (state[0].blendmode & FZ_BLEND_KNOCKOUT)
+			fz_knockout_end(ctx, dev);
+	}
+	fz_catch(ctx)
+	{
+		fz_rethrow(ctx);
 	}
-	fz_drop_pixmap(ctx, state[1].group_alpha);
-	/* The following test should not be required, but just occasionally
-	 * errors can cause the stack to get out of sync, and this might save
-	 * our bacon. */
-	assert(state[0].dest != state[1].dest);
-	if (state[0].dest != state[1].dest)
-		fz_drop_pixmap(ctx, state[1].dest);
-#ifdef DUMP_GROUP_BLENDS
-	fz_dump_blend(ctx, " to get ", state[0].dest);
-	if (state[0].shape)
-		fz_dump_blend(ctx, "/S=", state[0].shape);
-	if (state[0].group_alpha)
-		fz_dump_blend(ctx, "/GA=", state[0].group_alpha);
-	printf("\n");
-#endif
 
-	if (state[0].blendmode & FZ_BLEND_KNOCKOUT)
-		fz_knockout_end(ctx, dev);
 }
 
 typedef struct