Fix selection issues with visual features

Selections in drag mode now follow the following rules:

 * If a line is selected in the grid, at least one visual feature
   corresponding to the line is selected.

 * If a line has any features selected, that line is selected in the
   grid.

In addition, all control points now start out selected in the vector
clip tool, and all tools should no longer discard the current selection
at unpredictable or unintended times.

Updates #513.

Originally committed to SVN as r4363.

aegisub/src/base_grid.cpp

@@ -80,6 +80,7 @@ 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);
@@ -206,9 +207,8 @@ void BaseGrid::MakeCellVisible(int row, int col,bool center) {
 /// @param select        
 ///
 void BaseGrid::SelectRow(int row, bool addToSelected, bool select) {
-	if (!addToSelected) ClearSelection();
-
 	if (row < 0 || (size_t)row >= selMap.size()) return;
+	if (!addToSelected) ClearSelection();
 
 	if (select != selMap[row]) {
 		selMap[row] = select;
@@ -222,6 +222,8 @@ void BaseGrid::SelectRow(int row, bool addToSelected, bool select) {
 			GetClientSize(&w,&h);
 			RefreshRect(wxRect(0,(row+1-yPos)*lineHeight,w,lineHeight),false);
 		}
+
+		if (selChangeSub) selChangeSub->OnSelectionChange(!addToSelected, row, select);
 	}
 }
 

aegisub/src/base_grid.h

@@ -35,7 +35,7 @@
 ///
 
 
-
+#pragma once
 
 ////////////
 // Includes
@@ -58,6 +58,11 @@ class FrameMain;
 /// DOCME
 typedef std::list<AssEntry*>::iterator entryIter;
 
+class SelectionChangeSubscriber {
+public:
+	virtual void OnSelectionChange(bool clear, int row, bool selected) = 0;
+};
+
 
 
 /// DOCME
@@ -92,6 +97,8 @@ private:
 	/// DOCME
 	wxBitmap *bmp;
 
+	SelectionChangeSubscriber* selChangeSub;
+
 	void OnPaint(wxPaintEvent &event);
 	void OnSize(wxSizeEvent &event);
 	void OnScroll(wxScrollEvent &event);
@@ -162,6 +169,10 @@ 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();
 

aegisub/src/subs_edit_box.cpp

@@ -261,11 +261,10 @@ void SubsEditBox::SetSplitLineMode(wxSize newSize) {
 
 
 /// @brief Update function 
-/// @param timeOnly If true, only update the time fields
+/// @param timeOnly 
-/// @param weak     ?
+/// @param weak     
-/// @param video    If true, update the video display
 ///
-void SubsEditBox::Update (bool timeOnly,bool weak,bool video) {
+void SubsEditBox::Update (bool timeOnly,bool weak) {
 	if (enabled) {
 		AssDialogue *curdiag = grid->GetDialogue(linen);
 		if (curdiag) {
@@ -301,7 +300,7 @@ void SubsEditBox::Update (bool timeOnly,bool weak,bool video) {
 
 			// Video
 			VideoContext::Get()->curLine = curdiag;
-			if (video) VideoContext::Get()->UpdateDisplays(false);
+			VideoContext::Get()->UpdateDisplays(false);
 
 			TextEdit->EmptyUndoBuffer();
 		}
@@ -353,12 +352,10 @@ void SubsEditBox::SetToLine(int n,bool weak) {
 	// Set to nothing
 	if (n == -1) {
 		enabled = false;
-		SetControlsState(false);
-		return;
 	}
 
 	// Set line
-	if (grid->GetDialogue(n)) {
+	else if (grid->GetDialogue(n)) {
 		enabled = true;
 		if (n != linen) {
 			linen = n;
@@ -369,7 +366,7 @@ void SubsEditBox::SetToLine(int n,bool weak) {
 	}
 
 	// Update controls
-	Update(false, false, false);
+	Update();
 
 	// Set video
 	if (VideoContext::Get()->IsLoaded() && !weak) {

aegisub/src/subs_edit_box.h

@@ -251,7 +251,7 @@ public:
 
 	void SetSplitLineMode(wxSize size=wxSize(-1,-1));
 	void CommitText(bool weak=false);
-	void Update(bool timeOnly=false,bool weak=false,bool video=true);
+	void Update(bool timeOnly=false,bool weak=false);
 	void UpdateGlobals();
 	void SetToLine(int n,bool weak=false);
 	void UpdateFrameTiming();

aegisub/src/video_display.cpp

@@ -530,6 +530,7 @@ double VideoDisplay::GetZoom() const {
 
 template<class T>
 void VideoDisplay::SetTool() {
+	tool.reset();
 	tool.reset(new T(this, video, toolBar));
 	box->Bind(wxEVT_COMMAND_TOOL_CLICKED, &T::OnSubTool, static_cast<T*>(tool.get()), VISUAL_SUB_TOOL_START, VISUAL_SUB_TOOL_END);
 }

aegisub/src/visual_feature.cpp

@@ -45,7 +45,6 @@ VisualDraggableFeature::VisualDraggableFeature()
 , y(INT_MIN)
 , origX(INT_MIN)
 , origY(INT_MIN)
-, selected(false)
 , layer(0)
 , line(NULL)
 , lineN(-1)

aegisub/src/visual_feature.h

@@ -78,8 +78,6 @@ public:
 	int origX; /// x coordindate before the last operation began
 	int origY; /// y coordindate before the last operation began
 
-	bool selected; ///Iis this feature selected?
-
 	int layer; /// Layer; Higher = above
 
 	AssDialogue* line; /// The dialogue line this feature is for

aegisub/src/visual_tool.cpp

@@ -72,12 +72,12 @@ template<class FeatureType>
 VisualTool<FeatureType>::VisualTool(VideoDisplay *parent, VideoState const& video)
 : dragStartX(0)
 , dragStartY(0)
-, externalChange(true)
 , selChanged(false)
 , parent(parent)
 , holding(false)
 , curDiag(NULL)
 , dragging(false)
+, externalChange(true)
 , curFeature(NULL)
 , dragListOK(false)
 , frame_n(0)
@@ -90,6 +90,7 @@ VisualTool<FeatureType>::VisualTool(VideoDisplay *parent, VideoState const& vide
 {
 	if (VideoContext::Get()->IsLoaded()) {
 		frame_n = VideoContext::Get()->GetFrameN();
+		VideoContext::Get()->grid->RegisterSelectionChange(this);
 	}
 
 	PopulateFeatureList();
@@ -97,6 +98,7 @@ VisualTool<FeatureType>::VisualTool(VideoDisplay *parent, VideoState const& vide
 
 template<class FeatureType>
 VisualTool<FeatureType>::~VisualTool() {
+	VideoContext::Get()->grid->RegisterSelectionChange(NULL);
 }
 
 template<class FeatureType>
@@ -125,25 +127,28 @@ void VisualTool<FeatureType>::OnMouseEvent (wxMouseEvent &event) {
 		PopulateFeatureList();
 		dragListOK = true;
 	}
+	if (!dragging) {
+		GetHighlightedFeature();
+	}
 
 	if (dragging) {
 		// continue drag
 		if (event.LeftIsDown()) {
-			for (SelFeatureIter cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
+			for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-				(*cur)->x = (video.x - dragStartX + (*cur)->origX);
+				features[*cur].x = (video.x - dragStartX + features[*cur].origX);
-				(*cur)->y = (video.y - dragStartY + (*cur)->origY);
+				features[*cur].y = (video.y - dragStartY + features[*cur].origY);
 				if (shiftDown) {
 					if (abs(video.x - dragStartX) > abs(video.y - dragStartY)) {
-						(*cur)->y = (*cur)->origY;
+						features[*cur].y = features[*cur].origY;
 					}
 					else {
-						(*cur)->x = (*cur)->origX;
+						features[*cur].x = features[*cur].origX;
 					}
 				}
-				UpdateDrag(*cur);
+				UpdateDrag(&features[*cur]);
 
 				if (realTime) {
-					CommitDrag(*cur);
+					CommitDrag(&features[*cur]);
 				}
 			}
 			if (realTime) Commit();
@@ -160,26 +165,18 @@ void VisualTool<FeatureType>::OnMouseEvent (wxMouseEvent &event) {
 				if (!selChanged) {
 					if (ctrlDown) {
 						// deselect this feature
-						selFeatures.remove(curFeature);
+						RemoveSelection(curFeatureI);
-						curFeature->selected = false;
-						if (curFeature->lineN > -1) {
-							con->grid->SelectRow(curFeature->lineN, true, false);
-						}
 					}
 					else {
 						// deselect everything else
 						ClearSelection();
-						selFeatures.push_back(curFeature);
+						AddSelection(curFeatureI);
-						curFeature->selected = true;
-						if (curFeature->lineN > -1) {
-							con->grid->SelectRow(curFeature->lineN, false);
-						}
 					}
 				}
 			}
 			else {
-				for (SelFeatureIter cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
+				for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-					CommitDrag(*cur);
+					CommitDrag(&features[*cur]);
 				}
 				Commit(true);
 			}
@@ -213,34 +210,28 @@ void VisualTool<FeatureType>::OnMouseEvent (wxMouseEvent &event) {
 		}
 	}
 	else if (leftClick) {
-		curFeature = GetHighlightedFeature();
 		// start drag
 		if (curFeature) {
 			if (InitializeDrag(curFeature)) {
-				if (!curFeature->selected) {
+				if (selFeatures.find(curFeatureI) == selFeatures.end()) {
 					selChanged = true;
 					if (!ctrlDown) {
 						ClearSelection();
 					}
-					selFeatures.push_back(curFeature);
+					AddSelection(curFeatureI);
-					curFeature->selected = true;
-					if (curFeature->lineN != -1) {
-						con->grid->editBox->SetToLine(curFeature->lineN,true);
-						con->grid->SelectRow(curFeature->lineN, ctrlDown);
-					}
 				}
 				else {
 					selChanged = false;
-					if (curFeature->lineN != -1) {
-						con->grid->editBox->SetToLine(curFeature->lineN,true);
 				}
+				if (curFeature->lineN != -1) {
+					SetEditbox(curFeature->lineN);
 				}
 
 				dragStartX = video.x;
 				dragStartY = video.y;
-				for (SelFeatureIter cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
+				for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-					(*cur)->origX = (*cur)->x;
+					features[*cur].origX = features[*cur].x;
-					(*cur)->origY = (*cur)->y;
+					features[*cur].origY = features[*cur].y;
 				}
 
 				dragging = true;
@@ -250,7 +241,10 @@ void VisualTool<FeatureType>::OnMouseEvent (wxMouseEvent &event) {
 		}
 		// start hold
 		else {
-			if (!altDown) ClearSelection();
+			if (!altDown) {
+				ClearSelection();
+				SetEditbox();
+			}
 			curDiag = GetActiveDialogueLine();
 			if (curDiag && InitializeHold()) {
 				holding = true;
@@ -265,40 +259,40 @@ void VisualTool<FeatureType>::OnMouseEvent (wxMouseEvent &event) {
 }
 
 template<class FeatureType>
-void VisualTool<FeatureType>::Commit(bool full) {
+void VisualTool<FeatureType>::Commit(bool full, wxString message) {
 	SubtitlesGrid *grid = VideoContext::Get()->grid;
-	if (full) grid->ass->FlagAsModified(_("visual typesetting"));
+	if (full) {
+		if (message.empty()) {
+			message = _("visual typesetting");
+		}
+		grid->ass->FlagAsModified(message);
+	}
 	grid->CommitChanges(false,!full);
-	grid->editBox->Update(false, true, false);
+	grid->editBox->Update(false, true);
 }
 
 template<class FeatureType>
 AssDialogue* VisualTool<FeatureType>::GetActiveDialogueLine() {
 	SubtitlesGrid *grid = VideoContext::Get()->grid;
 	AssDialogue *diag = grid->GetDialogue(grid->editBox->linen);
-
+	if (grid->IsDisplayed(diag))
-	// Check if it's within range
-	if (diag) {
-		int f1 = VFR_Output.GetFrameAtTime(diag->Start.GetMS(),true);
-		int f2 = VFR_Output.GetFrameAtTime(diag->End.GetMS(),false);
-
-		if (f1 > frame_n || f2 < frame_n) return NULL;
-	}
-
 		return diag;
+	return NULL;
 }
 
 template<class FeatureType>
-FeatureType* VisualTool<FeatureType>::GetHighlightedFeature() {
+void VisualTool<FeatureType>::GetHighlightedFeature() {
 	int highestLayerFound = INT_MIN;
-	FeatureType* bestMatch = NULL;
+	curFeature = NULL;
-	for (FeatureIter cur = features.begin(); cur != features.end(); ++cur) {
+	curFeatureI = -1;
+	unsigned i = 0;
+	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur, ++i) {
 		if (cur->IsMouseOver(video.x, video.y) && cur->layer > highestLayerFound) {
-			bestMatch = &*cur;
+			curFeature = &*cur;
+			curFeatureI = i;
 			highestLayerFound = cur->layer;
 		}
 	}
-	return bestMatch;
 }
 
 template<class FeatureType>
@@ -308,31 +302,96 @@ void VisualTool<FeatureType>::DrawAllFeatures() {
 		dragListOK = true;
 	}
 
-	FeatureType* mouseOver = curFeature ?  curFeature : GetHighlightedFeature();
+	for (unsigned i = 0; i < features.size(); ++i) {
-
+		int fill;
-	for (FeatureCIter cur = features.begin(); cur != features.end(); ++cur) {
+		if (&features[i] == curFeature)
-		int fill = &*cur == mouseOver ? 2 :
+			fill = 2;
-		           cur->selected      ? 3 :
+		else if (selFeatures.find(i) != selFeatures.end())
-		                                1;
+			fill = 3;
+		else
+			fill = 1;
 		SetFillColour(colour[fill],0.6f);
 		SetLineColour(colour[0],1.0f,2);
-		cur->Draw(*this);
+		features[i].Draw(*this);
 	}
 }
 
 template<class FeatureType>
 void VisualTool<FeatureType>::Refresh() {
 	frame_n = VideoContext::Get()->GetFrameN();
-	if (externalChange) dragListOK = false;
+	if (externalChange) {
+		dragListOK = false;
 		DoRefresh();
+	}
+}
+template<class FeatureType>
+void VisualTool<FeatureType>::AddSelection(unsigned i) {
+	assert(i < features.size());
+
+	if (selFeatures.insert(i).second && features[i].line) {
+		lineSelCount[features[i].lineN] += 1;
+
+		SubtitlesGrid *grid = VideoContext::Get()->grid;
+		grid->SelectRow(features[i].lineN, true);
+	}
 }
 
 template<class FeatureType>
-void VisualTool<FeatureType>::ClearSelection() {
+void VisualTool<FeatureType>::RemoveSelection(unsigned i) {
-	for (SelFeatureIter cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
+	assert(i < features.size());
-		(*cur)->selected = false;
+
+	if (selFeatures.erase(i) > 0 && features[i].line) {
+		// Deselect a line only if all features for that line have been
+		// deselected
+		int lineN = features[i].lineN;
+		lineSelCount[lineN] -= 1;
+		assert(lineSelCount[lineN] >= 0);
+		if (lineSelCount[lineN] <= 0) {
+			SubtitlesGrid *grid = VideoContext::Get()->grid;
+			grid->SelectRow(lineN, true, false);
+
+			// We may have just deselected the active line, so make sure the
+			// edit box is set to something sane
+			SetEditbox();
+		}
+	}
+}
+
+template<class FeatureType>
+wxArrayInt VisualTool<FeatureType>::GetSelection() {
+	return VideoContext::Get()->grid->GetSelection();
+}
+
+
+template<class FeatureType>
+void VisualTool<FeatureType>::ClearSelection(bool hard) {
+	if (hard) {
+		VideoContext::Get()->grid->SelectRow(0, false, false);
 	}
 	selFeatures.clear();
+	lineSelCount.clear();
+}
+
+template<class FeatureType>
+void VisualTool<FeatureType>::SetEditbox(int lineN) {
+	VideoContext* con = VideoContext::Get();
+	if (lineN > -1) {
+		con->grid->editBox->SetToLine(lineN);
+		con->grid->SelectRow(lineN, true);
+	}
+	else {
+		wxArrayInt sel = GetSelection();
+		// If there is a selection and the edit box's line is in it, do nothing
+		// Otherwise set the edit box if there is a selection or the selection
+		// to the edit box if there is no selection
+		if (sel.empty()) {
+			con->grid->SelectRow(con->grid->editBox->linen, true);
+			return;
+		}
+		else if (!std::binary_search(sel.begin(), sel.end(), con->grid->editBox->linen)) {
+			con->grid->editBox->SetToLine(sel[0]);
+		}
+	}
 }
 
 /// @brief Get position of line 

aegisub/src/visual_tool.h

@@ -36,14 +36,15 @@
 #pragma once
 
 #ifndef AGI_PRE
-#include <list>
+#include <deque>
-#include <vector>
+#include <set>
 
 #include <wx/log.h>
 #include <wx/event.h>
 #include <wx/button.h>
 #endif
 
+#include "base_grid.h"
 #include "gl_wrap.h"
 
 class VideoDisplay;
@@ -74,30 +75,40 @@ public:
 /// @brief DOCME
 /// DOCME
 template<class FeatureType>
-class VisualTool : public IVisualTool {
+class VisualTool : public IVisualTool, public SelectionChangeSubscriber {
 private:
 	int dragStartX; /// Starting x coordinate of the current drag, if any
 	int dragStartY; /// Starting y coordinate of the current drag, if any
 
-	/// @brief Get the topmost visual feature under the mouse, or NULL if none are under the mouse
+	/// Set curFeature and curFeatureI to the topmost feature under the mouse,
-	FeatureType* GetHighlightedFeature();
+	/// or NULL and -1 if there are none
+	void GetHighlightedFeature();
 
-	typedef typename std::list<FeatureType*>::iterator SelFeatureIter;
+	typedef typename std::set<int>::iterator selection_iterator;
-	typedef typename std::list<FeatureType>::iterator FeatureIter;
-	typedef typename std::list<FeatureType>::const_iterator FeatureCIter;
 
-	std::list<FeatureType*> selFeatures; /// Currently selected visual features
+	std::set<int> selFeatures; /// Currently selected visual features
+	std::map<int, int> lineSelCount; /// Number of selected features for each line
+
+	/// @brief Set the edit box's active line, ensuring proper sync with grid
+	/// @param lineN Line number or -1 for automatic selection
+	///
+	/// This function ensures that the selection is not empty and that the line
+	/// displayed in the edit box is part of the selection, by either setting
+	/// the edit box to the selection or setting the selection to the edit
+	/// box's line, as is appropriate.
+	void SetEditbox(int lineN = -1);
 
-	bool externalChange; /// Only invalid drag lists when refreshing due to external changes
 	bool selChanged; /// Has the selection already been changed in the current click?
 protected:
 	VideoDisplay *parent; /// VideoDisplay which this belongs to, used to frame conversion
 	bool holding; /// Is a hold currently in progress?
 	AssDialogue *curDiag; /// Active dialogue line for a hold; only valid when holding = true
 	bool dragging; /// Is a drag currently in progress?
+	bool externalChange; /// Only invalid drag lists when refreshing due to external changes
 
-	FeatureType* curFeature; /// Topmost feature under the mouse; only valid during a drag?
+	FeatureType* curFeature; /// Topmost feature under the mouse; generally only valid during a drag
-	std::list<FeatureType> features; /// List of features which are drawn and can be clicked on
+	unsigned curFeatureI; /// Index of the current feature in the list
+	std::vector<FeatureType> features; /// List of features which are drawn and can be clicked on
 	bool dragListOK; /// Do the features not need to be regenerated?
 
 	int frame_n; /// Current frame number
@@ -121,8 +132,11 @@ protected:
 	/// @brief Get the dialogue line currently in the edit box
 	/// @return NULL if the line is not active on the current frame
 	AssDialogue *GetActiveDialogueLine();
+	/// Draw all of the features in the list
 	void DrawAllFeatures();
-	void Commit(bool full=false);
+	/// @brief Commit the current file state
+	/// @param message Description of changes for undo
+	void Commit(bool full=false, wxString message = L"");
 
 	/// @brief Called when a hold is begun
 	/// @return Should the hold actually happen?
@@ -152,8 +166,21 @@ protected:
 	/// @brief Called when there's stuff
 	virtual void DoRefresh() { }
 
-	/// @brief Must be called before removing entries from features
+	/// @brief Add a feature (and its line) to the selection
-	void ClearSelection();
+	/// @param i Index in the feature list
+	void AddSelection(unsigned i);
+	/// @brief Remove a feature from the selection
+	/// @param i Index in the feature list
+	/// Also deselects lines if all features for that line have been deselected
+	void RemoveSelection(unsigned i);
+	/// @brief Clear the selection
+	/// @param hard Should the grid's selection be cleared as well?
+	void ClearSelection(bool hard=true);
+	/// @brief Get the currently selected lines
+	wxArrayInt GetSelection();
+
+	typedef typename std::vector<FeatureType>::iterator feature_iterator;
+	typedef typename std::vector<FeatureType>::const_iterator feature_const_iterator;
 
 public:
 	/// @brief Handler for all mouse events
@@ -166,9 +193,12 @@ public:
 	virtual void Update() { };
 	/// @brief Draw stuff
 	virtual void Draw()=0;
-	/// @brief Called when there's stuff
+	/// @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
@@ -177,4 +207,3 @@ public:
 	/// @brief Destructor
 	virtual ~VisualTool();
 };
-

aegisub/src/visual_tool_clip.cpp

@@ -140,46 +140,42 @@ void VisualToolClip::CommitHold() {
 void VisualToolClip::PopulateFeatureList() {
 	// Clear
 	if (features.size() != 4) {
-		ClearSelection();
+		ClearSelection(false);
 		features.clear();
 		features.resize(4);
-		int i = 0;
-		for (std::list<ClipCorner>::iterator cur = features.begin(); cur != features.end(); ++cur, ++i) {
-			feat[i] = &*cur;
-		}
 	}
 
 	// Top-left
 	int i = 0;
-	feat[i]->x = curX1;
+	features[i].x = curX1;
-	feat[i]->y = curY1;
+	features[i].y = curY1;
-	feat[i]->horiz = feat[1];
+	features[i].horiz = &features[1];
-	feat[i]->vert = feat[2];
+	features[i].vert = &features[2];
-	feat[i]->type = DRAG_SMALL_CIRCLE;
+	features[i].type = DRAG_SMALL_CIRCLE;
 	i++;
 
 	// Top-right
-	feat[i]->x = curX2;
+	features[i].x = curX2;
-	feat[i]->y = curY1;
+	features[i].y = curY1;
-	feat[i]->horiz = feat[0];
+	features[i].horiz = &features[0];
-	feat[i]->vert = feat[3];
+	features[i].vert = &features[3];
-	feat[i]->type = DRAG_SMALL_CIRCLE;
+	features[i].type = DRAG_SMALL_CIRCLE;
 	i++;
 
 	// Bottom-left
-	feat[i]->x = curX1;
+	features[i].x = curX1;
-	feat[i]->y = curY2;
+	features[i].y = curY2;
-	feat[i]->horiz = feat[3];
+	features[i].horiz = &features[3];
-	feat[i]->vert = feat[0];
+	features[i].vert = &features[0];
-	feat[i]->type = DRAG_SMALL_CIRCLE;
+	features[i].type = DRAG_SMALL_CIRCLE;
 	i++;
 
 	// Bottom-right
-	feat[i]->x = curX2;
+	features[i].x = curX2;
-	feat[i]->y = curY2;
+	features[i].y = curY2;
-	feat[i]->horiz = feat[2];
+	features[i].horiz = &features[2];
-	feat[i]->vert = feat[1];
+	features[i].vert = &features[1];
-	feat[i]->type = DRAG_SMALL_CIRCLE;
+	features[i].type = DRAG_SMALL_CIRCLE;
 	i++;
 }
 
@@ -200,10 +196,10 @@ void VisualToolClip::UpdateDrag(ClipCorner* feature) {
 	feature->vert->x = feature->x;
 
 	// Get "cur" from features
-	curX1 = feat[0]->x;
+	curX1 = features[0].x;
-	curX2 = feat[3]->x;
+	curX2 = features[3].x;
-	curY1 = feat[0]->y;
+	curY1 = features[0].y;
-	curY2 = feat[3]->y;
+	curY2 = features[3].y;
 
 	// Make sure p1 < p2
 	if (curX1 > curX2) IntSwap(curX1,curX2);

aegisub/src/visual_tool_clip.h

@@ -75,9 +75,6 @@ private:
 	/// DOCME
 	bool inverse;
 
-	ClipCorner* feat[4];
-
-
 	/// @brief DOCME
 	/// @return 
 	///

aegisub/src/visual_tool_cross.cpp

@@ -79,9 +79,7 @@ void VisualToolCross::Update() {
 		SetOverride(line, L"\\pos", wxString::Format(L"(%i,%i)", x1 - dx, y1 - dy));
 	}
 
-	grid->ass->FlagAsModified(_("positioning"));
+	Commit(false, _("positioning"));
-	grid->CommitChanges(false,true);
-	grid->editBox->Update(false, true, false);
 }
 
 /// @brief Draw 

aegisub/src/visual_tool_drag.cpp

@@ -122,14 +122,38 @@ void VisualToolDrag::DoRefresh() {
 	UpdateToggleButtons();
 }
 
+void VisualToolDrag::OnSelectionChange(bool clear, int row, bool selected) {
+	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;
+			}
+		}
+	}
+	else {
+		for (size_t i = 0; i < features.size(); i++) {
+			if (features[i].lineN == row) {
+				RemoveSelection(i);
+			}
+		}
+	}
+	externalChange = true;
+}
+
 void VisualToolDrag::Draw() {
 	DrawAllFeatures();
 
 	// Draw arrows
-	for (std::list<VisualToolDragDraggableFeature>::iterator cur = features.begin(); cur != features.end(); ++cur) {
+	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur) {
 		if (cur->type == DRAG_START) continue;
 		VisualDraggableFeature *p2 = &*cur;
-		VisualDraggableFeature *p1 = cur->parent;
+		VisualDraggableFeature *p1 = &features[cur->parent];
 
 		// Has arrow?
 		bool hasArrow = p2->type == DRAG_END;
@@ -172,27 +196,22 @@ void VisualToolDrag::Draw() {
 	}
 }
 
-/// @brief Populate list 
 void VisualToolDrag::PopulateFeatureList() {
-	ClearSelection();
+	ClearSelection(false);
-	primary = NULL;
+	primary = -1;
+	GenerateFeatures();
+}
+void VisualToolDrag::GenerateFeatures() {
 	features.clear();
 
 	// Get video data
-	VideoContext* con = VideoContext::Get();
+	BaseGrid* grid = VideoContext::Get()->grid;
-	int numRows = con->grid->GetRows();
+	int numRows = grid->GetRows();
-	int framen = con->GetFrameN();
-	wxArrayInt sel = GetSelection();
 
-	// For each line
-	AssDialogue *diag;
 	for (int i=numRows;--i>=0;) {
-		diag = VideoContext::Get()->grid->GetDialogue(i);
+		AssDialogue *diag = grid->GetDialogue(i);
-		if (diag) {
+		if (!diag || !BaseGrid::IsDisplayed(diag)) continue;
-			// Line visible?
+
-			int f1 = VFR_Output.GetFrameAtTime(diag->Start.GetMS(),true);
-			int f2 = VFR_Output.GetFrameAtTime(diag->End.GetMS(),false);
-			if (f1 <= framen && f2 >= framen) {
 		// Get position
 		int x1,x2,y1,y2;
 		int t1=0;
@@ -212,7 +231,10 @@ void VisualToolDrag::PopulateFeatureList() {
 		feat.line = diag;
 		feat.lineN = i;
 		features.push_back(feat);
-				feat.parent = &features.back();
+		feat.parent = features.size() - 1;
+		if (grid->IsInSelection(i)) {
+			AddSelection(features.size() - 1);
+		}
 
 		// Create move destination feature
 		if (hasMove) {
@@ -224,7 +246,7 @@ void VisualToolDrag::PopulateFeatureList() {
 			feat.line = diag;
 			feat.lineN = i;
 			features.push_back(feat);
-					feat.parent->parent = &features.back();
+			features[feat.parent].parent = features.size() - 1;
 		}
 		// Create org feature
 		if (torgx != x1 || torgy != y1) {
@@ -238,11 +260,10 @@ void VisualToolDrag::PopulateFeatureList() {
 			features.push_back(feat);
 		}
 	}
-		}
-	}
 }
+
 bool VisualToolDrag::InitializeDrag(VisualToolDragDraggableFeature *feature) {
-	primary = feature;
+	primary = feature - &features[0];
 	return true;
 }
 
@@ -265,7 +286,7 @@ void VisualToolDrag::CommitDrag(VisualToolDragDraggableFeature* feature) {
 		return;
 	}
 
-	VisualToolDragDraggableFeature *p = feature->parent;
+	VisualToolDragDraggableFeature *p = feature->parent > -1 ? &features[feature->parent] : NULL;
 	if (feature->type == DRAG_END) {
 		std::swap(feature, p);
 	}
@@ -295,9 +316,9 @@ void VisualToolDrag::Update() {
 	int vx = video.x;
 	int vy = video.y;
 	parent->ToScriptCoords(&vx, &vy);
-	if (primary) {
+	if (primary > -1) {
-		dx = primary->x;
+		dx = features[primary].x;
-		dy = primary->y;
+		dy = features[primary].y;
 	}
 	else {
 		AssDialogue* line = GetActiveDialogueLine();
@@ -336,10 +357,7 @@ void VisualToolDrag::Update() {
 		}
 	}
 
-	grid->ass->FlagAsModified(_("positioning"));
+	Commit(false, _("positioning"));
-	grid->CommitChanges(false,true);
-	grid->editBox->Update(false, true, false);
 
-	/// @todo: should just move the existing features rather than remaking them all
+	GenerateFeatures();
-	PopulateFeatureList();
 }

aegisub/src/visual_tool_drag.h

@@ -47,11 +47,11 @@
 class VisualToolDragDraggableFeature : public VisualDraggableFeature {
 public:
 	int time;
-	VisualToolDragDraggableFeature* parent;
+	int parent;
 	VisualToolDragDraggableFeature()
 		: VisualDraggableFeature()
 		, time(0)
-		, parent(NULL)
+		, parent(-1)
 	{ }
 };
 
@@ -64,12 +64,15 @@ public:
 class VisualToolDrag : public VisualTool<VisualToolDragDraggableFeature> {
 private:
 	wxToolBar *toolBar; /// The subtoolbar
-	VisualToolDragDraggableFeature* primary; /// The feature last clicked on
+	int primary; /// The feature last clicked on
 
 	/// 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
 	bool toggleMoveOnMove;
 
+	/// Regenerage features without touching the selection
+	void GenerateFeatures();
+
 	void PopulateFeatureList();
 	bool InitializeDrag(VisualToolDragDraggableFeature* feature);
 	void UpdateDrag(VisualToolDragDraggableFeature* feature);
@@ -82,6 +85,8 @@ private:
 public:
 	VisualToolDrag(VideoDisplay *parent, VideoState const& video, wxToolBar *toolbar);
 
+	void OnSelectionChange(bool clear, int row, bool selected);
+
 	void Draw();
 	void Update();
 	void OnSubTool(wxCommandEvent &event);

aegisub/src/visual_tool_vector_clip.cpp

@@ -206,14 +206,16 @@ void VisualToolVectorClip::Draw() {
 
 /// @brief Populate feature list 
 void VisualToolVectorClip::PopulateFeatureList() {
-	// Clear
+	ClearSelection(false);
-	ClearSelection();
 	features.clear();
+	// This is perhaps a bit conservative as there can be up to 3N+1 features
+	features.reserve(spline.curves.size());
 	VisualToolVectorClipDraggableFeature feat;
 	
 	// Go through each curve
 	bool isFirst = true;
 	int i = 0;
+	int j = 0;
 	for (std::list<SplineCurve>::iterator cur=spline.curves.begin();cur!=spline.curves.end();cur++,i++) {
 		// First point
 		if (isFirst) {
@@ -224,9 +226,9 @@ void VisualToolVectorClip::PopulateFeatureList() {
 			feat.index = i;
 			feat.point = 0;
 			features.push_back(feat);
+			AddSelection(j++);
 		}
 
-		// Line
 		if (cur->type == CURVE_LINE) {
 			feat.x = (int)cur->p2.x;
 			feat.y = (int)cur->p2.y;
@@ -234,13 +236,10 @@ void VisualToolVectorClip::PopulateFeatureList() {
 			feat.index = i;
 			feat.point = 1;
 			features.push_back(feat);
+			AddSelection(j++);
 		}
 
-		// Bicubic
+		else if (cur->type == CURVE_BICUBIC) {
-		if (cur->type == CURVE_BICUBIC) {
-			// Current size
-			int size = features.size();
-
 			// Control points
 			feat.x = (int)cur->p2.x;
 			feat.y = (int)cur->p2.y;
@@ -259,6 +258,10 @@ void VisualToolVectorClip::PopulateFeatureList() {
 			feat.type = DRAG_SMALL_CIRCLE;
 			feat.point = 3;
 			features.push_back(feat);
+
+			AddSelection(j++);
+			AddSelection(j++);
+			AddSelection(j++);
 		}
 	}
 }
@@ -294,6 +297,7 @@ bool VisualToolVectorClip::InitializeDrag(VisualToolVectorClipDraggableFeature*
 				// Erase and save changes
 				spline.curves.erase(cur);
 				CommitDrag(feature);
+				PopulateFeatureList();
 				curFeature = NULL;
 				Commit(true);
 				return false;
@@ -390,7 +394,7 @@ bool VisualToolVectorClip::InitializeHold() {
 
 	// Freehand
 	if (mode == 6 || mode == 7) {
-		ClearSelection();
+		ClearSelection(false);
 		features.clear();
 		spline.curves.clear();
 		lastX = INT_MIN;
@@ -456,11 +460,13 @@ void VisualToolVectorClip::CommitHold() {
 
 	// Save it
 	if (mode != 3 && mode != 4) {
-		SetOverride(GetActiveDialogueLine(), inverse ? L"\\iclip" : L"\\clip", L"(" + spline.EncodeToASS() + L")");
+		SetOverride(curDiag, inverse ? L"\\iclip" : L"\\clip", L"(" + spline.EncodeToASS() + L")");
 	}
 
 	// End freedraw
 	if (!holding && (mode == 6 || mode == 7)) SetMode(0);
+
+	PopulateFeatureList();
 }
 
 /// @brief Refresh