[Glamor] [PATCH] fixup picture in SetWindowPixmap

Maarten Lankhorst maarten.lankhorst at canonical.com
Wed Nov 6 10:25:27 CET 2013


When creating a window with recordmydesktop running, the following may happen:

create picture 0x1cd457e0, with drawable 0x1327d1f0
(SetWindowPixmap is called)
destroy picture 0x1cd457e0, with drawable 0x1cd65820

Obtaining format for pixmap 0x1327d1f0 and picture 0x1cd457e0
==7989== Invalid read of size 4
==7989==    at 0x8CAA0CA: glamor_get_tex_format_type_from_pixmap (glamor_utils.h:1252)
==7989==    by 0x8CAD1B7: glamor_download_sub_pixmap_to_cpu (glamor_pixmap.c:1074)
==7989==    by 0x8CA8BB7: _glamor_get_image (glamor_getimage.c:66)
==7989==    by 0x8CA8D2F: glamor_get_image (glamor_getimage.c:92)
==7989==    by 0x29AEF2: miSpriteGetImage (misprite.c:413)
==7989==    by 0x1E7674: compGetImage (compinit.c:148)
==7989==    by 0x1F5E5B: ProcShmGetImage (shm.c:684)
==7989==    by 0x1F686F: ProcShmDispatch (shm.c:1121)
==7989==    by 0x15D00D: Dispatch (dispatch.c:432)
==7989==    by 0x14C569: main (main.c:298)
==7989==  Address 0x1cd457f0 is 16 bytes inside a block of size 120 free'd
==7989==    at 0x4C2B60C: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==7989==    by 0x228897: FreePicture (picture.c:1477)
==7989==    by 0x228B23: PictureDestroyWindow (picture.c:73)
==7989==    by 0x234C19: damageDestroyWindow (damage.c:1646)
==7989==    by 0x1E92C0: compDestroyWindow (compwindow.c:590)
==7989==    by 0x20FF85: DbeDestroyWindow (dbe.c:1389)
==7989==    by 0x185D46: FreeWindowResources (window.c:907)
==7989==    by 0x1889A7: DeleteWindow (window.c:975)
==7989==    by 0x17EBF1: doFreeResource (resource.c:873)
==7989==    by 0x17FC1B: FreeClientResources (resource.c:1139)
==7989==    by 0x15C4DE: CloseDownClient (dispatch.c:3402)
==7989==    by 0x2AB843: CheckConnections (connection.c:1008)
==7989== 
(II) fail to get matched format for dfdfdfdf

The fix is to update the picture pointer when the window pixmap is changed,
so it moves the picture around with the window rather than the pixmap.

This makes FreePicture work correctly.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71088
---
diff --git a/src/glamor.c b/src/glamor.c
index 7637f3b..e8e68be 100644
--- a/src/glamor.c
+++ b/src/glamor.c
@@ -413,6 +413,9 @@ glamor_init(ScreenPtr screen, unsigned int flags)
 	glamor_priv->saved_procs.create_picture = ps->CreatePicture;
 	ps->CreatePicture = glamor_create_picture;
 
+	glamor_priv->saved_procs.set_window_pixmap = screen->SetWindowPixmap;
+	screen->SetWindowPixmap = glamor_set_window_pixmap;
+
 	glamor_priv->saved_procs.destroy_picture = ps->DestroyPicture;
 	ps->DestroyPicture = glamor_destroy_picture;
 	glamor_init_composite_shaders(screen);
@@ -531,6 +534,7 @@ glamor_close_screen(CLOSE_SCREEN_ARGS_DECL)
 	ps->CompositeRects = glamor_priv->saved_procs.composite_rects;
 	ps->Glyphs = glamor_priv->saved_procs.glyphs;
 	ps->UnrealizeGlyph = glamor_priv->saved_procs.unrealize_glyph;
+	screen->SetWindowPixmap = glamor_priv->saved_procs.set_window_pixmap;
 #endif
 	screen_pixmap = screen->GetScreenPixmap(screen);
 	glamor_set_pixmap_private(screen_pixmap, NULL);
diff --git a/src/glamor_compositerects.c b/src/glamor_compositerects.c
index f1564a2..1a57699 100644
--- a/src/glamor_compositerects.c
+++ b/src/glamor_compositerects.c
@@ -215,8 +215,8 @@ glamor_composite_rectangles(CARD8	 op,
 	if (dst->pCompositeClip->data &&
 	    (!pixman_region_intersect(&region, &region, dst->pCompositeClip) ||
 	     region_is_empty(&region))) {
-		DEBUGF(("%s: zero-intersection between rectangles and clip\n",
-		     __FUNCTION__));
+		DEBUGF("%s: zero-intersection between rectangles and clip\n",
+		     __FUNCTION__);
 		pixman_region_fini(&region);
 		return;
 	}
diff --git a/src/glamor_fbo.c b/src/glamor_fbo.c
index a46a740..4838a27 100644
--- a/src/glamor_fbo.c
+++ b/src/glamor_fbo.c
@@ -212,7 +212,7 @@ glamor_pixmap_ensure_fb(glamor_pixmap_fbo *fbo)
 			break;
 		}
 
-		FatalError("destination is framebuffer incomplete: %s [%#x]\n",
+		FatalError("destination is framebuffer incomplete: %s [%x]\n",
 			   str, status);
 	}
 	glamor_put_dispatch(fbo->glamor_priv);
diff --git a/src/glamor_priv.h b/src/glamor_priv.h
index ffdd7fd..b6a1075 100644
--- a/src/glamor_priv.h
+++ b/src/glamor_priv.h
@@ -221,6 +221,7 @@ struct glamor_saved_procs {
 	CreatePictureProcPtr create_picture;
 	DestroyPictureProcPtr destroy_picture;
 	UnrealizeGlyphProcPtr unrealize_glyph;
+	SetWindowPixmapProcPtr set_window_pixmap;
 };
 
 #ifdef GLAMOR_GLES2
@@ -934,6 +935,8 @@ void glamor_destroy_upload_pixmap(PixmapPtr pixmap);
 
 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);
 
diff --git a/src/glamor_window.c b/src/glamor_window.c
index 3da11e4..b67c728 100644
--- a/src/glamor_window.c
+++ b/src/glamor_window.c
@@ -69,3 +69,35 @@ glamor_change_window_attributes(WindowPtr pWin, unsigned long mask)
 	}
 	return TRUE;
 }
+
+void
+glamor_set_window_pixmap(WindowPtr win, PixmapPtr pPixmap)
+{
+	ScreenPtr screen = win->drawable.pScreen;
+	glamor_screen_private *glamor_priv =
+		glamor_get_screen_private(screen);
+	PixmapPtr old = screen->GetWindowPixmap(win);
+
+	if (pPixmap != old) {
+		glamor_pixmap_private *pixmap_priv;
+		PicturePtr pic = NULL;
+
+		pixmap_priv = glamor_get_pixmap_private(old);
+		if (GLAMOR_PIXMAP_PRIV_IS_PICTURE(pixmap_priv) && pixmap_priv->base.picture->pDrawable == (DrawablePtr)win) {
+			pic = pixmap_priv->base.picture;
+			pixmap_priv->base.is_picture = 0;
+			pixmap_priv->base.picture = NULL;
+		}
+
+		pixmap_priv = glamor_get_pixmap_private(pPixmap);
+		if (pixmap_priv) {
+			pixmap_priv->base.is_picture = !!pic;
+			pixmap_priv->base.picture = pic;
+		}
+	}
+
+	screen->SetWindowPixmap = glamor_priv->saved_procs.set_window_pixmap;
+	(screen->SetWindowPixmap)(win, pPixmap);
+	glamor_priv->saved_procs.set_window_pixmap = screen->SetWindowPixmap;
+	screen->SetWindowPixmap = glamor_set_window_pixmap;
+}



More information about the Glamor mailing list