sna/glyphs: Fix computation of extents for long strings

And make sure we consider such overflowing strings for correct clipping
against Windows.

To offset the cost of doing a full extents check (~10% on aa10text), we
introduce an approximate extents query (~1% on aa10text). The disparity
should be rare, and should be an overestimate to force redundant
clipping.

Reported-by: Clemens Eisserer <linuxhippy@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=70541
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

2 files changed, 144 insertions, 49 deletions

diff --git a/src/sna/sna.h b/src/sna/sna.h
index cf02e9e3..07789ef6 100644
--- a/src/sna/sna.h
+++ b/src/sna/sna.h
@@ -919,6 +919,12 @@ inline static bool is_power_of_two(unsigned x)
 inline static bool is_clipped(const RegionRec *r,
 			      const DrawableRec *d)
 {
+	DBG(("%s: region[%ld]x(%d, %d),(%d, %d) against drawable %dx%d\n",
+	     __FUNCTION__,
+	     (long)RegionNumRects(r),
+	     r->extents.x1, r->extents.y1,
+	     r->extents.x2, r->extents.y2,
+	     d->width, d->height));
 	return (r->data ||
 		r->extents.x2 - r->extents.x1 != d->width ||
 		r->extents.y2 - r->extents.y1 != d->height);
diff --git a/src/sna/sna_glyphs.c b/src/sna/sna_glyphs.c
index b7ccb888..e656f417 100644
--- a/src/sna/sna_glyphs.c
+++ b/src/sna/sna_glyphs.c
@@ -310,8 +310,8 @@ glyph_extents(int nlist,
 	      GlyphPtr *glyphs,
 	      BoxPtr extents)
 {
-	int16_t x1, x2, y1, y2;
-	int16_t x, y;
+	int x1, x2, y1, y2;
+	int x, y;
 
 	x1 = y1 = MAXSHORT;
 	x2 = y2 = MINSHORT;
@@ -347,10 +347,60 @@ glyph_extents(int nlist,
 		}
 	}
 
-	extents->x1 = x1;
-	extents->x2 = x2;
-	extents->y1 = y1;
-	extents->y2 = y2;
+	extents->x1 = x1 > MINSHORT ? x1 : MINSHORT;
+	extents->y1 = y1 > MINSHORT ? y1 : MINSHORT;
+	extents->x2 = x2 < MAXSHORT ? x2 : MAXSHORT;
+	extents->y2 = y2 < MAXSHORT ? y2 : MAXSHORT;
+}
+
+static void
+glyph_approx_extents(int nlist,
+		     GlyphListPtr list,
+		     GlyphPtr *glyphs,
+		     BoxPtr extents)
+{
+	int x1, x2, y1, y2;
+	int x, y;
+
+	x1 = y1 = MAXSHORT;
+	x2 = y2 = MINSHORT;
+
+	x = y = 0;
+	while (nlist--) {
+		int n = list->len;
+		x += list->xOff;
+		if (x < x1)
+			x1 = x;
+		if (x > x2)
+			x2 = x;
+
+		y += list->yOff;
+		if (y < y1)
+			y1 = y;
+		if (y > y2)
+			y2 = y;
+
+		list++;
+		while (n--) {
+			GlyphPtr glyph = *glyphs++;
+			x += glyph->info.xOff;
+			y += glyph->info.yOff;
+		}
+
+		if (y < y1)
+			y1 = y;
+		if (y > y2)
+			y2 = y;
+		if (x < x1)
+			x1 = x;
+		if (x > x2)
+			x2 = x;
+	}
+
+	extents->x1 = x1 > MINSHORT ? x1 : MINSHORT;
+	extents->y1 = y1 > MINSHORT ? y1 : MINSHORT;
+	extents->x2 = x2 < MAXSHORT ? x2 : MAXSHORT;
+	extents->y2 = y2 < MAXSHORT ? y2 : MAXSHORT;
 }
 
 static inline unsigned int
@@ -517,6 +567,37 @@ static void apply_damage_clipped_to_dst(struct sna_composite_op *op,
 	sna_damage_add_box(op->damage, &box);
 }
 
+static inline bool clipped_glyphs(PicturePtr dst, int nlist, GlyphListPtr list, GlyphPtr *glyphs)
+{
+	PixmapPtr pixmap;
+	BoxRec box;
+
+	if (dst->pCompositeClip->data) {
+		DBG(("%s: yes, has complex region\n", __FUNCTION__));
+		return true;
+	}
+
+	pixmap = get_drawable_pixmap(dst->pDrawable);
+	if (dst->pCompositeClip->extents.x2 - dst->pCompositeClip->extents.x1 >= pixmap->drawable.width &&
+	    dst->pCompositeClip->extents.y2 - dst->pCompositeClip->extents.y1 >= pixmap->drawable.height) {
+		DBG(("%s: no, region matches drawable\n", __FUNCTION__));
+		return false;
+	}
+
+	glyph_approx_extents(nlist, list, glyphs, &box);
+	DBG(("%s? glyph extents (%d, %d), (%d, %d), region (%d, %d), (%d, %d): %s\n",
+	     __FUNCTION__, box.x1, box.y1, box.x2, box.y2,
+	     dst->pCompositeClip->extents.x1, dst->pCompositeClip->extents.y1, dst->pCompositeClip->extents.x2, dst->pCompositeClip->extents.y2,
+	     (box.x1 < dst->pCompositeClip->extents.x1 ||
+	      box.y1 < dst->pCompositeClip->extents.y1 ||
+	      box.x2 > dst->pCompositeClip->extents.x2 ||
+	      box.y2 > dst->pCompositeClip->extents.y2) ? "yes" : "no"));
+	return (box.x1 < dst->pCompositeClip->extents.x1 ||
+		box.y1 < dst->pCompositeClip->extents.y1 ||
+		box.x2 > dst->pCompositeClip->extents.x2 ||
+		box.y2 > dst->pCompositeClip->extents.y2);
+}
+
 flatten static bool
 glyphs_to_dst(struct sna *sna,
 	      CARD8 op,
@@ -541,7 +622,7 @@ glyphs_to_dst(struct sna *sna,
 	     __FUNCTION__, op, src_x, src_y, nlist,
 	     list->xOff, list->yOff, dst->pDrawable->x, dst->pDrawable->y));
 
-	if (is_clipped(dst->pCompositeClip, dst->pDrawable)) {
+	if (clipped_glyphs(dst, nlist, list, glyphs)) {
 		rects = REGION_RECTS(dst->pCompositeClip);
 		nrect = REGION_NUM_RECTS(dst->pCompositeClip);
 	} else
@@ -683,7 +764,7 @@ glyphs0_to_dst(struct sna *sna,
 	PicturePtr glyph_atlas;
 	BoxPtr rects;
 	int nrect;
-	int16_t x, y;
+	int x, y;
 
 	if (NO_GLYPHS_TO_DST)
 		return false;
@@ -694,7 +775,7 @@ glyphs0_to_dst(struct sna *sna,
 	     __FUNCTION__, op, src_x, src_y, nlist,
 	     list->xOff, list->yOff, dst->pDrawable->x, dst->pDrawable->y));
 
-	if (is_clipped(dst->pCompositeClip, dst->pDrawable)) {
+	if (clipped_glyphs(dst, nlist, list, glyphs)) {
 		rects = REGION_RECTS(dst->pCompositeClip);
 		nrect = REGION_NUM_RECTS(dst->pCompositeClip);
 	} else
@@ -750,46 +831,54 @@ glyphs0_to_dst(struct sna *sna,
 			}
 
 			if (nrect) {
-				for (i = 0; i < nrect; i++) {
-					struct sna_composite_rectangles r;
-					int16_t dx, dy;
-					int16_t x2, y2;
-
-					r.dst.x = x - glyph->info.x;
-					r.dst.y = y - glyph->info.y;
-					x2 = r.dst.x + glyph->info.width;
-					y2 = r.dst.y + glyph->info.height;
-					dx = dy = 0;
-
-					DBG(("%s: glyph=(%d, %d), (%d, %d), clip=(%d, %d), (%d, %d)\n",
-					     __FUNCTION__,
-					     r.dst.x, r.dst.y, x2, y2,
-					     rects[i].x1, rects[i].y1,
-					     rects[i].x2, rects[i].y2));
-					if (rects[i].y1 >= y2)
-						break;
-
-					if (r.dst.x < rects[i].x1)
-						dx = rects[i].x1 - r.dst.x, r.dst.x = rects[i].x1;
-					if (x2 > rects[i].x2)
-						x2 = rects[i].x2;
-					if (r.dst.y < rects[i].y1)
-						dy = rects[i].y1 - r.dst.y, r.dst.y = rects[i].y1;
-					if (y2 > rects[i].y2)
-						y2 = rects[i].y2;
-
-					if (r.dst.x < x2 && r.dst.y < y2) {
-						DBG(("%s: blt=(%d, %d), (%d, %d)\n",
-						     __FUNCTION__, r.dst.x, r.dst.y, x2, y2));
-
-						r.src.x = r.dst.x + src_x;
-						r.src.y = r.dst.y + src_y;
-						r.mask.x = dx + p->coordinate.x;
-						r.mask.y = dy + p->coordinate.y;
-						r.width  = x2 - r.dst.x;
-						r.height = y2 - r.dst.y;
-						tmp.blt(sna, &tmp, &r);
-						apply_damage(&tmp, &r);
+				int xi = x - glyph->info.x;
+				int yi = y - glyph->info.y;
+
+				if (xi < dst->pCompositeClip->extents.x2 &&
+				    yi < dst->pCompositeClip->extents.y2 &&
+				    xi + glyph->info.width  > dst->pCompositeClip->extents.x1 &&
+				    yi + glyph->info.height > dst->pCompositeClip->extents.y1) {
+					for (i = 0; i < nrect; i++) {
+						struct sna_composite_rectangles r;
+						int16_t dx, dy;
+						int16_t x2, y2;
+
+						r.dst.x = x - glyph->info.x;
+						r.dst.y = y - glyph->info.y;
+						x2 = r.dst.x + glyph->info.width;
+						y2 = r.dst.y + glyph->info.height;
+						dx = dy = 0;
+
+						DBG(("%s: glyph=(%d, %d), (%d, %d), clip=(%d, %d), (%d, %d)\n",
+						     __FUNCTION__,
+						     r.dst.x, r.dst.y, x2, y2,
+						     rects[i].x1, rects[i].y1,
+						     rects[i].x2, rects[i].y2));
+						if (rects[i].y1 >= y2)
+							break;
+
+						if (r.dst.x < rects[i].x1)
+							dx = rects[i].x1 - r.dst.x, r.dst.x = rects[i].x1;
+						if (x2 > rects[i].x2)
+							x2 = rects[i].x2;
+						if (r.dst.y < rects[i].y1)
+							dy = rects[i].y1 - r.dst.y, r.dst.y = rects[i].y1;
+						if (y2 > rects[i].y2)
+							y2 = rects[i].y2;
+
+						if (r.dst.x < x2 && r.dst.y < y2) {
+							DBG(("%s: blt=(%d, %d), (%d, %d)\n",
+							     __FUNCTION__, r.dst.x, r.dst.y, x2, y2));
+
+							r.src.x = r.dst.x + src_x;
+							r.src.y = r.dst.y + src_y;
+							r.mask.x = dx + p->coordinate.x;
+							r.mask.y = dy + p->coordinate.y;
+							r.width  = x2 - r.dst.x;
+							r.height = y2 - r.dst.y;
+							tmp.blt(sna, &tmp, &r);
+							apply_damage(&tmp, &r);
+						}
 					}
 				}
 			} else {