|Summary:||commit 0bfd2ac causes great rendering problem is some programs|
|Component:||general||Assignee:||Carl Worth <cworth>|
|Status:||RESOLVED FIXED||QA Contact:||cairo-bugs mailing list <cairo-bugs>|
|i915 platform:||i915 features:|
patch for fix all the problem in this report.
Description yuyichao 2012-09-08 01:58:04 UTC
After upgrading to cairo-git on archlinux, all gtk2 popup window/menu/tooltips have this kind of effect (Notice that the random stuff around the menu should actually be drop shadow). This happens when I am using QtCurve Theme and also causes another program that uses pango not able to display it's window. After compiling different commit of cairo-git I track down the single commit that causes the problem is this one. 0bfd2ac... xlib: Implement SHM fallbacks and fast upload paths I haven't tried to revert this single commit on git master but any commit before this one doesn't have the problem. http://i.imgur.com/3072e.png
Comment 2 Chris Wilson 2012-09-08 07:34:16 UTC
You graphic driver looks broken, please attach your Xorg.log.
Comment 4 Chris Wilson 2012-09-08 15:30:03 UTC
It's a missing cairo_surface_flush in gtk2-engines-qtcurve, and even worse they have a memory leak: --- shadowhelper.c.orig 2012-09-08 16:29:10.000000000 +0100 +++ shadowhelper.c 2012-09-08 16:28:36.000000000 +0100 @@ -58,6 +58,7 @@ cairo_rectangle(cr, 0, 0, shadowSize, shadowSize); cairo_fill(cr); cairo_destroy(cr); + cairo_surface_destroy(dest); g_object_unref(pixbuf); return pixmap; } Also note that cairo_rectangle(); cairo_fill(); is just cairo_paint();
Comment 5 Weng Xuetian 2012-09-12 22:17:56 UTC
I also experience problem with this commit. All the thing I do is just use cairo-xlib-surface to draw on xlib created window. After I add cairo_surface_flush to every cairo_destroy (Though I didn't use xlib to draa), case seems to be better. (I don't use it and it never have problem beforece). But cairo_xlib_surface_set_surface seems never have effect that it only draw content with the very origin size. Window I draw on is set to override_directted. The workaround for me is to destroy xlib-surface and recreate a new one.
Comment 6 Chris Wilson 2012-09-13 11:48:27 UTC
Please try commit e2c4bb9465e6261eb79f24af52d339df0b563b55 Author: Chris Wilson <firstname.lastname@example.org> Date: Thu Sep 13 12:40:49 2012 +0100 xlib: Fix regression in cairo_xlib_surface_set_drawable() In commit 0bfd2acd35547fc2bd0de99cc67d153f0170697d Author: Chris Wilson <email@example.com> Date: Mon Aug 13 01:34:12 2012 +0100 xlib: Implement SHM fallbacks and fast upload paths I made the mistake of inverting the logic for cairo_xlib_surface_set_drawable() causing it then to never update. Thanks to Uli Schlachter for spotting my error. References: https://bugs.freedesktop.org/show_bug.cgi?id=54657 Reported-by: Weng Xuetian <firstname.lastname@example.org> Signed-off-by: Chris Wilson <email@example.com> The issue the qtcurves engine ran afoul of was that it was mixing cairo and external access to its shadow Pixmaps, without flushing the cairo rendering. That behaviour has been documented since cairo-1.0, and with time we rely upon it for more optimisations. So long as you observe the requirement to flush/mark-dirty around non-cairo access to cairo surfaces' Drawable, it should work on any backend.
Comment 7 Weng Xuetian 2012-09-13 14:00:45 UTC
No, that didn't fix my problem. Let me rephrase my problem. My call sequence is: XCreateWindow (with ARGB visual and override redirect) create xlib surface. draw sth, flush surface. XResizeWindow xlib_surface_set_size (Enlarge it). draw sth, flush surface. Now the second draw will only happens in the first size. screen shot is something like this. http://wstaw.org/m/2012/09/13/plasma-desktopGm1163.png
Comment 8 Chris Wilson 2012-09-13 14:34:27 UTC
Getting closer. In this sequence, the issue is that although we flush the modifications, we didn't discard the backing storage for the fallback and so it retained the original size in later use. commit 6ee216000ae487492fceda0fb3fecb20bb9a41f6 Author: Chris Wilson <firstname.lastname@example.org> Date: Thu Sep 13 15:25:49 2012 +0100 xlib: Explicitly discard the fallback shm pixmap upon user modification If the user changes the size of the underlying drawable, we must make sure that we discard the current ShmPixmap in order to create a new fallback pixmap of the correct size next time.
Comment 9 Weng Xuetian 2012-09-13 15:39:23 UTC
Get this with new commit. cairo-xlib-surface-shm.c:837: _cairo_xlib_surface_get_shm: Assertion `surface->base.damage == ((void *)0)' failed.
Comment 10 Weng Xuetian 2012-09-13 15:46:01 UTC
Sometimes I also might get: cairo-xlib-surface.c:1494: get_compositor: Assertion `s->shm != ((void *)0)' failed.
Comment 11 Chris Wilson 2012-09-13 15:51:21 UTC
I think this is the last line of code in that function that has not been shown to be buggy... commit 69d97d97bea86e7f4223d857803fb7f0ec0d369f Author: Chris Wilson <email@example.com> Date: Thu Sep 13 16:45:46 2012 +0100 xlib: Force the fallback flush before updating the external Drawable _cairo_surface_begin_modification() performs an internal flush, for which the xlib backend skips flushing the fallback surface as it will continue to use it for the subsequent operation. In the case where we are flushing prior to updating the Drawable, we need to perform an external flush which will trigger the posting of the damage from the fallback surface. Reported-by: Weng Xuetian <firstname.lastname@example.org> References: https://bugs.freedesktop.org/show_bug.cgi?id=54657 Signed-off-by: Chris Wilson <email@example.com>
Comment 12 Weng Xuetian 2012-09-13 21:42:42 UTC
This still persists. cairo-xlib-surface-shm.c:837: _cairo_xlib_surface_get_shm: Assertion `surface->base.damage == ((void *)0)' failed.
Comment 13 Weng Xuetian 2012-09-13 21:45:01 UTC
Created attachment 67122 [details] [review] patch for fix all the problem in this report. Sorry but I don't have much knowledge about cairo. But IMHO when shm is released base.damage need also to be release, and in that case fallback should be clear.
Comment 14 Chris Wilson 2012-09-13 21:59:31 UTC
Yup, the patch is right. My expectation was that the flush would destroy the damage, but I had forgotten that it would not do so until the fallback was idle as well. (Migration avoidance). I pushed the same patch before noticing you had written one, sorry, or else I would have credited you with the complete fix. commit 5c59d989f9037f94c80ccd7929dc05f4a95be4df Author: Chris Wilson <firstname.lastname@example.org> Date: Thu Sep 13 22:50:11 2012 +0100 xlib: Destroy the fallback damage along with the fallback surface Whenever we discard the fallback surface, we need to destroy the associated damage tracking, so move this into the common discard routine. This should fix the issue when trying to flush the fallback before the user modifies any foreign Drawables. The current code issued the flush and then explicitly discard the fallback, but unless it was idle at the time of the flush the associated damage would not have also been destroyed. Asserts followed. References: https://bugs.freedesktop.org/show_bug.cgi?id=54657 Signed-off-by: Chris Wilson <email@example.com> commit 59248fb2628e86ff62abfbf122b88c2a299ec393 Author: Weng Xuetian <firstname.lastname@example.org> Date: Thu Sep 13 22:56:57 2012 +0100 xlib: Reset fallback counter when discarding the fallback References: https://bugs.freedesktop.org/show_bug.cgi?id=54657