Make visual tools pass around iterators for everything related to visual features rather than a mix of pointers and indices, and clean up feature creation. Fixes a large pile of selection-related bugs.

Originally committed to SVN as r4648.
2010-06-30 06:29:14 +00:00 · 2010-06-30 06:29:14 +00:00 · be1ed9e672
parent 8bd5b16699
commit be1ed9e672
14 changed files with 373 additions and 379 deletions
--- a/aegisub/src/visual_feature.cpp
+++ b/aegisub/src/visual_feature.cpp
@ -47,7 +47,6 @@ VisualDraggableFeature::VisualDraggableFeature()
 , origY(INT_MIN)
 , layer(0)
 , line(NULL)
-, lineN(-1)
 {
 }

--- a/aegisub/src/visual_feature.h
+++ b/aegisub/src/visual_feature.h
@ -81,7 +81,6 @@ public:
 	int layer; /// Layer; Higher = above

 	AssDialogue* line; /// The dialogue line this feature is for
-	int lineN; /// The line's index in the file

 	/// @brief Is the given point over this feature?
 	/// @param mx x coordinate to test
--- a/aegisub/src/visual_tool.cpp
+++ b/aegisub/src/visual_tool.cpp
@ -75,13 +75,12 @@ VisualTool<FeatureType>::VisualTool(VideoDisplay *parent, VideoState const& vide
 , dragStartX(0)
 , dragStartY(0)
 , selChanged(false)
+, selectedFeatures(selFeatures)
 , grid(VideoContext::Get()->grid)
 , parent(parent)
 , holding(false)
 , dragging(false)
 , externalChange(true)
-, curFeature(NULL)
-, dragListOK(false)
 , video(video)
 , leftClick(false)
 , leftDClick(false)
@ -92,6 +91,7 @@ VisualTool<FeatureType>::VisualTool(VideoDisplay *parent, VideoState const& vide
 	frameNumber = VideoContext::Get()->GetFrameN();
 	curDiag = GetActiveDialogueLine();
 	grid->AddSelectionListener(this);
+	curFeature = features.begin();
 }

 template<class FeatureType>
@ -124,35 +124,30 @@ void VisualTool<FeatureType>::OnMouseEvent(wxMouseEvent &event) {
 #endif
 	altDown = event.m_altDown;

-	if (!dragListOK) {
-		PopulateFeatureList();
-		dragListOK = true;
-		needRender = true;
-	}
 	if (!dragging) {
-		unsigned oldHigh = curFeatureI;
+		feature_iterator oldHigh = curFeature;
 		GetHighlightedFeature();
-		if (curFeatureI != oldHigh) needRender = true;
+		if (curFeature != oldHigh) needRender = true;
 	}

 	if (dragging) {
 		// continue drag
 		if (event.LeftIsDown()) {
 			for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-				features[*cur].x = (video.x - dragStartX + features[*cur].origX);
-				features[*cur].y = (video.y - dragStartY + features[*cur].origY);
+				(*cur)->x = (video.x - dragStartX + (*cur)->origX);
+				(*cur)->y = (video.y - dragStartY + (*cur)->origY);
 				if (shiftDown) {
 					if (abs(video.x - dragStartX) > abs(video.y - dragStartY)) {
-						features[*cur].y = features[*cur].origY;
+						(*cur)->y = (*cur)->origY;
 					}
 					else {
-						features[*cur].x = features[*cur].origX;
+						(*cur)->x = (*cur)->origX;
 					}
 				}
-				UpdateDrag(&features[*cur]);
+				UpdateDrag(*cur);

 				if (realTime) {
-					CommitDrag(&features[*cur]);
+					CommitDrag(*cur);
 				}
 			}
 			if (realTime) {
@ -172,21 +167,21 @@ void VisualTool<FeatureType>::OnMouseEvent(wxMouseEvent &event) {
 				if (!selChanged) {
 					if (ctrlDown) {
 						// deselect this feature
-						RemoveSelection(curFeatureI);
+						RemoveSelection(curFeature);
 					}
 					else {
-						SetSelection(curFeatureI);
+						SetSelection(curFeature);
 					}
 				}
 			}
 			else {
 				for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-					CommitDrag(&features[*cur]);
+					CommitDrag(*cur);
 				}
 				Commit(true);
 			}

-			curFeature = NULL;
+			curFeature = features.end();
 			parent->ReleaseMouse();
 			parent->SetFocus();
 		}
@ -216,27 +211,27 @@ void VisualTool<FeatureType>::OnMouseEvent(wxMouseEvent &event) {
 	}
 	else if (leftClick) {
 		// start drag
-		if (curFeature) {
-			if (InitializeDrag(curFeature)) {
-				if (selFeatures.find(curFeatureI) == selFeatures.end()) {
-					selChanged = true;
-					if (ctrlDown) {
-						AddSelection(curFeatureI);
-					}
-					else {
-						SetSelection(curFeatureI);
-					}
+		if (curFeature != features.end()) {
+			if (selFeatures.find(curFeature) == selFeatures.end()) {
+				selChanged = true;
+				if (ctrlDown) {
+					AddSelection(curFeature);
 				}
 				else {
-					selChanged = false;
+					SetSelection(curFeature);
 				}
-				if (curFeature->line) grid->SetActiveLine(curFeature->line);
+			}
+			else {
+				selChanged = false;
+			}
+			if (curFeature->line) grid->SetActiveLine(curFeature->line);

+			if (InitializeDrag(curFeature)) {
 				dragStartX = video.x;
 				dragStartY = video.y;
 				for (selection_iterator cur = selFeatures.begin(); cur != selFeatures.end(); ++cur) {
-					features[*cur].origX = features[*cur].x;
-					features[*cur].origY = features[*cur].y;
+					(*cur)->origX = (*cur)->x;
+					(*cur)->origY = (*cur)->y;
 				}

 				dragging = true;
@ -291,13 +286,10 @@ AssDialogue* VisualTool<FeatureType>::GetActiveDialogueLine() {
 template<class FeatureType>
 void VisualTool<FeatureType>::GetHighlightedFeature() {
 	int highestLayerFound = INT_MIN;
-	curFeature = NULL;
-	curFeatureI = -1;
-	unsigned i = 0;
-	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur, ++i) {
+	curFeature = features.end();
+	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur) {
 		if (cur->IsMouseOver(video.x, video.y) && cur->layer > highestLayerFound) {
-			curFeature = &*cur;
-			curFeatureI = i;
+			curFeature = cur;
 			highestLayerFound = cur->layer;
 		}
 	}
@ -305,30 +297,25 @@ void VisualTool<FeatureType>::GetHighlightedFeature() {

 template<class FeatureType>
 void VisualTool<FeatureType>::DrawAllFeatures() {
-	if (!dragListOK) {
-		PopulateFeatureList();
-		dragListOK = true;
-	}
-
 	SetLineColour(colour[0],1.0f,2);
-	for (unsigned i = 0; i < features.size(); ++i) {
+	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur) {
 		int fill;
-		if (&features[i] == curFeature)
+		if (cur == curFeature)
 			fill = 2;
-		else if (selFeatures.find(i) != selFeatures.end())
+		else if (selFeatures.find(cur) != selFeatures.end())
 			fill = 3;
 		else
 			fill = 1;
 		SetFillColour(colour[fill],0.6f);
-		features[i].Draw(*this);
+		cur->Draw(*this);
 	}
 }

 template<class FeatureType>
 void VisualTool<FeatureType>::Refresh() {
 	if (externalChange) {
-		dragListOK = false;
 		curDiag = GetActiveDialogueLine();
+		curFeature = features.end();
 		OnFileChanged();
 	}
 }
@ -336,6 +323,8 @@ void VisualTool<FeatureType>::Refresh() {
 template<class FeatureType>
 void VisualTool<FeatureType>::SetFrame(int newFrameNumber) {
 	if (frameNumber == newFrameNumber) return;
+	frameNumber = newFrameNumber;
+	curFeature = features.end();
 	OnFrameChanged();
 	AssDialogue *newCurDiag = GetActiveDialogueLine();
 	if (newCurDiag != curDiag) {
@ -357,14 +346,13 @@ void VisualTool<FeatureType>::OnActiveLineChanged(AssDialogue *new_line) {


 template<class FeatureType>
-void VisualTool<FeatureType>::SetSelection(unsigned i) {
-	assert(i < features.size());
-
+void VisualTool<FeatureType>::SetSelection(feature_iterator feat) {
 	selFeatures.clear();
 	lineSelCount.clear();
-	selFeatures.insert(i);

-	AssDialogue *line = features[i].line;
+	selFeatures.insert(feat);
+
+	AssDialogue *line = feat->line;
 	if (line) {
 		lineSelCount[line] = 1;

@ -376,23 +364,22 @@ void VisualTool<FeatureType>::SetSelection(unsigned i) {


 template<class FeatureType>
-void VisualTool<FeatureType>::AddSelection(unsigned i) {
-	assert(i < features.size());
-
-	if (selFeatures.insert(i).second && features[i].line) {
-		lineSelCount[features[i].line] += 1;
-		grid->SelectRow(features[i].lineN, true);
+void VisualTool<FeatureType>::AddSelection(feature_iterator feat) {
+	if (selFeatures.insert(feat).second && feat->line) {
+		lineSelCount[feat->line] += 1;
+		Selection sel = grid->GetSelectedSet();
+		if (sel.insert(feat->line).second) {
+			grid->SetSelectedSet(sel);
+		}
 	}
 }

 template<class FeatureType>
-void VisualTool<FeatureType>::RemoveSelection(unsigned i) {
-	assert(i < features.size());
-
-	if (selFeatures.erase(i) > 0 && features[i].line) {
+void VisualTool<FeatureType>::RemoveSelection(feature_iterator feat) {
+	if (selFeatures.erase(feat) > 0 && feat->line) {
 		// Deselect a line only if all features for that line have been
 		// deselected
-		AssDialogue* line = features[i].line;
+		AssDialogue* line = feat->line;
 		lineSelCount[line] -= 1;
 		assert(lineSelCount[line] >= 0);
 		if (lineSelCount[line] <= 0) {
--- a/aegisub/src/visual_tool.h
+++ b/aegisub/src/visual_tool.h
@ -76,28 +76,39 @@ public:
 	virtual ~IVisualTool() { };
 };

+struct ltaddr {
+	template<class T>
+	bool operator()(T lft, T rgt) const {
+		return &*lft < &*rgt;
+	}
+};
+
 /// DOCME
 /// @class VisualTool
 /// @brief DOCME
 /// DOCME
 template<class FeatureType>
 class VisualTool : public IVisualTool, protected SubtitleSelectionListener {
+protected:
+	typedef typename FeatureType Feature;
+	typedef typename std::list<FeatureType>::iterator feature_iterator;
+	typedef typename std::list<FeatureType>::const_iterator feature_const_iterator;
 private:
 	agi::OptionValue* realtime; /// Realtime updating option
 	int dragStartX; /// Starting x coordinate of the current drag, if any
 	int dragStartY; /// Starting y coordinate of the current drag, if any

-	/// Set curFeature and curFeatureI to the topmost feature under the mouse,
-	/// or NULL and -1 if there are none
+	/// Set curFeature to the topmost feature under the mouse, or end() if there
+	/// are none
 	void GetHighlightedFeature();

 	/// @brief Get the dialogue line currently in the edit box
 	/// @return NULL if the line is not active on the current frame
 	AssDialogue *GetActiveDialogueLine();

-	typedef typename std::set<int>::iterator selection_iterator;
+	typedef typename std::set<feature_iterator, ltaddr>::iterator selection_iterator;

-	std::set<int> selFeatures; /// Currently selected visual features
+	std::set<feature_iterator, ltaddr> selFeatures; /// Currently selected visual features
 	std::map<AssDialogue*, int> lineSelCount; /// Number of selected features for each line

 	bool selChanged; /// Has the selection already been changed in the current click?
@ -110,18 +121,15 @@ private:
 	/// @brief Called at the end of a hold
 	virtual void CommitHold() { }

-	/// @brief Called when the feature list needs to be (re)generated
-	virtual void PopulateFeatureList() { }
-
 	/// @brief Called at the beginning of a drag
 	/// @param feature The visual feature clicked on
 	/// @return Should the drag happen?
-	virtual bool InitializeDrag(FeatureType* feature) { return true; }
+	virtual bool InitializeDrag(feature_iterator feature) { return true; }
 	/// @brief Called on every mouse event during a drag
 	/// @param feature The current feature to process; not necessarily the one clicked on
-	virtual void UpdateDrag(FeatureType* feature) { }
+	virtual void UpdateDrag(feature_iterator feature) { }
 	/// @brief Called at the end of a drag
-	virtual void CommitDrag(FeatureType* feature) { }
+	virtual void CommitDrag(feature_iterator feature) { }

 	/// Called when the file is changed by something other than a visual tool
 	virtual void OnFileChanged() { DoRefresh(); }
@ -141,6 +149,9 @@ private:
 	virtual void Draw()=0;

 protected:
+	/// Read-only reference to the set of selected features for subclasses
+	const std::set<feature_iterator, ltaddr> &selectedFeatures;
+	typedef typename std::set<feature_iterator, ltaddr>::const_iterator sel_iterator;
 	SubtitlesGrid *grid;
 	VideoDisplay *parent; /// VideoDisplay which this belongs to, used to frame conversion
 	bool holding; /// Is a hold currently in progress?
@ -148,10 +159,8 @@ protected:
 	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; generally only valid during a drag
-	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?
+	feature_iterator 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

 	int frameNumber; /// Current frame number
 	VideoState const& video; /// Mouse and video information
@ -179,24 +188,20 @@ protected:

 	/// @brief Add a feature (and its line) to the selection
 	/// @param i Index in the feature list
-	void AddSelection(unsigned i);
+	void AddSelection(feature_iterator feat);

 	/// @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);
+	void RemoveSelection(feature_iterator feat);

 	/// @brief Set the selection to a single feature, deselecting everything else
 	/// @param i Index in the feature list
-	void SetSelection(unsigned i);
+	void SetSelection(feature_iterator feat);

 	/// @brief Clear the selection
 	void ClearSelection();

-	typedef typename std::vector<FeatureType>::iterator feature_iterator;
-	typedef typename std::vector<FeatureType>::const_iterator feature_const_iterator;
-
-protected:
 	// SubtitleSelectionListener implementation
 	void OnActiveLineChanged(AssDialogue *new_line);
 	virtual void OnSelectedSetChanged(const Selection &lines_added, const Selection &lines_removed) { }
--- a/aegisub/src/visual_tool_clip.cpp
+++ b/aegisub/src/visual_tool_clip.cpp
@ -58,6 +58,38 @@ VisualToolClip::VisualToolClip(VideoDisplay *parent, VideoState const& video, wx
 	if (curDiag) {
 		GetLineClip(curDiag,curX1,curY1,curX2,curY2,inverse);
 	}
+
+	Feature feat;
+	feat.type = DRAG_SMALL_CIRCLE;
+	features.resize(4, feat);
+
+	feature_iterator cur = features.begin();
+	feats[0] = &*(cur++);
+	feats[1] = &*(cur++);
+	feats[2] = &*(cur++);
+	feats[3] = &*(cur++);
+
+
+	// Top-left
+	int i = 0;
+	feats[i]->horiz = feats[1];
+	feats[i]->vert = feats[2];
+	i++;
+
+	// Top-right
+	feats[i]->horiz = feats[0];
+	feats[i]->vert = feats[3];
+	i++;
+
+	// Bottom-left
+	feats[i]->horiz = feats[3];
+	feats[i]->vert = feats[0];
+	i++;
+
+	// Bottom-right
+	feats[i]->horiz = feats[2];
+	feats[i]->vert = feats[1];
+	i++;
 }

 void VisualToolClip::Draw() {
@ -101,7 +133,6 @@ bool VisualToolClip::InitializeHold() {
 }

 void VisualToolClip::UpdateHold() {
-	// Coordinates
 	curX1 = startX;
 	curY1 = startY;
 	curX2 = video.x;
@ -117,7 +148,7 @@ void VisualToolClip::UpdateHold() {
 	curY1 = MID(0,curY1,video.h);
 	curY2 = MID(0,curY2,video.h);

-	PopulateFeatureList();
+	SetFeaturePositions();
 }

 void VisualToolClip::CommitHold() {
@ -130,83 +161,57 @@ void VisualToolClip::CommitHold() {
 	SetOverride(curDiag, inverse ? L"\\iclip" : L"\\clip",wxString::Format(L"(%i,%i,%i,%i)",x1,y1,x2,y2));
 }

-void VisualToolClip::PopulateFeatureList() {
-	if (features.size() != 4) {
-		ClearSelection();
-		features.clear();
-		features.resize(4);
-	}
-
-	// Top-left
-	int i = 0;
-	features[i].x = curX1;
-	features[i].y = curY1;
-	features[i].horiz = &features[1];
-	features[i].vert = &features[2];
-	features[i].type = DRAG_SMALL_CIRCLE;
-	i++;
-
-	// Top-right
-	features[i].x = curX2;
-	features[i].y = curY1;
-	features[i].horiz = &features[0];
-	features[i].vert = &features[3];
-	features[i].type = DRAG_SMALL_CIRCLE;
-	i++;
-
-	// Bottom-left
-	features[i].x = curX1;
-	features[i].y = curY2;
-	features[i].horiz = &features[3];
-	features[i].vert = &features[0];
-	features[i].type = DRAG_SMALL_CIRCLE;
-	i++;
-
-	// Bottom-right
-	features[i].x = curX2;
-	features[i].y = curY2;
-	features[i].horiz = &features[2];
-	features[i].vert = &features[1];
-	features[i].type = DRAG_SMALL_CIRCLE;
-	i++;
-}
-
-bool VisualToolClip::InitializeDrag(ClipCorner*) {
+bool VisualToolClip::InitializeDrag(feature_iterator) {
 	curDiag->StripTag(L"\\clip");
 	curDiag->StripTag(L"\\iclip");
 	return true;
 }

-void VisualToolClip::UpdateDrag(ClipCorner* feature) {
+void VisualToolClip::UpdateDrag(feature_iterator feature) {
 	// Update brothers
 	feature->horiz->y = feature->y;
 	feature->vert->x = feature->x;

 	// Get "cur" from features
-	curX1 = features[0].x;
-	curX2 = features[3].x;
-	curY1 = features[0].y;
-	curY2 = features[3].y;
+	curX1 = feats[0]->x;
+	curX2 = feats[3]->x;
+	curY1 = feats[0]->y;
+	curY2 = feats[3]->y;

 	// Make sure p1 < p2
 	if (curX1 > curX2) std::swap(curX1,curX2);
 	if (curY1 > curY2) std::swap(curY1,curY2);
 }

-void VisualToolClip::CommitDrag(ClipCorner*) {
+void VisualToolClip::CommitDrag(feature_iterator) {
 	CommitHold();
 }

-void VisualToolClip::OnLineChanged() {
-	if (curDiag) {
-		GetLineClip(curDiag,curX1,curY1,curX2,curY2,inverse);
-		PopulateFeatureList();
-	}
+void VisualToolClip::SetFeaturePositions() {
+	// Top-left
+	int i = 0;
+	feats[i]->x = curX1;
+	feats[i]->y = curY1;
+	i++;
+
+	// Top-right
+	feats[i]->x = curX2;
+	feats[i]->y = curY1;
+	i++;
+
+	// Bottom-left
+	feats[i]->x = curX1;
+	feats[i]->y = curY2;
+	i++;
+
+	// Bottom-right
+	feats[i]->x = curX2;
+	feats[i]->y = curY2;
 }

-void VisualToolClip::OnFileChanged() {
+void VisualToolClip::DoRefresh() {
 	if (curDiag) {
 		GetLineClip(curDiag,curX1,curY1,curX2,curY2,inverse);
-		PopulateFeatureList();
+		SetFeaturePositions();
 	}
 }
--- a/aegisub/src/visual_tool_clip.h
+++ b/aegisub/src/visual_tool_clip.h
@ -59,19 +59,20 @@ public:
 class VisualToolClip : public VisualTool<ClipCorner> {
 	int startX,startY,curX1,curY1,curX2,curY2;

+	ClipCorner *feats[4];
+
 	bool inverse;

 	bool InitializeHold();
 	void UpdateHold();
 	void CommitHold();

-	void OnLineChanged();
-	void OnFileChanged();
+	void DoRefresh();
+	void SetFeaturePositions();

-	void PopulateFeatureList();
-	bool InitializeDrag(ClipCorner* feature);
-	void UpdateDrag(ClipCorner* feature);
-	void CommitDrag(ClipCorner* feature);
+	bool InitializeDrag(feature_iterator feature);
+	void UpdateDrag(feature_iterator feature);
+	void CommitDrag(feature_iterator feature);

 	void Draw();
 public:
--- a/aegisub/src/visual_tool_drag.cpp
+++ b/aegisub/src/visual_tool_drag.cpp
@ -65,6 +65,9 @@ VisualToolDrag::VisualToolDrag(VideoDisplay *parent, VideoState const& video, wx
 	toolBar->AddTool(BUTTON_TOGGLE_MOVE, _("Toggle between \\move and \\pos"), GETIMAGE(visual_move_conv_move_24));
 	toolBar->Realize();
 	toolBar->Show(true);
+
+	grid->GetSelectedSet(selection);
+	OnFileChanged();
 }

 void VisualToolDrag::UpdateToggleButtons() {
@ -93,9 +96,8 @@ void VisualToolDrag::UpdateToggleButtons() {
 /// @brief Toggle button pressed 
 /// @param event 
 void VisualToolDrag::OnSubTool(wxCommandEvent &) {
-	Selection sel = grid->GetSelectedSet();
 	// Toggle \move <-> \pos
-	for (Selection::const_iterator cur = sel.begin(); cur != sel.end(); ++cur) {
+	for (Selection::const_iterator cur = selection.begin(); cur != selection.end(); ++cur) {
 		AssDialogue *line = *cur;
 		int x1,y1,x2,y2,t1,t2;
 		bool hasMove;
@ -118,31 +120,62 @@ void VisualToolDrag::OnLineChanged() {
 }

 void VisualToolDrag::OnFileChanged() {
-	/// @todo be less dumb and preserve selections when possible
-	PopulateFeatureList();
+	/// @todo it should be possible to preserve the selection in some cases
+	features.clear();
+	ClearSelection();
+	primary = NULL;
+
+	for (int i = grid->GetRows() - 1; i >=0; i--) {
+		AssDialogue *diag = grid->GetDialogue(i);
+		if (BaseGrid::IsDisplayed(diag)) {
+			MakeFeatures(diag);
+		}
+	}
 }

 void VisualToolDrag::OnFrameChanged() {
-	/// @todo be less dumb and preserve selections when possible
-	PopulateFeatureList();
+	if (primary && !BaseGrid::IsDisplayed(primary->line)) primary = NULL;
+
+	feature_iterator feat = features.begin();
+	feature_iterator end = features.end();
+
+	for (int i = grid->GetRows() - 1; i >=0; i--) {
+		AssDialogue *diag = grid->GetDialogue(i);
+		if (BaseGrid::IsDisplayed(diag)) {
+			// Features don't exist and should
+			if (feat == end || feat->line != diag) {
+				MakeFeatures(diag, feat);
+			}
+			// Move past already existing features for the line
+			else {
+				while (feat != end && feat->line == diag) ++feat;
+			}
+		}
+		else {
+			// Remove all features for this line (if any)
+			while (feat != end && feat->line == diag) {
+				feat->line = NULL;
+				RemoveSelection(feat);
+				feat = features.erase(feat);
+			}
+		}
+	}
 }

 void VisualToolDrag::OnSelectedSetChanged(const Selection &added, const Selection &removed) {
+	grid->GetSelectedSet(selection);
 	if (!externalChange) return;
 	externalChange = false;
 	grid->BeginBatch();

-	// Remove all deselected lines
-	for (size_t i = 0; i < features.size(); i++) {
-		if (removed.find(features[i].line) != removed.end()) {
-			RemoveSelection(i);
+	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur) {
+		// Remove all deselected lines
+		if (removed.find(cur->line) != removed.end()) {
+			RemoveSelection(cur);
 		}
-	}
-
-	// 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);
+		// And add all newly selected lines
+		else if (added.find(cur->line) != added.end() && cur->type == DRAG_START) {
+			AddSelection(cur);
 		}
 	}

@ -156,8 +189,8 @@ void VisualToolDrag::Draw() {
 	// Draw arrows
 	for (feature_iterator cur = features.begin(); cur != features.end(); ++cur) {
 		if (cur->type == DRAG_START) continue;
-		VisualDraggableFeature *p2 = &*cur;
-		VisualDraggableFeature *p1 = &features[cur->parent];
+		feature_iterator p2 = cur;
+		feature_iterator p1 = cur->parent;

 		// Has arrow?
 		bool hasArrow = p2->type == DRAG_END;
@ -199,89 +232,77 @@ void VisualToolDrag::Draw() {
 		}
 	}
 }
-
-void VisualToolDrag::PopulateFeatureList() {
-	ClearSelection();
-	primary = -1;
-	GenerateFeatures();
+void VisualToolDrag::MakeFeatures(AssDialogue *diag) {
+	MakeFeatures(diag, features.end());
 }
-void VisualToolDrag::GenerateFeatures() {
-	features.clear();
+void VisualToolDrag::MakeFeatures(AssDialogue *diag, feature_iterator pos) {
+	// Get position
+	int x1,x2,y1,y2;
+	int t1=0;
+	int t2=diag->End.GetMS()-diag->Start.GetMS();
+	int torgx,torgy;
+	bool hasMove;
+	GetLinePosition(diag,x1,y1,torgx,torgy);
+	GetLineMove(diag,hasMove,x1,y1,x2,y2,t1,t2);

-	// Get video data
-	BaseGrid* grid = VideoContext::Get()->grid;
-	int numRows = grid->GetRows();
+	// Create \pos feature
+	Feature feat;
+	feat.x = x1;
+	feat.y = y1;
+	feat.layer = 0;
+	feat.type = DRAG_START;
+	feat.time = t1;
+	feat.line = diag;
+	feat.parent = features.end();
+	features.insert(pos, feat);
+	feature_iterator cur = pos; --cur;
+	feat.parent = cur;
+	if (selection.find(diag) != selection.end()) {
+		AddSelection(cur);
+	}

-	for (int i=numRows;--i>=0;) {
-		AssDialogue *diag = grid->GetDialogue(i);
-		if (!diag || !BaseGrid::IsDisplayed(diag)) continue;
-
-		// Get position
-		int x1,x2,y1,y2;
-		int t1=0;
-		int t2=diag->End.GetMS()-diag->Start.GetMS();
-		int torgx,torgy;
-		bool hasMove;
-		GetLinePosition(diag,x1,y1,torgx,torgy);
-		GetLineMove(diag,hasMove,x1,y1,x2,y2,t1,t2);
-
-		// Create \pos feature
-		VisualToolDragDraggableFeature feat;
-		feat.x = x1;
-		feat.y = y1;
-		feat.layer = 0;
-		feat.type = DRAG_START;
-		feat.time = t1;
+	// Create move destination feature
+	if (hasMove) {
+		feat.x = x2;
+		feat.y = y2;
+		feat.layer = 1;
+		feat.type = DRAG_END;
+		feat.time = t2;
 		feat.line = diag;
-		feat.lineN = i;
-		features.push_back(feat);
-		feat.parent = features.size() - 1;
-		if (grid->IsInSelection(i)) {
-			AddSelection(features.size() - 1);
-		}
-
-		// Create move destination feature
-		if (hasMove) {
-			feat.x = x2;
-			feat.y = y2;
-			feat.layer = 1;
-			feat.type = DRAG_END;
-			feat.time = t2;
-			feat.line = diag;
-			feat.lineN = i;
-			features.push_back(feat);
-			features[feat.parent].parent = features.size() - 1;
-		}
-		// Create org feature
-		if (torgx != x1 || torgy != y1) {
-			feat.x = torgx;
-			feat.y = torgy;
-			feat.layer = -1;
-			feat.type = DRAG_ORIGIN;
-			feat.time = 0;
-			feat.line = diag;
-			feat.lineN = i;
-			features.push_back(feat);
-		}
+		features.insert(pos, feat);
+		feat.parent->parent = --pos; ++pos;
+	}
+	// Create org feature
+	if (torgx != x1 || torgy != y1) {
+		feat.x = torgx;
+		feat.y = torgy;
+		feat.layer = -1;
+		feat.type = DRAG_ORIGIN;
+		feat.time = 0;
+		feat.line = diag;
+		features.insert(pos, feat);
 	}
 }

-bool VisualToolDrag::InitializeDrag(VisualToolDragDraggableFeature *feature) {
-	primary = feature - &features[0];
+bool VisualToolDrag::InitializeDrag(feature_iterator feature) {
+	primary = &*feature;
+
+	// Set time of clicked feature to the current frame and shift all other
+	// selected features by the same amount
+	if (feature->type != DRAG_ORIGIN) {
+		int time = VFR_Output.GetTimeAtFrame(frameNumber,true,true) - feature->line->Start.GetMS();
+		int change = time - feature->time;
+
+		for (sel_iterator cur = selectedFeatures.begin(); cur != selectedFeatures.end(); ++cur) {
+			if ((*cur)->type != DRAG_ORIGIN) {
+				(*cur)->time += change;
+			}
+		}
+	}
 	return true;
 }

-/// @brief Update drag 
-/// @param feature 
-void VisualToolDrag::UpdateDrag(VisualToolDragDraggableFeature* feature) {
-	// Update "time" to reflect the time of the frame in which the feature is being dragged
-	int time = VFR_Output.GetTimeAtFrame(frameNumber,true,true);
-	feature->time = MID(0,time - feature->line->Start.GetMS(),feature->line->End.GetMS()-feature->line->Start.GetMS());
-}
-
-/// @brief Commit drag 
-/// @param feature 
-void VisualToolDrag::CommitDrag(VisualToolDragDraggableFeature* feature) {
+void VisualToolDrag::CommitDrag(feature_iterator feature) {
 	if (feature->type == DRAG_ORIGIN) {
 		int x = feature->x;
 		int y = feature->y;
@ -290,7 +311,7 @@ void VisualToolDrag::CommitDrag(VisualToolDragDraggableFeature* feature) {
 		return;
 	}

-	VisualToolDragDraggableFeature *p = feature->parent > -1 ? &features[feature->parent] : NULL;
+	feature_iterator p = feature->parent;
 	if (feature->type == DRAG_END) {
 		std::swap(feature, p);
 	}
@ -300,7 +321,7 @@ void VisualToolDrag::CommitDrag(VisualToolDragDraggableFeature* feature) {
 	parent->ToScriptCoords(&x1, &y1);

 	// Position
-	if (!p) {
+	if (feature->parent == features.end()) {
 		SetOverride(feature->line, L"\\pos", wxString::Format(L"(%i,%i)", x1, y1));
 	}
 	// Move
@ -309,7 +330,6 @@ void VisualToolDrag::CommitDrag(VisualToolDragDraggableFeature* feature) {
 		int y2 = p->y;
 		parent->ToScriptCoords(&x2, &y2);

-		// Set override
 		SetOverride(feature->line, L"\\move", wxString::Format(L"(%i,%i,%i,%i,%i,%i)", x1, y1, x2, y2, feature->time, p->time));
 	}
 }
@ -320,9 +340,9 @@ bool VisualToolDrag::Update() {
 	int vx = video.x;
 	int vy = video.y;
 	parent->ToScriptCoords(&vx, &vy);
-	if (primary > -1) {
-		dx = features[primary].x;
-		dy = features[primary].y;
+	if (primary) {
+		dx = primary->x;
+		dy = primary->y;
 	}
 	else {
 		if (!curDiag) return false;
@ -332,8 +352,7 @@ bool VisualToolDrag::Update() {
 	dx -= vx;
 	dy -= vy;

-	Selection sel = grid->GetSelectedSet();
-	for (Selection::const_iterator cur = sel.begin(); cur != sel.end(); ++cur) {
+	for (Selection::const_iterator cur = selection.begin(); cur != selection.end(); ++cur) {
 		int x1 = 0, y1 = 0, x2 = 0, y2 = 0, t1 = INT_MIN, t2 = INT_MIN, orgx, orgy;
 		bool isMove;

@ -359,6 +378,6 @@ bool VisualToolDrag::Update() {

 	Commit(true, _("positioning"));

-	GenerateFeatures();
+	OnFileChanged();
 	return false;
 }
--- a/aegisub/src/visual_tool_drag.h
+++ b/aegisub/src/visual_tool_drag.h
@ -47,12 +47,8 @@
 class VisualToolDragDraggableFeature : public VisualDraggableFeature {
 public:
 	int time;
-	int parent;
-	VisualToolDragDraggableFeature()
-		: VisualDraggableFeature()
-		, time(0)
-		, parent(-1)
-	{ }
+	std::list<VisualToolDragDraggableFeature>::iterator parent;
+	VisualToolDragDraggableFeature() : VisualDraggableFeature(), time(0) { }
 };


@ -62,21 +58,29 @@ public:
 ///
 /// DOCME
 class VisualToolDrag : public VisualTool<VisualToolDragDraggableFeature> {
-private:
-	wxToolBar *toolBar; /// The subtoolbar
-	int primary; /// The feature last clicked on
+	/// The subtoolbar for the move/pos conversion button
+	wxToolBar *toolBar;
+	/// The feature last clicked on for the double-click handler
+	/// Equal to curFeature during drags; possibly different at all other times
+	/// Null if no features have been clicked on or the last clicked on one no
+	/// longer exists
+	Feature *primary;
+	/// The last announced selection set
+	Selection selection;
+	int change;

 	/// 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();
+	/// @brief Create the features for a line
+	/// @param diag Line to create the features for
+	/// @param pos Insertion point in the feature list
+	void MakeFeatures(AssDialogue *diag, feature_iterator pos);
+	void MakeFeatures(AssDialogue *diag);

-	void PopulateFeatureList();
-	bool InitializeDrag(VisualToolDragDraggableFeature* feature);
-	void UpdateDrag(VisualToolDragDraggableFeature* feature);
-	void CommitDrag(VisualToolDragDraggableFeature* feature);
+	bool InitializeDrag(feature_iterator feature);
+	void CommitDrag(feature_iterator feature);

 	/// Set the pos/move button to the correct icon based on the active line
 	void UpdateToggleButtons();
--- a/aegisub/src/visual_tool_rotatexy.cpp
+++ b/aegisub/src/visual_tool_rotatexy.cpp
@ -51,6 +51,9 @@
 VisualToolRotateXY::VisualToolRotateXY(VideoDisplay *parent, VideoState const& video, wxToolBar *)
 : VisualTool<VisualDraggableFeature>(parent, video)
 {
+	features.resize(1);
+	org = &features.back();
+	org->type = DRAG_BIG_TRIANGLE;
 	DoRefresh();
 }

@ -60,9 +63,9 @@ void VisualToolRotateXY::Draw() {
 	// Pivot coordinates
 	int dx=0,dy=0;
 	if (dragging) GetLinePosition(curDiag,dx,dy);
-	else GetLinePosition(curDiag,dx,dy,orgx,orgy);
-	dx = orgx;
-	dy = orgy;
+	else GetLinePosition(curDiag,dx,dy,org->x,org->y);
+	dx = org->x;
+	dy = org->y;

 	SetLineColour(colour[0]);
 	SetFillColour(colour[1],0.3f);
@ -152,8 +155,8 @@ void VisualToolRotateXY::Draw() {
 }

 bool VisualToolRotateXY::InitializeHold() {
-	startAngleX = (orgy-video.y)*2.f;
-	startAngleY = (video.x-orgx)*2.f;
+	startAngleX = (org->y-video.y)*2.f;
+	startAngleY = (video.x-org->x)*2.f;
 	origAngleX = curAngleX;
 	origAngleY = curAngleY;

@ -161,8 +164,8 @@ bool VisualToolRotateXY::InitializeHold() {
 }

 void VisualToolRotateXY::UpdateHold() {
-	float screenAngleX = (orgy-video.y)*2.f;
-	float screenAngleY = (video.x-orgx)*2.f;
+	float screenAngleX = (org->y-video.y)*2.f;
+	float screenAngleY = (video.x-org->x)*2.f;

 	// Deltas
 	float deltaX = screenAngleX - startAngleX;
@ -193,36 +196,16 @@ void VisualToolRotateXY::CommitHold() {
 	}
 }

-void VisualToolRotateXY::PopulateFeatureList() {
-	if (!curDiag) return;
-
-	int posx, posy;
-	GetLinePosition(curDiag,posx,posy,orgx,orgy);
-
-	// Set features
-	features.resize(1);
-	VisualDraggableFeature &feat = features.back();
-	feat.x = orgx;
-	feat.y = orgy;
-	feat.line = curDiag;
-	feat.type = DRAG_BIG_TRIANGLE;
-}
-
-void VisualToolRotateXY::UpdateDrag(VisualDraggableFeature* feature) {
-	orgx = feature->x;
-	orgy = feature->y;
-}
-
-void VisualToolRotateXY::CommitDrag(VisualDraggableFeature* feature) {
+void VisualToolRotateXY::CommitDrag(feature_iterator feature) {
 	int x = feature->x;
 	int y = feature->y;
 	parent->ToScriptCoords(&x, &y);
-	SetOverride(feature->line, L"\\org",wxString::Format(L"(%i,%i)",x,y));
+	SetOverride(curDiag, L"\\org",wxString::Format(L"(%i,%i)",x,y));
 }

 void VisualToolRotateXY::DoRefresh() {
 	if (!curDiag) return;
 	int posx, posy;
-	GetLinePosition(curDiag,posx,posy,orgx,orgy);
+	GetLinePosition(curDiag,posx,posy,org->x,org->y);
 	GetLineRotation(curDiag,curAngleX,curAngleY,curAngleZ);
 }
--- a/aegisub/src/visual_tool_rotatexy.h
+++ b/aegisub/src/visual_tool_rotatexy.h
@ -44,16 +44,13 @@ class VisualToolRotateXY : public VisualTool<VisualDraggableFeature> {
 	float curAngleX,startAngleX,origAngleX;
 	float curAngleY,startAngleY,origAngleY;
 	float curAngleZ;
-	int orgx,orgy;
+	Feature *org;

 	bool InitializeHold();
 	void UpdateHold();
 	void CommitHold();

-
-	void PopulateFeatureList();
-	void UpdateDrag(VisualDraggableFeature* feature);
-	void CommitDrag(VisualDraggableFeature* feature);
+	void CommitDrag(feature_iterator feature);

 	void DoRefresh();

--- a/aegisub/src/visual_tool_rotatez.cpp
+++ b/aegisub/src/visual_tool_rotatez.cpp
@ -54,6 +54,9 @@ static const float rad2deg = 180.f / 3.1415926536f;
 VisualToolRotateZ::VisualToolRotateZ(VideoDisplay *parent, VideoState const& video, wxToolBar *)
 : VisualTool<VisualDraggableFeature>(parent, video)
 {
+	features.resize(1);
+	org = &features.back();
+	org->type = DRAG_BIG_TRIANGLE;
 	DoRefresh();
 }

@ -63,7 +66,7 @@ void VisualToolRotateZ::Draw() {
 	// Draw pivot
 	DrawAllFeatures();

-	int radius = (int)sqrt(double((posx-orgx)*(posx-orgx)+(posy-orgy)*(posy-orgy)));
+	int radius = (int)sqrt(double((posx-org->x)*(posx-org->x)+(posy-org->y)*(posy-org->y)));
 	int oRadius = radius;
 	if (radius < 50) radius = 50;

@ -78,7 +81,7 @@ void VisualToolRotateZ::Draw() {
 	glMatrixMode(GL_MODELVIEW);
 	glPushMatrix();
 	glLoadIdentity();
-	glTranslatef(orgx,orgy,-1.f);
+	glTranslatef(org->x,org->y,-1.f);
 	float matrix[16] = { 2500, 0, 0, 0, 0, 2500, 0, 0, 0, 0, 1, 1, 0, 0, 2500, 2500 };
 	glMultMatrixf(matrix);
 	glScalef(1.f,1.f,8.f);
@ -103,8 +106,8 @@ void VisualToolRotateZ::Draw() {
 	DrawLine(deltax,deltay,-deltax,-deltay);

 	// Draw the connection line
-	if (orgx != posx || orgy != posy) {
-		double angle = atan2(double(orgy-posy),double(posx-orgx)) + curAngle*deg2rad;
+	if (org->x != posx || org->y != posy) {
+		double angle = atan2(double(org->y-posy),double(posx-org->x)) + curAngle*deg2rad;
 		int fx = int(cos(angle)*oRadius);
 		int fy = -int(sin(angle)*oRadius);
 		DrawLine(0,0,fx,fy);
@ -121,14 +124,14 @@ void VisualToolRotateZ::Draw() {
 	glPopMatrix();

 	// Draw line to mouse
-	if (!dragging && !curFeature && video.x > INT_MIN && video.y > INT_MIN) {
+	if (!dragging && curFeature == features.end() && video.x > INT_MIN && video.y > INT_MIN) {
 		SetLineColour(colour[0]);
-		DrawLine(orgx,orgy,video.x,video.y);
+		DrawLine(org->x,org->y,video.x,video.y);
 	}
 }

 bool VisualToolRotateZ::InitializeHold() {
-	startAngle = atan2(double(orgy-video.y),double(video.x-orgx)) * rad2deg;
+	startAngle = atan2(double(org->y-video.y),double(video.x-org->x)) * rad2deg;
 	origAngle = curAngle;
 	curDiag->StripTag(L"\\frz");
 	curDiag->StripTag(L"\\fr");
@ -137,7 +140,7 @@ bool VisualToolRotateZ::InitializeHold() {
 }

 void VisualToolRotateZ::UpdateHold() {
-	float screenAngle = atan2(double(orgy-video.y),double(video.x-orgx)) * rad2deg;
+	float screenAngle = atan2(double(org->y-video.y),double(video.x-org->x)) * rad2deg;
 	curAngle = fmodf(screenAngle - startAngle + origAngle + 360.f, 360.f);

 	// Oh Snap
@ -154,33 +157,16 @@ void VisualToolRotateZ::CommitHold() {
 	}
 }

-void VisualToolRotateZ::PopulateFeatureList() {
-	if (!curDiag) return;
-
-	// Set features
-	features.resize(1);
-	VisualDraggableFeature &feat = features.back();
-	feat.x = orgx;
-	feat.y = orgy;
-	feat.line = curDiag;
-	feat.type = DRAG_BIG_TRIANGLE;
-}
-
-void VisualToolRotateZ::UpdateDrag(VisualDraggableFeature* feature) {
-	orgx = feature->x;
-	orgy = feature->y;
-}
-
-void VisualToolRotateZ::CommitDrag(VisualDraggableFeature* feature) {
+void VisualToolRotateZ::CommitDrag(feature_iterator feature) {
 	int x = feature->x;
 	int y = feature->y;
 	parent->ToScriptCoords(&x, &y);
-	SetOverride(feature->line, L"\\org",wxString::Format(L"(%i,%i)",x,y));
+	SetOverride(curDiag, L"\\org",wxString::Format(L"(%i,%i)",x,y));
 }

 void VisualToolRotateZ::DoRefresh() {
 	if (!curDiag) return;
-	GetLinePosition(curDiag, posx, posy, orgx, orgy);
+	GetLinePosition(curDiag, posx, posy, org->x, org->y);
 	GetLineRotation(curDiag, rx, ry, curAngle);
 	GetLineScale(curDiag, scaleX, scaleY);
 }
--- a/aegisub/src/visual_tool_rotatez.h
+++ b/aegisub/src/visual_tool_rotatez.h
@ -45,7 +45,7 @@
 /// DOCME
 class VisualToolRotateZ : public VisualTool<VisualDraggableFeature> {
 	float curAngle, startAngle, origAngle;
-	int orgx, orgy;
+	Feature *org;
 	int posx, posy;
 	float rx, ry;
 	float scaleX, scaleY;
@ -54,9 +54,7 @@ class VisualToolRotateZ : public VisualTool<VisualDraggableFeature> {
 	void UpdateHold();
 	void CommitHold();

-	void PopulateFeatureList();
-	void UpdateDrag(VisualDraggableFeature* feature);
-	void CommitDrag(VisualDraggableFeature* feature);
+	void CommitDrag(feature_iterator feature);

 	void DoRefresh();

--- a/aegisub/src/visual_tool_vector_clip.cpp
+++ b/aegisub/src/visual_tool_vector_clip.cpp
@ -53,6 +53,7 @@

 #include "ass_dialogue.h"
 #include "libresrc/libresrc.h"
+#include "utils.h"
 #include "video_display.h"
 #include "visual_tool_vector_clip.h"

@ -69,12 +70,19 @@ enum {
 	BUTTON_LAST // Leave this at the end and don't use it
 };

+template<class C, class O, class M>
+static void for_each_iter(C &container, O obj, M method) {
+	C::iterator end = container.end();
+	for (C::iterator cur = container.begin(); cur != end; ++cur) {
+		(obj ->* method)(cur);
+	}
+}
+
 VisualToolVectorClip::VisualToolVectorClip(VideoDisplay *parent, VideoState const& video, wxToolBar * toolBar)
 : VisualTool<VisualToolVectorClipDraggableFeature>(parent, video)
 , spline(*parent)
 , toolBar(toolBar)
 {
-	// Create toolbar
 	toolBar->AddTool(BUTTON_DRAG,_("Drag"),GETIMAGE(visual_vector_clip_drag_24),_("Drag control points."),wxITEM_CHECK);
 	toolBar->AddTool(BUTTON_LINE,_("Line"),GETIMAGE(visual_vector_clip_line_24),_("Appends a line."),wxITEM_CHECK);
 	toolBar->AddTool(BUTTON_BICUBIC,_("Bicubic"),GETIMAGE(visual_vector_clip_bicubic_24),_("Appends a bezier bicubic curve."),wxITEM_CHECK);
@ -184,7 +192,7 @@ void VisualToolVectorClip::Draw() {
 	spline.GetClosestParametricPoint(Vector2D(video.x, video.y), highCurve, t, pt);

 	// Draw highlighted line
-	if ((mode == 3 || mode == 4) && !curFeature && points.size() > 2) {
+	if ((mode == 3 || mode == 4) && curFeature == features.end() && points.size() > 2) {
 		std::vector<float> highPoints;
 		spline.GetPointList(highPoints, highCurve);
 		if (!highPoints.empty()) {
@ -223,7 +231,7 @@ void VisualToolVectorClip::Draw() {
 }

 void VisualToolVectorClip::MakeFeature(Spline::iterator cur) {
-	VisualToolVectorClipDraggableFeature feat;
+	Feature feat;
 	if (cur->type == CURVE_POINT) {
 		feat.x = (int)cur->p1.x;
 		feat.y = (int)cur->p1.y;
@ -250,6 +258,7 @@ void VisualToolVectorClip::MakeFeature(Spline::iterator cur) {
 		feat.point = 1;
 		feat.type = DRAG_SMALL_SQUARE;
 		features.push_back(feat);
+
 		feat.x = (int)cur->p3.x;
 		feat.y = (int)cur->p3.y;
 		feat.point = 2;
@ -264,28 +273,33 @@ void VisualToolVectorClip::MakeFeature(Spline::iterator cur) {
 	}
 }

-void VisualToolVectorClip::PopulateFeatureList() {
+void VisualToolVectorClip::MakeFeatures() {
+	ClearSelection();
 	features.clear();
-	// This is perhaps a bit conservative as there can be up to 3N+1 features
-	features.reserve(spline.size());
-
-	for (Spline::iterator cur = spline.begin(); cur != spline.end(); ++cur) {
-		MakeFeature(cur);
-	}
+	for_each_iter(spline, this, &VisualToolVectorClip::MakeFeature);
 }

-void VisualToolVectorClip::UpdateDrag(VisualToolVectorClipDraggableFeature* feature) {
-	spline.MovePoint(feature->curve,feature->point,Vector2D(feature->x,feature->y));
-}
-
-void VisualToolVectorClip::CommitDrag(VisualToolVectorClipDraggableFeature*) {
+void VisualToolVectorClip::Save() {
 	SetOverride(curDiag, inverse ? L"\\iclip" : L"\\clip", L"(" + spline.EncodeToASS() + L")");
 }

-bool VisualToolVectorClip::InitializeDrag(VisualToolVectorClipDraggableFeature* feature) {
-	// Delete a control point
-	if (mode == 5) {
-		// Update next
+void VisualToolVectorClip::UpdateDrag(feature_iterator feature) {
+	spline.MovePoint(feature->curve,feature->point,Vector2D(feature->x,feature->y));
+}
+
+void VisualToolVectorClip::CommitDrag(feature_iterator) {
+	Save();
+}
+
+bool VisualToolVectorClip::InitializeDrag(feature_iterator feature) {
+	if (mode != 5) return true;
+
+	if (feature->curve->type == CURVE_BICUBIC && (feature->point == 1 || feature->point == 2)) {
+		// Deleting bicubic curve handles, so convert to line
+		feature->curve->type = CURVE_LINE;
+		feature->curve->p2 = feature->curve->p4;
+	}
+	else {
 		Spline::iterator next = feature->curve;
 		next++;
 		if (next != spline.end()) {
@ -298,16 +312,15 @@ bool VisualToolVectorClip::InitializeDrag(VisualToolVectorClipDraggableFeature*
 			}
 		}

-		// Erase and save changes
 		spline.erase(feature->curve);
-		CommitDrag(feature);
-		curFeature = NULL;
-		PopulateFeatureList();
-		ClearSelection();
-		Commit(true);
-		return false;
 	}
-	return true;
+	curFeature = features.end();
+
+	Save();
+	MakeFeatures();
+	Commit(true, _("delete control point"));
+
+	return false;
 }

 bool VisualToolVectorClip::InitializeHold() {
@ -318,8 +331,7 @@ bool VisualToolVectorClip::InitializeHold() {
 		// Set start position
 		if (!spline.empty()) {
 			curve.p1 = spline.back().EndPoint();
-			if (mode == 1) curve.type = CURVE_LINE;
-			else curve.type = CURVE_BICUBIC;
+			curve.type = mode == 1 ? CURVE_LINE : CURVE_BICUBIC;
 		}
 		
 		// First point
@ -383,9 +395,9 @@ bool VisualToolVectorClip::InitializeHold() {
 		}

 		// Commit
-		SetOverride(curDiag, inverse ? L"\\iclip" : L"\\clip", L"(" + spline.EncodeToASS() + L")");
+		Save();
+		MakeFeatures();
 		Commit(true);
-		DoRefresh();
 		return false;
 	}

@ -430,7 +442,7 @@ void VisualToolVectorClip::UpdateHold() {
 		}
 		else curve.p2 = curve.p1 * 0.75 + curve.p4 * 0.25;
 		curve.p3 = curve.p1 * 0.25 + curve.p4 * 0.75;
-		PopulateFeatureList();
+		MakeFeatures();
 	}

 	// Freehand
@ -452,21 +464,19 @@ void VisualToolVectorClip::UpdateHold() {
 }

 void VisualToolVectorClip::CommitHold() {
+	if (mode == 3 || mode == 4) return;
+
 	// Smooth spline
 	if (!holding && mode == 7) {
 		spline.Smooth();
-		PopulateFeatureList();
 	}

-	// Save it
-	if (mode != 3 && mode != 4) {
-		SetOverride(curDiag, inverse ? L"\\iclip" : L"\\clip", L"(" + spline.EncodeToASS() + L")");
-	}
+	Save();

 	// End freedraw
 	if (!holding && (mode == 6 || mode == 7)) {
 		SetMode(0);
-		SelectAll();
+		MakeFeatures();
 	}
 }

@ -477,13 +487,12 @@ void VisualToolVectorClip::DoRefresh() {
 	int scale;
 	vect = GetLineVectorClip(curDiag,scale,inverse);
 	spline.DecodeFromASS(vect);
+
+	MakeFeatures();
 	SelectAll();
-	PopulateFeatureList();
 }

 void VisualToolVectorClip::SelectAll() {
 	ClearSelection();
-	for (size_t i = 0; i < features.size(); ++i) {
-		AddSelection(i);
-	}
+	for_each_iter(features, this, &VisualToolVectorClip::AddSelection);
 }
--- a/aegisub/src/visual_tool_vector_clip.h
+++ b/aegisub/src/visual_tool_vector_clip.h
@ -69,17 +69,19 @@ class VisualToolVectorClip : public VisualTool<VisualToolVectorClipDraggableFeat
 	/// @param mode 0-7
 	void SetMode(int mode);

+	void Save();
+
 	void SelectAll();
 	void MakeFeature(Spline::iterator cur);
+	void MakeFeatures();

 	bool InitializeHold();
 	void UpdateHold();
 	void CommitHold();

-	void PopulateFeatureList();
-	void UpdateDrag(VisualToolVectorClipDraggableFeature* feature);
-	void CommitDrag(VisualToolVectorClipDraggableFeature* feature);
-	bool InitializeDrag(VisualToolVectorClipDraggableFeature* feature);
+	void UpdateDrag(feature_iterator feature);
+	void CommitDrag(feature_iterator feature);
+	bool InitializeDrag(feature_iterator feature);

 	void DoRefresh();
 	void Draw();