[Shotwell] Final call for testing Shotwell 0.5

[Adam Dingle]

Martin,

first of all, our heartfelt thanks for your thorough and impressive 
testing efforts!  You've revealed a number of significant issues 
previously unknown to us.  Your feedback for both Shotwell 0.4 and 0.5 
has been an important contribution.  We deeply appreciate it!

I've filed tickets in our bug database for all of the significant issues 
below, and I've cc-ed you on the tickets so you can follow their 
progress.  (I know this may result in lots of email; if you'd rather not 
be cc-ed on tickets we file for bugs you've reported then just let me know.)

> 0.5.0 is looking really good so far..  below are some new issues I found.
> I'm testing using current Lucid bits so I used to have the issues that
> Roman Yepishev posted about recently. Using Shotwell trunk as of now they
> seem to be fixed which is great.
>   

That's good to hear.

> * What does the HIDE menu item do? I can see the images disappear alright but
> where do they go and what do I use this feature for? Even after messing around
> with it for several minutes, I don't really get it. (Update: a bit later while
> browsing around the menu I found the "View::Hidden" thing and I understand it
> now and it sort of makes sense. However, what about flashing and fading away
> the message "Use View::Hidden to show hidden images" somewhere in the UI when
> you first hide an image (conceptually sort of like how Firefox puts this yellow
> banner at the top of the page and says "FYI; I just blocked a pop up for you" etc.
> It could be just the first time you hide an image or maybe the first time in each
> session, so that it doesn't get too annoying. Another approach would be to show a
> small icon in the top left area of any "album" that has at least one hidden image;
> this icon should say "Click here to show hidden images as well" or similar. If you
> brainstorm a bit I'm sure you can come up with even better ways of cluing in the
> user as to where their images go when you "hide" them. Consider for example the
> special case where an event contains a single photo of a cat, thus that image
> will now be used for the event thumbnail despite being hidden and if you open
> that album you get an empty gray page which is a bit weird.
>   

Yes - we could possibly make it clearer where the hidden images have 
gone.  It's too late to address this in 0.5, but maybe in another release.

> * Import some photos, right click the first photo and select "Set as Desktop Background",
> press CTRL-ALT-D twice and verify that the image was set. Now click some other image and
> select "Set as Desktop Background" on that one and again look at the desktop. Now the old
> image will still be set as wallpaper. I suspect maybe this is because gconf doesn't emit a
> "changed" event if you set the exact same value again, i.e:
> picture_filename = /home/mnemo/.shotwell/wallpaper/wallpaper.jpg
>   

Good catch.  I see this only on Lucid.

> * After looking briefly at the set wallpaper code I'd suggest these changes:
> - in set_background() do "if (!set_string()) return false"
> - the function set_background() should return bool so that you can do
>   AppWindow.error_messag("txt") in set_desktop_background() like you
>   already do in the catch there.
>   

Thanks for the code suggestion - we'll look at this soon.

> * In the English translation, when you open the context menu on a photo both
> the "Modify Tags" and "Remove" items have keyboard accelerator "_m". Similar with
> "Favorites" and "Fullscreen" in the View menu.
>   

Right.

> * File::Publish is the only menu item in the main menu which does not have a "_X"-style
> keyboard accelerator.
>   

Yup.

> * If I have firefox running and I select "Help::Contents" in shotwell the page loads
> but the FF window is not raised (which typically means the user will not notice,
> I thought it did nothing at first). gtk_show_uri() doesn't throw though so maybe
> this is an GTK/Ubuntu bug (I've seen similar issues on Ubuntu before and discussions
> about that the "allowed window raising policy" is these days). Maybe you'd want to
> look into what's going on there or maybe deploy some sort of explicit "raise window"
> workaround though because Shotwell users will suffer from this for sure.
>   

This occurs only on Lucid, and indeed seems to be a GTK/Ubuntu bug: I 
see the same behavior when I click a URL in gedit's About box, for 
example.  As such, we're reluctant to implement a Shotwell-specific 
workaround.  I hope that Ubuntu will address this before the Lucid release.

> * Import a photo, right click it and select "Modify tags" (note: image should have no prior tags)
> and then type "blah" and press OK. Reproducible SEGV:
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff7ba3ce9 in gee_collection_contains (self=0x0, item=0x94c8a0) at collection.c:158
> 158		return GEE_COLLECTION_GET_INTERFACE (self)->contains (self, item);
> ...
>   

Great catch.  I think this worked until recently and is a recent regression.

> * From a "UI clutter" perspective I don't see why you need both "Add tags" and "Modify tags"?
>   

Add Tags can work on multiple photos, but Modify Tags works on only 
one.  It might be tricky to make Modify Tags work on multiple photos, 
since the photos can have different tags.

> * Reproducible SIGABRT. Import three JPGs and right click the first one and select "Hide". The use CTRL-A to select all remaining photos and right click selecting DUPLICATE.
> ERROR:src/CheckerboardLayout.c:2619:checkerboard_layout_repaint_item: assertion failed: (data_collection_contains (DATA_COLLECTION (self->priv->view), DATA_OBJECT (item)))
> ...

Great catch.

>
> * Switching between photos are a bit slow. I have a quad core 2.8ghz with 8GB RAM,
> speed raided 10K RPM discs and 12MB CPU cache; still it routinely takes >500ms to
> switch to the next image. Not sure what's going on there. Also when I import photos
> I can see them flash by extremely fast, like 10 images per second fast; so why does
> it have to be so slow to switch between images in the photo album (i.e. open a photo
> in full view and then pressing LEFT and RIGHT arrows). For example try holding down
> the RIGHT key (and try it in Picasa too). I think it would be useful to hit this with
> gprof/oprofile because something is fishy there and long term of course you could
> pre-render/load the JPGs (and maybe even add progressive loading even though I hate that)
> like Picasa does (which I assume you don't right now?). I want Shotwell to feel extremely
> light-weight so that when I try to find that special image that I want to show someone,
> then bottle-neck should be how fast _I_ can browse/inspect the images not how fast the
> machine is. Also note that if I launch a terminal with "htop" and mark it as "always on
> top" and the hold down the RIGHT key so make Shotwell constantly switch to the next
> image, it does not tax my CPU nor my HDD at 100% (not even a single core is saturated)
> so maybe there is something cheezy goign on with keyboard repeat rate or similar?
>   

Improving photo transition performance is an ongoing effort, and 
actually we've already profiled the code and we already do some 
prefetching as well.  But yes, this could be improved more and I agree 
we'd like to work toward instant photo transitions.  I've filed a ticket 
and hopefully we can improve this more in 0.6.

> * Double click any photo (from an event with multiple photos) to bring it up in
> full view. Then super quickly hit the RIGHT keyboard key twice. Shotwell only
> moves to the next image, not two images forward. This is really annoying if you
> want to quickly skip past that particular one image when you had too much to drink
> or whatever.
>   

Right.  We already have a ticket for this:  
http://trac.yorba.org/ticket/1465 .  Maybe in 0.6.

> * Download and unpack these two images to a separate dir:
> http://temp.minimum.se/shotwell_crashers/12.tgz
> Launch with "rm -rf rm -rf ~/.shotwell ; SHOTWELL_LOG=1 gdbr ./shotwell"
> and import from that dir, press OK when import finishes and immediately after
> you press OK you will get a reproducible SEGV with this stack:
> ...

Great catch.  Oddly, this seems to occur only when running under GDB.  
We'll investigate.

>
> * Launch Shotwell using "rm -rf ~/.shotwell ; SHOTWELL_LOG=1 gdbr ./shotwell" and import
> the photos from this tarball: http://temp.minimum.se/shotwell_crashers/catbug.tgz
> Click "Photos", select bug.jpg and then Events::NewEvent. At this point the bug gets
> sort under the 1970 event node which is unintuitive (unless you know that unix date zero
> is at 1970) but this is not something that makes sense for an end-user. How about having
> a special event tree node for "Unknown Year" or something like that?
>   

Right.  We've seen (and fixed) this bug before when adding date-less 
photos to existing events, but as you point out it also occurs when 
creating a new event, and so I've reopened the associated ticket: 
http://trac.yorba.org/ticket/1152 .  It's unlikely we'll fix this for 
0.5; hopefully in the next release.

> * Launch Shotwell using "rm -rf ~/.shotwell ; SHOTWELL_LOG=1 gdbr ./shotwell" and import
> the photos from this tarball: http://temp.minimum.se/shotwell_crashers/catbug.tgz
> Click "Photos", select cat.jpg and then Events::NewEvent. At this point the event tree
> first expands but is then automatically collapsed again and it leaves me in this weird
> state where no new event node has been added at all to the event node tree? In the terminal
> I can see this (below) but it's unclear to me _why_ Shotwell destroys the event "Event" ?
> [DBG] LibraryWindow.vala:221: Creating new event page for Event 2
> [DBG] Event.vala:192: Destroying event Event [1/6] Event 1
>   

This is expected behavior.  The event previously containing cat.jpg is 
destroyed (that was Event 1) since it no longer contains any photos: 
cat.jpg has been removed from it.  A new event is created for cat.jpg 
(which happens to look just like the old one).  If you find this unduly 
confusing, what alternative behavior would you suggest?

> * Are you aware that you're generating _extremely_ deep stacks during modest to large
> imports (which will most likely give you a stackoverflow for imports of a certain size
> and up)? I've seen cases where Shotwell used stacks 40000 frames deep. In these cases
> the frames follow the following repeating pattern, i.e:
>
> ...

No, we were not aware.  Stacks this deep are not good.  We'll 
investigate and see if there's a way to avoid this.

>
> * When I start with a deleted ~/.shotwell and using SHOTWELL_LOG=1, then it says:
> [DBG] DatabaseTables.vala:277: Database version -1 create by app version (null)
> ...is that supposed to be "created" maybe? And maybe a special message should be
> printed when a new database is created instead of saying "app version (null)" ?
>   

OK - we'll clean up this message if we can.

> * If you try to publish to Picasa Web Albums and select publish to a new album
> and you enter a single space character as the name, then it says "Service returned
> HTTP status code 400". I think there should be a "START OVER" or "BACK" button on
> this dialog page. Also I think you should prevent the user from pressing Publish
> if trim(album_name)="". Also using the name "<h6>INJECTION" didn't work, maybe
> some people will actually try stuff like "My <HAPPY> Birthday!" or whatever since
> they don't know "<" and ">" is special chars so maybe Shotwell should warm about
> that too?
>   

Reasonable suggestions.  We also saw your subsequent email about HTML 
encoding the album name; that also sounds like a good idea.

Once again, many thanks for the outstanding feedback!

adam