Am 2014-03-11 22:30, schrieb Eric Anholt:
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 <[email protected]>
---
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;
In my opinion, there should be a note that this will unmap on the
innermost call to finish_access, but the outer functions maybe still
want to access this pixmap.
+
+ 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;
Why haven't you used the enum glamor_access? Memory footprint shouldn't
matter that much.
unsigned char is_picture:1;
unsigned char gl_tex:1;
glamor_pixmap_fbo *fbo;
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: http://lists.x.org/mailman/listinfo/xorg-devel
