intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* UXA clipping reduction
@ 2011-06-01  7:02 Eric Anholt
  2011-06-01  7:02 ` [PATCH 1/3] uxa: Simplify BLT solid acceleration for spans filling by only clipping once Eric Anholt
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Eric Anholt @ 2011-06-01  7:02 UTC (permalink / raw)
  To: intel-gfx

There's this odd pattern in some UXA code that appears to have been
carried over from fbfillrect.c where for each thing to be filled we:

1) clip against the clip extents
2) trivial reject
3) clip that against each box of the clip
4) reject per box
5) draw.

I don't see a justification for this complexity, in terms of how I
expect cliprects to be (generally 1 rectangle) or in terms of how I
expect rendering to be (generally within cliprects if there are any).
And, we see from the history of uxa-accel.c, we've screwed up this
clipping up before.  So, I propose instead, we:

1) Clip against each box of the clip
2) Reject per box
3) draw.

Here are the patches.  No performance testing included, but one known
bug is deleted (which I actually hope we land the small submitted fix
for first, I'm just hoping to get a name/commit message on it).

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/3] uxa: Simplify BLT solid acceleration for spans filling by only clipping once.
  2011-06-01  7:02 UXA clipping reduction Eric Anholt
@ 2011-06-01  7:02 ` Eric Anholt
  2011-06-01  7:02 ` [PATCH 2/3] uxa: Simplify Composite solid acceleration for spans " Eric Anholt
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Anholt @ 2011-06-01  7:02 UTC (permalink / raw)
  To: intel-gfx

We were clipping each span against the bounds of the clip, throwing
out the span early if it was all clipped, and then walked the clip box
clipping against each of the cliprects.  We would expect spans to
typically be clipped against one box, and not thrown out, so we were
not saving any work there.  For multiple cliprects, we were adding
work.  Only for many spans clipped entirely out of a complicated clip
region would it have saved work, and it clearly didn't save bugs as
evidenced by the many fix attempts here.
---
 uxa/uxa-accel.c |   65 +++++++++++++++++-------------------------------------
 1 files changed, 21 insertions(+), 44 deletions(-)

diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 0650ac2..31c37e8 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -1,3 +1,4 @@
+
 /*
  * Copyright ® 2001 Keith Packard
  *
@@ -65,7 +66,7 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
 	BoxPtr pextent, pbox;
 	int nbox;
 	int extentX1, extentX2, extentY1, extentY2;
-	int fullX1, fullX2, fullY1;
+	int x1, x2, y, fullX1, fullX2, fullY1;
 	int partX1, partX2;
 	int off_x, off_y;
 	xRenderColor color;
@@ -216,58 +217,34 @@ solid:
 						 pGC->fgPixel))
 		goto fallback;
 
-	pextent = REGION_EXTENTS(pGC->screen, pClip);
-	extentX1 = pextent->x1;
-	extentY1 = pextent->y1;
-	extentX2 = pextent->x2;
-	extentY2 = pextent->y2;
 	while (n--) {
-		fullX1 = ppt->x;
-		fullY1 = ppt->y;
-		fullX2 = fullX1 + (int)*pwidth;
+		x1 = ppt->x;
+		y = ppt->y;
+		x2 = x1 + (int)*pwidth;
 		ppt++;
 		pwidth++;
 
-		if (fullY1 < extentY1 || extentY2 <= fullY1)
-			continue;
+		nbox = REGION_NUM_RECTS(pClip);
+		pbox = REGION_RECTS(pClip);
+		while (nbox--) {
+			if (pbox->y1 > y || pbox->y2 <= y)
+				continue;
 
-		if (fullX1 < extentX1)
-			fullX1 = extentX1;
+			if (x1 < pbox->x1)
+				x1 = pbox->x1;
 
-		if (fullX2 > extentX2)
-			fullX2 = extentX2;
+			if (x2 > pbox->x2)
+				x2 = pbox->x2;
 
-		if (fullX1 >= fullX2)
-			continue;
+			if (x2 <= x1)
+				continue;
 
-		nbox = REGION_NUM_RECTS(pClip);
-		if (nbox == 1) {
 			(*uxa_screen->info->solid) (dst_pixmap,
-						    fullX1 + off_x,
-						    fullY1 + off_y,
-						    fullX2 + off_x,
-						    fullY1 + 1 + off_y);
-		} else {
-			pbox = REGION_RECTS(pClip);
-			while (nbox--) {
-				if (pbox->y1 <= fullY1 && fullY1 < pbox->y2) {
-					partX1 = pbox->x1;
-					if (partX1 < fullX1)
-						partX1 = fullX1;
-					partX2 = pbox->x2;
-					if (partX2 > fullX2)
-						partX2 = fullX2;
-					if (partX2 > partX1) {
-						(*uxa_screen->info->
-						 solid) (dst_pixmap,
-							 partX1 + off_x,
-							 fullY1 + off_y,
-							 partX2 + off_x,
-							 fullY1 + 1 + off_y);
-					}
-				}
-				pbox++;
-			}
+						    x1 + off_x,
+						    y + off_y,
+						    x2 + off_x,
+						    y + 1 + off_y);
+			pbox++;
 		}
 	}
 	(*uxa_screen->info->done_solid) (dst_pixmap);
-- 
1.7.5.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] uxa: Simplify Composite solid acceleration for spans by only clipping once.
  2011-06-01  7:02 UXA clipping reduction Eric Anholt
  2011-06-01  7:02 ` [PATCH 1/3] uxa: Simplify BLT solid acceleration for spans filling by only clipping once Eric Anholt
@ 2011-06-01  7:02 ` Eric Anholt
  2011-06-01  7:02 ` [PATCH 3/3] uxa: Simplify uxa_poly_fill_rect " Eric Anholt
  2011-06-01 18:56 ` UXA clipping reduction Keith Packard
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Anholt @ 2011-06-01  7:02 UTC (permalink / raw)
  To: intel-gfx

Unlike the previous commit removing this style of code, the code in
this one was originally wrong, and would fail to clip in the second
pass of clipping when y was > pbox->y2.

Bug #37233.
---
 uxa/uxa-accel.c |   79 ++++++++++++++++--------------------------------------
 1 files changed, 24 insertions(+), 55 deletions(-)

diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 31c37e8..8f6da63 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -63,11 +63,9 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
 	uxa_screen_t *uxa_screen = uxa_get_screen(screen);
 	RegionPtr pClip = fbGetCompositeClip(pGC);
 	PixmapPtr dst_pixmap, src_pixmap = NULL;
-	BoxPtr pextent, pbox;
+	BoxPtr pbox;
 	int nbox;
-	int extentX1, extentX2, extentY1, extentY2;
-	int x1, x2, y, fullX1, fullX2, fullY1;
-	int partX1, partX2;
+	int x1, x2, y;
 	int off_x, off_y;
 	xRenderColor color;
 	PictFormatPtr format;
@@ -142,62 +140,35 @@ uxa_fill_spans(DrawablePtr pDrawable, GCPtr pGC, int n,
 		goto solid;
 	}
 
-	pextent = REGION_EXTENTS(pGC->screen, pClip);
-	extentX1 = pextent->x1;
-	extentY1 = pextent->y1;
-	extentX2 = pextent->x2;
-	extentY2 = pextent->y2;
 	while (n--) {
-		fullX1 = ppt->x;
-		fullY1 = ppt->y;
-		fullX2 = fullX1 + (int)*pwidth;
+		x1 = ppt->x;
+		y = ppt->y;
+		x2 = x1 + (int)*pwidth;
 		ppt++;
 		pwidth++;
 
-		if (fullY1 < extentY1 || extentY2 <= fullY1)
-			continue;
+		nbox = REGION_NUM_RECTS(pClip);
+		pbox = REGION_RECTS(pClip);
+		while (nbox--) {
+			if (pbox->y1 > y || pbox->y2 <= y)
+				continue;
 
-		if (fullX1 < extentX1)
-			fullX1 = extentX1;
+			if (x1 < pbox->x1)
+				x1 = pbox->x1;
 
-		if (fullX2 > extentX2)
-			fullX2 = extentX2;
+			if (x2 > pbox->x2)
+				x2 = pbox->x2;
 
-		if (fullX1 >= fullX2)
-			continue;
+			if (x2 <= x1)
+				continue;
 
-		nbox = REGION_NUM_RECTS(pClip);
-		if (nbox == 1) {
 			uxa_screen->info->composite(dst_pixmap,
-						    0, 0, 0, 0,
-						    fullX1 + off_x,
-						    fullY1 + off_y,
-						    fullX2 - fullX1, 1);
-		} else {
-			pbox = REGION_RECTS(pClip);
-			while (nbox--) {
-				if (pbox->y1 > fullY1)
-					break;
-
-				if (pbox->y1 <= fullY1) {
-					partX1 = pbox->x1;
-					if (partX1 < fullX1)
-						partX1 = fullX1;
-
-					partX2 = pbox->x2;
-					if (partX2 > fullX2)
-						partX2 = fullX2;
-
-					if (partX2 > partX1) {
-						uxa_screen->info->composite(dst_pixmap,
-									    0, 0, 0, 0,
-									    partX1 + off_x,
-									    fullY1 + off_y,
-									    partX2 - partX1, 1);
-					}
-				}
-				pbox++;
-			}
+						    0, 0,
+						    0, 0,
+						    x1 + off_x, y + off_y,
+						    x2 - x1, 1);
+
+			pbox++;
 		}
 	}
 
@@ -240,10 +211,8 @@ solid:
 				continue;
 
 			(*uxa_screen->info->solid) (dst_pixmap,
-						    x1 + off_x,
-						    y + off_y,
-						    x2 + off_x,
-						    y + 1 + off_y);
+						    x1 + off_x, y + off_y,
+						    x2 + off_x, y + 1 + off_y);
 			pbox++;
 		}
 	}
-- 
1.7.5.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] uxa: Simplify uxa_poly_fill_rect by only clipping once.
  2011-06-01  7:02 UXA clipping reduction Eric Anholt
  2011-06-01  7:02 ` [PATCH 1/3] uxa: Simplify BLT solid acceleration for spans filling by only clipping once Eric Anholt
  2011-06-01  7:02 ` [PATCH 2/3] uxa: Simplify Composite solid acceleration for spans " Eric Anholt
@ 2011-06-01  7:02 ` Eric Anholt
  2011-06-01 18:56 ` UXA clipping reduction Keith Packard
  3 siblings, 0 replies; 5+ messages in thread
From: Eric Anholt @ 2011-06-01  7:02 UTC (permalink / raw)
  To: intel-gfx

---
 uxa/uxa-accel.c |   87 +++++++++++++++++--------------------------------------
 1 files changed, 27 insertions(+), 60 deletions(-)

diff --git a/uxa/uxa-accel.c b/uxa/uxa-accel.c
index 8f6da63..dd83542 100644
--- a/uxa/uxa-accel.c
+++ b/uxa/uxa-accel.c
@@ -785,10 +785,7 @@ uxa_poly_fill_rect(DrawablePtr pDrawable,
 	RegionPtr pClip = fbGetCompositeClip(pGC);
 	PixmapPtr pPixmap;
 	register BoxPtr pbox;
-	BoxPtr pextent;
-	int extentX1, extentX2, extentY1, extentY2;
 	int fullX1, fullX2, fullY1, fullY2;
-	int partX1, partX2, partY1, partY2;
 	int xoff, yoff;
 	int xorg, yorg;
 	int n;
@@ -850,11 +847,6 @@ fallback:
 	xorg = pDrawable->x;
 	yorg = pDrawable->y;
 
-	pextent = REGION_EXTENTS(pGC->pScreen, pClip);
-	extentX1 = pextent->x1;
-	extentY1 = pextent->y1;
-	extentX2 = pextent->x2;
-	extentY2 = pextent->y2;
 	while (nrect--) {
 		fullX1 = prect->x + xorg;
 		fullY1 = prect->y + yorg;
@@ -862,62 +854,37 @@ fallback:
 		fullY2 = fullY1 + (int)prect->height;
 		prect++;
 
-		if (fullX1 < extentX1)
-			fullX1 = extentX1;
-
-		if (fullY1 < extentY1)
-			fullY1 = extentY1;
+		n = REGION_NUM_RECTS(pClip);
+		pbox = REGION_RECTS(pClip);
+		/*
+		 * clip the rectangle to each box in the clip region
+		 * this is logically equivalent to calling Intersect(),
+		 * but rectangles may overlap each other here.
+		 */
+		while (n--) {
+			int x1 = fullX1;
+			int x2 = fullX2;
+			int y1 = fullY1;
+			int y2 = fullY2;
 
-		if (fullX2 > extentX2)
-			fullX2 = extentX2;
+			if (pbox->x1 > x1)
+				x1 = pbox->x1;
+			if (pbox->x2 < x2)
+				x2 = pbox->x2;
+			if (pbox->y1 > y1)
+				y1 = pbox->y1;
+			if (pbox->y2 < y2)
+				y2 = pbox->y2;
+			pbox++;
 
-		if (fullY2 > extentY2)
-			fullY2 = extentY2;
+			if (x1 >= x2 || y1 >= y2)
+				continue;
 
-		if ((fullX1 >= fullX2) || (fullY1 >= fullY2))
-			continue;
-		n = REGION_NUM_RECTS(pClip);
-		if (n == 1) {
 			(*uxa_screen->info->solid) (pPixmap,
-						    fullX1 + xoff,
-						    fullY1 + yoff,
-						    fullX2 + xoff,
-						    fullY2 + yoff);
-		} else {
-			pbox = REGION_RECTS(pClip);
-			/*
-			 * clip the rectangle to each box in the clip region
-			 * this is logically equivalent to calling Intersect(),
-			 * but rectangles may overlap each other here.
-			 */
-			while (n--) {
-				partX1 = pbox->x1;
-				if (partX1 < fullX1)
-					partX1 = fullX1;
-				partY1 = pbox->y1;
-				if (partY1 < fullY1)
-					partY1 = fullY1;
-				partX2 = pbox->x2;
-				if (partX2 > fullX2)
-					partX2 = fullX2;
-				partY2 = pbox->y2;
-				if (partY2 > fullY2)
-					partY2 = fullY2;
-
-				pbox++;
-
-				if (partX1 < partX2 && partY1 < partY2) {
-					(*uxa_screen->info->solid) (pPixmap,
-								    partX1 +
-								    xoff,
-								    partY1 +
-								    yoff,
-								    partX2 +
-								    xoff,
-								    partY2 +
-								    yoff);
-				}
-			}
+						    x1 + xoff,
+						    y1 + yoff,
+						    x2 + xoff,
+						    y2 + yoff);
 		}
 	}
 	(*uxa_screen->info->done_solid) (pPixmap);
-- 
1.7.5.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: UXA clipping reduction
  2011-06-01  7:02 UXA clipping reduction Eric Anholt
                   ` (2 preceding siblings ...)
  2011-06-01  7:02 ` [PATCH 3/3] uxa: Simplify uxa_poly_fill_rect " Eric Anholt
@ 2011-06-01 18:56 ` Keith Packard
  3 siblings, 0 replies; 5+ messages in thread
From: Keith Packard @ 2011-06-01 18:56 UTC (permalink / raw)
  To: Eric Anholt, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 314 bytes --]

On Wed,  1 Jun 2011 00:02:53 -0700, Eric Anholt <eric@anholt.net> wrote:

> 1) Clip against each box of the clip
> 2) Reject per box
> 3) draw.

I didn't try to figure out the old code, but the new code looks correct
to me.

Reviewed-by: Keith Packard <keithp@keithp.com>

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2011-06-01 18:56 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-01  7:02 UXA clipping reduction Eric Anholt
2011-06-01  7:02 ` [PATCH 1/3] uxa: Simplify BLT solid acceleration for spans filling by only clipping once Eric Anholt
2011-06-01  7:02 ` [PATCH 2/3] uxa: Simplify Composite solid acceleration for spans " Eric Anholt
2011-06-01  7:02 ` [PATCH 3/3] uxa: Simplify uxa_poly_fill_rect " Eric Anholt
2011-06-01 18:56 ` UXA clipping reduction Keith Packard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).