From c4a27da4fc9b2d416229626670b634f718920467 Mon Sep 17 00:00:00 2001 From: Niels Martin Hansen Date: Sat, 26 Jun 2010 07:26:27 +0000 Subject: [PATCH] Change the grid to use more sensible maps to keep track of index/object mappings for subtitle lines, and follow the SelectionController model. (Hopefully the new maps can also give slightly better performance.) Try to keep the original API intact. Some redundant or unused functions in SubtitleGrid were removed. The LoadFromAss() function was renamed to UpdateMaps() since that was its real purpose. (Note that SubtitleGrid now has an UpdateMaps() function that overshadows BaseGrid::UpdateMaps(), but SubtitleGrid::UpdateMaps() calls the hidden function. They are not virtual.) This does not yet fix visual tool feature selection. Generally, things just feel more broken still. Further work will fix things. Originally committed to SVN as r4603. --- aegisub/src/base_grid.cpp | 190 +++++++++++------------ aegisub/src/base_grid.h | 18 +-- aegisub/src/dialog_styling_assistant.cpp | 3 +- aegisub/src/frame_main.cpp | 12 +- aegisub/src/frame_main_events.cpp | 6 +- aegisub/src/subs_grid.cpp | 178 +++++++-------------- aegisub/src/subs_grid.h | 12 +- 7 files changed, 173 insertions(+), 246 deletions(-) diff --git a/aegisub/src/base_grid.cpp b/aegisub/src/base_grid.cpp index 0b48e9e36..b861d86a2 100644 --- a/aegisub/src/base_grid.cpp +++ b/aegisub/src/base_grid.cpp @@ -100,6 +100,7 @@ BaseGrid::BaseGrid(wxWindow* parent, wxWindowID id, const wxPoint& pos, const wx /// @brief Destructor /// BaseGrid::~BaseGrid() { + ClearMaps(); delete bmp; } @@ -140,16 +141,16 @@ void BaseGrid::UpdateStyle() { /// @brief Clears grid /// -void BaseGrid::Clear () { - Selection lines_removed; - GetSelectedSet(lines_removed); - AnnounceSelectedSetChanged(Selection(), lines_removed); +void BaseGrid::ClearMaps() { + Selection old_selection(selection); - diagMap.clear(); - diagPtrMap.clear(); - selMap.clear(); + index_line_map.clear(); + line_index_map.clear(); + selection.clear(); yPos = 0; AdjustScrollbar(); + + AnnounceSelectedSetChanged(Selection(), old_selection); } @@ -211,34 +212,33 @@ void BaseGrid::MakeCellVisible(int row, int col,bool center) { /// @param select /// void BaseGrid::SelectRow(int row, bool addToSelected, bool select) { - if (row < 0 || (size_t)row >= selMap.size()) return; + if (row < 0 || (size_t)row >= index_line_map.size()) return; + + AssDialogue *line = index_line_map[row]; if (!addToSelected) { - // Sends change notifications for itself - ClearSelection(); + Selection old_selection(selection); + selection.clear(); + selection.insert(line); + AnnounceSelectedSetChanged(selection, old_selection); } - if (select != !!selMap[row]) { - selMap[row] = select; - - if (!addToSelected) { - Refresh(false); - } - else { - int w = 0; - int h = 0; - GetClientSize(&w,&h); - RefreshRect(wxRect(0,(row+1-yPos)*lineHeight,w,lineHeight),false); - } + else if (select && selection.find(line) == selection.end()) { + selection.insert(line); - Selection lines_added; - Selection lines_removed; - if (select) - lines_added.insert(diagPtrMap[row]); - else - lines_removed.insert(diagPtrMap[row]); + Selection added; + added.insert(line); - AnnounceSelectedSetChanged(lines_added, lines_removed); + AnnounceSelectedSetChanged(added, Selection()); + } + + else if (!select && selection.find(line) != selection.end()) { + selection.erase(line); + + Selection removed; + removed.insert(line); + + AnnounceSelectedSetChanged(Selection(), removed); } } @@ -276,16 +276,11 @@ void BaseGrid::SelectVisible() { /// @brief Unselects all cells /// void BaseGrid::ClearSelection() { - Selection lines_removed; - GetSelectedSet(lines_removed); + Selection old_selection(selection.begin(), selection.end()); - int rows = selMap.size(); - for (int i=0;i= selMap.size() || row < 0) return false; - return !!selMap[row]; + if ((size_t)row >= index_line_map.size() || row < 0) return false; + + return selection.find(index_line_map[row]) != selection.end(); } @@ -306,18 +302,27 @@ bool BaseGrid::IsInSelection(int row, int) const { /// @return /// int BaseGrid::GetNumberSelection() const { - return std::count(selMap.begin(), selMap.end(), 1); + return selection.size(); } -/// @brief Gets first selected row -/// @return +/// @brief Gets first selected row index +/// @return Row index of first selected row, -1 if no selection /// int BaseGrid::GetFirstSelRow() const { - std::vector::const_iterator first = std::find(selMap.begin(), selMap.end(), 1); - if (first == selMap.end()) return -1; - return std::distance(selMap.begin(), first); + Selection::const_iterator it = selection.begin(); + + if (it == selection.end()) return -1; + + int index = GetDialogueIndex(*it); + + for (; it != selection.end(); ++it) { + int other_index = GetDialogueIndex(*it); + if (other_index < index) index = other_index; + } + + return index; } @@ -340,24 +345,25 @@ int BaseGrid::GetLastSelRow() const { /// @return Array with indices of selected lines /// wxArrayInt BaseGrid::GetSelection(bool *cont) const { - // Prepare - int nrows = GetRows(); int last = -1; bool continuous = true; - wxArrayInt selections; - // Scan - for (int i=0;i sel_row_indices; + + for (Selection::const_iterator it = selection.begin(); it != selection.end(); ++it) { + sel_row_indices.insert(GetDialogueIndex(*it)); + } + + // Iterating the int set yields a sorted list + wxArrayInt res; + for (std::set::iterator it = sel_row_indices.begin(); it != sel_row_indices.end(); ++it) { + res.Add(*it); + if (last != -1 && *it != last+1) continuous = false; + last = *it; } - // Return if (cont) *cont = continuous; - return selections; + return res; } @@ -366,7 +372,7 @@ wxArrayInt BaseGrid::GetSelection(bool *cont) const { /// @return /// int BaseGrid::GetRows() const { - return diagMap.size(); + return index_line_map.size(); } @@ -981,18 +987,24 @@ void BaseGrid::SetColumnWidths() { -/// @brief Gets dialogue from map -/// @param n -/// @return +/// @brief Get dialogue by index +/// @param n Index to look up +/// @return Subtitle dialogue line for index, or 0 if invalid index /// AssDialogue *BaseGrid::GetDialogue(int n) const { - try { - if (n < 0 || (size_t)n >= diagMap.size()) return NULL; - return dynamic_cast(*diagMap.at(n)); - } - catch (...) { - return NULL; - } + if (n < 0 || n >= (int)index_line_map.size()) return 0; + return index_line_map[n]; +} + + + +/// @brief Get index by dialogue line +/// @param diag Dialogue line to look up +/// @return Subtitle index for object, or -1 if unknown subtitle +int BaseGrid::GetDialogueIndex(AssDialogue *diag) const { + std::map::const_iterator it = line_index_map.find(diag); + if (it != line_index_map.end()) return it->second; + else return -1; } @@ -1015,37 +1027,17 @@ bool BaseGrid::IsDisplayed(AssDialogue *line) { /// @brief Update maps /// void BaseGrid::UpdateMaps() { - // Store old - int len = selMap.size(); - std::vector tmpDiagPtrMap(diagPtrMap); - std::vector tmpSelMap(selMap); - - // Clear old - diagPtrMap.clear(); - diagMap.clear(); - selMap.clear(); + index_line_map.clear(); + line_index_map.clear(); - // Re-generate lines for (entryIter cur=AssFile::top->Line.begin();cur != AssFile::top->Line.end();cur++) { AssDialogue *curdiag = dynamic_cast(*cur); if (curdiag) { - // Find old pos - int sel = 0; - for (int i=0;i index_line_map; + std::map line_index_map; + protected: /// DOCME @@ -124,14 +128,11 @@ protected: /// DOCME int yPos; - /// DOCME - std::vector selMap; - public: // SelectionController implementation virtual void SetActiveLine(AssDialogue *new_line) { } virtual AssDialogue * GetActiveLine() const { return 0; } - virtual void SetSelectedSet(const Selection &new_selection) { } + virtual void SetSelectedSet(const Selection &new_selection); virtual void GetSelectedSet(Selection &selection) const; virtual void NextLine() { } virtual void PrevLine() { } @@ -145,12 +146,6 @@ public: /// DOCME bool byFrame; - /// DOCME - std::vector diagMap; - - /// DOCME - std::vector diagPtrMap; - void AdjustScrollbar(); void SetColumnWidths(); void BeginBatch(); @@ -167,7 +162,7 @@ public: void SelectVisible(); wxArrayInt GetSelection(bool *continuous=NULL) const; - void Clear(); + void ClearMaps(); void UpdateMaps(); void UpdateStyle(); @@ -176,6 +171,7 @@ public: void MakeCellVisible(int row, int col,bool center=true); AssDialogue *GetDialogue(int n) const; + int GetDialogueIndex(AssDialogue *diag) const; BaseGrid(wxWindow* parent, wxWindowID id, const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxDefaultSize, long style = wxWANTS_CHARS, const wxString& name = wxPanelNameStr); ~BaseGrid(); diff --git a/aegisub/src/dialog_styling_assistant.cpp b/aegisub/src/dialog_styling_assistant.cpp index 8670006a5..bea4e987e 100644 --- a/aegisub/src/dialog_styling_assistant.cpp +++ b/aegisub/src/dialog_styling_assistant.cpp @@ -274,8 +274,7 @@ void DialogStyling::OnActivate(wxActivateEvent &event) { PlayVideoButton->Enable(video->IsLoaded()); PlayAudioButton->Enable(audio->loaded); // Update grid - if (grid->ass != AssFile::top) - grid->LoadFromAss(AssFile::top,false,true); + grid->UpdateMaps(); // Fix style list Styles->Set(grid->ass->GetStyles()); // Fix line selection diff --git a/aegisub/src/frame_main.cpp b/aegisub/src/frame_main.cpp index 521e459ec..416f2df44 100644 --- a/aegisub/src/frame_main.cpp +++ b/aegisub/src/frame_main.cpp @@ -628,10 +628,11 @@ void FrameMain::InitContents() { void FrameMain::DeInitContents() { if (detachedVideo) detachedVideo->Destroy(); if (stylingAssistant) stylingAssistant->Destroy(); - AssFile::StackReset(); - delete AssFile::top; + SubsGrid->ClearMaps(); delete EditBox; delete videoBox; + AssFile::StackReset(); + delete AssFile::top; HelpButton::ClearPages(); } @@ -698,17 +699,18 @@ void FrameMain::LoadSubtitles (wxString filename,wxString charset) { } // Proceed into loading - SubsGrid->Clear(); + SubsGrid->ClearMaps(); AssFile::StackReset(); if (isFile) { AssFile::top->Load(filename,charset); - SubsGrid->LoadFromAss(AssFile::top,false,true); + SubsGrid->SetSelectedSet(SubtitleSelectionController::Selection()); + SubsGrid->UpdateMaps(); wxFileName fn(filename); StandardPaths::SetPathValue(_T("?script"),fn.GetPath()); OPT_SET("Path/Last/Subtitles")->SetString(STD_STR(fn.GetPath())); } else { - SubsGrid->LoadDefault(AssFile::top); + SubsGrid->LoadDefault(); StandardPaths::SetPathValue(_T("?script"),_T("")); } } diff --git a/aegisub/src/frame_main_events.cpp b/aegisub/src/frame_main_events.cpp index 857845370..f7795cd61 100644 --- a/aegisub/src/frame_main_events.cpp +++ b/aegisub/src/frame_main_events.cpp @@ -1051,7 +1051,7 @@ void FrameMain::OnAutomationMacro (wxCommandEvent &event) { 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->Clear(); + 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 @@ -1183,7 +1183,7 @@ void FrameMain::OnShiftToFrame (wxCommandEvent &) { void FrameMain::OnUndo(wxCommandEvent&) { VideoContext::Get()->Stop(); AssFile::StackPop(); - SubsGrid->LoadFromAss(AssFile::top,true); + SubsGrid->UpdateMaps(); AssFile::Popping = false; } @@ -1191,7 +1191,7 @@ void FrameMain::OnUndo(wxCommandEvent&) { void FrameMain::OnRedo(wxCommandEvent&) { VideoContext::Get()->Stop(); AssFile::StackRedo(); - SubsGrid->LoadFromAss(AssFile::top,true); + SubsGrid->UpdateMaps(); AssFile::Popping = false; } diff --git a/aegisub/src/subs_grid.cpp b/aegisub/src/subs_grid.cpp index bdd6ff90b..60ca2ce40 100644 --- a/aegisub/src/subs_grid.cpp +++ b/aegisub/src/subs_grid.cpp @@ -41,6 +41,7 @@ #ifndef AGI_PRE #include +#include #include #include @@ -121,6 +122,7 @@ SubtitlesGrid::SubtitlesGrid(FrameMain* parentFr, wxWindow *parent, wxWindowID i /// @brief Destructor /// SubtitlesGrid::~SubtitlesGrid() { + ClearMaps(); } @@ -793,94 +795,44 @@ void SubtitlesGrid::OnAudioClip(wxCommandEvent &event) { /// @brief Clears grid and sets it to default /// @param _ass /// -void SubtitlesGrid::LoadDefault (AssFile *_ass) { - if (_ass) { - ass = _ass; - } +void SubtitlesGrid::LoadDefault () { + ass = AssFile::top; ass->LoadDefault(); - LoadFromAss(NULL,false,true); + UpdateMaps(); } -/// @brief Read data from ASS file structure -/// @param _ass -/// @param keepSelection -/// @param dontModify -/// -void SubtitlesGrid::LoadFromAss (AssFile *_ass,bool keepSelection,bool dontModify) { - // Store selected rows - wxArrayInt srows; - if (keepSelection) { - srows = GetSelection(); - } +/// @brief Clear internal data structures +void SubtitlesGrid::ClearMaps() { + line_iter_map.clear(); + BaseGrid::ClearMaps(); +} - // Clear grid + + +/// @brief Update internal data structures +void SubtitlesGrid::UpdateMaps() { BeginBatch(); - int oldPos = yPos; - Clear(); - if (keepSelection) yPos = oldPos; - // Clear from video VideoContext::Get()->curLine = NULL; + line_iter_map.clear(); + BaseGrid::ClearMaps(); - // Get subtitles - if (_ass) ass = _ass; - else { - if (!ass) throw _T("Trying to set subs grid to current ass file, but there is none"); - } + ass = AssFile::top; - // Run through subs adding them - int n = 0; - AssDialogue *curdiag; - ready = false; - for (entryIter cur=ass->Line.begin();cur != ass->Line.end();cur++) { - curdiag = dynamic_cast(*cur); - if (curdiag) { - diagMap.push_back(cur); - diagPtrMap.push_back(curdiag); - selMap.push_back(false); - n++; - } - } - ready = true; + BaseGrid::UpdateMaps(); - // Restore selection - if (keepSelection) { - if (srows.empty() || srows[0] >= (signed)selMap.size()) { - SelectRow(selMap.size() - 1); - } - else { - for (size_t i=0;iLine.begin(); it != ass->Line.end(); ++it) { + AssDialogue *dlg = dynamic_cast(*it); + if (dlg) line_iter_map.insert(std::pair(dlg, it)); } - // Select first - else { - SelectRow(0); - } - - // Commit - if (!AssFile::Popping) { - if (dontModify) AssFile::StackPush(_("load")); - else ass->FlagAsModified(_("load")); - } - CommitChanges(); - // Set edit box if (editBox) { - int nrows = GetRows(); - int firstsel = -1; - for (int i=0;iUpdateGlobals(); - if (_ass) editBox->SetToLine(firstsel); + if (ass) editBox->SetToLine(firstsel); } // Finish setting layout @@ -896,20 +848,13 @@ void SubtitlesGrid::LoadFromAss (AssFile *_ass,bool keepSelection,bool dontModif /// @return /// void SubtitlesGrid::SwapLines(int n1,int n2) { - // Check bounds and get iterators - int rows = GetRows(); - if (n1 < 0 || n2 < 0 || n1 >= rows || n2 >= rows) return; - entryIter src1 = diagMap.at(n1); - entryIter src2 = diagMap.at(n2); + AssDialogue *dlg1 = GetDialogue(n1); + AssDialogue *dlg2 = GetDialogue(n2); + if (n1 == 0 || n2 == 0) return; - // Swaps - iter_swap(src1,src2); + std::swap(*dlg1, *dlg2); + UpdateMaps(); - // Update mapping - diagMap[n1] = src1; - diagPtrMap[n1] = (AssDialogue*) *src1; - diagMap[n2] = src2; - diagPtrMap[n2] = (AssDialogue*) *src2; ass->FlagAsModified(_("swap lines")); CommitChanges(); } @@ -923,20 +868,11 @@ void SubtitlesGrid::SwapLines(int n1,int n2) { /// @param update /// void SubtitlesGrid::InsertLine(AssDialogue *line,int n,bool after,bool update) { - // Check bounds and get iterators - entryIter pos = diagMap.at(n); + AssDialogue *rel_line = GetDialogue(n) + (after?1:0); + entryIter pos = line_iter_map[rel_line]; - // Insert - if (after) { - n++; - pos++; - } entryIter newIter = ass->Line.insert(pos,line); - //InsertRows(n); - //SetRowToLine(n,line); - diagMap.insert(diagMap.begin() + n,newIter); - diagPtrMap.insert(diagPtrMap.begin() + n,(AssDialogue*)(*newIter)); - selMap.insert(selMap.begin() + n,false); + UpdateMaps(); // Update if (update) { @@ -1100,19 +1036,20 @@ void SubtitlesGrid::PasteLines(int n,bool pasteOver) { 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; // Delete lines int size = target.Count(); for (int i=0;iLine.erase(diagMap.at(target[i])); + ass->Line.erase(line_iter_map[GetDialogue(target[i])]); deleted++; } // Add default line if file was wiped if (GetRows() == deleted) { AssDialogue *def = new AssDialogue; - ass->Line.push_back(def); + ++before_first; + ass->Line.insert(before_first, def); } // Update @@ -1124,7 +1061,7 @@ void SubtitlesGrid::DeleteLines(wxArrayInt target, bool flagModified) { } // Update selected line - int newSelected = MID(0, editBox->linen,GetRows() - 1); + int newSelected = ClampSignedInteger32(editBox->linen, 0, GetRows()-1); editBox->SetToLine(newSelected); SelectRow(newSelected); } @@ -1526,25 +1463,20 @@ void SubtitlesGrid::SetVideoToSubs(bool start) { -/// @brief (ie. not as displayed in the grid but as represented in the file) Retrieve a list of selected lines in the actual ASS file +/// @brief Retrieve a list of selected lines in the actual ASS file (ie. not as displayed in the grid but as represented in the file) /// @return /// std::vector SubtitlesGrid::GetAbsoluteSelection() { std::vector result; result.reserve(GetNumberSelection()); - int nrows = GetRows(); - for (int i = 0; i != nrows; ++i) { - if (selMap.at(i)) { - entryIter l = diagMap.at(i); - int n = 0; - for (std::list::iterator j = ass->Line.begin(); j != ass->Line.end(); ++j, ++n) { - if (j == l) { - result.push_back(n); - break; - } - } - } + Selection sel; + GetSelectedSet(sel); + + int line_index = 0; + for (entryIter it = ass->Line.begin(); it != ass->Line.end(); ++it, ++line_index) { + if (sel.find(dynamic_cast(*it)) != sel.end()) + result.push_back(line_index); } return result; @@ -1552,21 +1484,25 @@ std::vector SubtitlesGrid::GetAbsoluteSelection() { -/// @brief selection vector must be sorted Update list of selected lines from absolute selection +/// @brief Update list of selected lines from absolute selection /// @param selection /// void SubtitlesGrid::SetSelectionFromAbsolute(std::vector &selection) { + Selection newsel; - int nrows = GetRows(); + std::sort(selection.begin(), selection.end()); + + int i = 0; std::list::iterator j = ass->Line.begin(); - int index = 0; - for (int i = 0; i != nrows; ++i) { - entryIter l = diagMap.at(i); - while(j != l && j != ass->Line.end()) ++j, ++index; - if(j == l && binary_search(selection.begin(), selection.end(), index)) { - selMap[i] = true; - } else selMap[i] = false; + + for (size_t selveci = 0; selveci < selection.size(); ++selveci) { + while (i != selection[selveci] && j != ass->Line.end()) ++i, ++j; + if (j == ass->Line.end()) break; /// @todo Report error somehow + AssDialogue *dlg = dynamic_cast(*j); + if (dlg) newsel.insert(dlg); } + + SetSelectedSet(newsel); } diff --git a/aegisub/src/subs_grid.h b/aegisub/src/subs_grid.h index 4dd129050..5c8b27c41 100644 --- a/aegisub/src/subs_grid.h +++ b/aegisub/src/subs_grid.h @@ -78,10 +78,6 @@ typedef std::list::iterator entryIter; /// DOCME class SubtitlesGrid: public BaseGrid { private: - - /// DOCME - bool ready; - void OnPopupMenu(bool alternate=false); void OnKeyDown(wxKeyEvent &event); @@ -110,6 +106,8 @@ private: void OnAudioClip(wxCommandEvent &event); void OnShowColMenu(wxCommandEvent &event); + std::map line_iter_map; + public: /// DOCME @@ -118,10 +116,12 @@ public: SubtitlesGrid(FrameMain* parentFrame,wxWindow *parent, wxWindowID id, const wxPoint& pos = wxDefaultPosition, const wxSize& size = wxDefaultSize, long style = wxWANTS_CHARS, const wxString& name = wxPanelNameStr); ~SubtitlesGrid(); - void LoadDefault(AssFile *ass=NULL); - void LoadFromAss(AssFile *ass=NULL,bool keepSelection=false,bool dontModify=false); + void LoadDefault(); void CommitChanges(bool force=false,bool videoOnly=false); + void ClearMaps(); + void UpdateMaps(); + void SetVideoToSubs(bool start); void SetSubsToVideo(bool start);