GUI: fix paste overflow undo corruption, part 1

TODO: garbage collection

src/gui/editing.cpp

@@ -52,7 +52,16 @@ const char* FurnaceGUI::noteNameNormal(short note, short octave) {
   return noteNames[seek];
 }
 
-void FurnaceGUI::prepareUndo(ActionType action) {
+void FurnaceGUI::prepareUndo(ActionType action, UndoRegion region) {
+  if (region.begin.ord==-1) {
+    region.begin.ord=curOrder;
+    region.end.ord=curOrder;
+    region.begin.x=0;
+    region.end.x=e->getTotalChannelCount()-1;
+    region.begin.y=0;
+    region.end.y=e->curSubSong->patLen-1;
+  }
+
   switch (action) {
     case GUI_UNDO_CHANGE_ORDER:
       memcpy(&oldOrders,e->curOrders,sizeof(DivOrders));
@@ -74,8 +83,21 @@ void FurnaceGUI::prepareUndo(ActionType action) {
     case GUI_UNDO_PATTERN_COLLAPSE:
     case GUI_UNDO_PATTERN_EXPAND:
     case GUI_UNDO_PATTERN_DRAG:
-      for (int i=0; i<e->getTotalChannelCount(); i++) {
+      for (int h=region.begin.ord; h<=region.end.ord; h++) {
-        e->curPat[i].getPattern(e->curOrders->ord[i][curOrder],false)->copyOn(oldPat[i]);
+        for (int i=region.begin.x; i<=region.end.x; i++) {
+          unsigned short id=h|(i<<8);
+          DivPattern* p=NULL;
+
+          auto it=oldPatMap.find(id);
+          if (it==oldPatMap.end()) {
+            p=oldPatMap[id]=new DivPattern;
+            logV("oldPatMap: allocating for %.4x",id);
+          } else {
+            p=it->second;
+          }
+
+          e->curPat[i].getPattern(e->curOrders->ord[i][h],false)->copyOn(p);
+        }
       }
       break;
     case GUI_UNDO_PATTERN_COLLAPSE_SONG:
@@ -86,7 +108,7 @@ void FurnaceGUI::prepareUndo(ActionType action) {
   }
 }
 
-void FurnaceGUI::makeUndo(ActionType action) {
+void FurnaceGUI::makeUndo(ActionType action, UndoRegion region) {
   bool doPush=false;
   bool shallWalk=false;
   UndoStep s;
@@ -99,6 +121,16 @@ void FurnaceGUI::makeUndo(ActionType action) {
   s.newOrdersLen=e->curSubSong->ordersLen;
   s.nibble=curNibble;
   size_t subSong=e->getCurrentSubSong();
+
+  if (region.begin.ord==-1) {
+    region.begin.ord=curOrder;
+    region.end.ord=curOrder;
+    region.begin.x=0;
+    region.end.x=e->getTotalChannelCount()-1;
+    region.begin.y=0;
+    region.end.y=e->curSubSong->patLen-1;
+  }
+
   switch (action) {
     case GUI_UNDO_CHANGE_ORDER:
       for (int i=0; i<DIV_MAX_CHANS; i++) {
@@ -131,24 +163,37 @@ void FurnaceGUI::makeUndo(ActionType action) {
     case GUI_UNDO_PATTERN_COLLAPSE:
     case GUI_UNDO_PATTERN_EXPAND:
     case GUI_UNDO_PATTERN_DRAG:
-      for (int i=0; i<e->getTotalChannelCount(); i++) {
+      for (int h=region.begin.ord; h<=region.end.ord; h++) {
-        DivPattern* p=e->curPat[i].getPattern(e->curOrders->ord[i][curOrder],false);
+        for (int i=region.begin.x; i<=region.end.x; i++) {
-        for (int j=0; j<e->curSubSong->patLen; j++) {
+          DivPattern* p=e->curPat[i].getPattern(e->curOrders->ord[i][h],false);
-          for (int k=0; k<DIV_MAX_COLS; k++) {
+          DivPattern* op=NULL;
-            if (p->data[j][k]!=oldPat[i]->data[j][k]) {
+          unsigned short id=h|(i<<8);
-              s.pat.push_back(UndoPatternData(subSong,i,e->curOrders->ord[i][curOrder],j,k,oldPat[i]->data[j][k],p->data[j][k]));
 
-              if (k>=4) {
+          auto it=oldPatMap.find(id);
-                if (oldPat[i]->data[j][k&(~1)]==0x0b ||
+          if (it==oldPatMap.end()) {
-                    p->data[j][k&(~1)]==0x0b ||
+            logW("no data in oldPatMap for channel %d!",i);
-                    oldPat[i]->data[j][k&(~1)]==0x0d ||
+            continue;
-                    p->data[j][k&(~1)]==0x0d ||
+          } else {
-                    oldPat[i]->data[j][k&(~1)]==0xff ||
+            op=it->second;
-                    p->data[j][k&(~1)]==0xff) {
+          }
-                  shallWalk=true;
+
+          for (int j=region.begin.y; j<=region.end.y; j++) {
+            for (int k=0; k<DIV_MAX_COLS; k++) {
+              if (p->data[j][k]!=op->data[j][k]) {
+                s.pat.push_back(UndoPatternData(subSong,i,e->curOrders->ord[i][h],j,k,op->data[j][k],p->data[j][k]));
+
+                if (k>=4) {
+                  if (op->data[j][k&(~1)]==0x0b ||
+                      p->data[j][k&(~1)]==0x0b ||
+                      op->data[j][k&(~1)]==0x0d ||
+                      p->data[j][k&(~1)]==0x0d ||
+                      op->data[j][k&(~1)]==0xff ||
+                      p->data[j][k&(~1)]==0xff) {
+                    shallWalk=true;
+                  }
                 }
-              }
 
+              }
             }
           }
         }

src/gui/gui.cpp

@@ -6880,10 +6880,6 @@ bool FurnaceGUI::init() {
   ImGui::GetIO().ConfigFlags|=ImGuiConfigFlags_DockingEnable;
   toggleMobileUI(mobileUI,true);
 
-  for (int i=0; i<DIV_MAX_CHANS; i++) {
-    oldPat[i]=new DivPattern;
-  }
-
   firstFrame=true;
 
   userEvents=SDL_RegisterEvents(1);
@@ -7132,10 +7128,6 @@ bool FurnaceGUI::finish() {
     }
   }
 
-  for (int i=0; i<DIV_MAX_CHANS; i++) {
-    delete oldPat[i];
-  }
-
   if (backupTask.valid()) {
     backupTask.get();
   }

src/gui/gui.h

@@ -889,9 +889,17 @@ struct SelectionPoint {
 struct UndoRegion {
   struct UndoRegionPoint {
     int ord, x, y;
+    UndoRegionPoint(int o, int xp, int yp):
+      ord(o), x(xp), y(yp) {}
     UndoRegionPoint():
-      ord(0), x(0), y(0) {}
+      ord(-1), x(-1), y(-1) {}
   } begin, end;
+  UndoRegion(int o0, int x0, int y0, int o1, int x1, int y1):
+    begin(o0,x0,y0),
+    end(o1,x1,y1) {}
+  UndoRegion():
+    begin(),
+    end() {}
 };
 
 enum ActionType {
@@ -2179,7 +2187,7 @@ class FurnaceGUI {
 
   int oldOrdersLen;
   DivOrders oldOrders;
-  DivPattern* oldPat[DIV_MAX_CHANS];
+  std::map<unsigned short,DivPattern*> oldPatMap;
   FixedQueue<UndoStep,256> undoHist;
   FixedQueue<UndoStep,256> redoHist;
 
@@ -2536,8 +2544,8 @@ class FurnaceGUI {
   void moveCursorTop(bool select);
   void moveCursorBottom(bool select);
   void editAdvance();
-  void prepareUndo(ActionType action);
+  void prepareUndo(ActionType action, UndoRegion region=UndoRegion());
-  void makeUndo(ActionType action);
+  void makeUndo(ActionType action, UndoRegion region=UndoRegion());
   void doSelectAll();
   void doDelete();
   void doPullDelete();