Commit 0478f27b authored by Jim Nelson's avatar Jim Nelson

#728: Drag selection now works properly at the bottom of a large collection. ...

#728: Drag selection now works properly at the bottom of a large collection.  Related to a signed 16-bit 
overflow bug when GDK window grab is enabled (reported at https://bugzilla.gnome.org/show_bug.cgi?id=599937).  
Also, we discovered that signals must be disconnected manually, so have done that as well in places where 
required.
parent 1afb1900
......@@ -371,6 +371,7 @@ public class BatchImport {
return ImportResult.NOT_A_FILE;
}
#if !NO_DUPE_DETECTION
// duplicate detection: If EXIF data present, look for a match with either EXIF itself
// or the thumbnail
PhotoExif photo_exif = new PhotoExif(file);
......@@ -404,6 +405,7 @@ public class BatchImport {
if (full_md5 != null && PhotoTable.get_instance().has_full_md5(full_md5))
return ImportResult.PHOTO_EXISTS;
}
#endif
File import = file;
......
......@@ -298,6 +298,8 @@ public class CheckerboardLayout : Gtk.DrawingArea {
public CheckerboardLayout(ViewCollection view) {
this.view = view;
clear_drag_select();
reflow_scheduler = new OneShotScheduler(background_reflow);
// set existing items to be part of this layout
......@@ -317,6 +319,23 @@ public class CheckerboardLayout : Gtk.DrawingArea {
modify_bg(Gtk.StateType.NORMAL, AppWindow.BG_COLOR);
}
~CheckerboardLayout() {
view.contents_altered -= on_contents_altered;
view.items_state_changed -= on_items_state_changed;
view.items_visibility_changed -= on_items_visibility_changed;
view.ordering_changed -= on_ordering_changed;
view.item_view_altered -= on_item_view_altered;
view.item_geometry_altered -= on_item_geometry_altered;
view.views_altered -= on_views_altered;
view.geometries_altered -= on_geometries_altered;
if (hadjustment != null && vadjustment != null) {
hadjustment.value_changed -= on_viewport_shifted;
vadjustment.value_changed -= on_viewport_shifted;
parent.size_allocate -= on_viewport_resized;
}
}
public void set_adjustments(Gtk.Adjustment hadjustment, Gtk.Adjustment vadjustment) {
this.hadjustment = hadjustment;
this.vadjustment = vadjustment;
......@@ -712,10 +731,16 @@ public class CheckerboardLayout : Gtk.DrawingArea {
return intersection(selection_band);
}
public bool is_drag_select_active() {
return drag_origin.x >= 0 && drag_origin.y >= 0;
}
public void clear_drag_select() {
selection_band = Gdk.Rectangle();
drag_origin = Gdk.Point();
drag_endpoint = Gdk.Point();
drag_origin.x = -1;
drag_origin.y = -1;
drag_endpoint.x = -1;
drag_endpoint.y = -1;
// force a total repaint to clear the selection band
queue_draw();
......
......@@ -43,6 +43,10 @@ public class DataCollection {
owner.items_removed += on_items_removed;
}
~MarkerImpl() {
owner.items_removed -= on_items_removed;
}
public void mark(DataObject object) {
assert(owner.contains(object));
......@@ -492,6 +496,7 @@ public class ViewCollection : DataCollection {
public Gee.ArrayList<DataView> unselected = new Gee.ArrayList<DataView>();
}
private SourceCollection sources = null;
private ViewManager manager = null;
private ViewFilter filter = null;
private SortedList<DataView> selected = new SortedList<DataView>();
......@@ -547,7 +552,17 @@ public class ViewCollection : DataCollection {
visible.resort(get_order_added_comparator());
}
~ViewCollection () {
if (sources != null) {
sources.items_added -= on_sources_added;
sources.items_removed -= on_sources_removed;
sources.item_altered -= on_source_altered;
sources.item_metadata_altered -= on_source_altered;
}
}
public void monitor_source_collection(SourceCollection sources, ViewManager manager) {
this.sources = sources;
this.manager = manager;
// load in all items from the SourceCollection, filtering with the manager
......
......@@ -47,6 +47,11 @@ public class Event : EventSource {
primary_photo.thumbnail_altered += on_primary_thumbnail_altered;
}
~Event() {
if (primary_photo != null)
primary_photo.thumbnail_altered -= on_primary_thumbnail_altered;
}
public static void init() {
event_table = new EventTable();
global = new EventSourceCollection();
......
......@@ -26,7 +26,11 @@ class EventDirectoryItem : LayoutItem {
// monitor the event for changes
event.altered += on_event_altered;
}
~EventDirectoryItem() {
event.altered -= on_event_altered;
}
public override void exposed() {
if (is_exposed())
return;
......
......@@ -307,6 +307,10 @@ public class ImportPage : CheckerboardPage {
show_all();
}
~ImportPage() {
LibraryPhoto.global.contents_altered -= on_photos_added_removed;
}
private Gtk.ToggleActionEntry[] create_toggle_actions() {
Gtk.ToggleActionEntry[] toggle_actions = new Gtk.ToggleActionEntry[0];
......@@ -1097,6 +1101,12 @@ public class ImportQueuePage : SinglePhotoPage {
assert(removed);
assert(!queue.contains(batch_import));
// strip signal handlers
batch_import.starting -= on_starting;
batch_import.imported -= on_imported;
batch_import.import_complete -= on_import_complete;
// report the batch has been removed from the queue
batch_removed(batch_import);
// schedule next if available
......
......@@ -253,6 +253,17 @@ public class LibraryWindow : AppWindow {
sidebar.expand_first_branch_only(events_directory_page.get_marker());
}
~LibraryWindow() {
Event.global.items_added -= on_added_events;
Event.global.items_removed -= on_removed_events;
Event.global.item_altered -= on_event_altered;
CameraTable.get_instance().camera_added -= add_camera_page;
CameraTable.get_instance().camera_removed -= remove_camera_page;
unsubscribe_from_basic_information(get_current_page());
}
private Gtk.ActionEntry[] create_actions() {
Gtk.ActionEntry[] actions = new Gtk.ActionEntry[0];
......@@ -951,11 +962,7 @@ public class LibraryWindow : AppWindow {
new_action.set_active(old_action.get_active());
// old page unsubscribes to these signals (new page subscribes below)
get_current_page().get_view().items_state_changed -= on_selection_changed;
get_current_page().get_view().item_altered -= on_selection_changed;
get_current_page().get_view().item_metadata_altered -= on_selection_changed;
get_current_page().get_view().contents_altered -= on_selection_changed;
get_current_page().get_view().items_visibility_changed -= on_selection_changed;
unsubscribe_from_basic_information(get_current_page());
}
int pos = get_notebook_pos(page);
......@@ -982,11 +989,7 @@ public class LibraryWindow : AppWindow {
page.show_all();
// subscribe to these signals for each event page so basic properties display will update
get_current_page().get_view().items_state_changed += on_selection_changed;
get_current_page().get_view().item_altered += on_selection_changed;
get_current_page().get_view().item_metadata_altered += on_selection_changed;
get_current_page().get_view().contents_altered += on_selection_changed;
get_current_page().get_view().items_visibility_changed += on_selection_changed;
subscribe_for_basic_information(get_current_page());
page.switched_to();
}
......@@ -1060,6 +1063,26 @@ public class LibraryWindow : AppWindow {
// nothing recognized selected
}
}
private void subscribe_for_basic_information(Page page) {
ViewCollection view = page.get_view();
view.items_state_changed += on_selection_changed;
view.item_altered += on_selection_changed;
view.item_metadata_altered += on_selection_changed;
view.contents_altered += on_selection_changed;
view.items_visibility_changed += on_selection_changed;
}
private void unsubscribe_from_basic_information(Page page) {
ViewCollection view = page.get_view();
view.items_state_changed -= on_selection_changed;
view.item_altered -= on_selection_changed;
view.item_metadata_altered -= on_selection_changed;
view.contents_altered -= on_selection_changed;
view.items_visibility_changed -= on_selection_changed;
}
private void on_selection_changed() {
basic_properties.update_properties(get_current_page());
......
......@@ -51,13 +51,20 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
private ulong last_configure_ms = 0;
private bool report_move_finished = false;
private bool report_resize_finished = false;
private Gdk.Point last_down = Gdk.Point();
public Page(string page_name) {
this.page_name = page_name;
last_down = { -1, -1 };
set_flags(Gtk.WidgetFlags.CAN_FOCUS);
}
~Page() {
detach_event_source();
}
public string get_page_name() {
return page_name;
}
......@@ -122,6 +129,17 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
event_source.motion_notify_event += on_motion_internal;
}
private void detach_event_source() {
if (event_source == null)
return;
event_source.button_press_event -= on_button_pressed_internal;
event_source.button_release_event -= on_button_released_internal;
event_source.motion_notify_event -= on_motion_internal;
disable_drag_source();
}
public Gtk.Widget? get_event_source() {
return event_source;
}
......@@ -276,6 +294,32 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
return source_drag_failed(context, drag_result);
}
// Use this function rather than GDK or GTK's get_pointer, especially if called during a
// button-down mouse drag (i.e. a window grab).
//
// For more information, see: https://bugzilla.gnome.org/show_bug.cgi?id=599937
public bool get_event_source_pointer(out int x, out int y, out Gdk.ModifierType mask) {
if (event_source == null)
return false;
event_source.window.get_pointer(out x, out y, out mask);
if (last_down.x < 0 || last_down.y < 0)
return true;
// check for bogus values inside a drag which goes outside the window
// caused by (most likely) X windows signed 16-bit int overflow and fixup
// (https://bugzilla.gnome.org/show_bug.cgi?id=599937)
if ((x - last_down.x).abs() >= 0x7FFF)
x += 0xFFFF;
if ((y - last_down.y).abs() >= 0x7FFF)
y += 0xFFFF;
return true;
}
protected virtual bool on_left_click(Gdk.EventButton event) {
return false;
}
......@@ -306,6 +350,10 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
if (event_source != null)
event_source.grab_focus();
// stash location of mouse down for drag fixups
last_down.x = (int) event.x;
last_down.y = (int) event.y;
return on_left_click(event);
case 2:
......@@ -322,6 +370,9 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
private bool on_button_released_internal(Gdk.EventButton event) {
switch (event.button) {
case 1:
// clear when button released, only for drag fixups
last_down = { -1, -1 };
return on_left_released(event);
case 2:
......@@ -472,7 +523,7 @@ public abstract class Page : Gtk.ScrolledWindow, SidebarPage {
int x, y;
Gdk.ModifierType mask;
if (event.is_hint) {
event_source.window.get_pointer(out x, out y, out mask);
get_event_source_pointer(out x, out y, out mask);
} else {
x = (int) event.x;
y = (int) event.y;
......@@ -498,9 +549,6 @@ public abstract class CheckerboardPage : Page {
protected LayoutItem anchor = null;
protected LayoutItem cursor = null;
private LayoutItem highlighted = null;
// for drag selection
private bool drag_select = false;
private bool autoscroll_scheduled = false;
public CheckerboardPage(string page_name) {
......@@ -680,7 +728,12 @@ public abstract class CheckerboardPage : Page {
case Gdk.ModifierType.SHIFT_MASK:
get_view().unselect_all();
if (anchor == null)
anchor = item;
select_between_items(anchor, item);
cursor = item;
break;
......@@ -715,7 +768,6 @@ public abstract class CheckerboardPage : Page {
// Return true to block the DnD handler, false otherwise
if (item == null) {
drag_select = true;
layout.set_drag_select_origin((int) event.x, (int) event.y);
return true;
......@@ -726,8 +778,7 @@ public abstract class CheckerboardPage : Page {
protected override bool on_left_released(Gdk.EventButton event) {
// if drag-selecting, stop here and do nothing else
if (drag_select) {
drag_select = false;
if (layout.is_drag_select_active()) {
layout.clear_drag_select();
anchor = cursor;
......@@ -841,7 +892,7 @@ public abstract class CheckerboardPage : Page {
return false;
// go no further if not drag-selecting
if (!drag_select)
if (!layout.is_drag_select_active())
return false;
// set the new endpoint of the drag selection
......@@ -860,7 +911,7 @@ public abstract class CheckerboardPage : Page {
}
private void updated_selection_band() {
assert(drag_select);
assert(layout.is_drag_select_active());
// get all items inside the selection
Gee.List<LayoutItem>? intersection = layout.items_in_selection_band();
......@@ -891,7 +942,7 @@ public abstract class CheckerboardPage : Page {
}
private bool selection_autoscroll() {
if (!drag_select) {
if (!layout.is_drag_select_active()) {
autoscroll_scheduled = false;
return false;
......@@ -899,10 +950,10 @@ public abstract class CheckerboardPage : Page {
// as the viewport never scrolls horizontally, only interested in vertical
Gtk.Adjustment vadj = get_vadjustment();
int x, y;
Gdk.ModifierType mask;
layout.window.get_pointer(out x, out y, out mask);
get_event_source_pointer(out x, out y, out mask);
int new_value = (int) vadj.get_value();
switch (get_adjustment_relation(vadj, y)) {
......
......@@ -117,6 +117,10 @@ public abstract class EditingHostPage : SinglePhotoPage {
toolbar.insert(next_button, -1);
}
~EditingHostPage() {
sources.item_altered -= on_photo_altered;
}
public override void set_container(Gtk.Window container) {
base.set_container(container);
......
......@@ -33,6 +33,10 @@ public class Thumbnail : LayoutItem {
dim = original_dim.get_scaled(scale, true);
}
~Thumbnail() {
hq_scheduler.cancel();
}
public LibraryPhoto get_photo() {
return (LibraryPhoto) get_source();
}
......@@ -106,7 +110,7 @@ public class Thumbnail : LayoutItem {
private void on_low_quality_fetched(Gdk.Pixbuf? pixbuf, Dimensions dim, Gdk.InterpType interp,
Error? err) {
if (err != null)
error("Unable to fetch low-quality thumbnail for %s (scale: %d): %s", to_string(), scale,
critical("Unable to fetch low-quality thumbnail for %s (scale: %d): %s", to_string(), scale,
err.message);
if (pixbuf != null)
......@@ -118,7 +122,7 @@ public class Thumbnail : LayoutItem {
private void on_high_quality_fetched(Gdk.Pixbuf? pixbuf, Dimensions dim, Gdk.InterpType interp,
Error? err) {
if (err != null)
error("Unable to fetch high-quality thumbnail for %s (scale: %d): %s", to_string(), scale,
critical("Unable to fetch high-quality thumbnail for %s (scale: %d): %s", to_string(), scale,
err.message);
if (pixbuf != null)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment