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

Eric Anholt eric at anholt.net
Tue Mar 11 14:30:33 PDT 2014


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

Signed-off-by: Eric Anholt <eric at anholt.net>
---
 glamor/glamor_addtraps.c     |  2 +-
 glamor/glamor_copyarea.c     | 14 ++++++--------
 glamor/glamor_copyplane.c    | 13 +++++++------
 glamor/glamor_core.c         | 17 ++++++++---------
 glamor/glamor_fill.c         | 11 +++++------
 glamor/glamor_fillspans.c    | 11 +++++------
 glamor/glamor_getspans.c     |  2 +-
 glamor/glamor_picture.c      |  4 ++--
 glamor/glamor_polyfillrect.c | 11 +++++------
 glamor/glamor_polylines.c    | 11 +++++------
 glamor/glamor_priv.h         |  4 ++--
 glamor/glamor_putimage.c     |  2 +-
 glamor/glamor_render.c       | 27 +++++++++++----------------
 glamor/glamor_setspans.c     |  2 +-
 glamor/glamor_triangles.c    | 15 ++++++---------
 glamor/glamor_utils.h        | 33 ++++++++++++++++-----------------
 16 files changed, 82 insertions(+), 97 deletions(-)

diff --git a/glamor/glamor_addtraps.c b/glamor/glamor_addtraps.c
index 655d87e..fdc0f42 100644
--- a/glamor/glamor_addtraps.c
+++ b/glamor/glamor_addtraps.c
@@ -40,8 +40,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/glamor/glamor_copyarea.c b/glamor/glamor_copyarea.c
index d6bcacd..b367e03 100644
--- a/glamor/glamor_copyarea.c
+++ b/glamor/glamor_copyarea.c
@@ -570,15 +570,13 @@ _glamor_copy_n_to_n(DrawablePtr src,
                     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/glamor/glamor_copyplane.c b/glamor/glamor_copyplane.c
index c42d33e..50e9cf3 100644
--- a/glamor/glamor_copyplane.c
+++ b/glamor/glamor_copyplane.c
@@ -38,12 +38,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/glamor/glamor_core.c b/glamor/glamor_core.c
index 7a7ca08..c0994c1 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -303,7 +303,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 = glamor_get_pixmap_private(pixmap);
@@ -365,7 +365,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;
         }
     }
@@ -379,9 +379,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
@@ -455,7 +455,7 @@ glamor_validate_gc(GCPtr gc, unsigned long changes, DrawablePtr drawable)
                     (&old_tile->drawable, GLAMOR_ACCESS_RO)) {
                     new_tile =
                         fb24_32ReformatTile(old_tile, drawable->bitsPerPixel);
-                    glamor_finish_access(&old_tile->drawable, GLAMOR_ACCESS_RO);
+                    glamor_finish_access(&old_tile->drawable);
                 }
             }
             if (new_tile) {
@@ -478,8 +478,7 @@ glamor_validate_gc(GCPtr gc, unsigned long changes, DrawablePtr drawable)
                 if (glamor_prepare_access
                     (&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);
                 }
             }
         }
@@ -495,7 +494,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 {
@@ -539,7 +538,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/glamor/glamor_fill.c b/glamor/glamor_fill.c
index a3f0f29..d91dafb 100644
--- a/glamor/glamor_fill.c
+++ b/glamor/glamor_fill.c
@@ -112,13 +112,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/glamor/glamor_fillspans.c b/glamor/glamor_fillspans.c
index 7261d28..8cbd79f 100644
--- a/glamor/glamor_fillspans.c
+++ b/glamor/glamor_fillspans.c
@@ -79,13 +79,12 @@ _glamor_fill_spans(DrawablePtr drawable,
     }
     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/glamor/glamor_getspans.c b/glamor/glamor_getspans.c
index ff58725..42df87f 100644
--- a/glamor/glamor_getspans.c
+++ b/glamor/glamor_getspans.c
@@ -69,8 +69,8 @@ _glamor_get_spans(DrawablePtr drawable,
     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/glamor/glamor_picture.c b/glamor/glamor_picture.c
index 8bbe2e9..5fdc5f9 100644
--- a/glamor/glamor_picture.c
+++ b/glamor/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/glamor/glamor_polyfillrect.c b/glamor/glamor_polyfillrect.c
index a25fc4e..1e361a4 100644
--- a/glamor/glamor_polyfillrect.c
+++ b/glamor/glamor_polyfillrect.c
@@ -96,13 +96,12 @@ _glamor_poly_fill_rect(DrawablePtr drawable,
 
     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/glamor/glamor_polylines.c b/glamor/glamor_polylines.c
index b941617..697fc9e 100644
--- a/glamor/glamor_polylines.c
+++ b/glamor/glamor_polylines.c
@@ -105,13 +105,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:
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 24a3575..918f45c 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -578,7 +578,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);
@@ -905,7 +905,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/glamor/glamor_putimage.c b/glamor/glamor_putimage.c
index 702e89f..a4f97cd 100644
--- a/glamor/glamor_putimage.c
+++ b/glamor/glamor_putimage.c
@@ -229,8 +229,8 @@ glamor_put_image_xybitmap(DrawablePtr drawable, GCPtr gc,
  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/glamor/glamor_render.c b/glamor/glamor_render.c
index 086526d..93d9ead 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1788,22 +1788,17 @@ _glamor_composite(CARD8 op,
     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/glamor/glamor_setspans.c b/glamor/glamor_setspans.c
index 22fe88c..664f8ac 100644
--- a/glamor/glamor_setspans.c
+++ b/glamor/glamor_setspans.c
@@ -88,8 +88,8 @@ _glamor_set_spans(DrawablePtr drawable, GCPtr gc, char *src,
                     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/glamor/glamor_triangles.c b/glamor/glamor_triangles.c
index 693eef1..b89cb2d 100644
--- a/glamor/glamor_triangles.c
+++ b/glamor/glamor_triangles.c
@@ -41,16 +41,13 @@ _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/glamor/glamor_utils.h b/glamor/glamor_utils.h
index f9550b7..53b7d9b 100644
--- a/glamor/glamor_utils.h
+++ b/glamor/glamor_utils.h
@@ -1177,7 +1177,7 @@ glamor_dump_pixmap(PixmapPtr pixmap, int x, int y, int w, int h)
     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
@@ -1318,13 +1318,12 @@ 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.
@@ -1432,9 +1431,6 @@ 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) ||
@@ -1444,12 +1440,15 @@ 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.9.0



More information about the xorg-devel mailing list