summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJörg Frings-Fürst <debian@jff-webhosting.net>2014-10-03 14:53:52 +0200
committerJörg Frings-Fürst <debian@jff-webhosting.net>2014-10-03 14:53:52 +0200
commite97c1ca41f345bcf57417f6dc06e0d42f3547443 (patch)
treeb9afc8d1553a62369c373c703d4f479ad81188af
parent566dc060676b41e1e58a446b7dcc4159e242fee6 (diff)
Imported Upstream version 0.20.1
-rw-r--r--Makefile2
-rw-r--r--NEWS8
-rw-r--r--plugins/common/RESTSupport.vala2
-rw-r--r--src/AppWindow.vala6
-rw-r--r--src/Commands.vala10
-rw-r--r--src/Photo.vala250
-rw-r--r--src/PixbufCache.vala12
-rw-r--r--src/SearchFilter.vala2
-rw-r--r--src/Tag.vala2
-rw-r--r--src/direct/DirectPhoto.vala5
-rw-r--r--src/searches/SavedSearchDialog.vala2
-rw-r--r--src/tags/HierarchicalTagUtilities.vala2
12 files changed, 210 insertions, 93 deletions
diff --git a/Makefile b/Makefile
index bc1d5de..e3db565 100644
--- a/Makefile
+++ b/Makefile
@@ -2,7 +2,7 @@ PROGRAM = shotwell
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 --git a/NEWS b/NEWS
index 6c8b8e9..baa165d 100644
--- a/NEWS
+++ b/NEWS
@@ -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 --git a/plugins/common/RESTSupport.vala b/plugins/common/RESTSupport.vala
index 6a64025..7334de6 100644
--- a/plugins/common/RESTSupport.vala
+++ b/plugins/common/RESTSupport.vala
@@ -463,7 +463,7 @@ public class XmlDocument {
document = doc;
}
- ~RESTXmlDocument() {
+ ~XmlDocument() {
delete document;
}
diff --git a/src/AppWindow.vala b/src/AppWindow.vala
index 782f953..5a8e9c4 100644
--- a/src/AppWindow.vala
+++ b/src/AppWindow.vala
@@ -146,7 +146,11 @@ public class FullscreenWindow : PageWindow {
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 --git a/src/Commands.vala b/src/Commands.vala
index 0ad8ecb..04b771c 100644
--- a/src/Commands.vala
+++ b/src/Commands.vala
@@ -211,7 +211,7 @@ public abstract class GenericPhotoTransformationCommand : SingleDataSourceComman
base(photo, name, explanation);
}
- ~GenericPhotoTransformationState() {
+ ~GenericPhotoTransformationCommand() {
if (original_state != null)
original_state.broken.disconnect(on_state_broken);
@@ -795,8 +795,16 @@ public class StraightenCommand : GenericPhotoTransformationCommand {
}
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 --git a/src/Photo.vala b/src/Photo.vala
index 34b2676..98a3175 100644
--- a/src/Photo.vala
+++ b/src/Photo.vala
@@ -199,7 +199,11 @@ public abstract class Photo : PhotoSource, Dateable {
// 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 abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
}
notify_altered(new Alteration("image", "developer"));
- discard_prefetched(true);
+ discard_prefetched();
}
public RawDeveloper get_raw_developer() {
@@ -3204,64 +3233,123 @@ public abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
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 @@ public abstract class Photo : PhotoSource, Dateable {
//
// 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 @@ public abstract class Photo : PhotoSource, Dateable {
#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 @@ public abstract class Photo : PhotoSource, Dateable {
// 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 @@ public abstract class Photo : PhotoSource, Dateable {
// 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 class LibraryPhoto : Photo, Flaggable, Monitorable {
}
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 class LibraryPhoto : Photo, Flaggable, Monitorable {
}
public static void terminate() {
+ terminate_photo();
}
// This accepts a PhotoRow that was prepared with Photo.prepare_for_import and
diff --git a/src/PixbufCache.vala b/src/PixbufCache.vala
index 8b8f276..0708f5e 100644
--- a/src/PixbufCache.vala
+++ b/src/PixbufCache.vala
@@ -265,6 +265,10 @@ public class PixbufCache : Object {
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 @@ public class PixbufCache : Object {
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 --git a/src/SearchFilter.vala b/src/SearchFilter.vala
index 9769195..e8f0986 100644
--- a/src/SearchFilter.vala
+++ b/src/SearchFilter.vala
@@ -694,7 +694,7 @@ public class SearchFilterToolbar : Gtk.Toolbar {
this.add(button);
}
- ~ToggleActionButton() {
+ ~ToggleActionToolButton() {
button.clicked.disconnect(on_button_activate);
}
diff --git a/src/Tag.vala b/src/Tag.vala
index 1ae76ba..450af20 100644
--- a/src/Tag.vala
+++ b/src/Tag.vala
@@ -626,7 +626,7 @@ public class Tag : DataSource, ContainerSource, Proxyable, Indexable {
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 --git a/src/direct/DirectPhoto.vala b/src/direct/DirectPhoto.vala
index 98886ba..f7271ba 100644
--- a/src/direct/DirectPhoto.vala
+++ b/src/direct/DirectPhoto.vala
@@ -38,6 +38,8 @@ public class DirectPhoto : Photo {
}
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 class DirectPhoto : Photo {
}
public static void terminate() {
+ terminate_photo();
}
// Gets the dimensions of this photo's pixbuf when scaled to original
@@ -257,7 +260,7 @@ public class DirectPhotoSourceCollection : DatabaseSourceCollection {
}
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 --git a/src/searches/SavedSearchDialog.vala b/src/searches/SavedSearchDialog.vala
index b425cfb..da7f7db 100644
--- a/src/searches/SavedSearchDialog.vala
+++ b/src/searches/SavedSearchDialog.vala
@@ -522,7 +522,7 @@ public class SavedSearchDialog {
update_date_labels();
}
- ~SearchRowRating() {
+ ~SearchRowDate() {
context.changed.disconnect(on_changed);
}
diff --git a/src/tags/HierarchicalTagUtilities.vala b/src/tags/HierarchicalTagUtilities.vala
index 985491d..dbb7165 100644
--- a/src/tags/HierarchicalTagUtilities.vala
+++ b/src/tags/HierarchicalTagUtilities.vala
@@ -173,7 +173,7 @@ class HierarchicalTagUtilities {
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)