Hello community, here is the log from the commit of package shotwell for openSUSE:Factory checked in at 2014-10-14 08:59:06 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/shotwell (Old) and /work/SRC/openSUSE:Factory/.shotwell.new (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "shotwell" Changes: -------- --- /work/SRC/openSUSE:Factory/shotwell/shotwell.changes 2014-09-30 19:37:17.000000000 +0200 +++ /work/SRC/openSUSE:Factory/.shotwell.new/shotwell.changes 2014-10-14 08:59:15.000000000 +0200 @@ -1,0 +2,10 @@ +Fri Oct 3 07:46:55 UTC 2014 - [email protected] + +- Update to version 0.20.1: + + Correct problems with navigating photos in full-screen mode + (bgo#737092). + + Better memory utilization via more conservative pixbuf cache + (bgo#715198). + + Fixes minor bugs detected by better Vala code analysis. + +------------------------------------------------------------------- Old: ---- shotwell-0.20.0.tar.xz New: ---- shotwell-0.20.1.tar.xz ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ shotwell.spec ++++++ --- /var/tmp/diff_new_pack.rn6nAg/_old 2014-10-14 08:59:17.000000000 +0200 +++ /var/tmp/diff_new_pack.rn6nAg/_new 2014-10-14 08:59:17.000000000 +0200 @@ -17,7 +17,7 @@ Name: shotwell -Version: 0.20.0 +Version: 0.20.1 Release: 0 Summary: Photo Manager for GNOME License: LGPL-2.1+ ++++++ shotwell-0.20.0.tar.xz -> shotwell-0.20.1.tar.xz ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/Makefile new/shotwell-0.20.1/Makefile --- old/shotwell-0.20.0/Makefile 2014-09-19 20:28:48.000000000 +0200 +++ new/shotwell-0.20.1/Makefile 2014-10-02 22:06:50.000000000 +0200 @@ -2,7 +2,7 @@ PROGRAM_THUMBNAILER = shotwell-video-thumbnailer PROGRAM_MIGRATOR = shotwell-settings-migrator -VERSION = 0.20.0 +VERSION = 0.20.1 GITVER := $(shell git log -n 1 2>/dev/null | head -n 1 | awk '{print $$2}') GETTEXT_PACKAGE = $(PROGRAM) BUILD_ROOT = 1 diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/NEWS new/shotwell-0.20.1/NEWS --- old/shotwell-0.20.0/NEWS 2014-09-19 20:28:48.000000000 +0200 +++ new/shotwell-0.20.1/NEWS 2014-10-02 22:06:38.000000000 +0200 @@ -1,3 +1,11 @@ +Shotwell 0.20.1 - 2 October 2014 +-------------------------------- + + * Corrects problems with navigating photos in full-screen mode (#737092) + * Better memory utilization via more conservative pixbuf cache (#715198) + * Fixes minor bugs detected by better Vala code analysis + + Shotwell 0.20.0 - 16 September 2014 ----------------------------------- diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/plugins/common/RESTSupport.vala new/shotwell-0.20.1/plugins/common/RESTSupport.vala --- old/shotwell-0.20.0/plugins/common/RESTSupport.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/plugins/common/RESTSupport.vala 2014-10-01 03:15:12.000000000 +0200 @@ -463,7 +463,7 @@ document = doc; } - ~RESTXmlDocument() { + ~XmlDocument() { delete document; } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/AppWindow.vala new/shotwell-0.20.1/src/AppWindow.vala --- old/shotwell-0.20.0/src/AppWindow.vala 2014-08-21 22:44:19.000000000 +0200 +++ new/shotwell-0.20.1/src/AppWindow.vala 2014-10-01 03:14:12.000000000 +0200 @@ -146,7 +146,11 @@ return true; } - // Make sure this event gets propagated to the underlying window... + // propagate to this (fullscreen) window respecting "stop propagation" result... + if (base.key_press_event != null && base.key_press_event(event)) + return true; + + // ... then propagate to the underlying window hidden behind this fullscreen one return AppWindow.get_instance().key_press_event(event); } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/Commands.vala new/shotwell-0.20.1/src/Commands.vala --- old/shotwell-0.20.0/src/Commands.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/Commands.vala 2014-10-01 03:16:18.000000000 +0200 @@ -211,7 +211,7 @@ base(photo, name, explanation); } - ~GenericPhotoTransformationState() { + ~GenericPhotoTransformationCommand() { if (original_state != null) original_state.broken.disconnect(on_state_broken); @@ -795,8 +795,16 @@ } public override void execute_on_photo(Photo photo) { + // thaw collection so both alterations are signalled at the same time + DataCollection? collection = photo.get_membership(); + if (collection != null) + collection.freeze_notifications(); + photo.set_straighten(theta); photo.set_crop(crop); + + if (collection != null) + collection.thaw_notifications(); } } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/Photo.vala new/shotwell-0.20.1/src/Photo.vala --- old/shotwell-0.20.0/src/Photo.vala 2014-09-09 22:35:45.000000000 +0200 +++ new/shotwell-0.20.1/src/Photo.vala 2014-10-01 03:16:29.000000000 +0200 @@ -199,7 +199,11 @@ // The number of seconds we should hold onto a precached copy of the original image; if // it hasn't been accessed in this many seconds, discard it to conserve memory. - private const int PRECACHE_TIME_TO_LIVE = 180; + private const int SOURCE_PIXBUF_TIME_TO_LIVE_SEC = 10; + + // min and max size of source pixbuf cache LRU + private const int SOURCE_PIXBUF_MIN_LRU_COUNT = 1; + private const int SOURCE_PIXBUF_MAX_LRU_COUNT = 3; // Minimum raw embedded preview size we're willing to accept; any smaller than this, and // it's probably intended primarily for use only as a thumbnail and won't look good on the @@ -293,6 +297,35 @@ public PhotoFileReader editable; } + private class CachedPixbuf { + public Photo photo; + public Gdk.Pixbuf pixbuf; + public Timer last_touched = new Timer(); + + public CachedPixbuf(Photo photo, Gdk.Pixbuf pixbuf) { + this.photo = photo; + this.pixbuf = pixbuf; + } + } + + // The first time we have to run the pipeline on an image, we'll precache + // a copy of the unscaled, unmodified version; this allows us to operate + // directly on the image data quickly without re-fetching it at the top + // of the pipeline, which can cause significant lag with larger images. + // + // This adds a small amount of (automatically garbage-collected) memory + // overhead, but greatly simplifies the pipeline, since scaling can now + // be blithely ignored, and most of the pixel operations are fast enough + // that the app remains responsive, even with 10MP images. + // + // In order to make sure we discard unneeded precaches in a timely fashion, + // we spawn a timer when the unmodified pixbuf is first precached; if the + // timer elapses and the pixbuf hasn't been needed again since then, we'll + // discard it and free up the memory. The cache also has an LRU to prevent + // runaway amounts of memory from being stored (see SOURCE_PIXBUF_LRU_COUNT) + private static Gee.LinkedList<CachedPixbuf>? source_pixbuf_cache = null; + private static uint discard_source_id = 0; + // because fetching individual items from the database is high-overhead, store all of // the photo row in memory protected PhotoRow row; @@ -308,23 +341,6 @@ private OneShotScheduler remove_editable_scheduler = null; protected bool can_rotate_now = true; - - // The first time we have to run the pipeline on an image, we'll precache - // a copy of the unscaled, unmodified version; this allows us to operate - // directly on the image data quickly without re-fetching it at the top - // of the pipeline, which can cause significant lag with larger images. - // - // This adds a small amount of (automatically garbage-collected) memory - // overhead, but greatly simplifies the pipeline, since scaling can now - // be blithely ignored, and most of the pixel operations are fast enough - // that the app remains responsive, even with 10MP images. - // - // In order to make sure we discard unneeded precaches in a timely fashion, - // we spawn a timer when the unmodified pixbuf is first precached; if the - // timer elapses and the pixbuf hasn't been needed again since then, we'll - // discard it and free up the memory. - private Gdk.Pixbuf unmodified_precached = null; - private GLib.Timer secs_since_access = null; // RAW only: developed backing photos. private Gee.HashMap<RawDeveloper, BackingPhotoRow?>? developments = null; @@ -455,6 +471,19 @@ cached_exposure_time = this.row.exposure_time; } + protected static void init_photo() { + source_pixbuf_cache = new Gee.LinkedList<CachedPixbuf>(); + } + + protected static void terminate_photo() { + source_pixbuf_cache = null; + + if (discard_source_id != 0) { + Source.remove(discard_source_id); + discard_source_id = 0; + } + } + protected virtual void notify_editable_replaced(File? old_file, File? new_file) { editable_replaced(old_file, new_file); } @@ -785,7 +814,7 @@ if (!is_raw_developer_complete(d)) { develop_photo(d); try { - populate_prefetched(); + get_prefetched_copy(); } catch (Error e) { // couldn't reload the freshly-developed image, nothing to display return; @@ -826,7 +855,7 @@ } notify_altered(new Alteration("image", "developer")); - discard_prefetched(true); + discard_prefetched(); } public RawDeveloper get_raw_developer() { @@ -3204,64 +3233,123 @@ public override Gdk.Pixbuf get_pixbuf(Scaling scaling) throws Error { return get_pixbuf_with_options(scaling); } - + /** - * @brief Populates the cached version of the unmodified image. + * One-stop shopping for the source pixbuf cache. + * + * The source pixbuf cache holds untransformed, unscaled (full-sized) pixbufs of Photo objects. + * These can be rather large and shouldn't be held in memory for too long, nor should many be + * allowed to stack up. + * + * If locate is non-null, a source pixbuf is returned for the Photo. If keep is true, the + * pixbuf is stored in the cache. (Thus, passing a Photo w/ keep == false will drop the cached + * pixbuf.) If Photo is non-null but keep is false, null is returned. + * + * Whether locate is null or not, the cache is walked in its entirety, dropping expired pixbufs + * and dropping excessive pixbufs from the LRU. Locating a Photo "touches" the pixbuf, i.e. + * it moves to the head of the LRU. */ - public void populate_prefetched() throws Error { - lock (unmodified_precached) { - // If we don't have it already, precache the original... - if (unmodified_precached == null) { - unmodified_precached = load_raw_pixbuf(Scaling.for_original(), Exception.ALL, BackingFetchMode.SOURCE); - secs_since_access = new GLib.Timer(); - GLib.Timeout.add_seconds(5, (GLib.SourceFunc)discard_prefetched); - debug("spawning new precache timeout for %s", this.to_string()); + private static Gdk.Pixbuf? run_source_pixbuf_cache(Photo? locate, bool keep) throws Error { + lock (source_pixbuf_cache) { + CachedPixbuf? found = null; + + // walk list looking for photo to locate (if specified), dropping expired and LRU'd + // pixbufs along the way + double min_elapsed = double.MAX; + int count = 0; + Gee.Iterator<CachedPixbuf> iter = source_pixbuf_cache.iterator(); + while (iter.next()) { + CachedPixbuf cached_pixbuf = iter.get(); + + double elapsed = Math.trunc(cached_pixbuf.last_touched.elapsed()) + 1; + + if (locate != null && cached_pixbuf.photo.equals(locate)) { + // found it, remove and reinsert at head of LRU (below)... + iter.remove(); + found = cached_pixbuf; + + // ...that's why the counter is incremented + count++; + } else if (elapsed >= SOURCE_PIXBUF_TIME_TO_LIVE_SEC) { + iter.remove(); + } else if (count >= SOURCE_PIXBUF_MAX_LRU_COUNT) { + iter.remove(); + } else { + // find the item with the least elapsed time to reschedule a cache trim (to + // prevent onesy-twosy reschedules) + min_elapsed = double.min(elapsed, min_elapsed); + count++; + } + } + + // if not found and trying to locate one and keep it, generate now + if (found == null && locate != null && keep) { + found = new CachedPixbuf(locate, + locate.load_raw_pixbuf(Scaling.for_original(), Exception.ALL, BackingFetchMode.SOURCE)); + } else if (found != null) { + // since it was located, touch it so it doesn't expire + found.last_touched.start(); + } + + // if keeping it, insert at head of LRU + if (found != null && keep) { + source_pixbuf_cache.insert(0, found); + + // since this is (re-)inserted, count its elapsed time too ... w/ min_elapsed, this + // is almost guaranteed to be the min, since it was was touched mere clock cycles + // ago... + min_elapsed = double.min(found.last_touched.elapsed(), min_elapsed); + + // ...which means don't need to readjust the min_elapsed when trimming off excess + // due to adding back an element + while(source_pixbuf_cache.size > SOURCE_PIXBUF_MAX_LRU_COUNT) + source_pixbuf_cache.poll_tail(); } + + // drop expiration timer... + if (discard_source_id != 0) { + Source.remove(discard_source_id); + discard_source_id = 0; + } + + // ...only reschedule if there's something to expire + if (source_pixbuf_cache.size > SOURCE_PIXBUF_MIN_LRU_COUNT) { + assert(min_elapsed >= 0.0); + + // round-up to avoid a bunch of zero-second timeouts + uint retry_sec = SOURCE_PIXBUF_TIME_TO_LIVE_SEC - ((uint) Math.trunc(min_elapsed)); + discard_source_id = Timeout.add_seconds(retry_sec, trim_source_pixbuf_cache, Priority.LOW); + } + + return found != null ? found.pixbuf : null; } } - + + private static bool trim_source_pixbuf_cache() { + try { + run_source_pixbuf_cache(null, false); + } catch (Error err) { + } + + return false; + } + /** * @brief Get a copy of what's in the cache. * - * @return A Pixbuf with the image data from unmodified_precached. + * @return A copy of the Pixbuf with the image data from unmodified_precached. */ public Gdk.Pixbuf get_prefetched_copy() throws Error { - lock (unmodified_precached) { - if (unmodified_precached == null) { - try { - populate_prefetched(); - } catch (Error e) { - message("pixbuf for %s could not be loaded: %s", to_string(), e.message); - - throw e; - } - } - - return unmodified_precached.copy(); - } + return run_source_pixbuf_cache(this, true).copy(); } /** * @brief Discards the cached version of the unmodified image. - * - * @param immed Whether the cached version should be discarded now, or not. */ - public bool discard_prefetched(bool immed = false) { - lock (unmodified_precached) { - if (secs_since_access == null) - return false; - - double tmp; - if ((secs_since_access.elapsed(out tmp) > PRECACHE_TIME_TO_LIVE) || (immed)) { - debug("pipeline not run in over %d seconds or got immediate command, discarding " + - "cached original for %s", - PRECACHE_TIME_TO_LIVE, to_string()); - unmodified_precached = null; - secs_since_access = null; - return false; - } - - return true; + public void discard_prefetched() { + try { + run_source_pixbuf_cache(this, false); + } catch (Error err) { } } @@ -3283,7 +3371,7 @@ Timer timer = new Timer(); Timer total_timer = new Timer(); double redeye_time = 0.0, crop_time = 0.0, adjustment_time = 0.0, orientation_time = 0.0, - straighten_time = 0.0; + straighten_time = 0.0, scale_time = 0.0; total_timer.start(); #endif @@ -3329,13 +3417,8 @@ // // Image load-and-decode // - populate_prefetched(); - - Gdk.Pixbuf pixbuf = get_prefetched_copy(); - // remember to delete the cached copy if it isn't being used. - secs_since_access.start(); - debug("pipeline being run against %s, timer restarted.", this.to_string()); + Gdk.Pixbuf pixbuf = get_prefetched_copy(); // // Image transformation pipeline @@ -3405,15 +3488,15 @@ #endif } -#if MEASURE_PIPELINE - debug("PIPELINE %s (%s): redeye=%lf crop=%lf adjustment=%lf orientation=%lf total=%lf", - to_string(), scaling.to_string(), redeye_time, crop_time, adjustment_time, - orientation_time, total_timer.elapsed()); -#endif - // scale the scratch image, as needed. if (is_scaled) { +#if MEASURE_PIPELINE + timer.start(); +#endif pixbuf = pixbuf.scale_simple(scaled_to_viewport.width, scaled_to_viewport.height, Gdk.InterpType.BILINEAR); +#if MEASURE_PIPELINE + scale_time = timer.elapsed(); +#endif } // color adjustment; we do this dead last, since, if an image has been scaled down, @@ -3434,7 +3517,13 @@ // the pixbuf, and must be accounted for the test to be valid. if ((is_scaled) && (!is_straightened)) assert(scaled_to_viewport.approx_equals(Dimensions.for_pixbuf(pixbuf), SCALING_FUDGE)); - + +#if MEASURE_PIPELINE + debug("PIPELINE %s (%s): redeye=%lf crop=%lf adjustment=%lf orientation=%lf straighten=%lf scale=%lf total=%lf", + to_string(), scaling.to_string(), redeye_time, crop_time, adjustment_time, + orientation_time, straighten_time, scale_time, total_timer.elapsed()); +#endif + return pixbuf; } @@ -4032,12 +4121,12 @@ // at this point, any image date we have cached is stale, // so delete it and force the pipeline to re-fetch it - discard_prefetched(true); + discard_prefetched(); } private void on_reimport_editable() { // delete old image data and force the pipeline to load new from file. - discard_prefetched(true); + discard_prefetched(); debug("Reimporting editable for %s", to_string()); try { @@ -4890,6 +4979,8 @@ } public static void init(ProgressMonitor? monitor = null) { + init_photo(); + global = new LibraryPhotoSourceCollection(); // prefetch all the photos from the database and add them to the global collection ... @@ -4921,6 +5012,7 @@ } public static void terminate() { + terminate_photo(); } // This accepts a PhotoRow that was prepared with Photo.prepare_for_import and diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/PixbufCache.vala new/shotwell-0.20.1/src/PixbufCache.vala --- old/shotwell-0.20.0/src/PixbufCache.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/PixbufCache.vala 2014-10-01 03:16:18.000000000 +0200 @@ -265,6 +265,10 @@ return; } +#if TRACE_PIXBUF_CACHE + debug("%s %s fetched into pixbuf cache", type.to_string(), job.photo.to_string()); +#endif + encache(job.photo, job.pixbuf); // fire signal @@ -279,16 +283,14 @@ Photo photo = (Photo) object; if (in_progress.has_key(photo)) { - // Load is in progress, must cancel. + // Load is in progress, must cancel, but consider in-cache (since it was decached + // before being put into progress) in_progress.get(photo).cancel(); in_progress.unset(photo); + } else if (!cache.has_key(photo)) { continue; } - // only interested if in this cache - if (!cache.has_key(photo)) - continue; - decache(photo); #if TRACE_PIXBUF_CACHE diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/SearchFilter.vala new/shotwell-0.20.1/src/SearchFilter.vala --- old/shotwell-0.20.0/src/SearchFilter.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/SearchFilter.vala 2014-10-01 03:15:12.000000000 +0200 @@ -694,7 +694,7 @@ this.add(button); } - ~ToggleActionButton() { + ~ToggleActionToolButton() { button.clicked.disconnect(on_button_activate); } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/Tag.vala new/shotwell-0.20.1/src/Tag.vala --- old/shotwell-0.20.0/src/Tag.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/Tag.vala 2014-10-01 03:15:12.000000000 +0200 @@ -626,7 +626,7 @@ string built = builder.str; if (built.length >= separator.length) - if (built.substring(built.length - separator.length, separator.length) == separator); + if (built.substring(built.length - separator.length, separator.length) == separator) built = built.substring(0, built.length - separator.length); if (end != null) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/direct/DirectPhoto.vala new/shotwell-0.20.1/src/direct/DirectPhoto.vala --- old/shotwell-0.20.0/src/direct/DirectPhoto.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/direct/DirectPhoto.vala 2014-10-01 03:15:55.000000000 +0200 @@ -38,6 +38,8 @@ } public static void init(File initial_file) { + init_photo(); + global = new DirectPhotoSourceCollection(initial_file); DirectPhoto photo; string? reason = global.fetch(initial_file, out photo, false); @@ -47,6 +49,7 @@ } public static void terminate() { + terminate_photo(); } // Gets the dimensions of this photo's pixbuf when scaled to original @@ -257,7 +260,7 @@ } public void reimport_photo(DirectPhoto photo) { - photo.discard_prefetched(true); + photo.discard_prefetched(); DirectPhoto reimported_photo; fetch(photo.get_file(), out reimported_photo, true); } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/searches/SavedSearchDialog.vala new/shotwell-0.20.1/src/searches/SavedSearchDialog.vala --- old/shotwell-0.20.0/src/searches/SavedSearchDialog.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/searches/SavedSearchDialog.vala 2014-10-01 03:15:12.000000000 +0200 @@ -522,7 +522,7 @@ update_date_labels(); } - ~SearchRowRating() { + ~SearchRowDate() { context.changed.disconnect(on_changed); } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/shotwell-0.20.0/src/tags/HierarchicalTagUtilities.vala new/shotwell-0.20.1/src/tags/HierarchicalTagUtilities.vala --- old/shotwell-0.20.0/src/tags/HierarchicalTagUtilities.vala 2014-03-04 23:54:12.000000000 +0100 +++ new/shotwell-0.20.1/src/tags/HierarchicalTagUtilities.vala 2014-10-01 03:15:12.000000000 +0200 @@ -173,7 +173,7 @@ return; Tag? t = null; - if (Tag.global.exists(actual_path)); + if (Tag.global.exists(actual_path)) t = Tag.for_path(actual_path); if (t != null && t.get_hierarchical_children().size == 0) -- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
