[Glamor] [PATCH 22/34] glamor: Rely on nested mappings to handle src==dst and !prepare bugs.

Alex Deucher alexdeucher at gmail.com
Fri Feb 28 10:02:17 PST 2014


From: Eric Anholt <eric at anholt.net>

Now that the core deals with that for us, we can avoid all this extra
carefulness.

Ported from Eric's xserver glamor tree.

Signed-off-by: Eric Anholt <eric at anholt.net>
Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
---
 src/glamor_addtraps.c     |  2 +-
 src/glamor_copyarea.c     | 16 ++++++----------
 src/glamor_copyplane.c    | 13 +++++++------
 src/glamor_core.c         | 20 ++++++++------------
 src/glamor_fill.c         | 11 +++++------
 src/glamor_fillspans.c    | 12 +++++-------
 src/glamor_getspans.c     |  2 +-
 src/glamor_picture.c      |  4 ++--
 src/glamor_polyfillrect.c | 11 +++++------
 src/glamor_polylines.c    | 11 +++++------
 src/glamor_priv.h         |  4 ++--
 src/glamor_putimage.c     |  2 +-
 src/glamor_render.c       | 31 +++++++++++--------------------
 src/glamor_setspans.c     |  2 +-
 src/glamor_triangles.c    | 16 +++++-----------
 src/glamor_utils.h        | 34 ++++++++++++++++------------------
 16 files changed, 81 insertions(+), 110 deletions(-)

diff --git a/src/glamor_addtraps.c b/src/glamor_addtraps.c
index ac85296..ff3a9b8 100644
--- a/src/glamor_addtraps.c
+++ b/src/glamor_addtraps.c
@@ -41,8 +41,8 @@ _glamor_add_traps(PicturePtr pPicture,
 
 	if (glamor_prepare_access_picture(pPicture, GLAMOR_ACCESS_RW)) {
 		fbAddTraps(pPicture, x_off, y_off, ntrap, traps);
-		glamor_finish_access_picture(pPicture, GLAMOR_ACCESS_RW);
 	}
+	glamor_finish_access_picture(pPicture);
 
 	return TRUE;
 }
diff --git a/src/glamor_copyarea.c b/src/glamor_copyarea.c
index c767b6c..435d832 100644
--- a/src/glamor_copyarea.c
+++ b/src/glamor_copyarea.c
@@ -632,17 +632,13 @@ fall_back:
 			glamor_get_drawable_location(src),
 			glamor_get_drawable_location(dst));
 
-	if (glamor_prepare_access(dst, GLAMOR_ACCESS_RW)) {
-		if (dst == src
-		    || glamor_prepare_access(src, GLAMOR_ACCESS_RO)) {
-			fbCopyNtoN(src, dst, gc, box, nbox,
-				   dx, dy, reverse, upsidedown, bitplane,
-				   closure);
-			if (dst != src)
-				glamor_finish_access(src, GLAMOR_ACCESS_RO);
-		}
-		glamor_finish_access(dst, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access(dst, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access(src, GLAMOR_ACCESS_RO)) {
+		fbCopyNtoN(src, dst, gc, box, nbox,
+			   dx, dy, reverse, upsidedown, bitplane, closure);
 	}
+	glamor_finish_access(src);
+	glamor_finish_access(dst);
 	ok = TRUE;
 
       done:
diff --git a/src/glamor_copyplane.c b/src/glamor_copyplane.c
index 3f2652a..65d3244 100644
--- a/src/glamor_copyplane.c
+++ b/src/glamor_copyplane.c
@@ -39,12 +39,13 @@ _glamor_copy_plane(DrawablePtr pSrc, DrawablePtr pDst, GCPtr pGC,
 	    && glamor_ddx_fallback_check_pixmap(pDst))
 		goto fail;
 
-	glamor_prepare_access(pDst, GLAMOR_ACCESS_RW);
-	glamor_prepare_access(pSrc, GLAMOR_ACCESS_RO);
-	*pRegion = fbCopyPlane(pSrc, pDst, pGC, srcx, srcy, w, h,
-			  dstx, dsty, bitPlane);
-	glamor_finish_access(pSrc, GLAMOR_ACCESS_RO);
-	glamor_finish_access(pDst, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access(pDst, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access(pSrc, GLAMOR_ACCESS_RO)) {
+	    *pRegion = fbCopyPlane(pSrc, pDst, pGC, srcx, srcy, w, h,
+				   dstx, dsty, bitPlane);
+	}
+	glamor_finish_access(pSrc);
+	glamor_finish_access(pDst);
 	return TRUE;
 
  fail:
diff --git a/src/glamor_core.c b/src/glamor_core.c
index 33cbb72..3b68603 100644
--- a/src/glamor_core.c
+++ b/src/glamor_core.c
@@ -329,7 +329,7 @@ glamor_fini_finish_access_shaders(ScreenPtr screen)
 }
 
 void
-glamor_finish_access(DrawablePtr drawable, glamor_access_t access_mode)
+glamor_finish_access(DrawablePtr drawable)
 {
 	PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
 	glamor_pixmap_private *pixmap_priv =
@@ -395,9 +395,7 @@ glamor_prepare_access_gc(GCPtr gc)
 		if (!glamor_prepare_access(&gc->tile.pixmap->drawable,
 					   GLAMOR_ACCESS_RO)) {
 			if (gc->stipple)
-				glamor_finish_access(&gc->
-						     stipple->drawable, 
-						     GLAMOR_ACCESS_RO);
+				glamor_finish_access(&gc->stipple->drawable);
 			return FALSE;
 		}
 	}
@@ -411,9 +409,9 @@ void
 glamor_finish_access_gc(GCPtr gc)
 {
 	if (gc->fillStyle == FillTiled)
-		glamor_finish_access(&gc->tile.pixmap->drawable, GLAMOR_ACCESS_RO);
+		glamor_finish_access(&gc->tile.pixmap->drawable);
 	if (gc->stipple)
-		glamor_finish_access(&gc->stipple->drawable, GLAMOR_ACCESS_RO);
+		glamor_finish_access(&gc->stipple->drawable);
 }
 
 Bool
@@ -495,8 +493,7 @@ glamor_validate_gc(GCPtr gc, unsigned long changes, DrawablePtr drawable)
 					    fb24_32ReformatTile
 					    (old_tile,
 					     drawable->bitsPerPixel);
-					glamor_finish_access
-					    (&old_tile->drawable, GLAMOR_ACCESS_RO);
+					glamor_finish_access(&old_tile->drawable);
 				}
 			}
 			if (new_tile) {
@@ -521,8 +518,7 @@ glamor_validate_gc(GCPtr gc, unsigned long changes, DrawablePtr drawable)
 				    (&gc->tile.pixmap->drawable,
 				     GLAMOR_ACCESS_RW)) {
 					fbPadPixmap(gc->tile.pixmap);
-					glamor_finish_access
-					    (&gc->tile.pixmap->drawable, GLAMOR_ACCESS_RW);
+					glamor_finish_access(&gc->tile.pixmap->drawable);
 				}
 			}
 		}
@@ -539,7 +535,7 @@ glamor_validate_gc(GCPtr gc, unsigned long changes, DrawablePtr drawable)
 		if (glamor_prepare_access
 		    (&gc->stipple->drawable, GLAMOR_ACCESS_RW)) {
 			fbValidateGC(gc, changes, drawable);
-			glamor_finish_access(&gc->stipple->drawable, GLAMOR_ACCESS_RW);
+			glamor_finish_access(&gc->stipple->drawable);
 		}
 	} else {
 		fbValidateGC(gc, changes, drawable);
@@ -581,7 +577,7 @@ glamor_bitmap_to_region(PixmapPtr pixmap)
 	if (!glamor_prepare_access(&pixmap->drawable, GLAMOR_ACCESS_RO))
 		return NULL;
 	ret = fbPixmapToRegion(pixmap);
-	glamor_finish_access(&pixmap->drawable, GLAMOR_ACCESS_RO);
+	glamor_finish_access(&pixmap->drawable);
 	return ret;
 }
 
diff --git a/src/glamor_fill.c b/src/glamor_fill.c
index fb70ce4..6d8d9c5 100644
--- a/src/glamor_fill.c
+++ b/src/glamor_fill.c
@@ -114,13 +114,12 @@ glamor_fill(DrawablePtr drawable,
 		x = 0;
 		y = 0;
 	}
-	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
-		if (glamor_prepare_access_gc(gc)) {
-			fbFill(drawable, gc, x, y, width, height);
-			glamor_finish_access_gc(gc);
-		}
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access_gc(gc)) {
+	    fbFill(drawable, gc, x, y, width, height);
 	}
+	glamor_finish_access_gc(gc);
+	glamor_finish_access(drawable);
 
 	if (sub_pixmap != NULL) {
 		if (gc->fillStyle != FillSolid) {
diff --git a/src/glamor_fillspans.c b/src/glamor_fillspans.c
index 35e881f..df3cdc4 100644
--- a/src/glamor_fillspans.c
+++ b/src/glamor_fillspans.c
@@ -79,14 +79,12 @@ fail:
 	}
 	glamor_fallback("to %p (%c)\n", drawable,
 			glamor_get_drawable_location(drawable));
-	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
-		if (glamor_prepare_access_gc(gc)) {
-			fbFillSpans(drawable, gc, n, points, widths,
-				    sorted);
-			glamor_finish_access_gc(gc);
-		}
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access_gc(gc)) {
+	    fbFillSpans(drawable, gc, n, points, widths, sorted);
 	}
+	glamor_finish_access_gc(gc);
+	glamor_finish_access(drawable);
 	ret = TRUE;
 
 done:
diff --git a/src/glamor_getspans.c b/src/glamor_getspans.c
index dfb8ef4..06d274d 100644
--- a/src/glamor_getspans.c
+++ b/src/glamor_getspans.c
@@ -69,8 +69,8 @@ fail:
 	ret = TRUE;
 	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RO)) {
 		fbGetSpans(drawable, wmax, points, widths, count, dst);
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RO);
 	}
+	glamor_finish_access(drawable);
 done:
 	return ret;
 }
diff --git a/src/glamor_picture.c b/src/glamor_picture.c
index c55425d..77190d9 100644
--- a/src/glamor_picture.c
+++ b/src/glamor_picture.c
@@ -55,12 +55,12 @@ glamor_prepare_access_picture(PicturePtr picture, glamor_access_t access)
 }
 
 void
-glamor_finish_access_picture(PicturePtr picture, glamor_access_t access)
+glamor_finish_access_picture(PicturePtr picture)
 {
 	if (!picture || !picture->pDrawable)
 		return;
 
-	glamor_finish_access(picture->pDrawable, access);
+	glamor_finish_access(picture->pDrawable);
 }
 
 /* 
diff --git a/src/glamor_polyfillrect.c b/src/glamor_polyfillrect.c
index 4e1f7b3..2efbd76 100644
--- a/src/glamor_polyfillrect.c
+++ b/src/glamor_polyfillrect.c
@@ -98,13 +98,12 @@ fail:
 
 	glamor_fallback(" to %p (%c)\n",
 			drawable, glamor_get_drawable_location(drawable));
-	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
-		if (glamor_prepare_access_gc(gc)) {
-			fbPolyFillRect(drawable, gc, nrect, prect);
-			glamor_finish_access_gc(gc);
-		}
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access_gc(gc)) {
+	    fbPolyFillRect(drawable, gc, nrect, prect);
 	}
+	glamor_finish_access_gc(gc);
+	glamor_finish_access(drawable);
 	ret = TRUE;
 
 done:
diff --git a/src/glamor_polylines.c b/src/glamor_polylines.c
index 8891d3f..36636f0 100644
--- a/src/glamor_polylines.c
+++ b/src/glamor_polylines.c
@@ -231,13 +231,12 @@ _glamor_poly_lines(DrawablePtr drawable, GCPtr gc, int mode, int n,
 		return FALSE;
 
 	if (gc->lineWidth == 0) {
-		if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
-			if (glamor_prepare_access_gc(gc)) {
-				fbPolyLine(drawable, gc, mode, n, points);
-				glamor_finish_access_gc(gc);
-			}
-			glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
+		if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW) &&
+		    glamor_prepare_access_gc(gc)) {
+		    fbPolyLine(drawable, gc, mode, n, points);
 		}
+		glamor_finish_access_gc(gc);
+		glamor_finish_access(drawable);
 	} else {
 wide_line:
 		/* fb calls mi functions in the lineWidth != 0 case. */
diff --git a/src/glamor_priv.h b/src/glamor_priv.h
index 359604e..6803e0f 100644
--- a/src/glamor_priv.h
+++ b/src/glamor_priv.h
@@ -592,7 +592,7 @@ void glamor_copy_window(WindowPtr win, DDXPointRec old_origin,
 
 /* glamor_core.c */
 Bool glamor_prepare_access(DrawablePtr drawable, glamor_access_t access);
-void glamor_finish_access(DrawablePtr drawable, glamor_access_t access);
+void glamor_finish_access(DrawablePtr drawable);
 Bool glamor_prepare_access_window(WindowPtr window);
 void glamor_finish_access_window(WindowPtr window);
 Bool glamor_prepare_access_gc(GCPtr gc);
@@ -951,7 +951,7 @@ void glamor_set_window_pixmap(WindowPtr pWindow, PixmapPtr pPixmap);
 Bool
 glamor_prepare_access_picture(PicturePtr picture, glamor_access_t access);
 
-void glamor_finish_access_picture(PicturePtr picture, glamor_access_t access);
+void glamor_finish_access_picture(PicturePtr picture);
 
 void glamor_destroy_picture(PicturePtr picture);
 
diff --git a/src/glamor_putimage.c b/src/glamor_putimage.c
index b58d71e..9202e54 100644
--- a/src/glamor_putimage.c
+++ b/src/glamor_putimage.c
@@ -232,8 +232,8 @@ fail:
 	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
 		fbPutImage(drawable, gc, 1, x, y, w, h, left_pad, XYBitmap,
 			   bits);
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
 	}
+	glamor_finish_access(drawable);
 }
 #endif
 
diff --git a/src/glamor_render.c b/src/glamor_render.c
index 9bf1b07..aa8f99c 100644
--- a/src/glamor_render.c
+++ b/src/glamor_render.c
@@ -1860,26 +1860,17 @@ fail:
 	if (mask && mask->pDrawable && !mask->transform)
 		GET_SUB_PICTURE(mask, GLAMOR_ACCESS_RO);
 
-	if (glamor_prepare_access_picture(dest, GLAMOR_ACCESS_RW)) {
-		if (source_pixmap == dest_pixmap || glamor_prepare_access_picture
-		    (source, GLAMOR_ACCESS_RO)) {
-			if (!mask
-			    || glamor_prepare_access_picture(mask,
-							     GLAMOR_ACCESS_RO))
-			{
-				fbComposite(op,
-					    source, mask, dest,
-					    x_source, y_source,
-					    x_mask, y_mask, x_dest,
-					    y_dest, width, height);
-				if (mask)
-					glamor_finish_access_picture(mask, GLAMOR_ACCESS_RO);
-			}
-			if (source_pixmap != dest_pixmap)
-				glamor_finish_access_picture(source, GLAMOR_ACCESS_RO);
-		}
-		glamor_finish_access_picture(dest, GLAMOR_ACCESS_RW);
-	}
+	if (glamor_prepare_access_picture(dest, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access_picture(source, GLAMOR_ACCESS_RO) &&
+	    glamor_prepare_access_picture(mask, GLAMOR_ACCESS_RO)) {
+	    fbComposite(op,
+			source, mask, dest,
+			x_source, y_source,
+			x_mask, y_mask, x_dest, y_dest, width, height);
+	}
+	glamor_finish_access_picture(mask);
+	glamor_finish_access_picture(source);
+	glamor_finish_access_picture(dest);
 
 #define PUT_SUB_PICTURE(p, access)		do {				\
 	if (sub_ ##p ##_pixmap != NULL) {					\
diff --git a/src/glamor_setspans.c b/src/glamor_setspans.c
index 3d447b6..1349f80 100644
--- a/src/glamor_setspans.c
+++ b/src/glamor_setspans.c
@@ -86,8 +86,8 @@ fail:
 			drawable, glamor_get_drawable_location(drawable));
 	if (glamor_prepare_access(drawable, GLAMOR_ACCESS_RW)) {
 		fbSetSpans(drawable, gc, src, points, widths, numPoints, sorted);
-		glamor_finish_access(drawable, GLAMOR_ACCESS_RW);
 	}
+	glamor_finish_access(drawable);
 	ret = TRUE;
 
 done:
diff --git a/src/glamor_triangles.c b/src/glamor_triangles.c
index e0f4a97..3ae8cef 100644
--- a/src/glamor_triangles.c
+++ b/src/glamor_triangles.c
@@ -41,18 +41,12 @@ _glamor_triangles(CARD8 op,
 		|| glamor_ddx_fallback_check_pixmap(pSrc->pDrawable)))
 		return FALSE;
 
-	if (glamor_prepare_access_picture(pDst, GLAMOR_ACCESS_RW)) {
-		if (glamor_prepare_access_picture(pSrc,
-						  GLAMOR_ACCESS_RO)) {
-
-			fbTriangles(op, pSrc, pDst, maskFormat, xSrc,
-				    ySrc, ntris, tris);
-
-			glamor_finish_access_picture(pSrc, GLAMOR_ACCESS_RO);
-		}
-
-		glamor_finish_access_picture(pDst, GLAMOR_ACCESS_RW);
+	if (glamor_prepare_access_picture(pDst, GLAMOR_ACCESS_RW) &&
+	    glamor_prepare_access_picture(pSrc, GLAMOR_ACCESS_RO)) {
+	    fbTriangles(op, pSrc, pDst, maskFormat, xSrc, ySrc, ntris, tris);
 	}
+	glamor_finish_access_picture(pSrc);
+	glamor_finish_access_picture(pDst);
 	return TRUE;
 }
 
diff --git a/src/glamor_utils.h b/src/glamor_utils.h
index d5c7b5f..eac3597 100644
--- a/src/glamor_utils.h
+++ b/src/glamor_utils.h
@@ -1193,7 +1193,7 @@ static inline void glamor_dump_pixmap(PixmapPtr pixmap, int x, int y, int w, int
 	default:
 		ErrorF("dump depth %d, not implemented.\n", pixmap->drawable.depth);
 	}
-	glamor_finish_access(&pixmap->drawable, GLAMOR_ACCESS_RO);
+	glamor_finish_access(&pixmap->drawable);
 }
 
 static inline void _glamor_compare_pixmaps(PixmapPtr pixmap1, PixmapPtr pixmap2,
@@ -1320,13 +1320,12 @@ static inline void glamor_compare_pixmaps(PixmapPtr pixmap1, PixmapPtr pixmap2,
 {
 	assert(pixmap1->drawable.depth == pixmap2->drawable.depth);
 
-	glamor_prepare_access(&pixmap1->drawable, GLAMOR_ACCESS_RO);
-	glamor_prepare_access(&pixmap2->drawable, GLAMOR_ACCESS_RO);
-
-	_glamor_compare_pixmaps(pixmap1, pixmap2, x, y, w, h, -1, all, diffs);
-
-	glamor_finish_access(&pixmap1->drawable, GLAMOR_ACCESS_RO);
-	glamor_finish_access(&pixmap2->drawable, GLAMOR_ACCESS_RO);
+	if (glamor_prepare_access(&pixmap1->drawable, GLAMOR_ACCESS_RO) &&
+	    glamor_prepare_access(&pixmap2->drawable, GLAMOR_ACCESS_RO)) {
+	    _glamor_compare_pixmaps(pixmap1, pixmap2, x, y, w, h, -1, all, diffs);
+	}
+	glamor_finish_access(&pixmap1->drawable);
+	glamor_finish_access(&pixmap2->drawable);
 }
 
 /* This function is used to compare two pictures.
@@ -1438,9 +1437,6 @@ static inline void glamor_compare_pictures( ScreenPtr screen,
 		return;
 	}
 
-	glamor_prepare_access(&fst_pixmap->drawable, GLAMOR_ACCESS_RO);
-	glamor_prepare_access(&snd_pixmap->drawable, GLAMOR_ACCESS_RO);
-
 	if ((fst_type == SourcePictTypeLinear) ||
 	     (fst_type == SourcePictTypeRadial) ||
 	     (fst_type == SourcePictTypeConical) ||
@@ -1450,13 +1446,15 @@ static inline void glamor_compare_pictures( ScreenPtr screen,
 		x_source = y_source = 0;
 	}
 
-	_glamor_compare_pixmaps(fst_pixmap, snd_pixmap,
-	                        x_source, y_source,
-	                        width, height,
-	                        fst_picture->format, all, diffs);
-
-	glamor_finish_access(&fst_pixmap->drawable, GLAMOR_ACCESS_RO);
-	glamor_finish_access(&snd_pixmap->drawable, GLAMOR_ACCESS_RO);
+	if (glamor_prepare_access(&fst_pixmap->drawable, GLAMOR_ACCESS_RO) &&
+	    glamor_prepare_access(&snd_pixmap->drawable, GLAMOR_ACCESS_RO)) {
+	    _glamor_compare_pixmaps(fst_pixmap, snd_pixmap,
+				    x_source, y_source,
+				    width, height, fst_picture->format,
+				    all, diffs);
+	}
+	glamor_finish_access(&fst_pixmap->drawable);
+	glamor_finish_access(&snd_pixmap->drawable);
 
 	if (fst_generated)
 		glamor_destroy_picture(fst_picture);
-- 
1.8.3.1



More information about the Glamor mailing list