After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 771698 - [BISECTED] Path layer does not get exported to cairo surface
[BISECTED] Path layer does not get exported to cairo surface
Status: RESOLVED FIXED
Product: libchamplain
Classification: Core
Component: view
unspecified
Other Linux
: Normal normal
: ---
Assigned To: libchamplain-maint
libchamplain-maint
Depends on:
Blocks:
 
 
Reported: 2016-09-20 09:51 UTC by Jonas Danielsson
Modified: 2016-09-29 21:42 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix surface exporting when horizontal wrap is disabled. (582 bytes, patch)
2016-09-21 21:26 UTC, Marius Stanciu
none Details | Review
Fix path rendering when no clones are visible. (944 bytes, patch)
2016-09-23 09:54 UTC, Marius Stanciu
none Details | Review
Fix surface exporting when no clones are visible. (915 bytes, patch)
2016-09-23 09:57 UTC, Marius Stanciu
none Details | Review
Fix path-layer surface exporting when no clone is visible. (924 bytes, patch)
2016-09-29 18:41 UTC, Marius Stanciu
none Details | Review

Description Jonas Danielsson 2016-09-20 09:51:44 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
Comment 1 Marius Stanciu 2016-09-21 21:17:24 UTC
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!
Comment 2 Marius Stanciu 2016-09-21 21:26:19 UTC
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.
Comment 3 Marcus Lundblad 2016-09-21 21:33:16 UTC
Just tested the patch (git bz apply worked fine).
Works!
Thanks!
Comment 4 Marcus Lundblad 2016-09-22 22:10:03 UTC
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.
Comment 5 Marius Stanciu 2016-09-23 09:54:17 UTC
Created attachment 336149 [details] [review]
Fix path rendering when no clones are visible.
Comment 6 Marius Stanciu 2016-09-23 09:57:02 UTC
Created attachment 336150 [details] [review]
Fix surface exporting when no clones are visible.
Comment 7 Marius Stanciu 2016-09-23 09:58:05 UTC
Good point! changed the comment.
Comment 8 Marcus Lundblad 2016-09-23 12:12:55 UTC
Review of attachment 336150 [details] [review]:

LGTM
Comment 9 Jiri Techet 2016-09-26 22:45:54 UTC
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.
Comment 10 Marius Stanciu 2016-09-29 18:41:16 UTC
Created attachment 336536 [details] [review]
Fix path-layer surface exporting when no clone is visible.
Comment 11 Marius Stanciu 2016-09-29 18:48:07 UTC
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 :(
Comment 12 Marcus Lundblad 2016-09-29 19:53:23 UTC
(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
Comment 13 Jiri Techet 2016-09-29 21:18:17 UTC
> 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.
Comment 14 Jiri Techet 2016-09-29 21:42:00 UTC
(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!