GNOME Bugzilla – Bug 771698
[BISECTED] Path layer does not get exported to cairo surface
Last modified: 2016-09-29 21:42:00 UTC
Reproduce: Run launcher-gtk demo, press screenshot icon, look at champlain-map.png. The route is not in the image. Bisected to: 46cc00ab5bec50005bf0ccdb479a6c7fd58937a4 is the first bad commit commit 46cc00ab5bec50005bf0ccdb479a6c7fd58937a4 Author: Marius Stanciu <stanciumarius94@gmail.com> Date: Mon Aug 8 19:51:40 2016 +0300 Make path layer exportablility compatible with recent wrap changes. :040000 040000 f3da6b1a68f0c1246b947007f9d845e5072bde65 dc644f976c4a005295d74677bc197ff79ec52c0a M champlain
Thanks for pointing this out! I got so "wrapped up"(heh) with horizontal wrapping, I forgot to treat the base case (when it's disabled). Attached a fix!
Created attachment 336036 [details] [review] Fix surface exporting when horizontal wrap is disabled. git-bz attach gives me a "failed to update bug" for this one for some reason. I hope manually adding it won't cause much trouble.
Just tested the patch (git bz apply worked fine). Works! Thanks!
Review of attachment 336036 [details] [review]: ::: champlain/champlain-path-layer.c @@ +843,3 @@ cairo_destroy (cr); } + /* If horizontal wrapping is disabled, only the right actor is visible, so no merging is needed */ Maybe I'm misunderstanding this, but isn't this comment a bit misleading? Isn't it the case that this code-path is taken when the current view is not involving the "wrap-around point"? The comment, I think, kinda makes it sound like this happens when the "wrap" property is set to false on the map view.
Created attachment 336149 [details] [review] Fix path rendering when no clones are visible.
Created attachment 336150 [details] [review] Fix surface exporting when no clones are visible.
Good point! changed the comment.
Review of attachment 336150 [details] [review]: LGTM
Marius, could you use CLUTTER_ACTOR_IS_VISIBLE() instead? I'm testing on an older linux where clutter_actor_is_visible() is missing and I think bumping clutter version dependency because of this isn't necessary. Otherwise LGTM. I'd swear I tested this but I think I got confused by the fact that launcher-gtk doesn't overwrite previous export - I fixed this in master.
Created attachment 336536 [details] [review] Fix path-layer surface exporting when no clone is visible.
Sorry for the slow response, I'm in full process of moving to another city. Changed to CLUTTER_ACTOR_IS_VISIBLE(). There's a new problem though: After the last commit (launcher-gtk: allow replacing exported map) exporting image in launcher-gtk gives me a segmentation fault. I would give you a stack backtrace but I have no idea how to run launcher-gtk (a shell script) in gdb :(
(In reply to Marius Stanciu from comment #11) > Sorry for the slow response, I'm in full process of moving to another city. > > Changed to CLUTTER_ACTOR_IS_VISIBLE(). > > There's a new problem though: > After the last commit (launcher-gtk: allow replacing exported map) > exporting image in launcher-gtk gives me a segmentation fault. > > I would give you a stack backtrace but I have no idea how to run > launcher-gtk (a shell script) in gdb :( Something like this should work: libtool --mode=execute gdb --args /bin/bash -c ./launcher-gtk
> There's a new problem though: > After the last commit (launcher-gtk: allow replacing exported map) > exporting image in launcher-gtk gives me a segmentation fault. I tried in a different linux distribution and get the crash too. Will have a look at it.
(In reply to Jiri Techet from comment #13) > > There's a new problem though: > > After the last commit (launcher-gtk: allow replacing exported map) > > exporting image in launcher-gtk gives me a segmentation fault. > > I tried in a different linux distribution and get the crash too. Will have a > look at it. It was just missing NULL in the last parameter of gdk_pixbuf_save_to_stream(). Fixed and merged the patch from this pull request. Thanks!