From 4c9a20072552c52b3763bd73e7a7e9b9cb8b4993 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 10 Jan 2014 23:43:09 +0800 Subject: [PATCH] glamor: Allow nested mapping of pixmaps. 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. v2: Add a comment about nested unmap behavior, and just reuse the glamor_access_t enum. Signed-off-by: Eric Anholt Reviewed-by: Markus Wick --- glamor/glamor_core.c | 23 ++++++++++++++++++++++- glamor/glamor_priv.h | 6 ++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/glamor/glamor_core.c b/glamor/glamor_core.c index 6b10e97ab..a6a603973 100644 --- a/glamor/glamor_core.c +++ b/glamor/glamor_core.c @@ -106,6 +106,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.map_access = access; return glamor_download_pixmap_to_cpu(pixmap, access); } @@ -301,7 +314,15 @@ 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. 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); } diff --git a/glamor/glamor_priv.h b/glamor/glamor_priv.h index 3cc0d2407..776de06ef 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(). + */ + glamor_access_t map_access; unsigned char is_picture:1; unsigned char gl_tex:1; glamor_pixmap_fbo *fbo;