From 70d41d31b2f6e2c9117a36d7368369443c305c98 Mon Sep 17 00:00:00 2001 From: Niels Martin Hansen Date: Sat, 26 Jun 2010 04:38:02 +0000 Subject: [PATCH] Remove the SelectionChangeSubscriber mechanism from the grid and implement some basic selection change notification through SelectionController. Change SelectionListener interface so it receives the set of lines added and removed from selection, instead of just the complete new selection. Update VisualTool<> to use SelectionListener to receive selection change notifications. This change (temporarily, I hope) breaks feature selection in visual drag mode, when changing selection via the grid. This is caused by the grid selection change first clearing the entire selection, which sends a separate notification about selection clear. This causes the last visual feature to be deselected, and then the visual tool base reselects the active line, causing a new notification for selection to be sent. The active line happens to be the newly clicked line, and the selection notification enters during the externalChange guard being set, and is then ignored for feature update purposes. When control returns to the original SelectRow call in the grid, the line to be selected has already been selected and then nothing happens. The best fix is to avoid two notifications being required to deselect all then reselect one line in the first place, so making the grid selection handling saner is the best fix. Originally committed to SVN as r4602. --- aegisub/src/base_grid.cpp | 50 +++++++++++++++++++++++++++--- aegisub/src/base_grid.h | 20 ++++-------- aegisub/src/selection_controller.h | 20 ++++++++---- aegisub/src/visual_tool.cpp | 4 +-- aegisub/src/visual_tool.h | 11 ++++--- aegisub/src/visual_tool_drag.cpp | 27 +++++++--------- aegisub/src/visual_tool_drag.h | 6 ++-- 7 files changed, 90 insertions(+), 48 deletions(-) diff --git a/aegisub/src/base_grid.cpp b/aegisub/src/base_grid.cpp index bebad4b8d..0b48e9e36 100644 --- a/aegisub/src/base_grid.cpp +++ b/aegisub/src/base_grid.cpp @@ -80,7 +80,6 @@ BaseGrid::BaseGrid(wxWindow* parent, wxWindowID id, const wxPoint& pos, const wx holding = false; byFrame = false; lineHeight = 1; // non-zero to avoid div by 0 - selChangeSub = NULL; // Set scrollbar scrollBar = new wxScrollBar(this,GRID_SCROLLBAR,wxDefaultPosition,wxDefaultSize,wxSB_VERTICAL); @@ -142,6 +141,10 @@ void BaseGrid::UpdateStyle() { /// @brief Clears grid /// void BaseGrid::Clear () { + Selection lines_removed; + GetSelectedSet(lines_removed); + AnnounceSelectedSetChanged(Selection(), lines_removed); + diagMap.clear(); diagPtrMap.clear(); selMap.clear(); @@ -209,7 +212,11 @@ void BaseGrid::MakeCellVisible(int row, int col,bool center) { /// void BaseGrid::SelectRow(int row, bool addToSelected, bool select) { if (row < 0 || (size_t)row >= selMap.size()) return; - if (!addToSelected) ClearSelection(); + + if (!addToSelected) { + // Sends change notifications for itself + ClearSelection(); + } if (select != !!selMap[row]) { selMap[row] = select; @@ -224,7 +231,14 @@ void BaseGrid::SelectRow(int row, bool addToSelected, bool select) { RefreshRect(wxRect(0,(row+1-yPos)*lineHeight,w,lineHeight),false); } - if (selChangeSub) selChangeSub->OnSelectionChange(!addToSelected, row, select); + Selection lines_added; + Selection lines_removed; + if (select) + lines_added.insert(diagPtrMap[row]); + else + lines_removed.insert(diagPtrMap[row]); + + AnnounceSelectedSetChanged(lines_added, lines_removed); } } @@ -233,6 +247,9 @@ void BaseGrid::SelectRow(int row, bool addToSelected, bool select) { /// @brief Selects visible lines /// void BaseGrid::SelectVisible() { + Selection lines_removed; + GetSelectedSet(lines_removed); + int rows = GetRows(); bool selectedOne = false; for (int i=0;i::iterator entryIter; -class SelectionChangeSubscriber { -public: - virtual void OnSelectionChange(bool clear, int row, bool selected) = 0; -}; +typedef SelectionController SubtitleSelectionController; +typedef SelectionListener SubtitleSelectionListener; /// DOCME @@ -72,7 +70,7 @@ public: /// @brief DOCME /// /// DOCME -class BaseGrid : public wxWindow, public BaseSelectionController { +class BaseGrid : public wxWindow, public BaseSelectionController { private: /// DOCME @@ -99,8 +97,6 @@ private: /// DOCME wxBitmap *bmp; - SelectionChangeSubscriber* selChangeSub; - void OnPaint(wxPaintEvent &event); void OnSize(wxSizeEvent &event); void OnScroll(wxScrollEvent &event); @@ -133,10 +129,10 @@ protected: public: // SelectionController implementation - virtual void SetActiveLine(AssEntry *new_line) { } - virtual AssEntry * GetActiveLine() const { return 0; } + virtual void SetActiveLine(AssDialogue *new_line) { } + virtual AssDialogue * GetActiveLine() const { return 0; } virtual void SetSelectedSet(const Selection &new_selection) { } - virtual void GetSelectedSet(Selection &selection) const { } + virtual void GetSelectedSet(Selection &selection) const; virtual void NextLine() { } virtual void PrevLine() { } @@ -181,10 +177,6 @@ public: AssDialogue *GetDialogue(int n) const; - void RegisterSelectionChange(SelectionChangeSubscriber* sel) { - selChangeSub = sel; - } - 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/selection_controller.h b/aegisub/src/selection_controller.h index 39ac82987..dffdd18ce 100644 --- a/aegisub/src/selection_controller.h +++ b/aegisub/src/selection_controller.h @@ -47,13 +47,21 @@ public: /// Virtual destructor for safety virtual ~SelectionListener() { } - /// @brief Called when the active subtitle line changes - /// @param new_line The subtitle line that is now the active line + /// @brief Called when the active line changes + /// @param new_line The line that is now the active line + /// + /// In case new_line is 0, the active line was changed to none. virtual void OnActiveLineChanged(ItemDataType *new_line) = 0; /// @brief Called when the selected set changes - /// @param new_selection The subtitle line set that is now the selected set - virtual void OnSelectedSetChanged(const Selection &new_selection) = 0; + /// @param lines_added Lines added to the selection + /// @param lines_removed Lines removed from the selection + /// + /// Conceptually, removal happens before addition, for the purpose of this change notification. + /// + /// In case the two sets overlap, the intersection are lines that were previously selected + /// and remain selected. + virtual void OnSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed) = 0; }; @@ -165,11 +173,11 @@ protected: } /// Call OnSelectedSetChangedon all listeners - void AnnounceSelectedSetChanged(const Selection &new_selection) + void AnnounceSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed) { for (SelectionListenerSet::iterator listener = listeners.begin(); listener != listeners.end(); ++listener) { - (*listener)->OnSelectedSetChanged(new_selection); + (*listener)->OnSelectedSetChanged(lines_added, lines_removed); } } diff --git a/aegisub/src/visual_tool.cpp b/aegisub/src/visual_tool.cpp index 7c807ce1b..2c51ebbaa 100644 --- a/aegisub/src/visual_tool.cpp +++ b/aegisub/src/visual_tool.cpp @@ -92,7 +92,7 @@ VisualTool::VisualTool(VideoDisplay *parent, VideoState const& vide { if (VideoContext::Get()->IsLoaded()) { frame_n = VideoContext::Get()->GetFrameN(); - VideoContext::Get()->grid->RegisterSelectionChange(this); + VideoContext::Get()->grid->AddSelectionListener(this); } PopulateFeatureList(); @@ -100,7 +100,7 @@ VisualTool::VisualTool(VideoDisplay *parent, VideoState const& vide template VisualTool::~VisualTool() { - VideoContext::Get()->grid->RegisterSelectionChange(NULL); + VideoContext::Get()->grid->RemoveSelectionListener(this); } template diff --git a/aegisub/src/visual_tool.h b/aegisub/src/visual_tool.h index 4684c9fcf..5a783324b 100644 --- a/aegisub/src/visual_tool.h +++ b/aegisub/src/visual_tool.h @@ -79,7 +79,7 @@ public: /// @brief DOCME /// DOCME template -class VisualTool : public IVisualTool, public SelectionChangeSubscriber { +class VisualTool : public IVisualTool, protected SubtitleSelectionListener { private: agi::OptionValue* realtime; /// Realtime updating option int dragStartX; /// Starting x coordinate of the current drag, if any @@ -187,6 +187,12 @@ protected: typedef typename std::vector::iterator feature_iterator; typedef typename std::vector::const_iterator feature_const_iterator; +protected: + // SubtitleSelectionListener implementation + // (overridden by deriving classes too) + virtual void OnActiveLineChanged(AssDialogue *new_line) { } + virtual void OnSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed) { } + public: /// @brief Handler for all mouse events /// @param event Shockingly enough, the mouse event @@ -202,9 +208,6 @@ public: /// @brief Called by stuff when there's stuff void Refresh(); - /// Called by the grid when the selection changes - virtual void OnSelectionChange(bool, int, bool) { } - /// @brief Constructor /// @param parent The VideoDisplay to use for coordinate conversion /// @param video Video and mouse information passing blob diff --git a/aegisub/src/visual_tool_drag.cpp b/aegisub/src/visual_tool_drag.cpp index 16c8356c6..c66eff395 100644 --- a/aegisub/src/visual_tool_drag.cpp +++ b/aegisub/src/visual_tool_drag.cpp @@ -121,27 +121,24 @@ void VisualToolDrag::DoRefresh() { UpdateToggleButtons(); } -void VisualToolDrag::OnSelectionChange(bool clear, int row, bool selected) { +void VisualToolDrag::OnSelectedSetChanged(const Selection &added, const Selection &removed) { if (!externalChange) return; externalChange = false; - if (clear) { - ClearSelection(false); - } - if (selected) { - for (size_t i = 0; i < features.size(); i++) { - if (features[i].lineN == row && features[i].type == DRAG_START) { - AddSelection(i); - break; - } + + // Remove all deselected lines + for (size_t i = 0; i < features.size(); i++) { + if (removed.find(features[i].line) != removed.end()) { + RemoveSelection(i); } } - else { - for (size_t i = 0; i < features.size(); i++) { - if (features[i].lineN == row) { - RemoveSelection(i); - } + + // And add all newly selected lines + for (size_t i = 0; i < features.size(); i++) { + if (added.find(features[i].line) != added.end() && features[i].type == DRAG_START) { + AddSelection(i); } } + externalChange = true; } diff --git a/aegisub/src/visual_tool_drag.h b/aegisub/src/visual_tool_drag.h index dde2e3289..f082a8705 100644 --- a/aegisub/src/visual_tool_drag.h +++ b/aegisub/src/visual_tool_drag.h @@ -82,11 +82,13 @@ private: void UpdateToggleButtons(); void DoRefresh(); +protected: + // Overriding SubtitleSelectionListener inherited from base VisualTool<> + virtual void OnSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed); + public: VisualToolDrag(VideoDisplay *parent, VideoState const& video, wxToolBar *toolbar); - void OnSelectionChange(bool clear, int row, bool selected); - void Draw(); bool Update(); void OnSubTool(wxCommandEvent &event);