glamor: Replace fallback preparation code

These offer a simpler and more efficient means for temporarily
transitioning to CPU-accessible memory for fallback implementations.

v2: Do not attempt fallbacks with GLAMOR_DRM_ONLY pixmaps

    glamor cannot transfer pixels for GLAMOR_DRM_ONLY pixmaps using
    glReadPixels and glTexSubImage2D, and so there's no way to perform
    fallback operations with these pixmaps.

v3: Clear ->pbo field when deleting the PBO.  Otherwise, we'd reuse
    the old name next time we fall back on the pixmap, which would
    potentially conflict with some other pixmap that genned a new
    name, or just do a lazy allocation of the name (compat GL context,
    like we currently use) or error out (core GL context, like we hope
    to use some day).  Also, style fixes.  Changes by anholt, acked by
    keithp.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Eric Anholt <eric@anholt.net>

9 files changed, 346 insertions, 190 deletions

diff --git a/glamor/Makefile.am b/glamor/Makefile.am
index bde58b632..cb81872b4 100644
--- a/glamor/Makefile.am
+++ b/glamor/Makefile.am
@@ -21,6 +21,8 @@ libglamor_la_SOURCES = \
 	glamor_image.c \
 	glamor_render.c \
 	glamor_gradient.c \
+	glamor_prepare.c \
+	glamor_prepare.h \
 	glamor_program.c \
 	glamor_program.h \
 	glamor_rects.c \
diff --git a/glamor/glamor.c b/glamor/glamor.c
index 08f6ba174..6aaf1b40f 100644
--- a/glamor/glamor.c
+++ b/glamor/glamor.c
@@ -397,6 +397,10 @@ glamor_init(ScreenPtr screen, unsigned int flags)
         }
     }
 
+    glamor_priv->has_rw_pbo = FALSE;
+    if (glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP)
+        glamor_priv->has_rw_pbo = TRUE;
+
     glamor_priv->has_khr_debug = epoxy_has_gl_extension("GL_KHR_debug");
     glamor_priv->has_pack_invert =
         epoxy_has_gl_extension("GL_MESA_pack_invert");
diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index b34943761..ffb6eba4b 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -114,27 +114,6 @@ glamor_link_glsl_prog(ScreenPtr screen, GLint prog, const char *format, ...)
     }
 }
 
-Bool
-glamor_prepare_access(DrawablePtr drawable, glamor_access_t access)
-{
-    PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
-    glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
-
-    if (pixmap->devPrivate.ptr) {
-        /* Already mapped, nothing needs to be done.  Note that we
-         * aren't allowing promotion from RO to RW, because it would
-         * require re-mapping the PBO.
-         */
-        assert(!GLAMOR_PIXMAP_PRIV_HAS_FBO(pixmap_priv) ||
-               access == GLAMOR_ACCESS_RO ||
-               pixmap_priv->base.map_access == GLAMOR_ACCESS_RW);
-        return TRUE;
-    }
-    pixmap_priv->base.map_access = access;
-
-    return glamor_download_pixmap_to_cpu(pixmap, access);
-}
-
 /*
  *  When downloading a unsupported color format to CPU memory,
     we need to shuffle the color elements and then use a supported
@@ -313,91 +292,6 @@ glamor_fini_finish_access_shaders(ScreenPtr screen)
     glDeleteProgram(glamor_priv->finish_access_prog[1]);
 }
 
-void
-glamor_finish_access(DrawablePtr drawable)
-{
-    PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
-    glamor_pixmap_private *pixmap_priv = glamor_get_pixmap_private(pixmap);
-    glamor_screen_private *glamor_priv =
-        glamor_get_screen_private(drawable->pScreen);
-
-    if (!GLAMOR_PIXMAP_PRIV_HAS_FBO_DOWNLOADED(pixmap_priv))
-        return;
-
-    /* If we are doing a series of unmaps from a nested map, we're
-     * done.  None of the callers do any rendering to maps after
-     * starting an unmap sequence, so we don't need to delay until the
-     * last nested unmap.
-     */
-    if (!pixmap->devPrivate.ptr)
-        return;
-
-    if (pixmap_priv->base.map_access == GLAMOR_ACCESS_RW) {
-        glamor_restore_pixmap_to_texture(pixmap);
-    }
-
-    if (pixmap_priv->base.fbo->pbo != 0 && pixmap_priv->base.fbo->pbo_valid) {
-        assert(glamor_priv->gl_flavor == GLAMOR_GL_DESKTOP);
-
-        glamor_make_current(glamor_priv);
-        glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
-        glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
-        glDeleteBuffers(1, &pixmap_priv->base.fbo->pbo);
-
-        pixmap_priv->base.fbo->pbo_valid = FALSE;
-        pixmap_priv->base.fbo->pbo = 0;
-    }
-    else {
-        free(pixmap->devPrivate.ptr);
-    }
-
-    if (pixmap_priv->type == GLAMOR_TEXTURE_DRM)
-        pixmap->devKind = pixmap_priv->base.drm_stride;
-
-    if (pixmap_priv->base.gl_fbo == GLAMOR_FBO_DOWNLOADED)
-        pixmap_priv->base.gl_fbo = GLAMOR_FBO_NORMAL;
-
-    pixmap->devPrivate.ptr = NULL;
-}
-
-/**
- * Calls uxa_prepare_access with UXA_PREPARE_SRC for the tile, if that is the
- * current fill style.
- *
- * Solid doesn't use an extra pixmap source, so we don't worry about them.
- * Stippled/OpaqueStippled are 1bpp and can be in fb, so we should worry
- * about them.
- */
-Bool
-glamor_prepare_access_gc(GCPtr gc)
-{
-    if (gc->stipple) {
-        if (!glamor_prepare_access(&gc->stipple->drawable, GLAMOR_ACCESS_RO))
-            return FALSE;
-    }
-    if (gc->fillStyle == FillTiled) {
-        if (!glamor_prepare_access(&gc->tile.pixmap->drawable,
-                                   GLAMOR_ACCESS_RO)) {
-            if (gc->stipple)
-                glamor_finish_access(&gc->stipple->drawable);
-            return FALSE;
-        }
-    }
-    return TRUE;
-}
-
-/**
- * Finishes access to the tile in the GC, if used.
- */
-void
-glamor_finish_access_gc(GCPtr gc)
-{
-    if (gc->fillStyle == FillTiled)
-        glamor_finish_access(&gc->tile.pixmap->drawable);
-    if (gc->stipple)
-        glamor_finish_access(&gc->stipple->drawable);
-}
-
 Bool
 glamor_stipple(PixmapPtr pixmap, PixmapPtr stipple,
                int x, int y, int width, int height,
diff --git a/glamor/glamor_picture.c b/glamor/glamor_picture.c
index 5fdc5f9b0..cbbc19406 100644
--- a/glamor/glamor_picture.c
+++ b/glamor/glamor_picture.c
@@ -45,24 +45,6 @@ glamor_upload_picture_to_texture(PicturePtr picture)
     return glamor_upload_pixmap_to_texture(pixmap);
 }
 
-Bool
-glamor_prepare_access_picture(PicturePtr picture, glamor_access_t access)
-{
-    if (!picture || !picture->pDrawable)
-        return TRUE;
-
-    return glamor_prepare_access(picture->pDrawable, access);
-}
-
-void
-glamor_finish_access_picture(PicturePtr picture)
-{
-    if (!picture || !picture->pDrawable)
-        return;
-
-    glamor_finish_access(picture->pDrawable);
-}
-
 /* 
  * We should already have drawable attached to it, if it has one.
  * Then set the attached pixmap to is_picture format, and set
diff --git a/glamor/glamor_prepare.c b/glamor/glamor_prepare.c
new file mode 100644
index 000000000..561c55d19
--- /dev/null
+++ b/glamor/glamor_prepare.c
@@ -0,0 +1,274 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#include "glamor_priv.h"
+#include "glamor_prepare.h"
+#include "glamor_transfer.h"
+
+/*
+ * Make a pixmap ready to draw with fb by
+ * creating a PBO large enough for the whole object
+ * and downloading all of the FBOs into it.
+ */
+
+static Bool
+glamor_prep_pixmap_box(PixmapPtr pixmap, glamor_access_t access, BoxPtr box)
+{
+    ScreenPtr                   screen = pixmap->drawable.pScreen;
+    glamor_screen_private       *glamor_priv = glamor_get_screen_private(screen);
+    glamor_pixmap_private       *priv = glamor_get_pixmap_private(pixmap);
+    int                         gl_access, gl_usage;
+    RegionRec                   region;
+
+    if (priv->type == GLAMOR_DRM_ONLY)
+        return FALSE;
+
+    if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(priv))
+        return TRUE;
+
+    RegionInit(&region, box, 1);
+
+    /* See if it's already mapped */
+    if (pixmap->devPrivate.ptr) {
+        /*
+         * Someone else has mapped this pixmap;
+         * we'll assume that it's directly mapped
+         * by a lower level driver
+         */
+        if (!priv->base.prepared)
+            return TRUE;
+
+        /* In X, multiple Drawables can be stored in the same Pixmap (such as
+         * each individual window in a non-composited screen pixmap, or the
+         * reparented window contents inside the window-manager-decorated window
+         * pixmap on a composited screen).
+         *
+         * As a result, when doing a series of mappings for a fallback, we may
+         * need to add more boxes to the set of data we've downloaded, as we go.
+         */
+        RegionSubtract(&region, &region, &priv->base.prepare_region);
+        if (!RegionNotEmpty(&region))
+            return TRUE;
+
+        if (access == GLAMOR_ACCESS_RW)
+            FatalError("attempt to remap buffer as writable");
+
+        if (priv->base.pbo) {
+            glBindBuffer(GL_PIXEL_PACK_BUFFER, priv->base.pbo);
+            glUnmapBuffer(GL_PIXEL_PACK_BUFFER);
+            pixmap->devPrivate.ptr = NULL;
+        }
+    } else {
+        RegionInit(&priv->base.prepare_region, box, 1);
+
+        if (glamor_priv->has_rw_pbo) {
+            if (priv->base.pbo == 0)
+                glGenBuffers(1, &priv->base.pbo);
+
+            if (access == GLAMOR_ACCESS_RW)
+                gl_usage = GL_DYNAMIC_DRAW;
+            else
+                gl_usage = GL_STREAM_READ;
+
+            glBindBuffer(GL_PIXEL_PACK_BUFFER, priv->base.pbo);
+            glBufferData(GL_PIXEL_PACK_BUFFER,
+                         pixmap->devKind * pixmap->drawable.height, NULL,
+                         gl_usage);
+        } else {
+            pixmap->devPrivate.ptr = malloc(pixmap->devKind *
+                                            pixmap->drawable.height);
+            if (!pixmap->devPrivate.ptr)
+                return FALSE;
+        }
+        priv->base.map_access = access;
+    }
+
+    glamor_download_boxes(pixmap, RegionRects(&region), RegionNumRects(&region),
+                          0, 0, 0, 0, pixmap->devPrivate.ptr, pixmap->devKind);
+
+    RegionUninit(&region);
+
+    if (glamor_priv->has_rw_pbo) {
+        if (priv->base.map_access == GLAMOR_ACCESS_RW)
+            gl_access = GL_READ_WRITE;
+        else
+            gl_access = GL_READ_ONLY;
+
+        pixmap->devPrivate.ptr = glMapBuffer(GL_PIXEL_PACK_BUFFER, gl_access);
+        glBindBuffer(GL_PIXEL_PACK_BUFFER, 0);
+    }
+
+    priv->base.prepared = TRUE;
+    return TRUE;
+}
+
+/*
+ * When we're done with the drawable, unmap the PBO, reupload
+ * if we were writing to it and then unbind it to release the memory
+ */
+
+static void
+glamor_fini_pixmap(PixmapPtr pixmap)
+{
+    ScreenPtr                   screen = pixmap->drawable.pScreen;
+    glamor_screen_private       *glamor_priv = glamor_get_screen_private(screen);
+    glamor_pixmap_private       *priv = glamor_get_pixmap_private(pixmap);
+
+    if (!GLAMOR_PIXMAP_PRIV_HAS_FBO(priv))
+        return;
+
+    if (!priv->base.prepared)
+        return;
+
+    if (glamor_priv->has_rw_pbo) {
+        glBindBuffer(GL_PIXEL_UNPACK_BUFFER, priv->base.pbo);
+        glUnmapBuffer(GL_PIXEL_UNPACK_BUFFER);
+        pixmap->devPrivate.ptr = NULL;
+    }
+
+    if (priv->base.map_access == GLAMOR_ACCESS_RW) {
+        glamor_upload_boxes(pixmap,
+                            RegionRects(&priv->base.prepare_region),
+                            RegionNumRects(&priv->base.prepare_region),
+                            0, 0, 0, 0, pixmap->devPrivate.ptr, pixmap->devKind);
+    }
+
+    RegionUninit(&priv->base.prepare_region);
+
+    if (glamor_priv->has_rw_pbo) {
+        glBindBuffer(GL_PIXEL_UNPACK_BUFFER, 0);
+        glDeleteBuffers(1, &priv->base.pbo);
+        priv->base.pbo = 0;
+    } else {
+        free(pixmap->devPrivate.ptr);
+        pixmap->devPrivate.ptr = NULL;
+    }
+
+    priv->base.prepared = FALSE;
+}
+
+Bool
+glamor_prepare_access(DrawablePtr drawable, glamor_access_t access)
+{
+    PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+    BoxRec box;
+    int off_x, off_y;
+
+    glamor_get_drawable_deltas(drawable, pixmap, &off_x, &off_y);
+
+    box.x1 = drawable->x + off_x;
+    box.x2 = box.x1 + drawable->width;
+    box.y1 = drawable->y + off_y;
+    box.y2 = box.y1 + drawable->height;
+    return glamor_prep_pixmap_box(pixmap, access, &box);
+}
+
+Bool
+glamor_prepare_access_box(DrawablePtr drawable, glamor_access_t access,
+                         int x, int y, int w, int h)
+{
+    PixmapPtr pixmap = glamor_get_drawable_pixmap(drawable);
+    BoxRec box;
+    int off_x, off_y;
+
+    glamor_get_drawable_deltas(drawable, pixmap, &off_x, &off_y);
+    box.x1 = drawable->x + x + off_x;
+    box.x2 = box.x1 + w;
+    box.y1 = drawable->y + y + off_y;
+    box.y2 = box.y1 + h;
+    return glamor_prep_pixmap_box(pixmap, access, &box);
+}
+
+void
+glamor_finish_access(DrawablePtr drawable)
+{
+    glamor_fini_pixmap(glamor_get_drawable_pixmap(drawable));
+}
+
+/*
+ * Make a picture ready to use with fb.
+ */
+
+Bool
+glamor_prepare_access_picture(PicturePtr picture, glamor_access_t access)
+{
+    if (!picture || !picture->pDrawable)
+        return TRUE;
+
+    return glamor_prepare_access(picture->pDrawable, access);
+}
+
+Bool
+glamor_prepare_access_picture_box(PicturePtr picture, glamor_access_t access,
+                        int x, int y, int w, int h)
+{
+    if (!picture || !picture->pDrawable)
+        return TRUE;
+    return glamor_prepare_access_box(picture->pDrawable, access,
+                                    x, y, w, h);
+}
+
+void
+glamor_finish_access_picture(PicturePtr picture)
+{
+    if (!picture || !picture->pDrawable)
+        return;
+
+    glamor_finish_access(picture->pDrawable);
+}
+
+/*
+ * Make a GC ready to use with fb. This just
+ * means making sure the appropriate fill pixmap is
+ * in CPU memory again
+ */
+
+Bool
+glamor_prepare_access_gc(GCPtr gc)
+{
+    switch (gc->fillStyle) {
+    case FillTiled:
+        return glamor_prepare_access(&gc->tile.pixmap->drawable,
+                                     GLAMOR_ACCESS_RO);
+    case FillStippled:
+    case FillOpaqueStippled:
+        return glamor_prepare_access(&gc->stipple->drawable, GLAMOR_ACCESS_RO);
+    }
+    return TRUE;
+}
+
+/*
+ * Free any temporary CPU pixmaps for the GC
+ */
+void
+glamor_finish_access_gc(GCPtr gc)
+{
+    switch (gc->fillStyle) {
+    case FillTiled:
+        glamor_finish_access(&gc->tile.pixmap->drawable);
+        break;
+    case FillStippled:
+    case FillOpaqueStippled:
+        glamor_finish_access(&gc->stipple->drawable);
+        break;
+    }
+}
diff --git a/glamor/glamor_prepare.h b/glamor/glamor_prepare.h
new file mode 100644
index 000000000..85fa79574
--- /dev/null
+++ b/glamor/glamor_prepare.h
@@ -0,0 +1,52 @@
+/*
+ * Copyright © 2014 Keith Packard
+ *
+ * Permission to use, copy, modify, distribute, and sell this software and its
+ * documentation for any purpose is hereby granted without fee, provided that
+ * the above copyright notice appear in all copies and that both that copyright
+ * notice and this permission notice appear in supporting documentation, and
+ * that the name of the copyright holders not be used in advertising or
+ * publicity pertaining to distribution of the software without specific,
+ * written prior permission.  The copyright holders make no representations
+ * about the suitability of this software for any purpose.  It is provided "as
+ * is" without express or implied warranty.
+ *
+ * THE COPYRIGHT HOLDERS DISCLAIM ALL WARRANTIES WITH REGARD TO THIS SOFTWARE,
+ * INCLUDING ALL IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS, IN NO
+ * EVENT SHALL THE COPYRIGHT HOLDERS BE LIABLE FOR ANY SPECIAL, INDIRECT OR
+ * CONSEQUENTIAL DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE,
+ * DATA OR PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
+ * TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE
+ * OF THIS SOFTWARE.
+ */
+
+#ifndef _GLAMOR_PREPARE_H_
+#define _GLAMOR_PREPARE_H_
+
+Bool
+glamor_prepare_access(DrawablePtr drawable, glamor_access_t access);
+
+Bool
+glamor_prepare_access_box(DrawablePtr drawable, glamor_access_t access,
+                         int x, int y, int w, int h);
+
+void
+glamor_finish_access(DrawablePtr drawable);
+
+Bool
+glamor_prepare_access_picture(PicturePtr picture, glamor_access_t access);
+
+Bool
+glamor_prepare_access_picture_box(PicturePtr picture, glamor_access_t access,
+                        int x, int y, int w, int h);
+
+void
+glamor_finish_access_picture(PicturePtr picture);
+
+Bool
+glamor_prepare_access_gc(GCPtr gc);
+
+void
+glamor_finish_access_gc(GCPtr gc);
+
+#endif /* _GLAMOR_PREPARE_H_ */
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index c56c55973..531d25d17 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -209,6 +209,7 @@ typedef struct glamor_screen_private {
     int has_buffer_storage;
     int has_khr_debug;
     int max_fbo_size;
+    int has_rw_pbo;
 
     struct xorg_list
         fbo_cache[CACHE_FORMAT_COUNT][CACHE_BUCKET_WCOUNT][CACHE_BUCKET_HCOUNT];
@@ -430,6 +431,9 @@ typedef struct glamor_pixmap_private_base {
     int drm_stride;
     glamor_screen_private *glamor_priv;
     PicturePtr picture;
+    GLuint pbo;
+    RegionRec prepare_region;
+    Bool prepared;
 #if GLAMOR_HAS_GBM
     EGLImageKHR image;
 #endif
@@ -624,14 +628,9 @@ void glamor_copy_n_to_n(DrawablePtr src, DrawablePtr dst, GCPtr gc,
                         Bool upsidedown, Pixel bitplane, void *closure);
 
 /* glamor_core.c */
-Bool glamor_prepare_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);
-void glamor_finish_access_gc(GCPtr gc);
 void glamor_init_finish_access_shaders(ScreenPtr screen);
 void glamor_fini_finish_access_shaders(ScreenPtr screen);
+
 const Bool glamor_get_drawable_location(const DrawablePtr drawable);
 void glamor_get_drawable_deltas(DrawablePtr drawable, PixmapPtr pixmap,
                                 int *x, int *y);
@@ -919,10 +918,6 @@ int glamor_create_picture(PicturePtr picture);
 
 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);
-
 void glamor_destroy_picture(PicturePtr picture);
 
 /* fixup a fbo to the exact size as the pixmap. */
diff --git a/glamor/glamor_render.c b/glamor/glamor_render.c
index 5a7a23880..8952023f1 100644
--- a/glamor/glamor_render.c
+++ b/glamor/glamor_render.c
@@ -1586,15 +1586,6 @@ _glamor_composite(CARD8 op,
     RegionRec region;
     BoxPtr extent;
     int nbox, ok = FALSE;
-    PixmapPtr sub_dest_pixmap = NULL;
-    PixmapPtr sub_source_pixmap = NULL;
-    PixmapPtr sub_mask_pixmap = NULL;
-    int dest_x_off, dest_y_off, saved_dest_x, saved_dest_y;
-    int source_x_off, source_y_off, saved_source_x, saved_source_y;
-    int mask_x_off, mask_y_off, saved_mask_x, saved_mask_y;
-    DrawablePtr saved_dest_drawable;
-    DrawablePtr saved_source_drawable;
-    DrawablePtr saved_mask_drawable;
     int force_clip = 0;
 
     dest_pixmap_priv = glamor_get_pixmap_private(dest_pixmap);
@@ -1737,34 +1728,13 @@ _glamor_composite(CARD8 op,
          dest->pDrawable->width, dest->pDrawable->height,
          glamor_get_picture_location(dest));
 
-#define GET_SUB_PICTURE(p, access)		do {					\
-	glamor_get_drawable_deltas(p->pDrawable, p ##_pixmap,				\
-				   & p ##_x_off, & p ##_y_off);				\
-	sub_ ##p ##_pixmap = glamor_get_sub_pixmap(p ##_pixmap,				\
-					      x_ ##p + p ##_x_off + p->pDrawable->x,	\
-					      y_ ##p + p ##_y_off + p->pDrawable->y,	\
-					      width, height, access);			\
-	if (sub_ ##p ##_pixmap != NULL) {						\
-		saved_ ##p ##_drawable = p->pDrawable;					\
-		saved_ ##p ##_x = x_ ##p;						\
-		saved_ ##p ##_y = y_ ##p;						\
-		if (p->pCompositeClip)							\
-			pixman_region_translate (p->pCompositeClip,			\
-						 -p->pDrawable->x - x_ ##p,		\
-						 -p->pDrawable->y - y_ ##p);		\
-		p->pDrawable = &sub_ ##p ##_pixmap->drawable;				\
-		x_ ##p = 0;								\
-		y_ ##p = 0;								\
-	} } while(0)
-    GET_SUB_PICTURE(dest, GLAMOR_ACCESS_RW);
-    if (source->pDrawable && !source->transform)
-        GET_SUB_PICTURE(source, GLAMOR_ACCESS_RO);
-    if (mask && mask->pDrawable && !mask->transform)
-        GET_SUB_PICTURE(mask, GLAMOR_ACCESS_RO);
-
-    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)) {
+    if (glamor_prepare_access_picture_box(dest, GLAMOR_ACCESS_RW,
+                                          x_dest, y_dest, width, height) &&
+        glamor_prepare_access_picture_box(source, GLAMOR_ACCESS_RO,
+                                          x_source, y_source, width, height) &&
+        glamor_prepare_access_picture_box(mask, GLAMOR_ACCESS_RO,
+                                          x_mask, y_mask, width, height))
+    {
         fbComposite(op,
                     source, mask, dest,
                     x_source, y_source,
@@ -1774,25 +1744,6 @@ _glamor_composite(CARD8 op,
     glamor_finish_access_picture(source);
     glamor_finish_access_picture(dest);
 
-#define PUT_SUB_PICTURE(p, access)		do {				\
-	if (sub_ ##p ##_pixmap != NULL) {					\
-		x_ ##p = saved_ ##p ##_x;					\
-		y_ ##p = saved_ ##p ##_y;					\
-		p->pDrawable = saved_ ##p ##_drawable;				\
-		if (p->pCompositeClip)						\
-			pixman_region_translate (p->pCompositeClip,		\
-						 p->pDrawable->x + x_ ##p,	\
-						 p->pDrawable->y + y_ ##p);	\
-		glamor_put_sub_pixmap(sub_ ##p ##_pixmap, p ##_pixmap,		\
-				      x_ ##p + p ##_x_off + p->pDrawable->x,	\
-				      y_ ##p + p ##_y_off + p->pDrawable->y,	\
-				      width, height, access);			\
-	}} while(0)
-    if (mask && mask->pDrawable)
-        PUT_SUB_PICTURE(mask, GLAMOR_ACCESS_RO);
-    if (source->pDrawable)
-        PUT_SUB_PICTURE(source, GLAMOR_ACCESS_RO);
-    PUT_SUB_PICTURE(dest, GLAMOR_ACCESS_RW);
  done:
     return ret;
 }
diff --git a/glamor/glamor_utils.h b/glamor/glamor_utils.h
index 4c1581ef5..5a568f1c2 100644
--- a/glamor/glamor_utils.h
+++ b/glamor/glamor_utils.h
@@ -32,6 +32,8 @@
 #ifndef __GLAMOR_UTILS_H__
 #define __GLAMOR_UTILS_H__
 
+#include "glamor_prepare.h"
+
 #define v_from_x_coord_x(_xscale_, _x_)          ( 2 * (_x_) * (_xscale_) - 1.0)
 #define v_from_x_coord_y(_yscale_, _y_)          (-2 * (_y_) * (_yscale_) + 1.0)
 #define v_from_x_coord_y_inverted(_yscale_, _y_) (2 * (_y_) * (_yscale_) - 1.0)