1961: Restore undo collation for vertex operations (TrenchBroom#1962)

* 1961: Restore undo collation for vertex commands. * 1961: Minor cleanup.
lineCode · Jan 9, 2018 · 05e49a4 · 05e49a4
1 parent 20c49d4
commit 05e49a4
Show file tree

Hide file tree

Showing 6 changed files with 62 additions and 18 deletions.
diff --git a/common/src/View/MapDocumentCommandFacade.cpp b/common/src/View/MapDocumentCommandFacade.cpp
@@ -727,6 +727,7 @@ namespace TrenchBroom {
 
             invalidateSelectionBounds();
 
+            VectorUtils::sortAndRemoveDuplicates(newVertexPositions);
             return newVertexPositions;
         }
 
@@ -744,7 +745,10 @@ namespace TrenchBroom {
                 const Edge3::List newPositions = brush->moveEdges(m_worldBounds, oldPositions, delta);
                 VectorUtils::append(newEdgePositions, newPositions);
             }
-
+
+            invalidateSelectionBounds();
+
+            VectorUtils::sortAndRemoveDuplicates(newEdgePositions);
             return newEdgePositions;
         }
 
@@ -765,6 +769,7 @@ namespace TrenchBroom {
 
             invalidateSelectionBounds();
 
+            VectorUtils::sortAndRemoveDuplicates(newFacePositions);
             return newFacePositions;
         }
 

diff --git a/common/src/View/MoveBrushEdgesCommand.cpp b/common/src/View/MoveBrushEdgesCommand.cpp
@@ -62,9 +62,15 @@ namespace TrenchBroom {
         }
 
         bool MoveBrushEdgesCommand::doCollateWith(UndoableCommand::Ptr command) {
-            // Don't collate vertex moves. Collation changes the path along which the vertices are moved, and as a result
-            // changes the outcome of the entire operation.
-            return false;
+            MoveBrushEdgesCommand* other = static_cast<MoveBrushEdgesCommand*>(command.get());
+
+            if (!VectorUtils::equals(m_newEdgePositions, other->m_oldEdgePositions))
+                return false;
+
+            m_newEdgePositions = other->m_newEdgePositions;
+            m_delta += other->m_delta;
+
+            return true;
         }
 
         void MoveBrushEdgesCommand::doSelectNewHandlePositions(VertexHandleManagerBaseT<Edge3>& manager) const {

diff --git a/common/src/View/MoveBrushFacesCommand.cpp b/common/src/View/MoveBrushFacesCommand.cpp
@@ -63,9 +63,15 @@ namespace TrenchBroom {
         }
 
         bool MoveBrushFacesCommand::doCollateWith(UndoableCommand::Ptr command) {
-            // Don't collate vertex moves. Collation changes the path along which the vertices are moved, and as a result
-            // changes the outcome of the entire operation.
-            return false;
+            MoveBrushFacesCommand* other = static_cast<MoveBrushFacesCommand*>(command.get());
+
+            if (!VectorUtils::equals(m_newFacePositions, other->m_oldFacePositions))
+                return false;
+
+            m_newFacePositions = other->m_newFacePositions;
+            m_delta += other->m_delta;
+
+            return true;
         }
 
 

diff --git a/common/src/View/MoveBrushVerticesCommand.cpp b/common/src/View/MoveBrushVerticesCommand.cpp
@@ -65,9 +65,15 @@ namespace TrenchBroom {
         }
 
         bool MoveBrushVerticesCommand::doCollateWith(UndoableCommand::Ptr command) {
-            // Don't collate vertex moves. Collation changes the path along which the vertices are moved, and as a result
-            // changes the outcome of the entire operation.
-            return false;
+            MoveBrushVerticesCommand* other = static_cast<MoveBrushVerticesCommand*>(command.get());
+
+            if (!VectorUtils::equals(m_newVertexPositions, other->m_oldVertexPositions))
+                return false;
+
+            m_newVertexPositions = other->m_newVertexPositions;
+            m_delta += other->m_delta;
+
+            return true;
         }
 
         void MoveBrushVerticesCommand::doSelectNewHandlePositions(VertexHandleManagerBaseT<Vec3>& manager) const {

diff --git a/common/src/View/VertexCommand.cpp b/common/src/View/VertexCommand.cpp
@@ -119,20 +119,40 @@ namespace TrenchBroom {
         }
 
         bool VertexCommand::doPerformDo(MapDocumentCommandFacade* document) {
-            if (!doCanDoVertexOperation(document))
-                return false;
-
-            takeSnapshot();
-            return doVertexOperation(document);
+            if (m_snapshot != nullptr) {
+                restoreAndTakeNewSnapshot(document);
+                return true;
+            } else {
+                if (!doCanDoVertexOperation(document))
+                    return false;
+
+                takeSnapshot();
+                return doVertexOperation(document);
+            }
         }
 
         bool VertexCommand::doPerformUndo(MapDocumentCommandFacade* document) {
-            ensure(m_snapshot != nullptr, "snapshot is null");
-            document->restoreSnapshot(m_snapshot);
-            deleteSnapshot();
+            restoreAndTakeNewSnapshot(document);
             return true;
         }
 
+        void VertexCommand::restoreAndTakeNewSnapshot(MapDocumentCommandFacade* document) {
+            ensure(m_snapshot != nullptr, "snapshot is null");
+
+            Model::Snapshot *snapshot = nullptr;
+            try {
+                using std::swap;
+                swap(m_snapshot, snapshot);
+                takeSnapshot();
+
+                document->restoreSnapshot(snapshot);
+                delete snapshot;
+            } catch (...) {
+                delete snapshot;
+                throw;
+            }
+        }
+
         bool VertexCommand::doIsRepeatable(MapDocumentCommandFacade* document) const {
             return false;
         }

diff --git a/common/src/View/VertexCommand.h b/common/src/View/VertexCommand.h
@@ -70,6 +70,7 @@ namespace TrenchBroom {
         private:
             bool doPerformDo(MapDocumentCommandFacade* document);
             bool doPerformUndo(MapDocumentCommandFacade* document);
+            void restoreAndTakeNewSnapshot(MapDocumentCommandFacade* document);
             bool doIsRepeatable(MapDocumentCommandFacade* document) const;
         private:
             void takeSnapshot();