Show saved undo state in Edit > Undo History window (fix #3578)

Some extra changes introduced:
* DocUndo & CmdTransaction were simplified: removing the saved
  counter, and storing a specific UndoState pointing to the state that
  matches the version in the disk
* DocUndo::onDeleteUndoState() can generate a
  impossibleToBackToSavedState() if the saved state is deleted. This
  might fix some bugs where a "save changes" dialog weren't displayed
  after undoing and making changes (probably related to #3542, but not
  sure)

Some extra work is needed to avoid showing the "save changes" dialog
if we are close to the saved state and only non-modification undo
states separate us from there. E.g. if we open a file, select the
canvas, and close it, Aseprite now shows the "save changes" dialog,
this wasn't true in previous versions.

data/extensions/aseprite-theme/dark/theme.xml

@@ -997,6 +997,14 @@
             <text color="listitem_selected_text" align="left middle" x="1" state="selected" />
             <text color="disabled" align="left middle" x="1" state="disabled" />
         </style>
+        <style id="undo_saved_item" extends="list_item">
+            <text color="listitem_normal_text" align="left middle" x="11" />
+            <text color="listitem_selected_text" align="left middle" x="11" state="selected" />
+            <text color="disabled" align="left middle" x="11" state="disabled" />
+            <icon part="icon_save" color="listitem_normal_text" align="left middle" x="1" />
+            <icon part="icon_save" color="listitem_selected_text" align="left middle" x="1" state="selected" />
+            <icon part="icon_save" color="disabled" align="left middle" x="1" state="disabled" />
+        </style>
         <style id="aseprite_face">
             <icon part="aseprite_face" />
             <icon part="aseprite_face_mouse" state="mouse" />

data/extensions/aseprite-theme/theme.xml

@@ -987,6 +987,14 @@
             <text color="listitem_selected_text" align="left middle" x="1" state="selected" />
             <text color="disabled" align="left middle" x="1" state="disabled" />
         </style>
+        <style id="undo_saved_item" extends="list_item">
+            <text color="listitem_normal_text" align="left middle" x="11" />
+            <text color="listitem_selected_text" align="left middle" x="11" state="selected" />
+            <text color="disabled" align="left middle" x="11" state="disabled" />
+            <icon part="icon_save" color="listitem_normal_text" align="left middle" x="1" />
+            <icon part="icon_save" color="listitem_selected_text" align="left middle" x="1" state="selected" />
+            <icon part="icon_save" color="disabled" align="left middle" x="1" state="disabled" />
+        </style>
         <style id="aseprite_face">
             <icon part="aseprite_face" />
             <icon part="aseprite_face_mouse" state="mouse" />

src/app/cmd_transaction.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019-2020  Igara Studio S.A.
+// Copyright (C) 2019-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -21,21 +21,15 @@
 
 namespace app {
 
-CmdTransaction::CmdTransaction(const std::string& label,
-                               bool changeSavedState,
-                               int* savedCounter)
+CmdTransaction::CmdTransaction(const std::string& label)
   : m_ranges(nullptr)
   , m_label(label)
-  , m_changeSavedState(changeSavedState)
-  , m_savedCounter(savedCounter)
 {
 }
 
 CmdTransaction* CmdTransaction::moveToEmptyCopy()
 {
-  CmdTransaction* copy = new CmdTransaction(m_label,
-                                            m_changeSavedState,
-                                            m_savedCounter);
+  CmdTransaction* copy = new CmdTransaction(m_label);
   copy->m_spritePositionBefore = m_spritePositionBefore;
   copy->m_spritePositionAfter = m_spritePositionAfter;
   if (m_ranges) {
@@ -99,25 +93,16 @@ void CmdTransaction::onExecute()
 
   // Execute the sequence of "cmds"
   CmdSequence::onExecute();
-
-  if (m_changeSavedState)
-    ++(*m_savedCounter);
 }
 
 void CmdTransaction::onUndo()
 {
   CmdSequence::onUndo();
-
-  if (m_changeSavedState)
-    --(*m_savedCounter);
 }
 
 void CmdTransaction::onRedo()
 {
   CmdSequence::onRedo();
-
-  if (m_changeSavedState)
-    ++(*m_savedCounter);
 }
 
 std::string CmdTransaction::onLabel() const

src/app/cmd_transaction.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019  Igara Studio S.A.
+// Copyright (C) 2019-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -22,8 +22,7 @@ namespace app {
   // The whole DocUndo contains a list of these CmdTransaction.
   class CmdTransaction : public CmdSequence {
   public:
-    CmdTransaction(const std::string& label,
-      bool changeSavedState, int* savedCounter);
+    CmdTransaction(const std::string& label);
 
     // Moves the CmdTransaction internals to a new copy in case that
     // we want to rollback this CmdTransaction and start again with
@@ -60,8 +59,6 @@ namespace app {
     SpritePosition m_spritePositionAfter;
     std::unique_ptr<Ranges> m_ranges;
     std::string m_label;
-    bool m_changeSavedState;
-    int* m_savedCounter;
   };
 
 } // namespace app

src/app/commands/cmd_undo_history.cpp

@@ -10,6 +10,7 @@
 #endif
 
 #include "app/cmd.h"
+#include "app/cmd_transaction.h"
 #include "app/commands/command.h"
 #include "app/console.h"
 #include "app/context.h"
@@ -52,8 +53,12 @@ public:
       initTheme();
     }
 
-    void setUndoHistory(DocUndo* history) {
+    void setUndoHistory(Doc* doc, DocUndo* history) {
+      m_doc = doc;
       m_undoHistory = history;
+
+      updateSavedState();
+
       invalidate();
     }
 
@@ -94,6 +99,26 @@ public:
       view->setViewScroll(scroll);
     }
 
+    void updateSavedState() {
+      const auto oldIsAssociatedToFile = m_isAssociatedToFile;
+      const auto oldSavedState = m_savedState;
+
+      m_isAssociatedToFile =
+        (m_doc && m_undoHistory ?
+         m_doc->isAssociatedToFile() && !m_undoHistory->isSavedStateIsLost() :
+         false);
+
+      // Get the state where this sprite was saved.
+      m_savedState = nullptr;
+      if (m_undoHistory && m_isAssociatedToFile)
+        m_savedState = m_undoHistory->savedState();
+
+      if (oldIsAssociatedToFile != m_isAssociatedToFile ||
+          oldSavedState != m_savedState) {
+        invalidate();
+      }
+    }
+
     obs::signal<void(const undo::UndoState*)> Change;
 
   protected:
@@ -283,6 +308,9 @@ public:
         return;
 
       auto style = theme->styles.listItem();
+      if (m_isAssociatedToFile && m_savedState == state) {
+        style = theme->styles.undoSavedItem();
+      }
 
       ui::PaintWidgetPartInfo info;
       info.text = &itemText;
@@ -291,7 +319,10 @@ public:
     }
 
     UndoHistoryWindow* m_window;
+    Doc* m_doc = nullptr;
     DocUndo* m_undoHistory = nullptr;
+    bool m_isAssociatedToFile = false;
+    const undo::UndoState* m_savedState = nullptr;
     int m_itemHeight;
   };
 
@@ -348,7 +379,7 @@ private:
         m_actions.invalidate();
       }
       catch (const std::exception& ex) {
-        selectState(m_doc->undoHistory()->currentState());
+        selectCurrentState();
         Console::showException(ex);
       }
     }
@@ -376,19 +407,21 @@ private:
 
     ++m_nitems;
 
+    m_actions.updateSavedState();
     m_actions.invalidate();
     view()->updateView();
 
-    selectState(history->currentState());
+    selectCurrentState();
   }
 
   void onDeleteUndoState(DocUndo* history,
                          undo::UndoState* state) override {
+    m_actions.updateSavedState();
     --m_nitems;
   }
 
   void onCurrentUndoStateChange(DocUndo* history) override {
-    selectState(history->currentState());
+    selectCurrentState();
   }
 
   void onClearRedo(DocUndo* history) override {
@@ -399,6 +432,10 @@ private:
     updateTitle();
   }
 
+  void onNewSavedState(DocUndo* history) override {
+    m_actions.updateSavedState();
+  }
+
   void attachDocument(Doc* doc) {
     if (m_doc == doc)
       return;
@@ -428,15 +465,15 @@ private:
 
   void setUndoHistory(DocUndo* history) {
     m_nitems = 0;
-    m_actions.setUndoHistory(history);
+    m_actions.setUndoHistory(m_doc, history);
     view()->updateView();
 
     if (history)
-      m_actions.selectState(history->currentState());
+      selectCurrentState();
   }
 
-  void selectState(const undo::UndoState* state) {
-    m_actions.selectState(state);
+  void selectCurrentState() {
+    m_actions.selectState(m_doc->undoHistory()->currentState());
   }
 
   void updateTitle() {

src/app/doc.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2021  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -276,7 +276,7 @@ void Doc::notifyLayerGroupCollapseChange(Layer* layer)
 
 bool Doc::isModified() const
 {
-  return !m_undo->isSavedState();
+  return !m_undo->isInSavedState();
 }
 
 bool Doc::isAssociatedToFile() const
@@ -286,8 +286,8 @@ bool Doc::isAssociatedToFile() const
 
 void Doc::markAsSaved()
 {
-  m_undo->markSavedState();
   m_flags |= kAssociatedToFile;
+  m_undo->markSavedState();
 }
 
 void Doc::impossibleToBackToSavedState()

src/app/doc_undo.cpp

@@ -31,10 +31,6 @@ namespace app {
 
 DocUndo::DocUndo()
   : m_undoHistory(this)
-  , m_ctx(nullptr)
-  , m_totalUndoSize(0)
-  , m_savedCounter(0)
-  , m_savedStateIsLost(false)
 {
 }
 
@@ -144,20 +140,25 @@ void DocUndo::clearRedo()
   notify_observers(&DocUndoObserver::onClearRedo, this);
 }
 
-bool DocUndo::isSavedState() const
+bool DocUndo::isInSavedState() const
 {
-  return (!m_savedStateIsLost && m_savedCounter == 0);
+  return (!m_savedStateIsLost &&
+          m_savedState == currentState());
 }
 
 void DocUndo::markSavedState()
 {
-  m_savedCounter = 0;
+  m_savedState = currentState();
   m_savedStateIsLost = false;
+  notify_observers(&DocUndoObserver::onNewSavedState, this);
 }
 
 void DocUndo::impossibleToBackToSavedState()
 {
+  // Now there is no state related to the disk state.
+  m_savedState = nullptr;
   m_savedStateIsLost = true;
+  notify_observers(&DocUndoObserver::onNewSavedState, this);
 }
 
 std::string DocUndo::nextUndoLabel() const
@@ -270,6 +271,11 @@ void DocUndo::onDeleteUndoState(undo::UndoState* state)
 
   m_totalUndoSize -= cmd->memSize();
   notify_observers(&DocUndoObserver::onDeleteUndoState, this, state);
+
+  // Mark this document as impossible to match the version on disk
+  // because we're just going to delete the saved state.
+  if (m_savedState == state)
+    impossibleToBackToSavedState();
 }
 
 } // namespace app

src/app/doc_undo.h

@@ -44,9 +44,11 @@ namespace app {
 
     void clearRedo();
 
-    bool isSavedState() const;
+    bool isInSavedState() const;
+    bool isSavedStateIsLost() const { return m_savedStateIsLost; }
     void markSavedState();
     void impossibleToBackToSavedState();
+    const undo::UndoState* savedState() const { return m_savedState; }
 
     std::string nextUndoLabel() const;
     std::string nextRedoLabel() const;
@@ -58,8 +60,6 @@ namespace app {
 
     Cmd* lastExecutedCmd() const;
 
-    int* savedCounter() { return &m_savedCounter; }
-
     const undo::UndoState* firstState() const   { return m_undoHistory.firstState(); }
     const undo::UndoState* lastState() const    { return m_undoHistory.lastState(); }
     const undo::UndoState* currentState() const { return m_undoHistory.currentState(); }
@@ -74,19 +74,13 @@ namespace app {
     void onDeleteUndoState(undo::UndoState* state) override;
 
     undo::UndoHistory m_undoHistory;
-    Context* m_ctx;
-    size_t m_totalUndoSize;
-
-    // This counter is equal to 0 if we are in the "saved state", i.e.
-    // the document on memory is equal to the document on disk. This
-    // value is less than 0 if we're in a past version of the document
-    // (due undoes), or greater than 0 if we are in a future version
-    // (due redoes).
-    int m_savedCounter;
+    const undo::UndoState* m_savedState = nullptr;
+    Context* m_ctx = nullptr;
+    size_t m_totalUndoSize = 0;
 
     // True if the saved state was invalidated/corrupted/lost in some
     // way. E.g. If the save process fails.
-    bool m_savedStateIsLost;
+    bool m_savedStateIsLost = false;
 
     DISABLE_COPYING(DocUndo);
   };

src/app/doc_undo_observer.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2021  Igara Studio S.A.
+// Copyright (C) 2021-2022  Igara Studio S.A.
 // Copyright (C) 2015-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -26,6 +26,7 @@ namespace app {
     virtual void onCurrentUndoStateChange(DocUndo* history) { }
     virtual void onClearRedo(DocUndo* history) { }
     virtual void onTotalUndoSizeChange(DocUndo* history) { }
+    virtual void onNewSavedState(DocUndo* history) { }
   };
 
 } // namespace app

src/app/transaction.cpp

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2018-2019  Igara Studio S.A.
+// Copyright (C) 2018-2022  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -45,9 +45,7 @@ Transaction::Transaction(
   m_doc->add_observer(this);
   m_undo = m_doc->undoHistory();
 
-  m_cmds = new CmdTransaction(label,
-    modification == Modification::ModifyDocument,
-    m_undo->savedCounter());
+  m_cmds = new CmdTransaction(label);
 
   // Here we are executing an empty CmdTransaction, just to save the
   // SpritePosition. Sub-cmds are executed then one by one, in

src/undo

@@ -1 +1 @@
-Subproject commit 3cf8f20d08bf38b06b8e0e75757a18e31ddb12d9
+Subproject commit c868a0238973f04564253133c1cd3689f9aa3913