instrument editor undo: don't check delta if no user input has come in that could potentially have dirtied the editor

src/engine/instrument.cpp

@@ -376,6 +376,8 @@ bool MemPatch::calcDiff(const void* pre, const void* post, size_t inputSize) {
   const unsigned char* preBytes=(const unsigned char*)pre;
   const unsigned char* postBytes=(const unsigned char*)post;
 
+  // @NOTE: consider/profile using a memcmp==0 check to early-out, if it's potentially faster
+  // for the common case, which is no change
   for (size_t ii=0; ii<inputSize; ++ii) {
     if (preBytes[ii] != postBytes[ii]) {
       lastDiff=ii;
@@ -439,7 +441,7 @@ bool DivInstrumentUndoStep::makeUndoPatch(size_t processTime_, const DivInstrume
   return nameValid || podPatch.isValid();
 }
 
-void DivInstrument::recordUndoStepIfChanged(size_t processTime, const DivInstrument* old) {
+bool DivInstrument::recordUndoStepIfChanged(size_t processTime, const DivInstrument* old) {
   DivInstrumentUndoStep step;
 
   // generate a patch to go back to old
@@ -464,7 +466,10 @@ void DivInstrument::recordUndoStepIfChanged(size_t processTime, const DivInstrum
     undoHist.push_back(stepPtr);
 
     // logI("DivInstrument::undoHist push (%u off, %u size)", stepPtr->podPatch.offset, stepPtr->podPatch.size);
+    return true;
   }
+
+  return false;
 }
 
 int DivInstrument::undo() {

src/engine/instrument.h

@@ -934,7 +934,7 @@ struct DivInstrument : DivInstrumentPOD {
    */
   FixedQueue<DivInstrumentUndoStep*, 128> undoHist;
   FixedQueue<DivInstrumentUndoStep*, 128> redoHist;
-  void recordUndoStepIfChanged(size_t processTime, const DivInstrument* old);
+  bool recordUndoStepIfChanged(size_t processTime, const DivInstrument* old);
   int undo();
   int redo();
 

src/gui/gui.cpp

@@ -3704,6 +3704,7 @@ bool FurnaceGUI::loop() {
       ImGui::GetIO().AddKeyEvent(ImGuiKey_Backspace,false);
       injectBackUp=false;
     }
+
     while (SDL_PollEvent(&ev)) {
       WAKE_UP;
       ImGui_ImplSDL2_ProcessEvent(&ev);
@@ -3720,13 +3721,16 @@ bool FurnaceGUI::loop() {
         }
         case SDL_MOUSEBUTTONUP:
           pointUp(ev.button.x,ev.button.y,ev.button.button);
+          insEditMayBeDirty=true;
           break;
         case SDL_MOUSEBUTTONDOWN:
           pointDown(ev.button.x,ev.button.y,ev.button.button);
+          insEditMayBeDirty=true;
           break;
         case SDL_MOUSEWHEEL:
           wheelX+=ev.wheel.x;
           wheelY+=ev.wheel.y;
+          insEditMayBeDirty=true;
           break;
         case SDL_WINDOWEVENT:
           switch (ev.window.event) {
@@ -3803,12 +3807,14 @@ bool FurnaceGUI::loop() {
           if (!ImGui::GetIO().WantCaptureKeyboard) {
             keyDown(ev);
           }
+          insEditMayBeDirty=true;
 #ifdef IS_MOBILE
           injectBackUp=true;
 #endif
           break;
         case SDL_KEYUP:
           // for now
+          insEditMayBeDirty=true;
           break;
         case SDL_DROPFILE:
           if (ev.drop.file!=NULL) {
@@ -7145,6 +7151,11 @@ bool FurnaceGUI::loop() {
       willCommit=false;
     }
 
+    // To check for instrument editor modification, we need an up-to-date `insEditMayBeDirty`
+    // (based on incoming user input events), and we want any possible instrument modifications
+    // to already have been made.
+    checkRecordInstrumentUndoStep();
+
     if (shallDetectScale) {
       if (--shallDetectScale<1) {
         if (settings.dpiScale<0.5f) {
@@ -8311,6 +8322,7 @@ FurnaceGUI::FurnaceGUI():
   localeRequiresKorean(false),
   prevInsData(NULL),
   cachedCurInsPtr(NULL),
+  insEditMayBeDirty(false),
   pendingLayoutImport(NULL),
   pendingLayoutImportLen(0),
   pendingLayoutImportStep(0),

src/gui/gui.h

@@ -2258,6 +2258,7 @@ class FurnaceGUI {
   DivInstrument* prevInsData;
   DivInstrument cachedCurIns;
   DivInstrument* cachedCurInsPtr;
+  bool insEditMayBeDirty;
 
   unsigned char* pendingLayoutImport;
   size_t pendingLayoutImportLen;
@@ -2924,6 +2925,7 @@ class FurnaceGUI {
   void doUndoSample();
   void doRedoSample();
 
+  void checkRecordInstrumentUndoStep();
   void doUndoInstrument();
   void doRedoInstrument();
 

src/gui/insEdit.cpp

@@ -7740,31 +7740,44 @@ void FurnaceGUI::drawInsEdit() {
       
       ImGui::EndPopup();
     }
-    
-    if (ins) {
-      bool insChanged=ins!=cachedCurInsPtr;
-      bool delayDiff=ImGui::IsMouseDown(ImGuiMouseButton_Left) || ImGui::IsMouseDown(ImGuiMouseButton_Right) || ImGui::GetIO().WantCaptureKeyboard;
-
-      // check against the last cached to see if diff -- note that modifications to instruments happen outside
-      // drawInsEdit (e.g. cursor inputs are processed and can directly modify macro data)
-      if (!insChanged && !delayDiff) {
-        ins->recordUndoStepIfChanged(e->processTime, &cachedCurIns);
-      }
-
-      if (insChanged || !delayDiff) {
-        cachedCurIns=*ins;
-      }
-
-      cachedCurInsPtr=ins;
-    } else {
-      cachedCurInsPtr=NULL;
-    }
   }
-  
+
   if (ImGui::IsWindowFocused(ImGuiFocusedFlags_ChildWindows)) curWindow=GUI_WINDOW_INS_EDIT;
   ImGui::End();
 }
 
+void FurnaceGUI::checkRecordInstrumentUndoStep() {
+  if (curIns>=0 && curIns<(int)e->song.ins.size()) {
+    DivInstrument* ins=e->song.ins[curIns];
+
+    // invalidate cachedCurIns/any possible changes if the cachedCurIns was referencing a different
+    // instrument altgoether
+    bool insChanged=ins!=cachedCurInsPtr;
+    if (insChanged) {
+      insEditMayBeDirty=false;
+      cachedCurInsPtr=ins;
+      cachedCurIns=*ins;
+    }
+
+    cachedCurInsPtr=ins;
+
+    // check against the last cached to see if diff -- note that modifications to instruments
+    // happen outside drawInsEdit (e.g. cursor inputs are processed and can directly modify
+    // macro data).  but don't check until we think the user input is complete.
+    bool delayDiff=ImGui::IsMouseDown(ImGuiMouseButton_Left) || ImGui::IsMouseDown(ImGuiMouseButton_Right) || ImGui::GetIO().WantCaptureKeyboard;
+    if (!delayDiff && insEditMayBeDirty) {
+      bool hasChange=ins->recordUndoStepIfChanged(e->processTime, &cachedCurIns);
+      if (hasChange) {
+        cachedCurIns=*ins;
+      }
+      insEditMayBeDirty=false;
+    }
+  } else {
+    cachedCurInsPtr=NULL;
+    insEditMayBeDirty=false;
+  }
+}
+
 void FurnaceGUI::doUndoInstrument() {
   if (!insEditOpen) return;
   if (curIns<0 || curIns>=(int)e->song.ins.size()) return;