Remove the added/removed items arguments from the selection changed signal

They're only used by one thing (the drag visual tool), so calculating
them is pointless overhead most of the time, and it simplifies the
visual tool code much less than it complicates the grid code.

aegisub/src/audio_timing_dialogue.cpp

@@ -373,9 +373,8 @@ class AudioTimingControllerDialogue : public AudioTimingController {
 	/// @param user_triggered Is this a user-initiated commit or an autocommit
 	void DoCommit(bool user_triggered);
 
-	// SubtitleSelectionListener interface
-	void OnActiveLineChanged(AssDialogue *new_line);
-	void OnSelectedSetChanged(const SubtitleSelection &lines_added, const SubtitleSelection &lines_removed);
+	void OnActiveLineChanged();
+	void OnSelectedSetChanged();
 
 	// AssFile events
 	void OnFileChanged(int type);
@@ -457,12 +456,12 @@ void AudioTimingControllerDialogue::GetMarkers(const TimeRange &range, AudioMark
 	video_position_provider.GetMarkers(range, out_markers);
 }
 
-void AudioTimingControllerDialogue::OnActiveLineChanged(AssDialogue *new_line)
+void AudioTimingControllerDialogue::OnActiveLineChanged()
 {
 	Revert();
 }
 
-void AudioTimingControllerDialogue::OnSelectedSetChanged(SubtitleSelection const&, SubtitleSelection const&)
+void AudioTimingControllerDialogue::OnSelectedSetChanged()
 {
 	RegenerateSelectedLines();
 	RegenerateInactiveLines();

aegisub/src/base_grid.cpp

@@ -249,15 +249,13 @@ void BaseGrid::UpdateStyle() {
 }
 
 void BaseGrid::ClearMaps() {
-	Selection old_selection(selection);
-
 	index_line_map.clear();
 	line_index_map.clear();
 	selection.clear();
 	yPos = 0;
 	AdjustScrollbar();
 
-	AnnounceSelectedSetChanged(Selection(), old_selection);
+	AnnounceSelectedSetChanged();
 }
 
 void BaseGrid::UpdateMaps() {
@@ -307,10 +305,9 @@ void BaseGrid::EndBatch() {
 		if (batch_active_line_changed)
 			AnnounceActiveLineChanged(active_line);
 		batch_active_line_changed = false;
-		if (!batch_selection_added.empty() || !batch_selection_removed.empty())
-			AnnounceSelectedSetChanged(batch_selection_added, batch_selection_removed);
-		batch_selection_added.clear();
-		batch_selection_removed.clear();
+		if (batch_selection_changed)
+			AnnounceSelectedSetChanged();
+		batch_selection_changed = false;
 	}
 
 	AdjustScrollbar();
@@ -339,19 +336,11 @@ void BaseGrid::SelectRow(int row, bool addToSelected, bool select) {
 
 	if (select && selection.find(line) == selection.end()) {
 		selection.insert(line);
-
-		Selection added;
-		added.insert(line);
-
-		AnnounceSelectedSetChanged(added, Selection());
+		AnnounceSelectedSetChanged();
 	}
 	else if (!select && selection.find(line) != selection.end()) {
 		selection.erase(line);
-
-		Selection removed;
-		removed.insert(line);
-
-		AnnounceSelectedSetChanged(Selection(), removed);
+		AnnounceSelectedSetChanged();
 	}
 
 	int w = GetClientSize().GetWidth();
@@ -976,11 +965,8 @@ void BaseGrid::SetByFrame(bool state) {
 }
 
 void BaseGrid::SetSelectedSet(const Selection &new_selection) {
-	Selection inserted, removed;
-	set_difference(new_selection, selection, inserted);
-	set_difference(selection, new_selection, removed);
 	selection = new_selection;
-	AnnounceSelectedSetChanged(inserted, removed);
+	AnnounceSelectedSetChanged();
 	Refresh(false);
 }
 
@@ -1023,23 +1009,9 @@ void BaseGrid::AnnounceActiveLineChanged(AssDialogue *new_line) {
 		SubtitleSelectionController::AnnounceActiveLineChanged(new_line);
 }
 
-void BaseGrid::AnnounceSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed) {
-	if (batch_level > 0) {
-		// Remove all previously added lines that are now removed
-		Selection temp;
-		set_difference(batch_selection_added, lines_removed, temp);
-		std::swap(temp, batch_selection_added);
-		temp.clear();
-
-		// Remove all previously removed lines that are now added
-		set_difference(batch_selection_removed, lines_added, temp);
-		std::swap(temp, batch_selection_removed);
-
-		// Add new stuff to batch sets
-		batch_selection_added.insert(lines_added.begin(), lines_added.end());
-		batch_selection_removed.insert(lines_removed.begin(), lines_removed.end());
-	}
-	else {
-		SubtitleSelectionController::AnnounceSelectedSetChanged(lines_added, lines_removed);
-	}
+void BaseGrid::AnnounceSelectedSetChanged() {
+	if (batch_level > 0)
+		batch_selection_changed = true;
+	else
+		SubtitleSelectionController::AnnounceSelectedSetChanged();
 }

aegisub/src/base_grid.h

@@ -71,12 +71,8 @@ class BaseGrid : public wxWindow, public SubtitleSelectionController {
 	int batch_level = 0;
 	/// Has the active line been changed in the current batch?
 	bool batch_active_line_changed = false;
-	/// Lines which will be added to the selection when the current batch is
-	/// completed; should be disjoint from selection
-	Selection batch_selection_added;
-	/// Lines which will be removed from the selection when the current batch
-	/// is completed; should be a subset of selection
-	Selection batch_selection_removed;
+	/// Has the selection been changed in the current batch?
+	bool batch_selection_changed = false;
 
 	/// Connection for video seek event. Stored explicitly so that it can be
 	/// blocked if the relevant option is disabled
@@ -121,7 +117,7 @@ class BaseGrid : public wxWindow, public SubtitleSelectionController {
 
 	// Re-implement functions from BaseSelectionController to add batching
 	void AnnounceActiveLineChanged(AssDialogue *new_line);
-	void AnnounceSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed);
+	void AnnounceSelectedSetChanged();
 
 protected:
 	agi::Context *context; ///< Current project context

aegisub/src/selection_controller.h

@@ -65,7 +65,7 @@ public:
 
 protected:
 	agi::signal::Signal<ItemDataType> AnnounceActiveLineChanged;
-	agi::signal::Signal<Selection const&, Selection const&> AnnounceSelectedSetChanged;
+	agi::signal::Signal<> AnnounceSelectedSetChanged;
 
 public:
 	/// Virtual destructor for safety

aegisub/src/subs_edit_box.cpp

@@ -378,7 +378,7 @@ void SubsEditBox::OnActiveLineChanged(AssDialogue *new_line) {
 	}
 }
 
-void SubsEditBox::OnSelectedSetChanged(const SubtitleSelection &, const SubtitleSelection &) {
+void SubsEditBox::OnSelectedSetChanged() {
 	sel = c->selectionController->GetSelectedSet();
 	initial_times.clear();
 }

aegisub/src/subs_edit_box.h

@@ -148,7 +148,7 @@ class SubsEditBox : public wxPanel {
 	void OnKeyDown(wxKeyEvent &event);
 
 	void OnActiveLineChanged(AssDialogue *new_line);
-	void OnSelectedSetChanged(const SubtitleSelection &, const SubtitleSelection &);
+	void OnSelectedSetChanged();
 	void OnLineInitialTextChanged(std::string const& new_text);
 
 	void OnFrameTimeRadio(wxCommandEvent &event);

aegisub/src/visual_tool_drag.cpp

@@ -36,6 +36,7 @@
 
 #include <algorithm>
 #include <boost/format.hpp>
+#include <boost/range/algorithm/binary_search.hpp>
 #include <functional>
 
 #include <wx/toolbar.h>
@@ -49,8 +50,9 @@ static const DraggableFeatureType DRAG_END = DRAG_BIG_CIRCLE;
 VisualToolDrag::VisualToolDrag(VideoDisplay *parent, agi::Context *context)
 : VisualTool<VisualToolDragDraggableFeature>(parent, context)
 {
-	c->selectionController->GetSelectedSet(selection);
 	connections.push_back(c->selectionController->AddSelectionListener(&VisualToolDrag::OnSelectedSetChanged, this));
+	auto const& sel_set = c->selectionController->GetSelectedSet();
+	selection.insert(begin(selection), begin(sel_set), end(sel_set));
 }
 
 void VisualToolDrag::SetToolbar(wxToolBar *tb) {
@@ -155,17 +157,20 @@ template<class C, class T> static bool line_not_present(C const& set, T const& i
 	});
 }
 
-void VisualToolDrag::OnSelectedSetChanged(const SubtitleSelection &added, const SubtitleSelection &removed) {
-	c->selectionController->GetSelectedSet(selection);
+void VisualToolDrag::OnSelectedSetChanged() {
+	auto const& new_sel_set = c->selectionController->GetSelectedSet();
+	std::vector<AssDialogue *> new_sel(begin(new_sel_set), end(new_sel_set));
 
 	bool any_changed = false;
 	for (auto it = features.begin(); it != features.end(); ) {
-		if (removed.count(it->line)) {
+		bool was_selected = boost::binary_search(selection, it->line);
+		bool is_selected = boost::binary_search(new_sel, it->line);
+		if (was_selected && !is_selected) {
 			sel_features.erase(&*it++);
 			any_changed = true;
 		}
 		else {
-			if (added.count(it->line) && it->type == DRAG_START && line_not_present(sel_features, it)) {
+			if (is_selected && !was_selected && it->type == DRAG_START && line_not_present(sel_features, it)) {
 				sel_features.insert(&*it);
 				any_changed = true;
 			}
@@ -175,6 +180,7 @@ void VisualToolDrag::OnSelectedSetChanged(const SubtitleSelection &added, const
 
 	if (any_changed)
 		parent->Render();
+	selection = std::move(new_sel);
 }
 
 void VisualToolDrag::Draw() {
@@ -232,7 +238,7 @@ void VisualToolDrag::MakeFeatures(AssDialogue *diag, feature_list::iterator pos)
 	feat->type = DRAG_START;
 	feat->line = diag;
 
-	if (selection.count(diag))
+	if (boost::binary_search(selection, diag))
 		sel_features.insert(feat.get());
 	features.insert(pos, *feat.release());
 

aegisub/src/visual_tool_drag.h

@@ -45,7 +45,7 @@ class VisualToolDrag : public VisualTool<VisualToolDragDraggableFeature> {
 	/// longer exists
 	Feature *primary = nullptr;
 	/// The last announced selection set
-	SubtitleSelection selection;
+	std::vector<AssDialogue *> selection;
 
 	/// When the button is pressed, will it convert the line to a move (vs. from
 	/// move to pos)? Used to avoid changing the button's icon unnecessarily
@@ -57,7 +57,7 @@ class VisualToolDrag : public VisualTool<VisualToolDragDraggableFeature> {
 	void MakeFeatures(AssDialogue *diag, feature_list::iterator pos);
 	void MakeFeatures(AssDialogue *diag);
 
-	void OnSelectedSetChanged(SubtitleSelection const& lines_added, SubtitleSelection const& lines_removed);
+	void OnSelectedSetChanged();
 
 	void OnFrameChanged() override;
 	void OnFileChanged() override;