Add selection-preserving logic to BaseGrid::UpdateMaps and eliminate some unnecessary updates and clears of the maps. Fixes a pile of cases where operations would result in incorrect or no selections, or scrolling to the top of the file.

Originally committed to SVN as r4677.

aegisub/src/base_grid.cpp

@@ -161,6 +161,85 @@ void BaseGrid::ClearMaps() {
 	AnnounceSelectedSetChanged(Selection(), old_selection);
 }
 
+/// @brief Update maps 
+///
+void BaseGrid::UpdateMaps(bool preserve_selected_rows) {
+	BeginBatch();
+	int active_row = line_index_map[active_line];
+
+	std::vector<int> sel_rows;
+	if (preserve_selected_rows) {
+		sel_rows.reserve(selection.size());
+		std::transform(selection.begin(), selection.end(), std::back_inserter(sel_rows),
+			std::bind1st(std::mem_fun(&BaseGrid::GetDialogueIndex), this));
+	}
+
+	index_line_map.clear();
+	line_index_map.clear();
+
+	for (entryIter cur=AssFile::top->Line.begin();cur != AssFile::top->Line.end();cur++) {
+		AssDialogue *curdiag = dynamic_cast<AssDialogue*>(*cur);
+		if (curdiag) {
+			line_index_map[curdiag] = (int)index_line_map.size();
+			index_line_map.push_back(curdiag);
+		}
+	}
+
+	if (preserve_selected_rows) {
+		Selection sel;
+
+		// If the file shrank enough that no selected rows are left, select the
+		// last row
+		if (sel_rows.empty()) {
+			sel_rows.push_back(index_line_map.size() - 1);
+		}
+		else if (sel_rows[0] >= (int)index_line_map.size()) {
+			sel_rows[0] = index_line_map.size() - 1;
+		}
+		for (size_t i = 0; i < sel_rows.size(); i++) {
+			if (sel_rows[i] >= (int)index_line_map.size()) break;
+			sel.insert(index_line_map[sel_rows[i]]);
+		}
+
+		SetSelectedSet(sel);
+	}
+	else {
+		Selection lines;
+		std::copy(index_line_map.begin(), index_line_map.end(), std::inserter(lines, lines.begin()));
+		Selection new_sel;
+		// Remove lines which no longer exist from the selection
+		set_intersection(selection.begin(), selection.end(),
+			lines.begin(), lines.end(),
+			std::inserter(new_sel, new_sel.begin()));
+
+		SetSelectedSet(new_sel);
+	}
+
+	// The active line may have ceased to exist; pick a new one if so
+	if (line_index_map.find(active_line) == line_index_map.end()) {
+		if (active_row < (int)index_line_map.size()) {
+			SetActiveLine(index_line_map[active_row]);
+		}
+		else if (preserve_selected_rows && !selection.empty()) {
+			SetActiveLine(index_line_map[sel_rows[0]]);
+		}
+		else {
+			SetActiveLine(index_line_map.back());
+		}
+	}
+
+	if (selection.empty() && active_line) {
+		Selection sel;
+		sel.insert(active_line);
+		SetSelectedSet(sel);
+	}
+
+	EndBatch();
+
+	Refresh(false);
+}
+
+
 
 
 /// @brief Begin batch 
@@ -858,7 +937,6 @@ void BaseGrid::ScrollTo(int y) {
 /// @brief Adjust scrollbar 
 ///
 void BaseGrid::AdjustScrollbar() {
-	// Variables
 	int w,h,sw,sh;
 	GetClientSize(&w,&h);
 	int drawPerScreen = h/lineHeight;
@@ -866,15 +944,12 @@ void BaseGrid::AdjustScrollbar() {
 	bool barToEnable = drawPerScreen < rows+2;
 	bool barEnabled = scrollBar->IsEnabled();
 
-	// Set yPos
 	yPos = MID(0,yPos,rows - drawPerScreen);
 
-	// Set size
 	scrollBar->Freeze();
 	scrollBar->GetSize(&sw,&sh);
 	scrollBar->SetSize(w-sw,0,sw,h);
 
-	// Set parameters
 	if (barEnabled) {
 		scrollBar->SetScrollbar(yPos,drawPerScreen,rows+2,drawPerScreen-2,true);
 	}
@@ -1043,32 +1118,6 @@ bool BaseGrid::IsDisplayed(AssDialogue *line) {
 	return false;
 }
 
-
-
-/// @brief Update maps 
-///
-void BaseGrid::UpdateMaps() {
-	index_line_map.clear();
-	line_index_map.clear();
-	
-	for (entryIter cur=AssFile::top->Line.begin();cur != AssFile::top->Line.end();cur++) {
-		AssDialogue *curdiag = dynamic_cast<AssDialogue*>(*cur);
-		if (curdiag) {
-			line_index_map[curdiag] = (int)index_line_map.size();
-			index_line_map.push_back(curdiag);
-		}
-	}
-
-	if (line_index_map.find(active_line) == line_index_map.end()) {
-		// this isn't supposed to happen
-		SetActiveLine(0);
-	}
-
-	Refresh(false);
-}
-
-
-
 /// @brief Key press 
 /// @param event 
 /// @return 

aegisub/src/base_grid.h

@@ -169,7 +169,10 @@ public:
 	wxArrayInt GetSelection(bool *continuous=NULL) const;
 
 	void ClearMaps();
-	void UpdateMaps();
+	/// @brief Update the row <-> AssDialogue mappings
+	/// @param preserve_selected_rows Try to keep the same rows selected rather
+	///                               rather than the same lines
+	void UpdateMaps(bool preserve_selected_rows = false);
 	void UpdateStyle();
 
 	int GetRows() const;

aegisub/src/dialog_shift_times.cpp

@@ -312,7 +312,6 @@ void DialogShiftTimes::OnOK(wxCommandEvent &event) {
 	// End dialog
 	grid->ass->Commit(_("shifting"));
 	grid->CommitChanges();
-	grid->UpdateMaps();
 	grid->editBox->Update();
 	EndModal(0);
 }

aegisub/src/dialog_styling_assistant.cpp

@@ -273,8 +273,6 @@ void DialogStyling::OnActivate(wxActivateEvent &event) {
 	// Enable/disable play video/audio buttons
 	PlayVideoButton->Enable(video->IsLoaded());
 	PlayAudioButton->Enable(audio->loaded);
-	// Update grid
-	grid->UpdateMaps();
 	// Fix style list
 	Styles->Set(grid->ass->GetStyles());
 	// Fix line selection

aegisub/src/frame_main_events.cpp

@@ -1049,19 +1049,14 @@ void FrameMain::OnAutomationMacro (wxCommandEvent &event) {
 #ifdef WITH_AUTOMATION
 	SubsGrid->BeginBatch();
 	// First get selection data
-	// This much be done before clearing the maps, since selection data are lost during that
 	std::vector<int> selected_lines = SubsGrid->GetAbsoluteSelection();
 	int first_sel = SubsGrid->GetFirstSelRow();
-	// Clear all maps from the subs grid before running the macro
-	// The stuff done by the macro might invalidate some of the iterators held by the grid, which will cause great crashing
-	SubsGrid->ClearMaps();
 	// Run the macro...
 	activeMacroItems[event.GetId()-Menu_Automation_Macro]->Process(SubsGrid->ass, selected_lines, first_sel, this);
 	// Have the grid update its maps, this properly refreshes it to reflect the changed subs
 	SubsGrid->UpdateMaps();
 	SubsGrid->SetSelectionFromAbsolute(selected_lines);
 	SubsGrid->CommitChanges(true, false);
-	SubsGrid->AdjustScrollbar();
 	SubsGrid->EndBatch();
 #endif
 }
@@ -1167,25 +1162,17 @@ void FrameMain::OnShiftToFrame (wxCommandEvent &) {
 /// @brief Undo 
 void FrameMain::OnUndo(wxCommandEvent&) {
 	VideoContext::Get()->Stop();
-	std::vector<int> selected_lines = SubsGrid->GetAbsoluteSelection();
-	int active_line = SubsGrid->GetDialogueIndex(SubsGrid->GetActiveLine());
 	ass->Undo();
 	UpdateTitle();
-	SubsGrid->UpdateMaps();
-	SubsGrid->SetSelectionFromAbsolute(selected_lines);
-	SubsGrid->SetActiveLine(SubsGrid->GetDialogue(active_line));
+	SubsGrid->UpdateMaps(true);
 }
 
 /// @brief Redo 
 void FrameMain::OnRedo(wxCommandEvent&) {
 	VideoContext::Get()->Stop();
-	std::vector<int> selected_lines = SubsGrid->GetAbsoluteSelection();
-	int active_line = SubsGrid->GetDialogueIndex(SubsGrid->GetActiveLine());
 	ass->Redo();
 	UpdateTitle();
-	SubsGrid->UpdateMaps();
-	SubsGrid->SetSelectionFromAbsolute(selected_lines);
-	SubsGrid->SetActiveLine(SubsGrid->GetDialogue(active_line));
+	SubsGrid->UpdateMaps(true);
 }
 
 /// @brief Find 

aegisub/src/hilimod_textctrl.cpp

@@ -42,7 +42,6 @@
 #include "compat.h"
 #include "hilimod_textctrl.h"
 #include "main.h"
-#include "options.h"
 
 
 /// @brief Constructor 

aegisub/src/subs_edit_box.cpp

@@ -333,8 +333,8 @@ void SubsEditBox::UpdateGlobals () {
 	ActorBox->Thaw();
 
 	// Set subs update
+	OnActiveLineChanged(grid->GetActiveLine());
 	TextEdit->SetSelection(0,0);
-	grid->SetActiveLine(grid->GetDialogue(grid->GetFirstSelRow()));
 }
 
 

aegisub/src/subs_grid.cpp

@@ -701,7 +701,6 @@ void SubtitlesGrid::OnRecombine(wxCommandEvent &) {
 	ass->Commit(_("combining"));
 	UpdateMaps();
 	CommitChanges();
-	AdjustScrollbar();
 
 	// Remove now non-existent lines from the selection
 	Selection lines;
@@ -809,44 +808,20 @@ void SubtitlesGrid::OnAudioClip(wxCommandEvent &event) {
 ///
 void SubtitlesGrid::LoadDefault () {
 	ass->LoadDefault();
+	ClearMaps();
 	UpdateMaps();
 
-	assert(!line_iter_map.empty());
 	SetActiveLine(GetDialogue(0));
 	SelectRow(0);
 }
 
-
-
-/// @brief Clear internal data structures
-void SubtitlesGrid::ClearMaps() {
-	line_iter_map.clear();
-	BaseGrid::ClearMaps();
-}
-
-
-
 /// @brief Update internal data structures
-void SubtitlesGrid::UpdateMaps() {
-	BeginBatch();
-
-	line_iter_map.clear();
-	BaseGrid::ClearMaps();
-
-	BaseGrid::UpdateMaps();
-
-	for (entryIter it = ass->Line.begin(); it != ass->Line.end(); ++it) {
-		AssDialogue *dlg = dynamic_cast<AssDialogue*>(*it);
-		if (dlg) line_iter_map.insert(std::pair<AssDialogue*,entryIter>(dlg, it));
-	}
+void SubtitlesGrid::UpdateMaps(bool preserve_selected_rows) {
+	BaseGrid::UpdateMaps(preserve_selected_rows);
 
 	if (editBox) {
 		editBox->UpdateGlobals();
 	}
-
-	// Finish setting layout
-	AdjustScrollbar();
-	EndBatch();
 }
 
 
@@ -862,7 +837,6 @@ void SubtitlesGrid::SwapLines(int n1,int n2) {
 	if (n1 == 0 || n2 == 0) return;
 	
 	std::swap(*dlg1, *dlg2);
-	UpdateMaps();
 
 	ass->Commit(_("swap lines"));
 	CommitChanges();
@@ -878,19 +852,15 @@ void SubtitlesGrid::SwapLines(int n1,int n2) {
 ///
 void SubtitlesGrid::InsertLine(AssDialogue *line,int n,bool after,bool update) {
 	AssDialogue *rel_line = GetDialogue(n + (after?1:0));
-	entryIter pos;
-	if (rel_line) pos = line_iter_map[rel_line];
-	else pos = ass->Line.end();
+	entryIter pos = std::find(ass->Line.begin(), ass->Line.end(), rel_line);
 
 	entryIter newIter = ass->Line.insert(pos,line);
-	line_iter_map[line] = newIter;
 	BaseGrid::UpdateMaps();
 
 	// Update
 	if (update) {
 		ass->Commit(_("line insertion"));
 		CommitChanges();
-		AdjustScrollbar();
 	}
 }
 
@@ -1020,7 +990,6 @@ void SubtitlesGrid::PasteLines(int n,bool pasteOver) {
 		if (inserted > 0) {
 			// Commit
 			UpdateMaps();
-			AdjustScrollbar();
 			ass->Commit(_("paste"));
 			CommitChanges();
 
@@ -1047,16 +1016,20 @@ void SubtitlesGrid::PasteLines(int n,bool pasteOver) {
 /// @param flagModified 
 ///
 void SubtitlesGrid::DeleteLines(wxArrayInt target, bool flagModified) {
-	// Check if it's wiping file
-	int deleted = 0;
-	entryIter before_first = line_iter_map[GetDialogue(0)]; --before_first;
+	entryIter before_first = std::find_if(ass->Line.begin(), ass->Line.end(), cast<AssDialogue*>()); --before_first;
 	int old_active_line_index = GetDialogueIndex(GetActiveLine());
 
-	// Delete lines
-	int size = target.Count();
-	for (int i=0;i<size;i++) {
-		ass->Line.erase(line_iter_map[GetDialogue(target[i])]);
-		deleted++;
+	int row = -1;
+	int deleted = 0;
+	for (entryIter cur = ass->Line.begin(); cur != ass->Line.end();) {
+		if (dynamic_cast<AssDialogue*>(*cur) && ++row == target[deleted]) {
+			cur = ass->Line.erase(cur);
+			++deleted;
+			if (deleted == target.size()) break;
+		}
+		else {
+			++cur;
+		}
 	}
 
 	// Add default line if file was wiped
@@ -1067,21 +1040,12 @@ void SubtitlesGrid::DeleteLines(wxArrayInt target, bool flagModified) {
 		old_active_line_index = 0;
 	}
 
-	// Update
 	UpdateMaps();
-	AdjustScrollbar();
+
 	if (flagModified) {
 		ass->Commit(_("delete"));
 		CommitChanges();
 	}
-
-	if (old_active_line_index >= GetRows()) {
-		old_active_line_index = GetRows() - 1;
-	}
-
-	// Update selected line
-	SelectRow(old_active_line_index);
-	SetActiveLine(GetDialogue(old_active_line_index));
 }
 
 

aegisub/src/subs_grid.h

@@ -94,8 +94,6 @@ private:
 	void OnAudioClip(wxCommandEvent &event);
 	void OnShowColMenu(wxCommandEvent &event);
 
-	std::map<AssDialogue*,entryIter> line_iter_map;
-
 public:
 
 	/// DOCME
@@ -107,8 +105,7 @@ public:
 	void LoadDefault();
 	void CommitChanges(bool force=false,bool videoOnly=false);
 
-	void ClearMaps();
-	void UpdateMaps();
+	void UpdateMaps(bool preserve_selected_rows = false);
 
 	void SetVideoToSubs(bool start);
 	void SetSubsToVideo(bool start);