[PATCH 18/27] glamor: Allow nested mapping of pixmaps.

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


The common pattern is to do nested if statements making calls to
prepare_access() and then pop those mappings back off in each set of
braces.  Some cases checked for src == dst to avoid leaking mappings,
but others didn't.  Others didn't even do the nested mappings, so a
failure in the outer map would result in trying to umap the inner and
failing.

By allowing nested mappings, we can fix both problems by not requiring
the care from the caller, plus we can allow a simpler nesting of all
the prepares in one if statement.

Signed-off-by: Eric Anholt <eric at anholt.net>
---
 glamor/glamor_core.c | 19 ++++++++++++++++++-
 glamor/glamor_priv.h |  6 ++++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c
index 5883809..7a7ca08 100644
--- a/glamor/glamor_core.c
+++ b/glamor/glamor_core.c
@@ -105,6 +105,19 @@ 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.mapped_for_write);
+        return TRUE;
+    }
+    pixmap_priv->base.mapped_for_write = (access == GLAMOR_ACCESS_RW);
 
     return glamor_download_pixmap_to_cpu(pixmap, access);
 }
@@ -300,7 +313,11 @@ glamor_finish_access(DrawablePtr drawable, glamor_access_t access_mode)
     if (!GLAMOR_PIXMAP_PRIV_HAS_FBO_DOWNLOADED(pixmap_priv))
         return;
 
-    if (access_mode != GLAMOR_ACCESS_RO) {
+    /* If we are doing a series of unmaps from a nested map, we're done. */
+    if (!pixmap->devPrivate.ptr)
+        return;
+
+    if (pixmap_priv->base.mapped_for_write) {
         glamor_restore_pixmap_to_texture(pixmap);
     }
 
diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h
index 7f41025..24a3575 100644
--- a/glamor/glamor_priv.h
+++ b/glamor/glamor_priv.h
@@ -410,6 +410,12 @@ typedef struct glamor_pixmap_clipped_regions {
 typedef struct glamor_pixmap_private_base {
     glamor_pixmap_type_t type;
     enum glamor_fbo_state gl_fbo;
+    /**
+     * If devPrivate.ptr is non-NULL (meaning we're within
+     * glamor_prepare_access), determies whether we should re-upload
+     * that data on glamor_finish_access().
+     */
+    bool mapped_for_write;
     unsigned char is_picture:1;
     unsigned char gl_tex:1;
     glamor_pixmap_fbo *fbo;
-- 
1.9.0



More information about the xorg-devel mailing list