---- Reported by shotwell-maint@gnome.bugs 2010-11-24 10:45:00 -0800 ----

Original Redmine bug id: 2875
Original URL: http://redmine.yorba.org/issues/2875
Searchable id: yorba-bug-2875
Original author: andreas -
Original description:

Attached patch implements the
[libchamplain](http://projects.gnome.org/libchamplain/) widget into shotwell
to display marker pins on a map for geo-coded pictures.

unlike the request for google-maps in #2570, this actually uses
[OpenStreetMap](http://www.openstreetmap.org) maps which are license-wise less
problematic and allowed to be embedded. Libchamplain also caches the tiles, so
that the markers also display on a map when used offline.

improvement possibilities / TODO (feel free to hack on):

- Allow for zoom-out (zoom-in works by double-clicking, courtesy of libchamplain). Basically just requires zoom-in/out buttons and signal bindings.

- Allow for drag-n-drop geo-coding of pictures without geo data

- Menu option to switch the the different maps that libchamplain can display (cycle map, height map,..).

- Make the layer clickable (and display pictures by selecting the marker - Easy task)

- Increase the marker pin size when many pictures on small area instead of cluttering the map with many pins

- …

On my intel card on ubuntu 10.10 i need to set the environment variable
CLUTTER_VBLANK=none to disable vsync or the map will not render! see
http://www.google.com/search?q=CLUTTER_VBLANK%3Dnone

so use 'CLUTTER_VBLANK=none shotwell' when testing if it doesn't seem to work.

I sometimes get "BadDrawable" crashes (async X api errors) - not sure what
it's about. Might be related to the mesa/clutter issue.

The current clutter-gtk requires a minor fix (patch submitted upstream at
https://bugzilla.gnome.org/show_bug.cgi?id=635522 and attached to this ticket)

I'd be happy to get some more feedback (esp. if you encounter the same crashes
on systems different from ubuntu 10.10 (11.04 or fedora might be better))

-> Kudos to the libchamplin guys for the widget in the first place!

Related issues:
related to shotwell - Feature #1473: geographic tree in sidebar (Open)



---- Additional Comments From shotwell-maint@gnome.bugs 2013-09-17 15:24:00 -0700 ----

### History

####

#1

Updated by andreas - almost 3 years ago

  * **Keywords** changed from _map gps_ to _map gps geocoding_

####

#2

Updated by andreas - almost 3 years ago

Of course this feature adds additional dependencies on

- libchamplain >= 0.4.6

- clutter >= 1.0

- clutter-gtk >= 0.10

(might work with lower versions, but these are the ones i'm using)

The occasional crash results in this message on stdout:

The program 'shotwell' received an X Window System error.

This probably reflects a bug in the program.

The error was 'BadDrawable (invalid Pixmap or Window parameter)'.

(Details: serial 57947 error_code 9 request_code 137 minor_code 7)

(Note to programmers: normally, X errors are reported asynchronously;

that is, you will receive the error a while after causing it.

To debug your program, run it with the --sync command line

option to change this behavior. You can then get a meaningful

backtrace from your debugger if you break on the gdk_x_error() function.)

####

#3

Updated by Adam Dingle almost 3 years ago

Andreas,

it's nice to see you're working on this. I'd love to try out your patch. I
tried applying it to the current trunk, but that failed, so probably the trunk
has changed since you generated it. I looked at your patch to see which
Subversion revision number it had been generated from, but that's not evident
since you generated it using git. So please regenerate the patch from the
latest Shotwell trunk and resubmit – thanks.

(By the way, Yorba is planning to switch from Subversion to git some time
after we ship Shotwell 0.8.)

####

#4

Updated by Adam Dingle almost 3 years ago

Never mind: I didn't realize that I needed to run 'patch -p1' since there was
an extra directory component in filenames in the patch file. The patch applies
cleanly to the current trunk. I'll try to build and run it soon.

####

#5

Updated by andreas - almost 3 years ago

Replying to [comment:5 adam]:

> Never mind: I didn't realize that I needed to run 'patch -p1' since there
was an extra directory component in filenames in the patch file. The patch
applies cleanly to the current trunk. I'll try to build and run it soon.

no problem. git makes it easier to maintain (local) branches which is why i
use it over svn.. would love to see a native git repo.

I'll make sure to note the revision next time and indicate which patch level
to use.

####

#6

Updated by Adam Dingle almost 3 years ago

Andreas,

I was finally able to get your code to build and run. I had to make the
following changes:

  * In the Makefile, I had to change champlain-0.10 to champlain-0.4.
  * In add_gps_marker() in Properties.vala, I had to change the calls to get_latitude() and get_longitude() to property accessors (e.g. base_marker.latitude).

Are you really sure that you're using libchamplain 0.4.6 as you mentioned in
your comment above?

Also, please update the EXT_PKG_VERSIONS define in the Makefile so that it
will check for the minimum required versions of all the libraries you
mentioned above.

When I run Shotwell with your changes, I see a libchamplain map in the Basic
Information area in the lower left. As soon as I select a photo without GPS
info, however, the map goes away. When I then select a photo with GPS info,
the map does not return, but I see duplicate sets of text fields in the Basic
Information area. Unfortunately I don't seen any GPS marker on the map, even
when I select a photo with GPS information. Not sure what's wrong here.

The Shotwell team hasn't yet decided where in our user interface we'd like
maps to display. The Basic Information pane is one possibility, but
alternatively we might want this to be a separate window. We're currently busy
finishing up 0.8, so I think we might not get a chance to discuss this for
another week or two. In the meantime, you can keep working on the feature in
the Basic Information pane if you like, though we may ask you to move it
somewhere else in the interface at some point.

Thanks again for your efforts so far!

####

#7

Updated by andreas - almost 3 years ago

Hey adam

thanks for the interest and sorry for wasting your time with the library
version mixup.. i was trying out how it worked on champlain-0.10-trunk and
forgot to switch back (btw. the BadDrawable crash also occurs there for me).

The properties you needed to change to make it compile also work on the trunk
so we'll go with that.

The issue with the map not being displayed again on the second time is related
to the markers not displaying. Make sure to set the environment variable
_CLUTTER_VBLANK=none_ (google the string for more infos).

Good luck on the 0.8 release, I'm looking forward to it.

####

#8

Updated by andreas - almost 3 years ago

i just rebased on trunk (0.8, assuming it's released based on the downloadable
version on the main shotwell page – you really need a tag to pin the release
version though)

..just noticed i could've include the version checks to EXT_PKG_VERSIONS..
next time

happy new year :)

####

#9

Updated by andreas - almost 3 years ago

..ok so the EXT_PKG_VERSIONS are in – the previous rebase needed other 0.8
tweaks anyway..

the “BadDrawable?†crashes are worse for me now – anyone NOT
experiencing these -> i'd be interested in video card, driver, distribution,
clutter and champlain version

####

#10

Updated by Adam Dingle over 2 years ago

  * **Target version** set to _0.10_
  * **Priority** set to _High_

We'll think about whether we could take this in some form for 0.10.

####

#11

Updated by Adam Dingle over 2 years ago

  * **Target version** deleted (<strike>_0.10_</strike>)

####

#12

Updated by andreas - over 2 years ago

yay i spent the last two days on the feature, since i really want to see it in
shotwell.

I think i finally figured out why i had these bad drawable errors. They seem
to be related to some kind of race between the event handlers and the
reparenting ops on the gtk widget.. so the solution is to not put it into the
table with the other props but rather create a vbox that contains both the
table and the map, that solves the existing issue.

Too bad the 0.10 milestone was removed but it's understandable.. Also i
wouldn't wanna hold back on all other cool features that are to come..

downside of the current situation on ubuntu is that natty ships with
libchamplain 0.8 (instead of 0.10, the current stable which is compatible with
0.11) and requires gtk3 (well clutter-gtk-1.0 requires it, but that's a
dependency of libchamplain-0.8) and since gtk3 conflicts with gtk2 on which
shotwell depends, it's not actually buildable – doh.

Still that's not a reason to give up so here comes a new patch against the
newest libchamplain (0.11 which will end up being stable 0.12) –
unfortunately you'll also need to patch libchamplain-0.11 to build against
clutter-gtk-0.10 (instead of 1.0) since that still depends on Gtk2 and
additionally a patch against the vala bindings of libchamplain.

As a cookie i've added

- rebased against current master tip

- zooming out on right-doubleclicks (or use the mousewheel if you have one)

- add gps tags to the DB since startup was about 5x slower when displaying the library page with all my 1700 pics (half of them with gps tags) – with gps data in the DB it's now on par with an unpatched shotwell

Seems like this ticket will remain dependent on #3455 (Port to GTK+3)

TODO:

- if somebody could just help me figuring out why the map widget is not hiding when i call hide() on it, then the map would not constantly be showing (esp. when it's not supposed to).

- figure out how to update the DB to store the gps data of all existing pics. I'll post that to the mailing list..

- it doesn't show up as an issue in the profiler performance wise but i'd like to use Clutter.Clone() on the gps marker texture since they're all using the same backing texture and are not modifying it.

- wait for the gtk3 port to happen (#3455) at least with git i don't feel it's going to be such a pain to merge anymore :)

..and as always testing makes your sad days a little brighter :)

####

#13

Updated by andreas - over 2 years ago

  * **File** _add_champlain_map-gtk3_rebase.patch_ added
  * **Description** updated (diff)

here's a new update, this time (mostly) a rebase against the gtk3 branch -
since gtk3 is dependency anyway.

Same stuff as before applies (esp. the NoAccessorMethods patch)

####

#14

Updated by Sander Deryckere about 2 years ago

  * **Description** updated (diff)

For the case you don't know it, there's a €30 reward for this feature on
Elveos (not by me btw, I just saw it and thought I should mention it).

https://elveos.org/en/features/343/contributions?title=show-geolocalied-
photos-on-a-map

####

#15

Updated by andreas - about 2 years ago

..i actually wanted to post that on elveos but I'd need a login.

This feature heavily depends on how fast clutter gets integrated into
mainstream distributions (i.e. ubuntu) - and they only plan it for 12.04
precise pangolin. This dependency on clutter is due to libchamplain. So don't
expect an upstream shotwell release with this feature within the next 5
months. But as mentioned above, you can of course compile it yourself - it's
working but could use some more features.

and of course thanks for the pledges but I'm doing it for fun :) If someone
wants to hack on that, feel free to claim the pledge - should it get over the
$1000, I'd however ask that at least half the amount be donated to yorba.

cheers

andreas

####

#16

Updated by andreas - almost 2 years ago

  * **File** deleted (<strike>_fix-clutter-gtk-binding.patch_</strike>)

####

#17

Updated by andreas - almost 2 years ago

  * **File** deleted (<strike>_use-gtk_-2.0.patch_</strike>)

####

#18

Updated by andreas - almost 2 years ago

  * **File** deleted (<strike>_add_champlain_map-gtk3_rebase.patch_</strike>)

####

#19

Updated by andreas - almost 2 years ago

  * **File** deleted (<strike>_add-noaccessormethod.patch_</strike>)

####

#20

Updated by andreas - almost 2 years ago

  * **File** _add-champlain-map.patch_ added

Finally - the first no hacks, no external patches release. Just apply, copy
the marker png and try.

No additional features in this release, just a rebase to current master

Tested with native Ubuntu Precise Pangolin packages libchamplain-0.12 and
libclutter-gtk-1.0-0

####

#21

Updated by andreas - almost 2 years ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#22

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#23

Updated by andreas - over 1 year ago

  * **File** _add-champlain-map.patch_ added

New release which saw a general code overhaul and separates the map stuff from
the preferences into its own file/classes.

Runs with the stock ubuntu precise libs.

New features

- Hovering over the markers on the map highlights the thumbnail  
- Drag a photo or thumbnail onto the map to set its position (can't drag markers on the map itself)  
- Load the marker texture once and reuse it instead of loading it from file for every marker

Caveats:

- Hovering over the thumbnail highlights the marker on the map.. kinda. The code is called, but marker isn't highlighted when using the marker texture, with the simple dot - the fallback when the marker file isn't available - this works.  
- Imports of geo tagged photos are slow because the map transition blocks until completed - not sure why as i tell it to cancel. Could be a libchamplain bug.  
- DONT USE ON PRODUCTION LIBRARIES AS IT MODIFIES THE DB STRUCTURE

####

#24

Updated by Roumano - over 1 year ago

It's will be very nice to have gps maps view in the trunk of shotwell

As some patch are available, can anyone review it and/or add a version target
?

####

#25

Updated by Roumano - over 1 year ago

Hi Andreas,

I want attempt to test your patch as it was for me a important improvement in
shotwell ...

I have successfully patched & compile shotwell.

But when i try to launch the patched shotwell i got this crash :

ERROR:/home/roumano/bin/shotwell/src/db/PhotoTable.vala:437:photo_table_get_al
l: assertion failed: (_tmp3_ == SQLITE_OK)

Abandon

####

#26

Updated by andreas - over 1 year ago

Hi Roumano,

[edit] ..scratch that, i can reproduce it on a db of an actual shotwell
instance.

Feel free to try it out on a new db meanwhile (e.g. shotwell -d ~/shotwell-
test)

####

#27

Updated by Lucas Beeler over 1 year ago

  * **Target version** set to _0.13_

Thanks for the patch, gentlemen. This is a big patch and there's a fair degree
of product management that will need to happen before it can be landed. Those
of us on the Shotwell team will have to think very seriously about what we
want the Shotwell geo data user experience to look like and whether this patch
is in line with our vision. As you guys know, we take user experience very
seriously at Yorba.

That said, this is shaping up to be an awesome patch. I'll talk to the rest of
the team and we'll see if we can't do something with this for Shotwell 0.13.
Keep up the good work!

Lucas

####

#28

Updated by Adam Dingle over 1 year ago

  * **Status** changed from _Review_ to _Open_

Andreas,

I just tried your latest patch. This is looking promising - I'd love to see
this land in 0.13 if possible.

Issues I saw:

  * When I first ran, I saw the same database error Roumano reported.
  * When I run with your patch, I see a bunch of critical assertions (in gdk_pixbuf_get_has_alpha, gdk_pixbuf_get_pixels and so on) at startup.
  * There should be a separator line between the map and the other items in the Basic Information pane.
  * There needs to be a View menu item to show or hide the map.
  * It's cool that the map jumps from one place to another when I select different photos, however that sometimes takes a few seconds which can be annoying. I think the animation should always complete within 1.5 seconds or so.
  * I saw Shotwell crash twice within a few minutes: "GLib-ERROR **: Creating pipes for GWakeup: Too many open files - Trace/breakpoint trap (core dumped)" 
  * When I mouse over a photo the corresponding map marker turns red, but when I mouse away it remains red and never goes back to blue.

Please address these issues in the next iteration of your patch. Thanks!

####

#29

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#30

Updated by andreas - over 1 year ago

  * **File** _add-champlain-map.patch_ added

Here's a new patch bumping the db schema - that will cause the db to be
upgraded.

again, I seriously recommend using a new test DB as it will mess with the
schema! Furthermore the coordinates of pictures are only saved to db on import
- therefor all geotagged images must be removed and reimported anyway for them
to show up on the map.

####

#31

Updated by andreas - over 1 year ago

Thanks Lucas and Adam for your review and consideration

Adam Dingle wrote:

>   * When I first ran, I saw the same database error Roumano reported.

fixed.

>   * When I run with your patch, I see a bunch of critical assertions (in
gdk_pixbuf_get_has_alpha, gdk_pixbuf_get_pixels and so on) at startup.

i never saw any of these on my setup. ubuntu precise with intel graphics.

>   * There should be a separator line between the map and the other items in
the Basic Information pane.

>   * There needs to be a View menu item to show or hide the map.

I'm not that much of a UI guy though i might be able to handle that. i was
also thinking about an easier and faster way to zoom in and out (though mouse-
wheel would probably work). Though these are certainly the last things I'll
add.

>   * It's cool that the map jumps from one place to another when I select
different photos, however that sometimes takes a few seconds which can be
annoying. I think the animation should always complete within 1.5 seconds or
so.

Would have done so if it was configurable. This will require changes in
libchamplain and if you plan on releasing 0.13 with ubuntu's stock
libchamplain that's not unless it gets updated upstream.

The animation cancellation is also something highly desirable to make imports
faster. As a workaround, a switch could probably be added to disable
animations during imports.

>   * I saw Shotwell crash twice within a few minutes: "GLib-ERROR **:
Creating pipes for GWakeup: Too many open files - Trace/breakpoint trap (core
dumped)"

Never had those either

>   * When I mouse over a photo the corresponding map marker turns red, but
when I mouse away it remains red and never goes back to blue.

Not sure why this is, but I'm getting those too. With the gps-marker.png
installed it doesn't highlight at all. Could be a libchamplain issue.

> Please address these issues in the next iteration of your patch. Thanks!

As you'll understand, this all depends on the time at my disposition which can
strongly vary due to my current job. I try to release as often as possible so
that others could pick up at any given time if interest arises. Unfortunately
I'm not able to make any promises though, only best effort.

cheers

####

#32

Updated by Roumano - over 1 year ago

Hi,

Thanks both for the quick respond.

I can run now patched shotwell,

I just have a question i couldn't respond, where is the db file of the patches
shotwell ?

if i remove my ~/.shotwell/data/photo.db (and .bak) , i run the patches
shotwell still see lot of image, but normal shotwell see it empty ...

I thing i have found a bug to normal shotwell (same issue with
patches/unpatched shotwell) :

select all (Ctrl-A) , then edit/remove to library, (only remove to library)

When it at 100%, shotwell hang and never finish : (because it can't remove
video of the library...)

Relaunch shotwell, you will see all your video & zero pictures

####

#33

Updated by Adam Dingle over 1 year ago

Roumano,

in the trunk build of Shotwell, the database is at
~/.local/share/shotwell/data/photo.db .

If you've found an unrelated bug then please file a new ticket - we prefer
that each ticket refer to only one bug. Thanks!

####

#34

Updated by Adam Dingle over 1 year ago

Roumano wrote via email:

> if you have a lot of near location, all blue point on the map are like a
potetoes & difficult to find what you want/need

>

> I don't known if it possible but it will be great if all near location can
be grouped (depending of the zoom view) & degrouped when making a

zoom to the map Like what it's done with the plugin "RV Maps & Earth" on
Piwigo website (it's use gmaps as api)

>

> You can have a exemple on my website
:http://roumano.com/images/map.php?/categories

I agree. It would be really cool if Shotwell could automatically group nearby
locations and ungroup them when you zoom in.

####

#35

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#36

Updated by andreas - over 1 year ago

  * **File** _add-champlain-map.patch_ added
  * **File** _gps-marker-selected.png_ added

Good news for all of you who have been waiting patiently..

This iteration of the patch resolves the technical issues with the previous
iterations:

- Importing pictures is not slow as hell because the map first has to move to the marker before being able to continue (+)  
- GPS metadata of pictures imported with previous shotwell versions are now updated into the db  
- The marker is reset when the cursor is not over the thumbnail any more  
as always apply on current master (da6e2fa25d65af18c678c32a5db21607bf1fd84a)

The first issue was a tough nut to crack.. the problem was that the priority
of "idle" queued tasks was lower than the priority libchamplain uses, so the
new request (to cancel the current move) didn't get in before the whole
transition was done.. i therefor upped the priority of "idle" tasks a bit
(reviewers please check that this behavior doesn't have other adverse effects
- i didn't discover any)

The critical assertions (in gdk_pixbuf_get_has_alpha, gdk_pixbuf_get_pixels)
come from not having the marker png's installed (put them in the "icons" dir -
the source patch only contains the svgs from which the pngs are made, but
those are not used!)

Still don't know what the GLib Errors are that adam came across.

Thanks for testing and as said before, if someone wants to tackle the easy GUI
tasks that adam wished for: go ahead, it's unlikely i'll do them. I'd rather
get the grouping of markers done first.

####

#37

Updated by Alexander Wilms over 1 year ago

Thanks for you work! It'd be great if Shotwell had a new view featuring a
bigger map like Aperture:
http://images.apple.com/euro/aperture/images/whatis_organize_places.jpg, maybe
combining the normal OSM tiles with a relief layer.

Do you think that something like this could be done for version 0.14?

####

#38

Updated by Adam Dingle over 1 year ago

  * **Status** changed from _Open_ to _Review_

####

#39

Updated by Adam Dingle over 1 year ago

  * **Target version** deleted (<strike>_0.13_</strike>)

####

#40

Updated by Adam Dingle over 1 year ago

Alexander: yes, I agree that a bigger map view would be nice. As to what we
can achieve for 0.14, well, it depends as always on our resources (which are
limited) and the priority of other tasks. Patches are always welcome. :)

####

#41

Updated by Alexander Wilms over 1 year ago

  * **File** shotwell.svg added

Unfortunately I'm not a developer, I can only do translations & a bit of
design.

I created these less skeuomorphic pins, IMHO they'd rather fit into Shotwell's
UI:

####

#42

Updated by andreas - over 1 year ago

Thanks Alexander, they look really good! I'll put in these for the next patch
iteration. For that I'd just need a quick written confirmation and a statement
that you created them yourself or derived them from a permissible source. (You
know, the legal stuff..)

I won't be able to guarantee that they'll be the finally used icons, should
the patch be merged into shotwell, but I really do appreciate the effort.

####

#43

Updated by Alexander Wilms over 1 year ago

I did create these icons myself and license them under cc0. And thanks for
including them :)

####

#44

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_gps-marker.png_</strike>)

####

#45

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_gps-marker-selected.png_</strike>)

####

#46

Updated by andreas - over 1 year ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#47

Updated by andreas - over 1 year ago

  * **File** _add-champlain-map.patch_ added

New patch iteration

- Group markers when they are too close (set to group when < 30px)  
- Alexander's new icons included in the diff. No more external .pngs needed.

As always, don't use on your production DB, create a new one with shotwell -d
/home/foo/my-test-shotwell-folder

####

#48

Updated by Roumano - over 1 year ago

Sorry, i can't test anymore (maybe for a while) these patch as the git master
of shotwell now require vala 0.17 & in gentoo only .16 is available ....

####

#49

Updated by andreas - over 1 year ago

The patch also applies to shotwell-0.12.3 (and runs)

    
    
    patch -p1 < ~/path/to/add-champlain-map.patch
    

  
However I only test-compiled it with valac-0.17.4

In other news, it seems that the import is still waiting on the map to finish
the transition before moving on.. thought I fixed that

####

#50

Updated by Roumano - over 1 year ago

It's no compile for me (shotwell-0.12.3) :

/home/roumano/bin/shotwell/src/MapWidget.vala:41: undefined reference to
`clutter_actor_get_first_child'

/home/roumano/bin/shotwell/src/MapWidget.vala:41: undefined reference to
`clutter_actor_get_first_child'

( i have the clutter version 1.8.4 )

####

#51

Updated by andreas - over 1 year ago

So i forgot to adjust the minimal version (which is 1.10.0) in the Makefile
then - Thanks for trying, fixed it in my branch.

A workaround for clutter 1.8 is probably ClutterGroup.get_nth_child which
might need typecasting to ClutterGroup.
http://developer.gnome.org/clutter/1.8/ClutterGroup.html#clutter-group-get-
nth-child

####

#52

Updated by Roumano - 12 months ago

Not sure if it's can help, but a another software on Linux make a very good
step forward about map implementation : it's darktable software (version 1.1,
writing in C)

Now, they can

- see pictures on the map   
- with several map source (like openstreetmap, googlemap, google satellite, opencycle map, ...  
- degrouping them with make a zoom  
- add a gps location by moving a picture (or several) on the map  
- add gps location with correlation from a .gpx file  
- find a location & update the map with the result (like search london city)  
- display gps information of a image (available in 2 format, first for human : N 45°22,771' & the second more userful : in computer format : N 45.3796509)

####

#53

Updated by Jim Nelson 12 months ago

  * **Target version** set to _0.14.0_

Interesting. Let's see if we can get this in for 0.14.

####

#54

Updated by Jim Nelson 11 months ago

  * **Category** set to _library-mode_

####

#55

Updated by Lucas Beeler 11 months ago

  * **Status** changed from _Review_ to _Open_
  * **Assignee** set to _andreas -_

@andreas:

I took another look at this patch last night and I really like the
functionality of having the map – complete with GPS pins – in the Shotwell
info pane. There are, however, two things that I think we need to do to get
this patch to the point that it's ready for a comprehensive pre-landing
review. First, could you update the diff so that it applies against today's
Shotwell master? An even better idea would be for you to set up your own clone
of the Shotwell repo on GitHub and keep your work in it, rebasing changes from
Shotwell master onto it as needed. This is a great way to keep a patch from
going stale. Second, could we avoid storing geolocation information in the
Shotwell database and instead just read it out of the PhotoMetadata object
that's associated with every Photo as-needed? I ask for this because we go out
of our way to minimize database schema changes here at Yorba. Thank you so
much for your work. Overall, this looks really promising!

####

#56

Updated by andreas - 10 months ago

Hi Lucas,

I actually already put it on github a month ago but rebased it against today's
master.

https://github.com/abrauchli/shotwell

Two your second request: this will make the whole thing unusable! i tried in
the first iterations of this patch, before i decided to put it into the db.
Quote from comment #12:

"- add gps tags to the DB since startup was about 5x slower when displaying
the library page with all my 1700 pics (half of them with gps tags) – with
gps data in the DB it's now on par with an unpatched shotwell"

If it's just about displaying single pictures it'll work and won't make that
much of a difference, but even one single event will be much slower if
metadata has to be fetched.

You can probably try it out by populating the gps_coords struct with get_gps()
instead of database-data in "public GpsCoords
get_gps_coords()"@src/photos/PhotoMetadata.vala:999

Also when thinking about expanding this patch for location-based tags it'll be
much harder without quick access to location data. One thing that might be
worth investigating is moving the data to a separate table. That will avoid
column-explosion if even more location-related columns are added in the future
(this patch already adds 3 columns) - what i had in mind is adding the
country/city/region with reverse-geolookup for automatic location-based tags.

Cheers

####

#57

Updated by Lucas Beeler 10 months ago

  * **Status** changed from _Open_ to _Review_

####

#58

Updated by Lucas Beeler 10 months ago

  * **Status** changed from _Review_ to _Open_

Hi Andreas,

Jim and I met to discuss how we'd like to proceed with adding geolocation
support to Shotwell. Your patch is great work and represents a phenomenal
start. In fact, our only serious issue with your patch is that it tries to do
too much! ;-) Shotwell has grown into a big product and as it's grown we've
taken a more conservative, iterative, incremental approach to integrating new
features. We definitely want geolocation support in Shotwell, and we think
your work is great. We just want to scale the first iteration of the feature
back to the minimal set of functionality that the average Shotwell user will
find useful. We've come up with the following design:

(i)

The Gtk+ map widget should appear only in the single photo view (i.e., the
view that appears when you double-click on a photo in the thumbnail overview).

(ii)

The map widget should show the location of the current photo (the nice thing
about single photo view is that there's only one photo to deal with ;-) ) with
a location marking pin. The location marking pin should be centered in the map
view. When the user moves forward or backward in the photo sequence (say, by
pressing the left or right arrow keys on the keyboard), the map should scroll
to show the location of the new current photo. Once again, the location should
be marked with a with a centered marking pin.

####

#59

Updated by Jim Nelson 9 months ago

  * **Target version** changed from _0.14.0_ to _0.15.0_

The time frame for getting this in to 0.14 is closing. It doesn't look like
this will be ready. We'll revisit this for 0.15.

####

#60

Updated by Jim Nelson 8 months ago

  * **Target version** changed from _0.15.0_ to _0.16.0_

####

#61

Updated by Jim Nelson 6 months ago

  * **Target version** deleted (<strike>_0.16.0_</strike>)

####

#62

Updated by andreas - 4 months ago

Hi Lucas and Jim,

sorry about the delay, life happened off-the-line.

I've rebased my branch (https://github.com/abrauchli/shotwell) against the
latest master in case anybody is interested in using it. During the process
I've updated the code to reflect deprecation changes in libchamplain, clutter
and gtk3. I'll delete the patch on this ticket as it's growing old.

While I'm still very much interested in seeing this work merged, I do see
myself constrained in my available time. I have, thus, currently not
implemented any of your suggested changes but I completely agree that this
minimalistic approach is well delimited and represents a good introductory
step.

Implementing it is mostly about removing (db) code from the current
implementation. I'd be happy to review and merge into my repo in case anybody
wants to give it a go.

cheers

####

#63

Updated by andreas - 4 months ago

  * **File** deleted (<strike>_add-champlain-map.patch_</strike>)

####

#64

Updated by Jim Nelson 4 months ago

  * **Status** changed from _Open_ to _Review_

####

#65

Updated by Joe Bylund 4 months ago

Andreas,

This crashes with

    
    **
    ERROR:/home/jbylund/Desktop/shotwell/src/db/PhotoTable.vala:437:photo_table_get_all: assertion failed: (res == Sqlite.OK)
    Aborted (core dumped)

  
on an existing database. I an re-importing some photos on a fresh db to give
it a try now, but to me that would be a showstopper.

-Joe

####

#66

Updated by andreas - 4 months ago

Hey Joe, thanks for testing!

I forgot to bump the hardcoded schema version to trigger the db upgrade and I
only tested it with new pristine db or one that already had the columns.

It's fixed in https://github.com/abrauchli/shotwell/commit/6017b2e58e3237c80a7
d4a48dc8199e14df4b8d8

cheers

####

#67

Updated by Joe Bylund 4 months ago

So at the moment it does not write the gps location to file even if you have
writing tags to file enabled? One of the first things I tried was dropping on
a map and then publishing to flickr.

####

#68

Updated by andreas - 4 months ago

Yes that was true. Although it's implemented now.

As side note: while examining what I need to support the writeback I
discovered that (around Photo.vala:2382/2410) public void
set_comment_persistent and set_title_persistent seem stale. They aren't called
from anywhere in the code.

####

#69

Updated by Joe Bylund 4 months ago

Andreas,

Tested again, worked great. I'll let you know if I see anything else, and let
me know if there's anything you'd like me to test. Thank you so much for this
patch, I'm super excited to use it.

-Joe

####

#70

Updated by F M 3 months ago

I have compiled https://github.com/abrauchli/shotwell/ on Ubuntu 13.04 without
a problem.

It is a great addition to shotwell, I would be very happy to see this
integrated soon.

####

#71

Updated by Alexander Wilms 3 months ago

I think it would look great if a photo's meta-data and the map widget would be
displayed in a side pane like on Google+

####

#72

Updated by Jens . 2 months ago

I would like to test this and report back but cannot build Shotwell on Ubuntu
12.04. The current daily build does not contain this patch yet (right?). Is it
possible to provide a build for Ubuntu 12.04? Thank you!

####

#73

Updated by Joe Bylund 2 months ago

Jens,

This is not in the current build yet. The easiest way is to

    
    git clone https://github.com/abrauchli/shotwell.git

  
and then build shotwell as you would otherwise. I recommend backing up your
shotwell library before doing this because it triggers a db upgrade, so you
can't go back to the current unless you either restore a previous library or
wipe your library and start from scratch.

-Joe

####

#74

Updated by Jens . 2 months ago

As I said - I cannot build Shotwell on Ubuntu 12.04 because there are too many
missing dependancies. I actually cloned the above repo, added the valac PPA
and tried to find `libclutter` etc. in up to date versions, but I don't want
to mess with my whole system just because of one application. The only
alternative would be to install a VM with Ubuntu 13.04 or whatever is needed,
I'll do that if there really is no alternative. But in the end I want to get
Shotwell running on my personal machine, which will stay on ubuntu LTS
versions.

Can maybe somebody running Ubuntu 12.04 run `fakeroot dpkg-buildpackage -b`
and provide the .deb files?

####

#75

Updated by andreas - 2 months ago

Dear Jens, thank you for your interest.

Please note that this feature is a developer preview intended for developers
to enhance the started work for eventual merging into mainline. It is not
something that I will (or others should) make available as finished build as
it will only create support requests (breaks DB compatibility with main
shotwell) and thus burden the yorba team.

Thanks for understanding

####

#76

Updated by Jens . 2 months ago

I understand!

However, I have a large interest in moving to Shotwell (from iPhoto, btw) and
thus I am not so easily scared away. ;)

So I installed an Ubuntu 13.04 VM just to compile Shotwell and have a look.
Here are some comments. I know the code in the Git repository is not finished
yet and some of my thoughts below create a lot of work - please don't
misunderstand. I just want to help by providing some user feedback. Also I'm
perfectly willing to test some more if updates appear. I tried several photo
managers and Shotwell seems to be the best so far (Digikam is overloaded and
installs 180MB of KDE libraries with it, Darkroom is more editor than
organizer, FSpot crashes too often, ...).

Shotwell is nice, clean, fast (ok, it hasn't seen my 32'000 120GB photo
library yet). This is why I'm here.

So, my comments regarding the map after playing with current CVS for an
evening:

  * The map is based on OpenStreetmap. This is good (more detail than e.g. Google map) and bad (less detail than Google satellite view). I think the advantages weigh more but this is perhaps a matter of taste.
  * The map size is adjustable (by adjusting the sidebar width). This is great and it's one major gripe I have with iPhoto - finding locations in a 5x5cm map in a sidebar with a fixed size is horrible. When geotagging, I want to see a larger map than when browsing images.
  * The automatic zoom level is not ideal (yet), for single images the map should zoom further in so that at least individual streets are visible. (In the Google Maps API, which I am familiar with, this would correspond to a zoom level of 15 on a scale of 0..20.)
  * Importing images with existing GPS data takes a lot longer because of the map animation. For me the animation is not necessary and during import the map could also be completely disabled. (Also, what happens if you import without a network connection?)
  * The markers on the map should be movable (adjusting the GPS location) and perhaps even show the GPS location as a tooltip when moving. (Or better yet: the geolocated location name, e.g. street or facility, if possible.) 
  * During import of roughly 100 iPhone 4S photos, the current build crashed twice with a segfault (at different photos). If you like I can research this further (eg. compiling with debug symbols), please provide the command line to do this best.
Some additional thoughts:

  * The whole idea of geolocation is to **do** something with this data. So when you have a thousand photos with GPS locations, what is the benefit for the user? 
    * The most obvious usage is to filter by locality - for example, provide another sidebar tree containing continents, countries, districts, cities, ghettos, streets etc which are in use. To organize this kind of data the first thing coming to my mind is the xAL (extensible Address Language) standard combined with the appropriate IPTC fields (for JPEG data at least). Then, when the user selects an item in the sidebar, update the map view to contain markers for each photo (grouped if too many) and the main view to contain all matching photos. To do this, Shotwell would need to request this data from OSM for each photo and save it locally (otherwise it's probably too slow).
    * Also good (and probably relatively easy to implement) would be searching by GPS data (and localities) and creating saved searches based on this data.
    * Another nice idea is a travel map: if I have a lot of geotagged photos I'd like to see them displayed on a map in chronological order, like a directions route. Granted this is probably not going to be core functionality, but could this be done as a plugin?
  * Most cameras, especially smartphones, have inaccurate GPS data - so if you take many shots in a single location the GPS locations will be spread out over a few hundred meters / yards. Correcting this is one step in my import workflow. In iPhoto I can manage my locations by name and radius, and imported pictures whose GPS coordinates are within the set radius of a known/saved/named location will automatically be assigned to this location (of course the locations cannot overlap). This is a major work saver. The rest I update by selecting them and typing in a new location name (which is searched via Google). IMHO an equivalent or better solution (in addition to the location manager with a radius option) could be selecting multiple markers on the map and dragging them (together) to a new location.

Thank you for listening. ;)



--- Bug imported by chaz@yorba.org 2013-11-25 21:48 UTC  ---

This bug was previously known as _bug_ 2875 at http://redmine.yorba.org/show_bug.cgi?id=2875
Imported an attachment (id=261867)

Unknown version " in product shotwell. 
   Setting version to "!unspecified".
Unknown milestone "unknown in product shotwell. 
   Setting to default milestone for this product, "---".
Setting qa contact to the default for this product.
   This bug either had no qa contact or an invalid one.
Resolution set on an open status.
   Dropping resolution 
CC member joseph.bylund+shotwell@gmail.com does not have an account here