[Glamor] R: [PATCH] fixup picture in SetWindowPixmap
Zhigang Gong
zhigang.gong at gmail.com
Wed Nov 6 06:45:31 PST 2013
Just pushed this patch. Thanks.
On Wed, Nov 06, 2013 at 11:59:31AM +0100, Maarten Lankhorst wrote:
> op 06-11-13 11:32, Fabio Pedretti schreef:
> > You should rebase patch on git master.
> >
> The failed hunk can be ignored . I can resend, but the only difference is that glamor_fbo.c is removed from the diff.
>
> ~Maarten
> -- 8< --
> Subject: [PATCH] fixup picture in SetWindowPixmap
>
> 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(®ion, ®ion, dst->pCompositeClip) ||
> region_is_empty(®ion))) {
> - DEBUGF(("%s: zero-intersection between rectangles and clip\n",
> - __FUNCTION__));
> + DEBUGF("%s: zero-intersection between rectangles and clip\n",
> + __FUNCTION__);
> pixman_region_fini(®ion);
> return;
> }
> 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;
> +}
>
> _______________________________________________
> Glamor mailing list
> Glamor at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/glamor
More information about the Glamor
mailing list