sna/damage: Fix union of extents with dirty damage but no region

By failing to account for certain paths which would create a damage elt
without fully initialisating the damage region (only the damage extents),
we would later overwrite the damage extents with only the extents for
this operation (rather than the union of this operation with the current
damage). This fixes a regression from 098592ca5d,
(sna: Remove the independent tracking of elts from boxes).

Include the associated damage migration debugging code of the callers.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

3 files changed, 168 insertions, 152 deletions

diff --git a/src/sna/sna_accel.c b/src/sna/sna_accel.c
index 8aa30fcf..30da328c 100644
--- a/src/sna/sna_accel.c
+++ b/src/sna/sna_accel.c
@@ -64,6 +64,10 @@
 #define USE_ZERO_SPANS 1
 #define USE_BO_FOR_SCRATCH_PIXMAP 1
 
+#define MIGRATE_ALL 0
+
+#define ACCEL_PUT_IMAGE 1
+
 static int sna_font_key;
 
 static const uint8_t copy_ROP[] = {
@@ -1142,8 +1146,33 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 		goto done;
 	}
 
+	if (MIGRATE_ALL && priv->gpu_damage) {
+		BoxPtr box;
+		int n = sna_damage_get_boxes(priv->gpu_damage, &box);
+		if (n) {
+			Bool ok;
+
+			DBG(("%s: forced migration\n", __FUNCTION__));
+
+			assert(pixmap_contains_damage(pixmap, priv->gpu_damage));
+
+			ok = FALSE;
+			if (priv->cpu_bo && sna->kgem.gen >= 60)
+				ok = sna->render.copy_boxes(sna, GXcopy,
+							    pixmap, priv->gpu_bo, 0, 0,
+							    pixmap, priv->cpu_bo, 0, 0,
+							    box, n);
+			if (!ok)
+				sna_read_boxes(sna,
+					       priv->gpu_bo, 0, 0,
+					       pixmap, 0, 0,
+					       box, n);
+		}
+		sna_damage_destroy(&priv->gpu_damage);
+	}
+
 	if (sna_damage_contains_box(priv->gpu_damage,
-				    REGION_EXTENTS(NULL, region)) != PIXMAN_REGION_OUT) {
+				    &region->extents) != PIXMAN_REGION_OUT) {
 		DBG(("%s: region (%dx%d) intersects gpu damage\n",
 		     __FUNCTION__,
 		     region->extents.x2 - region->extents.x1,
@@ -1194,11 +1223,13 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 			}
 
 			if (region_subsumes_damage(r, priv->gpu_damage)) {
-				BoxPtr box = REGION_RECTS(&DAMAGE_PTR(priv->gpu_damage)->region);
-				int n = REGION_NUM_RECTS(&DAMAGE_PTR(priv->gpu_damage)->region);
+				BoxPtr box;
+				int n;
 				Bool ok;
 
 				ok = FALSE;
+				n = sna_damage_get_boxes(priv->gpu_damage,
+							 &box);
 				if (priv->cpu_bo && sna->kgem.gen >= 30)
 					ok = sna->render.copy_boxes(sna, GXcopy,
 								    pixmap, priv->gpu_bo, 0, 0,
@@ -1238,7 +1269,7 @@ sna_drawable_move_region_to_cpu(DrawablePtr drawable,
 				if (sna_damage_intersect(priv->gpu_damage, r, &need)) {
 					BoxPtr box = REGION_RECTS(&need);
 					int n = REGION_NUM_RECTS(&need);
-					Bool ok = FALSE;
+					Bool ok;
 
 					ok = FALSE;
 					if (priv->cpu_bo && sna->kgem.gen >= 30)
@@ -1369,11 +1400,11 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, BoxPtr box)
 		goto done;
 
 	region_set(&r, box);
-	if (region_subsumes_damage(&r, priv->cpu_damage)) {
-		int n = REGION_NUM_RECTS(&DAMAGE_PTR(priv->cpu_damage)->region);
+	if (MIGRATE_ALL || region_subsumes_damage(&r, priv->cpu_damage)) {
 		Bool ok;
+		int n;
 
-		box = REGION_RECTS(&DAMAGE_PTR(priv->cpu_damage)->region);
+		n = sna_damage_get_boxes(priv->cpu_damage, &box);
 		ok = FALSE;
 		if (priv->cpu_bo)
 			ok = sna->render.copy_boxes(sna, GXcopy,
@@ -1386,8 +1417,7 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, BoxPtr box)
 			    box->x2 >= pixmap->drawable.width &&
 			    box->y2 >= pixmap->drawable.height) {
 				priv->gpu_bo =
-					sna_replace(sna,
-						    pixmap,
+					sna_replace(sna, pixmap,
 						    priv->gpu_bo,
 						    pixmap->devPrivate.ptr,
 						    pixmap->devKind);
@@ -1403,7 +1433,7 @@ sna_pixmap_move_area_to_gpu(PixmapPtr pixmap, BoxPtr box)
 
 		sna_damage_destroy(&priv->cpu_damage);
 		list_del(&priv->list);
-	} else if (DAMAGE_IS_ALL(priv->gpu_damage) ||
+	} else if (DAMAGE_IS_ALL(priv->cpu_damage) ||
 		   sna_damage_contains_box__no_reduce(priv->cpu_damage, box)) {
 		Bool ok = FALSE;
 		if (priv->cpu_bo)
@@ -1817,8 +1847,7 @@ sna_pixmap_move_to_gpu(PixmapPtr pixmap, unsigned flags)
 			    (box->x2 - box->x1) >= pixmap->drawable.width &&
 			    (box->y2 - box->y1) >= pixmap->drawable.height) {
 				priv->gpu_bo =
-					sna_replace(sna,
-						    pixmap,
+					sna_replace(sna, pixmap,
 						    priv->gpu_bo,
 						    pixmap->devPrivate.ptr,
 						    pixmap->devKind);
@@ -2609,6 +2638,9 @@ sna_put_image(DrawablePtr drawable, GCPtr gc, int depth,
 	if (wedged(sna))
 		goto fallback;
 
+	if (!ACCEL_PUT_IMAGE)
+		goto fallback;
+
 	switch (format) {
 	case ZPixmap:
 		if (!PM_IS_SOLID(drawable, gc->planemask))
@@ -2714,7 +2746,10 @@ sna_self_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 	if (dst != src)
 		get_drawable_deltas(dst, pixmap, &tx, &ty);
 
-	if (priv && priv->gpu_bo) {
+	if (priv == NULL || DAMAGE_IS_ALL(priv->cpu_damage))
+		goto fallback;
+
+	if (priv->gpu_bo) {
 		if (!sna_pixmap_move_to_gpu(pixmap, MOVE_WRITE | MOVE_READ)) {
 			DBG(("%s: fallback - not a pure copy and failed to move dst to GPU\n",
 			     __FUNCTION__));
@@ -3040,8 +3075,7 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 				assert(src_dx + box->x1 + dst_pixmap->drawable.width <= src_pixmap->drawable.width);
 
 				dst_priv->gpu_bo =
-					sna_replace(sna,
-						    dst_pixmap,
+					sna_replace(sna, dst_pixmap,
 						    dst_priv->gpu_bo,
 						    bits, stride);
 
@@ -3052,7 +3086,7 @@ sna_copy_boxes(DrawablePtr src, DrawablePtr dst, GCPtr gc,
 						       dst_pixmap->drawable.height);
 				}
 			} else {
-				DBG(("%s: dst is on the GPU, src is on the CPU, uploading\n",
+				DBG(("%s: dst is on the GPU, src is on the CPU, uploading into dst\n",
 				     __FUNCTION__));
 				sna_write_boxes(sna, dst_pixmap,
 						dst_priv->gpu_bo, dst_dx, dst_dy,
diff --git a/src/sna/sna_damage.c b/src/sna/sna_damage.c
index ea981573..6e456cf5 100644
--- a/src/sna/sna_damage.c
+++ b/src/sna/sna_damage.c
@@ -248,7 +248,7 @@ _sna_damage_create_elt(struct sna_damage *damage,
 	 return damage;
 }
 
-static void
+static struct sna_damage *
 _sna_damage_create_elt_from_boxes(struct sna_damage *damage,
 				  const BoxRec *boxes, int count,
 				  int16_t dx, int16_t dy)
@@ -273,13 +273,13 @@ _sna_damage_create_elt_from_boxes(struct sna_damage *damage,
 		count -=n;
 		boxes += n;
 		if (count == 0)
-			return;
+			return damage;
 	}
 
 	DBG(("    %s(): new elt\n", __FUNCTION__));
 
 	if (!_sna_damage_create_boxes(damage, count))
-		return;
+		return damage;
 
 	for (i = 0; i < count; i++) {
 		damage->box[i].x1 = boxes[i].x1 + dx;
@@ -289,9 +289,11 @@ _sna_damage_create_elt_from_boxes(struct sna_damage *damage,
 	}
 	damage->box += i;
 	damage->remain -= i;
+
+	return damage;
 }
 
-static void
+static struct sna_damage *
 _sna_damage_create_elt_from_rectangles(struct sna_damage *damage,
 				       const xRectangle *r, int count,
 				       int16_t dx, int16_t dy)
@@ -317,13 +319,13 @@ _sna_damage_create_elt_from_rectangles(struct sna_damage *damage,
 		count -=n;
 		r += n;
 		if (count == 0)
-			return;
+			return damage;
 	}
 
 	DBG(("    %s(): new elt\n", __FUNCTION__));
 
 	if (!_sna_damage_create_boxes(damage, count))
-		return;
+		return damage;
 
 	for (i = 0; i < count; i++) {
 		damage->box[i].x1 = r[i].x + dx;
@@ -333,9 +335,11 @@ _sna_damage_create_elt_from_rectangles(struct sna_damage *damage,
 	}
 	damage->box += n;
 	damage->remain -= n;
+
+	return damage;
 }
 
-static void
+static struct sna_damage *
 _sna_damage_create_elt_from_points(struct sna_damage *damage,
 				   const DDXPointRec *p, int count,
 				   int16_t dx, int16_t dy)
@@ -361,13 +365,13 @@ _sna_damage_create_elt_from_points(struct sna_damage *damage,
 		count -=n;
 		p += n;
 		if (count == 0)
-			return;
+			return damage;
 	}
 
 	DBG(("    %s(): new elt\n", __FUNCTION__));
 
 	if (! _sna_damage_create_boxes(damage, count))
-		return;
+		return damage;
 
 	for (i = 0; i < count; i++) {
 		damage->box[i].x1 = p[i].x + dx;
@@ -377,6 +381,8 @@ _sna_damage_create_elt_from_points(struct sna_damage *damage,
 	}
 	damage->box += count;
 	damage->remain -= count;
+
+	return damage;
 }
 
 static void free_list(struct list *head)
@@ -407,6 +413,21 @@ static void __sna_damage_reduce(struct sna_damage *damage)
 	nboxes -= damage->remain;
 	if (nboxes == 0)
 		goto done;
+	if (nboxes == 1) {
+		pixman_region16_t tmp;
+
+		tmp.extents = damage->embedded_box.box[0];
+		tmp.data = NULL;
+
+		if (damage->mode == DAMAGE_ADD)
+			pixman_region_union(region, region, &tmp);
+		else
+			pixman_region_subtract(region, region, &tmp);
+		damage->extents = region->extents;
+
+		goto done;
+	}
+
 	if (damage->mode == DAMAGE_ADD)
 		nboxes += REGION_NUM_RECTS(region);
 
@@ -496,6 +517,23 @@ done:
 	DBG(("    reduce: after region.n=%d\n", REGION_NUM_RECTS(region)));
 }
 
+static void damage_union(struct sna_damage *damage, const BoxRec *box)
+{
+	if (damage->extents.x2 < damage->extents.x1) {
+		damage->extents = *box;
+	} else {
+		if (damage->extents.x1 > box->x1)
+			damage->extents.x1 = box->x1;
+		if (damage->extents.x2 < box->x2)
+			damage->extents.x2 = box->x2;
+
+		if (damage->extents.y1 > box->y1)
+			damage->extents.y1 = box->y1;
+		if (damage->extents.y2 < box->y2)
+			damage->extents.y2 = box->y2;
+	}
+}
+
 inline static struct sna_damage *__sna_damage_add(struct sna_damage *damage,
 						  RegionPtr region)
 {
@@ -516,7 +554,7 @@ inline static struct sna_damage *__sna_damage_add(struct sna_damage *damage,
 
 	if (REGION_NUM_RECTS(&damage->region) <= 1) {
 		pixman_region_union(&damage->region, &damage->region, region);
-		damage->extents = damage->region.extents;
+		damage_union(damage, &region->extents);
 		return damage;
 	}
 
@@ -524,17 +562,7 @@ inline static struct sna_damage *__sna_damage_add(struct sna_damage *damage,
 					     &region->extents) == PIXMAN_REGION_IN)
 		return damage;
 
-
-	if (damage->extents.x1 > region->extents.x1)
-		damage->extents.x1 = region->extents.x1;
-	if (damage->extents.x2 < region->extents.x2)
-		damage->extents.x2 = region->extents.x2;
-
-	if (damage->extents.y1 > region->extents.y1)
-		damage->extents.y1 = region->extents.y1;
-	if (damage->extents.y2 < region->extents.y2)
-		damage->extents.y2 = region->extents.y2;
-
+	damage_union(damage, &region->extents);
 	return _sna_damage_create_elt(damage,
 				      REGION_RECTS(region),
 				      REGION_NUM_RECTS(region));
@@ -566,6 +594,50 @@ fastcall struct sna_damage *_sna_damage_add(struct sna_damage *damage,
 }
 #endif
 
+static void _pixman_region_union_box(RegionRec *region, const BoxRec *box)
+{
+	RegionRec u = { *box, NULL };
+	pixman_region_union(region, region, &u);
+}
+
+static struct sna_damage *__sna_damage_add_box(struct sna_damage *damage,
+					       const BoxRec *box)
+{
+	if (box->y2 <= box->y1 || box->x2 <= box->x1)
+		return damage;
+
+	if (!damage) {
+		damage = _sna_damage_create();
+		if (damage == NULL)
+			return NULL;
+	} else switch (damage->mode) {
+	case DAMAGE_ALL:
+		return damage;
+	case DAMAGE_SUBTRACT:
+		__sna_damage_reduce(damage);
+	case DAMAGE_ADD:
+		break;
+	}
+
+	switch (REGION_NUM_RECTS(&damage->region)) {
+	case 0:
+		pixman_region_init_rects(&damage->region, box, 1);
+		damage_union(damage, box);
+		return damage;
+	case 1:
+		_pixman_region_union_box(&damage->region, box);
+		damage_union(damage, box);
+		return damage;
+	}
+
+	if (pixman_region_contains_rectangle(&damage->region,
+					     (BoxPtr)box) == PIXMAN_REGION_IN)
+		return damage;
+
+	damage_union(damage, box);
+	return _sna_damage_create_elt(damage, box, 1);
+}
+
 inline static struct sna_damage *
 __sna_damage_add_boxes(struct sna_damage *damage,
 		       const BoxRec *box, int n,
@@ -608,29 +680,15 @@ __sna_damage_add_boxes(struct sna_damage *damage,
 	extents.y1 += dy;
 	extents.y2 += dy;
 
+	if (n == 1)
+		return __sna_damage_add_box(damage, &extents);
+
 	if (pixman_region_contains_rectangle(&damage->region,
 					     &extents) == PIXMAN_REGION_IN)
 		return damage;
 
-	_sna_damage_create_elt_from_boxes(damage, box, n, dx, dy);
-
-	if (REGION_NUM_RECTS(&damage->region) == 0) {
-		damage->region.extents = damage->embedded_box.box[0];
-		damage->region.data = NULL;
-		damage->extents = extents;
-	} else {
-		if (damage->extents.x1 > extents.x1)
-			damage->extents.x1 = extents.x1;
-		if (damage->extents.x2 < extents.x2)
-			damage->extents.x2 = extents.x2;
-
-		if (damage->extents.y1 > extents.y1)
-			damage->extents.y1 = extents.y1;
-		if (damage->extents.y2 < extents.y2)
-			damage->extents.y2 = extents.y2;
-	}
-
-	return damage;
+	damage_union(damage, &extents);
+	return _sna_damage_create_elt_from_boxes(damage, box, n, dx, dy);
 }
 
 #if DEBUG_DAMAGE
@@ -660,12 +718,6 @@ struct sna_damage *_sna_damage_add_boxes(struct sna_damage *damage,
 }
 #endif
 
-static void _pixman_region_union_box(RegionRec *region, const BoxRec *box)
-{
-	RegionRec u = { *box, NULL };
-	pixman_region_union(region, region, &u);
-}
-
 inline static struct sna_damage *
 __sna_damage_add_rectangles(struct sna_damage *damage,
 			    const xRectangle *r, int n,
@@ -698,6 +750,9 @@ __sna_damage_add_rectangles(struct sna_damage *damage,
 	extents.y1 += dy;
 	extents.y2 += dy;
 
+	if (n == 1)
+		return __sna_damage_add_box(damage, &extents);
+
 	if (!damage) {
 		damage = _sna_damage_create();
 		if (damage == NULL)
@@ -715,25 +770,8 @@ __sna_damage_add_rectangles(struct sna_damage *damage,
 					     &extents) == PIXMAN_REGION_IN)
 		return damage;
 
-	_sna_damage_create_elt_from_rectangles(damage, r, n, dx, dy);
-
-	if (REGION_NUM_RECTS(&damage->region) == 0) {
-		damage->region.extents = damage->embedded_box.box[0];
-		damage->region.data = NULL;
-		damage->extents = extents;
-	} else {
-		if (damage->extents.x1 > extents.x1)
-			damage->extents.x1 = extents.x1;
-		if (damage->extents.x2 < extents.x2)
-			damage->extents.x2 = extents.x2;
-
-		if (damage->extents.y1 > extents.y1)
-			damage->extents.y1 = extents.y1;
-		if (damage->extents.y2 < extents.y2)
-			damage->extents.y2 = extents.y2;
-	}
-
-	return damage;
+	damage_union(damage, &extents);
+	return _sna_damage_create_elt_from_rectangles(damage, r, n, dx, dy);
 }
 
 #if DEBUG_DAMAGE
@@ -792,6 +830,9 @@ __sna_damage_add_points(struct sna_damage *damage,
 	extents.y1 += dy;
 	extents.y2 += dy + 1;
 
+	if (n == 1)
+		return __sna_damage_add_box(damage, &extents);
+
 	if (!damage) {
 		damage = _sna_damage_create();
 		if (damage == NULL)
@@ -809,24 +850,9 @@ __sna_damage_add_points(struct sna_damage *damage,
 					     &extents) == PIXMAN_REGION_IN)
 		return damage;
 
+	damage_union(damage, &extents);
 	_sna_damage_create_elt_from_points(damage, p, n, dx, dy);
 
-	if (REGION_NUM_RECTS(&damage->region) == 0) {
-		damage->region.extents = damage->embedded_box.box[0];
-		damage->region.data = NULL;
-		damage->extents = extents;
-	} else {
-		if (damage->extents.x1 > extents.x1)
-			damage->extents.x1 = extents.x1;
-		if (damage->extents.x2 < extents.x2)
-			damage->extents.x2 = extents.x2;
-
-		if (damage->extents.y1 > extents.y1)
-			damage->extents.y1 = extents.y1;
-		if (damage->extents.y2 < extents.y2)
-			damage->extents.y2 = extents.y2;
-	}
-
 	return damage;
 }
 
@@ -857,53 +883,6 @@ struct sna_damage *_sna_damage_add_points(struct sna_damage *damage,
 }
 #endif
 
-inline static struct sna_damage *__sna_damage_add_box(struct sna_damage *damage,
-						      const BoxRec *box)
-{
-	if (box->y2 <= box->y1 || box->x2 <= box->x1)
-		return damage;
-
-	if (!damage) {
-		damage = _sna_damage_create();
-		if (damage == NULL)
-			return NULL;
-	} else switch (damage->mode) {
-	case DAMAGE_ALL:
-		return damage;
-	case DAMAGE_SUBTRACT:
-		__sna_damage_reduce(damage);
-	case DAMAGE_ADD:
-		break;
-	}
-
-	switch (REGION_NUM_RECTS(&damage->region)) {
-	case 0:
-		pixman_region_init_rects(&damage->region, box, 1);
-		damage->extents = *box;
-		return damage;
-	case 1:
-		_pixman_region_union_box(&damage->region, box);
-		damage->extents = damage->region.extents;
-		return damage;
-	}
-
-	if (pixman_region_contains_rectangle(&damage->region,
-					     (BoxPtr)box) == PIXMAN_REGION_IN)
-		return damage;
-
-	if (damage->extents.x1 > box->x1)
-		damage->extents.x1 = box->x1;
-	if (damage->extents.x2 < box->x2)
-		damage->extents.x2 = box->x2;
-
-	if (damage->extents.y1 > box->y1)
-		damage->extents.y1 = box->y1;
-	if (damage->extents.y2 < box->y2)
-		damage->extents.y2 = box->y2;
-
-	return _sna_damage_create_elt(damage, box, 1);
-}
-
 #if DEBUG_DAMAGE
 fastcall struct sna_damage *_sna_damage_add_box(struct sna_damage *damage,
 						const BoxRec *box)
@@ -971,12 +950,16 @@ struct sna_damage *_sna_damage_is_all(struct sna_damage *damage,
 	if (damage->region.data)
 		return damage;
 
+	DBG(("%s: (%d, %d), (%d, %d)\n", __FUNCTION__,
+	     damage->extents.x1, damage->extents.y1,
+	     damage->extents.x2, damage->extents.y2));
+
 	assert(damage->extents.x1 == 0 &&
 	       damage->extents.y1 == 0 &&
 	       damage->extents.x2 == width &&
 	       damage->extents.y2 == height);
 
-	return _sna_damage_all(damage, width, height);
+	return __sna_damage_all(damage, width, height);
 }
 
 static bool box_contains(const BoxRec *a, const BoxRec *b)
@@ -1213,16 +1196,15 @@ int _sna_damage_contains_box(struct sna_damage *damage,
 bool _sna_damage_contains_box__no_reduce(const struct sna_damage *damage,
 					 const BoxRec *box)
 {
-	int ret;
-
 	assert(damage && damage->mode != DAMAGE_ALL);
+	if (damage->mode == DAMAGE_SUBTRACT)
+		return false;
+
 	if (!sna_damage_maybe_contains_box(damage, box))
 		return false;
 
-	ret = pixman_region_contains_rectangle((RegionPtr)&damage->region,
-					       (BoxPtr)box);
-	return (!damage->dirty || damage->mode == DAMAGE_ADD) &&
-		ret == PIXMAN_REGION_IN;
+	return pixman_region_contains_rectangle((RegionPtr)&damage->region,
+						(BoxPtr)box) == PIXMAN_REGION_IN;
 }
 
 static Bool __sna_damage_intersect(struct sna_damage *damage,
diff --git a/src/sna/sna_render.c b/src/sna/sna_render.c
index 142c1aa9..304ff0f6 100644
--- a/src/sna/sna_render.c
+++ b/src/sna/sna_render.c
@@ -69,7 +69,7 @@ no_render_composite(struct sna *sna,
 		    int16_t width, int16_t height,
 		    struct sna_composite_op *tmp)
 {
-	DBG(("%s ()\n", __FUNCTION__));
+	DBG(("%s (op=%d, mask? %d)\n", __FUNCTION__, op, mask != NULL));
 
 	if (mask == NULL &&
 	    sna_blt_composite(sna,