From c9582fe7cf4619547efdb5451caf87770980f85a Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 5 Mar 2014 08:28:14 -0800 Subject: [PATCH] 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 | 9 +++-- aegisub/src/base_grid.cpp | 52 +++++++-------------------- aegisub/src/base_grid.h | 10 ++---- aegisub/src/selection_controller.h | 2 +- aegisub/src/subs_edit_box.cpp | 2 +- aegisub/src/subs_edit_box.h | 2 +- aegisub/src/visual_tool_drag.cpp | 18 ++++++---- aegisub/src/visual_tool_drag.h | 4 +-- 8 files changed, 36 insertions(+), 63 deletions(-) diff --git a/aegisub/src/audio_timing_dialogue.cpp b/aegisub/src/audio_timing_dialogue.cpp index 433a3f985..7944f2f50 100644 --- a/aegisub/src/audio_timing_dialogue.cpp +++ b/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(); diff --git a/aegisub/src/base_grid.cpp b/aegisub/src/base_grid.cpp index 92c6f0963..5d1091018 100644 --- a/aegisub/src/base_grid.cpp +++ b/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(); } diff --git a/aegisub/src/base_grid.h b/aegisub/src/base_grid.h index 76cbf982a..7468f0511 100644 --- a/aegisub/src/base_grid.h +++ b/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 diff --git a/aegisub/src/selection_controller.h b/aegisub/src/selection_controller.h index d08dc8c7c..af2ec17af 100644 --- a/aegisub/src/selection_controller.h +++ b/aegisub/src/selection_controller.h @@ -65,7 +65,7 @@ public: protected: agi::signal::Signal AnnounceActiveLineChanged; - agi::signal::Signal AnnounceSelectedSetChanged; + agi::signal::Signal<> AnnounceSelectedSetChanged; public: /// Virtual destructor for safety diff --git a/aegisub/src/subs_edit_box.cpp b/aegisub/src/subs_edit_box.cpp index 39d59f0cc..55a40b4c4 100644 --- a/aegisub/src/subs_edit_box.cpp +++ b/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(); } diff --git a/aegisub/src/subs_edit_box.h b/aegisub/src/subs_edit_box.h index a75001b24..a0e3d2f77 100644 --- a/aegisub/src/subs_edit_box.h +++ b/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); diff --git a/aegisub/src/visual_tool_drag.cpp b/aegisub/src/visual_tool_drag.cpp index 3a1e6385e..4f8c75e84 100644 --- a/aegisub/src/visual_tool_drag.cpp +++ b/aegisub/src/visual_tool_drag.cpp @@ -36,6 +36,7 @@ #include #include +#include #include #include @@ -49,8 +50,9 @@ static const DraggableFeatureType DRAG_END = DRAG_BIG_CIRCLE; VisualToolDrag::VisualToolDrag(VideoDisplay *parent, agi::Context *context) : VisualTool(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 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 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()); diff --git a/aegisub/src/visual_tool_drag.h b/aegisub/src/visual_tool_drag.h index ce1e604ab..a8cfe8af9 100644 --- a/aegisub/src/visual_tool_drag.h +++ b/aegisub/src/visual_tool_drag.h @@ -45,7 +45,7 @@ class VisualToolDrag : public VisualTool { /// longer exists Feature *primary = nullptr; /// The last announced selection set - SubtitleSelection selection; + std::vector 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 { 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;