Commit cb1c5228 authored by Jim Nelson's avatar Jim Nelson

#2302: On startup, now checking all photos at startup for online/offline...

#2302: On startup, now checking all photos at startup for online/offline status, including those outside the library directory.  This patch also includes various optimizations in linking/unlinking and freezing/thawing DataCollections, which improves the performance of marking online/offline status as well as moving to the trash.
parent 0b242b7e
......@@ -122,7 +122,7 @@ public abstract class CollectionPage : CheckerboardPage {
init_ui_bind("/CollectionMenuBar");
init_item_context_menu("/CollectionContextMenu");
get_view().set_comparator(get_sort_comparator());
get_view().set_comparator(get_sort_comparator(), get_sort_comparator_predicate());
get_view().contents_altered.connect(on_contents_altered);
get_view().selection_group_altered.connect(on_selection_changed);
get_view().items_visibility_changed.connect(on_contents_altered);
......@@ -1701,7 +1701,7 @@ public abstract class CollectionPage : CheckerboardPage {
}
private void on_sort_changed() {
get_view().set_comparator(get_sort_comparator());
get_view().set_comparator(get_sort_comparator(), get_sort_comparator_predicate());
set_config_photos_sort(get_sort_order() == SORT_ORDER_ASCENDING, get_sort_criteria());
}
......@@ -1724,15 +1724,33 @@ public abstract class CollectionPage : CheckerboardPage {
if (is_sort_ascending())
return Thumbnail.rating_ascending_comparator;
else
return Thumbnail.rating_descending_comparator;
return Thumbnail.rating_descending_comparator;
default:
error("Unknown sort criteria: %d", get_sort_criteria());
error("Unknown sort criteria: %s", get_sort_criteria().to_string());
return Thumbnail.title_ascending_comparator;
}
}
private ComparatorPredicate get_sort_comparator_predicate() {
switch (get_sort_criteria()) {
case SortBy.TITLE:
return Thumbnail.title_comparator_predicate;
case SortBy.EXPOSURE_DATE:
return Thumbnail.exposure_time_comparator_predicate;
case SortBy.RATING:
return Thumbnail.rating_comparator_predicate;
default:
error("Unknown sort criteria: %s", get_sort_criteria().to_string());
return Thumbnail.title_comparator_predicate;
}
}
private override void set_display_titles(bool display) {
base.set_display_titles(display);
......@@ -1811,7 +1829,7 @@ public abstract class CollectionPage : CheckerboardPage {
}
if (resort_needed)
get_view().set_comparator(get_sort_comparator());
get_view().set_comparator(get_sort_comparator(), get_sort_comparator_predicate());
}
private void on_new_event() {
......
......@@ -1321,21 +1321,25 @@ public class TrashUntrashPhotosCommand : PageCommand {
private void trash(ProgressMonitor? monitor) {
int ctr = 0;
int count = photos.size;
LibraryPhoto.global.freeze_notifications();
foreach (LibraryPhoto photo in photos) {
photo.trash();
if (monitor != null)
monitor(++ctr, count);
}
LibraryPhoto.global.thaw_notifications();
}
private void untrash(ProgressMonitor? monitor) {
int ctr = 0;
int count = photos.size;
LibraryPhoto.global.freeze_notifications();
foreach (LibraryPhoto photo in photos) {
photo.untrash();
if (monitor != null)
monitor(++ctr, count);
}
LibraryPhoto.global.thaw_notifications();
}
private void on_photo_destroyed(DataSource source) {
......
......@@ -22,10 +22,15 @@
// operation is probably best done by a Gee.ArrayList.
//
// ComparatorPredicate is used to determine if a re-sort operation is necessary; it has no
// effect on adding a DataObject to a DataSet in sorted order.
public delegate bool ComparatorPredicate(DataObject object, Alteration alteration);
public class DataSet {
private SortedList<DataObject> list = new SortedList<DataObject>();
private Gee.HashSet<DataObject> hash_set = new Gee.HashSet<DataObject>();
private Comparator user_comparator = null;
private ComparatorPredicate? comparator_predicate = null;
public DataSet() {
reset_comparator();
......@@ -60,11 +65,13 @@ public class DataSet {
public void reset_comparator() {
user_comparator = null;
comparator_predicate = null;
list.resort(order_added_comparator);
}
public void set_comparator(Comparator user_comparator) {
public void set_comparator(Comparator user_comparator, ComparatorPredicate? comparator_predicate) {
this.user_comparator = user_comparator;
this.comparator_predicate = comparator_predicate;
list.resort(comparator_wrapper);
}
......@@ -147,7 +154,12 @@ public class DataSet {
}
// Returns true if the item has moved.
public bool resort_object(DataObject object) {
public bool resort_object(DataObject object, Alteration? alteration) {
if (comparator_predicate != null && alteration != null
&& !comparator_predicate(object, alteration)) {
return false;
}
return list.resort_item(object);
}
}
......@@ -451,8 +463,7 @@ public class DataCollection {
}
// This signal fires whenever any (or multiple) items in the collection signal they've been
// altered. This is more useful than item_altered() because it isn't blocked when notifications
// are frozen and is called when they are thawed.
// altered. This is more efficient than being called for each altered item.
public virtual signal void items_altered(Gee.Map<DataObject, Alteration> items) {
}
......@@ -539,8 +550,8 @@ public class DataCollection {
return true;
}
public virtual void set_comparator(Comparator comparator) {
dataset.set_comparator(comparator);
public virtual void set_comparator(Comparator comparator, ComparatorPredicate? predicate) {
dataset.set_comparator(comparator, predicate);
notify_ordering_changed();
}
......@@ -801,7 +812,7 @@ public class DataCollection {
public void internal_notify_altered(DataObject object, Alteration alteration) {
assert(internal_contains(object));
bool resort_occurred = dataset.resort_object(object);
bool resort_occurred = dataset.resort_object(object, alteration);
if (are_notifications_frozen()) {
if (frozen_items_altered == null)
......@@ -878,14 +889,11 @@ public class DataCollection {
// fired at once.
//
// DataObject/DataSource/DataView should also "eat" their signals as well, to prevent observers
// from being notified while their collection is frozen.
// from being notified while their collection is frozen, and only fire them when
// internal_collection_thawed is called.
//
// For DataCollection, the signals affected are item_altered, item_metadata_altered, and
// ordering_changed (and their corresponding signals in DataObject).
//
// WARNING: In current implementation, this drops incoming signals from the DataObjects, relying
// on aggregate signals (items_altered rather than item_altered, etc.) to notify observers.
// This should be used selectively, and with caution.
public void freeze_notifications() {
if (notifies_frozen++ == 0)
frozen();
......@@ -912,16 +920,24 @@ public class DataCollection {
// should issue caught notifications.
protected virtual void thawed() {
if (frozen_items_altered != null) {
foreach (DataObject object in frozen_items_altered.keys)
notify_item_altered(object, frozen_items_altered.get(object));
notify_items_altered(frozen_items_altered);
// refs are swapped around due to reentrancy
Gee.Map<DataObject, Alteration> copy = frozen_items_altered;
frozen_items_altered = null;
foreach (DataObject object in copy.keys)
notify_item_altered(object, copy.get(object));
notify_items_altered(copy);
}
if (fire_ordering_changed) {
fire_ordering_changed = false;
notify_ordering_changed();
}
// notify all members as well
int count = get_count();
for (int ctr = 0; ctr < count; ctr++)
get_at(ctr).internal_collection_thawed();
}
}
......@@ -1207,8 +1223,6 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
Gee.Collection<DataSource> added) {
// if source is in holding tank, remove it now and relink to collection
if (holding_tank.contains(container)) {
debug("Adding %s from holding tank in %s", container.to_string(), to_string());
bool removed = holding_tank.remove(container);
assert(removed);
......@@ -1255,6 +1269,8 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
}
private void on_contained_sources_unlinking(Gee.Collection<DataSource> unlinking) {
contained_sources.freeze_notifications();
foreach (DataSource source in unlinking) {
Gee.Collection<ContainerSource>? containers = get_containers_holding_source(source);
if (containers == null || containers.size == 0)
......@@ -1266,9 +1282,13 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
foreach (ContainerSource container in containers)
container.break_link(source);
}
contained_sources.thaw_notifications();
}
private void on_contained_sources_relinked(Gee.Collection<DataSource> relinked) {
contained_sources.freeze_notifications();
foreach (DataSource source in relinked) {
Gee.List<SourceBacklink>? backlinks = source.get_backlinks(backlink_name);
if (backlinks == null || backlinks.size == 0)
......@@ -1283,6 +1303,8 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
backlink.to_string());
}
}
contained_sources.thaw_notifications();
}
private void on_contained_source_destroyed(DataSource source) {
......@@ -1290,9 +1312,6 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
while (iter.next()) {
ContainerSource container = iter.get();
if (!container.has_links()) {
debug("Destroying %s in %s holding tank: no more backlinks", container.to_string(),
to_string());
iter.remove();
container.destroy_orphan(true);
}
......@@ -1311,14 +1330,10 @@ public abstract class ContainerSourceCollection : DatabaseSourceCollection {
// destroyed.
public void evaporate(ContainerSource container) {
if (contained_sources.has_backlink(container.get_backlink())) {
debug("Unlinking %s to %s holding tank", container.to_string(), to_string());
unlink_marked(mark(container));
bool added = holding_tank.add(container);
assert(added);
} else {
debug("Destroying %s in %s", container.to_string(), to_string());
destroy_marked(mark(container), true);
}
}
......@@ -1513,14 +1528,14 @@ public class ViewCollection : DataCollection {
// those changes in this collection
sources.items_added.connect(on_sources_added);
sources.items_removed.connect(on_sources_removed);
sources.item_altered.connect(on_source_altered);
sources.items_altered.connect(on_sources_altered);
}
public void halt_monitoring() {
if (sources != null) {
sources.items_added.disconnect(on_sources_added);
sources.items_removed.disconnect(on_sources_removed);
sources.item_altered.disconnect(on_source_altered);
sources.items_altered.disconnect(on_sources_altered);
}
sources = null;
......@@ -1656,9 +1671,12 @@ public class ViewCollection : DataCollection {
if (!base.add(object))
return false;
((DataView) object).internal_set_visible(true);
add_many_visible((Gee.Collection<DataView>) get_singleton(object));
filter_altered_item(object);
DataView view = (DataView) object;
Gee.Collection<DataView> views = (Gee.Collection<DataView>) get_singleton(view);
view.internal_set_visible(true);
add_many_visible(views);
filter_altered_items(views);
return true;
}
......@@ -1667,15 +1685,11 @@ public class ViewCollection : DataCollection {
ProgressMonitor? monitor = null) {
Gee.Collection<DataObject> return_list = base.add_many(objects, monitor);
foreach (DataObject object in return_list) {
foreach (DataObject object in return_list)
((DataView) object).internal_set_visible(true);
}
add_many_visible((Gee.Collection<DataView>) return_list);
foreach (DataObject object in return_list) {
filter_altered_item(object);
}
filter_altered_items((Gee.Collection<DataView>) return_list);
return return_list;
}
......@@ -1699,28 +1713,48 @@ public class ViewCollection : DataCollection {
remove_marked(marker);
}
private void on_source_altered(DataObject object, Alteration alteration) {
DataSource source = (DataSource) object;
private void on_sources_altered(Gee.Map<DataObject, Alteration> items) {
// let ViewManager decide whether or not to keep, but only add if not already present
// and only remove if already present
bool include = manager.include_in_view(source);
if (include && !has_view_for_source(source)) {
add(manager.create_view(source));
} else if (!include && has_view_for_source(source)) {
Marker marker = mark(get_view_for_source(source));
remove_marked(marker);
} else if (include && has_view_for_source(source)) {
DataView view = get_view_for_source(source);
if (selected.contains(view))
selected.resort_object(view);
Gee.ArrayList<DataView> to_add = null;
Gee.ArrayList<DataView> to_remove = null;
bool ordering_changed = false;
foreach (DataObject object in items.keys) {
DataSource source = (DataSource) object;
Alteration alteration = items.get(object);
if (visible != null && is_visible(view)) {
if (visible.resort_object(view))
notify_ordering_changed();
bool include = manager.include_in_view(source);
if (include && !has_view_for_source(source)) {
if (to_add == null)
to_add = new Gee.ArrayList<DataView>();
to_add.add(manager.create_view(source));
} else if (!include && has_view_for_source(source)) {
if (to_remove == null)
to_remove = new Gee.ArrayList<DataView>();
to_remove.add(get_view_for_source(source));
} else if (include && has_view_for_source(source)) {
DataView view = get_view_for_source(source);
if (selected.contains(view))
selected.resort_object(view, alteration);
if (visible != null && is_visible(view)) {
if (visible.resort_object(view, alteration))
ordering_changed = true;
}
}
}
if (to_add != null)
add_many(to_add);
if (to_remove != null)
remove_marked(mark_many(to_remove));
if (ordering_changed)
notify_ordering_changed();
}
private void on_mirror_contents_added(Gee.Iterable<DataObject> added) {
......@@ -1806,41 +1840,52 @@ public class ViewCollection : DataCollection {
base.notify_items_removed(removed);
}
private void filter_altered_item(DataObject object) {
private void filter_altered_items(Gee.Collection<DataView> views) {
if (filter == null)
return;
DataView view = (DataView) object;
// Can't use the marker system because ViewCollection completely overrides DataCollection
// and hidden items cannot be marked.
if (filter(view)) {
if (!view.is_visible()) {
Gee.ArrayList<DataView> to_show = new Gee.ArrayList<DataView>();
to_show.add(view);
show_items(to_show);
}
} else {
if (view.is_visible()) {
Gee.ArrayList<DataView> to_hide = new Gee.ArrayList<DataView>();
to_hide.add(view);
hide_items(to_hide);
Gee.ArrayList<DataView> to_show = null;
Gee.ArrayList<DataView> to_hide = null;
foreach (DataView view in views) {
if (filter(view)) {
if (!view.is_visible()) {
if (to_show == null)
to_show = new Gee.ArrayList<DataView>();
to_show.add(view);
}
} else {
if (view.is_visible()) {
if (to_hide == null)
to_hide = new Gee.ArrayList<DataView>();
to_hide.add(view);
}
}
}
if (to_show != null)
show_items(to_show);
if (to_hide != null)
hide_items(to_hide);
}
public override void item_altered(DataObject object, Alteration alteration) {
filter_altered_item(object);
public override void items_altered(Gee.Map<DataObject, Alteration> map) {
filter_altered_items(map.keys);
base.item_altered(object, alteration);
base.items_altered(map);
}
public override void set_comparator(Comparator comparator) {
selected.set_comparator(comparator);
public override void set_comparator(Comparator comparator, ComparatorPredicate? predicate) {
selected.set_comparator(comparator, predicate);
if (visible != null)
visible.set_comparator(comparator);
visible.set_comparator(comparator, predicate);
base.set_comparator(comparator);
base.set_comparator(comparator, predicate);
}
public override void reset_comparator() {
......
......@@ -51,7 +51,7 @@ public class Alteration {
assert(pairs.length >= 1);
foreach (string pair in pairs) {
string[] subject_detail = pair.split(":", 1);
string[] subject_detail = pair.split(":", 2);
assert(subject_detail.length == 2);
add_detail(subject_detail[0], subject_detail[1]);
......@@ -136,6 +136,28 @@ public class Alteration {
return false;
}
public string to_string() {
if (subject != null) {
assert(detail != null);
return "%s:%s".printf(subject, detail);
}
assert(map != null);
string str = "";
foreach (string key in map.get_keys()) {
foreach (string value in map.get(key)) {
if (str.length != 0)
str += ", ";
str += "%s:%s".printf(key, value);
}
}
return str;
}
public bool equals(Alteration other) {
// identity
if (this == other)
......@@ -240,6 +262,7 @@ public abstract class DataObject : Object {
private int64 object_id = INVALID_OBJECT_ID;
private DataCollection member_of = null;
private int64 ordinal = DataCollection.INVALID_OBJECT_ORDINAL;
private Alteration frozen_alteration = null;
// This signal is fired when the source of the data is altered in a way that's significant
// to how it's represented in the application. This base signal must be called by child
......@@ -258,6 +281,9 @@ public abstract class DataObject : Object {
if (member_of != null) {
if (!member_of.are_notifications_frozen())
altered(alteration);
else
frozen_alteration = (frozen_alteration == null) ? alteration
: frozen_alteration.compress(alteration);
} else {
altered(alteration);
}
......@@ -327,6 +353,21 @@ public abstract class DataObject : Object {
return ordinal;
}
// This method is only called by DataCollection
public virtual void internal_collection_thawed() {
// if captured one or more alterations while frozen, fire them now
if (frozen_alteration == null)
return;
// swap due to possible reentrancy
Alteration copy = frozen_alteration;
frozen_alteration = null;
// don't call notify_altered(), as that will redirect the Alteration to the DataCollection,
// which is already handling this for its observers
altered(copy);
}
public inline int64 get_object_id() {
return object_id;
......@@ -445,6 +486,7 @@ public abstract class DataSource : DataObject {
private bool in_contact = false;
private bool marked_for_destroy = false;
private bool is_destroyed = false;
private Alteration frozen_alteration = null;
// This signal is fired after the DataSource has been unlinked from its SourceCollection.
public virtual signal void unlinked(SourceCollection sources) {
......@@ -470,12 +512,31 @@ public abstract class DataSource : DataObject {
}
public override void notify_altered(Alteration alteration) {
// signal reflection
contact_subscribers_alteration(subscriber_altered, alteration);
// if SourceCollection is frozen, freeze notifications to subscribers as well
if (get_membership() != null && get_membership().are_notifications_frozen()) {
frozen_alteration = (frozen_alteration == null) ? alteration
: frozen_alteration.compress(alteration);
} else {
// signal reflection
contact_subscribers_alteration(subscriber_altered, alteration);
}
// call base class in all cases
base.notify_altered(alteration);
}
public override void internal_collection_thawed() {
if (frozen_alteration != null) {
// swap out due to possible reentrancy
Alteration alteration = frozen_alteration;
frozen_alteration = null;
contact_subscribers_alteration(subscriber_altered, alteration);
}
base.internal_collection_thawed();
}
private void subscriber_altered(DataView view, Alteration alteration) {
view.notify_altered(alteration);
}
......
......@@ -391,7 +391,8 @@ public class DirectoryMonitor : Object {
public virtual signal void directory_discovered(File file, FileInfo info) {
}
// reason is a user-visible string
// reason is a user-visible string. May be called more than once during discovery.
// Discovery always completes with discovery-completed.
public virtual signal void discovery_failed(string reason) {
}
......@@ -772,6 +773,8 @@ public class DirectoryMonitor : Object {
} catch (Error err) {
warning("Unable to retrieve info on %s: %s", dir.get_path(), err.message);
explore_directory_completed(in_discovery);
return;
}
}
......@@ -780,6 +783,8 @@ public class DirectoryMonitor : Object {
if (local_dir_info.get_file_type() != FileType.DIRECTORY) {
notify_discovery_failed(_("Unable to monitor %s: Not a directory").printf(dir.get_path()));
explore_directory_completed(in_discovery);
return;
}
......@@ -824,6 +829,8 @@ public class DirectoryMonitor : Object {
} catch (Error err2) {
warning("Aborted directory traversal of %s: %s", dir.get_path(), err2.message);
explore_directory_completed(in_discovery);
return;
}
......@@ -856,7 +863,12 @@ public class DirectoryMonitor : Object {
}
}
// if last directory, report that discovery is complete
explore_directory_completed(in_discovery);
}
// called whenever exploration of a directory is completed, to know when to signal that
// discovery has ended
private void explore_directory_completed(bool in_discovery) {
if (in_discovery) {
assert(outstanding_exploration_dirs > 0);
if (--outstanding_exploration_dirs == 0)
......
......@@ -63,7 +63,7 @@ public class Event : EventSource, ContainerSource, Proxyable {
}
public override bool include_in_view(DataSource source) {
return ((TransformablePhoto) source).get_event_id().id == event_id.id;
return ((Photo) source).get_event_id().id == event_id.id;
}
public override DataView create_view(DataSource source) {
......@@ -152,7 +152,7 @@ public class Event : EventSource, ContainerSource, Proxyable {
event_photos.add(LibraryPhoto.global.fetch(photo_id));
view = new ViewCollection("ViewCollection for Event %lld".printf(event_id.id));
view.set_comparator(view_comparator);
view.set_comparator(view_comparator, view_comparator_predicate);
view.monitor_source_collection(LibraryPhoto.global, new EventManager(event_id), event_photos);
// need to do this manually here because only want to monitor ViewCollection contents after
......@@ -244,6 +244,10 @@ public class Event : EventSource, ContainerSource, Proxyable {
- ((PhotoView *) b)->get_photo_source().get_exposure_time();
}
private static bool view_comparator_predicate(DataObject object, Alteration alteration) {
return alteration.has_detail("metadata", "exposure-time");
}
private Gee.ArrayList<LibraryPhoto> views_to_photos(Gee.Iterable<DataObject> views) {
Gee.ArrayList<LibraryPhoto> photos = new Gee.ArrayList<LibraryPhoto>();
foreach (DataObject object in views)
......@@ -257,7 +261,7 @@ public class Event : EventSource, ContainerSource, Proxyable {
global.notify_container_contents_added(this, photos);
global.notify_container_contents_altered(this, photos, null);
notify_altered(new Alteration("contents", "added"));
notify_altered(new Alteration.from_list("contents:added, metadata:time"));
}
// Event needs to know whenever a photo is removed from the system to update the event
......@@ -288,7 +292,7 @@ public class Event : EventSource, ContainerSource, Proxyable {
return;
}
notify_altered(new Alteration("contents", "removed"));
notify_altered(new Alteration.from_list("contents:removed, metadata:time"));
}
public override void notify_relinking(SourceCollection sources) {
......
......@@ -151,7 +151,7 @@ public class EventsDirectoryPage : CheckerboardPage {
base(page_name);
// set comparator before monitoring source collection, to prevent a re-sort
get_view().set_comparator(get_event_comparator());
get_view().set_comparator(get_event_comparator(), event_comparator_predicate);
get_view().monitor_source_collection(Event.global, view_manager, initial_events);
init_ui_start("events_directory.ui", "EventsDirectoryActionGroup", create_actions());
......@@ -180,13 +180,17 @@ public class EventsDirectoryPage : CheckerboardPage {
get_view().items_state_changed.disconnect(on_selection_changed);
}
private int64 event_ascending_comparator(void *a, void *b) {
private static int64 event_ascending_comparator(void *a, void *b) {
time_t start_a = ((EventDirectoryItem *) a)->event.get_start_time();
time_t start_b = ((EventDirectoryItem *) b)->event.get_start_time();
return start_a - start_b;
}
protected static bool event_comparator_predicate(DataObject object, Alteration alteration) {
return alteration.has_detail("metadata", "time");
}
private int64 event_descending_comparator(void *a, void *b) {
return event_ascending_comparator(b, a);
}
......@@ -278,7 +282,7 @@ public class EventsDirectoryPage : CheckerboardPage {
}
public void notify_sort_changed() {
get_view().set_comparator(get_event_comparator());
get_view().set_comparator(get_event_comparator(), event_comparator_predicate);
}
private void on_view_menu() {
......
......@@ -344,7 +344,7 @@ public class ImportPage : CheckerboardPage {
get_view().monitor_source_collection(import_sources, new ImportViewManager(this));
// sort by exposure time
get_view().set_comparator(preview_comparator);
get_view().set_comparator(preview_comparator, preview_comparator_predicate);
// monitor selection for UI
get_view().items_state_changed.connect(on_view_changed);
......@@ -432,11 +432,15 @@ public class ImportPage : CheckerboardPage {
return Resources.ICON_SINGLE_PHOTO;
}
private int64 preview_comparator(void *a, void *b) {
private static int64 preview_comparator(void *a, void *b) {
return ((ImportPreview *) a)->get_import_source().get_exposure_time()
- ((ImportPreview *) b)->get_import_source().get_exposure_time();
}
private static bool preview_comparator_predicate(DataObject object, Alteration alteration) {
return alteration.has_detail("metadata", "exposure-time");
}