Fix Doc::isModified() when we are in a similar UndoState to the saved one

If the current UndoState doesn't modify the "saved state" (e.g. there
is a sequence of undoes/redoes that doesn't modify the saved version
of the sprite compared to the current one), we can indicate that we
are in the saved state anyway (!Doc::isModified).

src/app/cmd_transaction.cpp

@@ -21,15 +21,18 @@
 
 namespace app {
 
-CmdTransaction::CmdTransaction(const std::string& label)
+CmdTransaction::CmdTransaction(const std::string& label,
+                               bool changeSavedState)
   : m_ranges(nullptr)
   , m_label(label)
+  , m_changeSavedState(changeSavedState)
 {
 }
 
 CmdTransaction* CmdTransaction::moveToEmptyCopy()
 {
-  CmdTransaction* copy = new CmdTransaction(m_label);
+  CmdTransaction* copy = new CmdTransaction(m_label,
+                                            m_changeSavedState);
   copy->m_spritePositionBefore = m_spritePositionBefore;
   copy->m_spritePositionAfter = m_spritePositionAfter;
   if (m_ranges) {

src/app/cmd_transaction.h

@@ -22,7 +22,10 @@ namespace app {
   // The whole DocUndo contains a list of these CmdTransaction.
   class CmdTransaction : public CmdSequence {
   public:
-    CmdTransaction(const std::string& label);
+    CmdTransaction(const std::string& label,
+                   bool changeSavedState);
+
+    bool doesChangeSavedState() const { return m_changeSavedState; }
 
     // Moves the CmdTransaction internals to a new copy in case that
     // we want to rollback this CmdTransaction and start again with
@@ -59,6 +62,7 @@ namespace app {
     SpritePosition m_spritePositionAfter;
     std::unique_ptr<Ranges> m_ranges;
     std::string m_label;
+    bool m_changeSavedState;
   };
 
 } // namespace app

src/app/commands/cmd_undo_history.cpp

@@ -9,6 +9,7 @@
 #include "config.h"
 #endif
 
+#include "app/app.h"
 #include "app/cmd.h"
 #include "app/cmd_transaction.h"
 #include "app/commands/command.h"
@@ -24,6 +25,7 @@
 #include "app/modules/palettes.h"
 #include "app/site.h"
 #include "app/ui/skin/skin_theme.h"
+#include "app/ui/workspace.h"
 #include "base/mem_utils.h"
 #include "fmt/format.h"
 #include "ui/init_theme_event.h"
@@ -422,6 +424,9 @@ private:
 
   void onCurrentUndoStateChange(DocUndo* history) override {
     selectCurrentState();
+
+    // TODO DocView should be an DocUndoObserver and update its state automatically
+    App::instance()->workspace()->updateTabs();
   }
 
   void onClearRedo(DocUndo* history) override {

src/app/doc.cpp

@@ -276,7 +276,7 @@ void Doc::notifyLayerGroupCollapseChange(Layer* layer)
 
 bool Doc::isModified() const
 {
-  return !m_undo->isInSavedState();
+  return !m_undo->isInSavedStateOrSimilar();
 }
 
 bool Doc::isAssociatedToFile() const

src/app/doc_undo.cpp

@@ -140,10 +140,50 @@ void DocUndo::clearRedo()
   notify_observers(&DocUndoObserver::onClearRedo, this);
 }
 
-bool DocUndo::isInSavedState() const
+bool DocUndo::isInSavedStateOrSimilar() const
 {
-  return (!m_savedStateIsLost &&
+  if (m_savedStateIsLost)
-          m_savedState == currentState());
+    return false;
+
+  // Here we try to find if we can reach the saved state from the
+  // currentState() undoing or redoing and the sprite is exactly the
+  // same as the saved state, e.g. this can happen if the undo states
+  // don't modify the sprite (like actions that change the current
+  // selection/mask boundaries).
+  bool savedStateWithUndoes = true;
+
+  auto state = currentState();
+  while (state) {
+    if (m_savedState == state) {
+      return true;
+    }
+    else if (STATE_CMD(state)->doesChangeSavedState()) {
+      savedStateWithUndoes = false;
+      break;
+    }
+    state = state->prev();
+  }
+
+  // If we reached the end of the undo history (e.g. because all undo
+  // states do not modify the sprite), the only way to be in the saved
+  // state is if the initial point of history is the saved state too
+  // i.e. when m_savedState is nullptr (and m_savedStateIsLost is
+  // false).
+  if (savedStateWithUndoes && m_savedState == nullptr)
+    return true;
+
+  // Now we try with redoes.
+  state = (currentState() ? currentState()->next(): firstState());
+  while (state) {
+    if (STATE_CMD(state)->doesChangeSavedState()) {
+      return false;
+    }
+    else if (m_savedState == state) {
+      return true;
+    }
+    state = state->next();
+  }
+  return false;
 }
 
 void DocUndo::markSavedState()

src/app/doc_undo.h

@@ -44,11 +44,33 @@ namespace app {
 
     void clearRedo();
 
-    bool isInSavedState() const;
+    // Returns true we are in the UndoState that matches the sprite
+    // version on the disk (or we are in a similar state that doesn't
+    // modify that same state, e.g. if the current state modifies the
+    // selection but not the pixels, we are in a similar state)
+    bool isInSavedStateOrSimilar() const;
+
+    // Returns true if the saved state was lost, e.g. because we
+    // deleted the redo history and the saved state was there.
     bool isSavedStateIsLost() const { return m_savedStateIsLost; }
+
+    // Marks current UndoState as the one that matches the sprite on
+    // the disk (this is used after saving the file).
     void markSavedState();
+
+    // Indicates that now it's impossible to back to the version of
+    // the sprite that matches the saved version. This can be because
+    // the save process fails or because we deleted the redo history
+    // where the saved state was available.
     void impossibleToBackToSavedState();
-    const undo::UndoState* savedState() const { return m_savedState; }
+
+    // Returns the position in the undo history where this sprite was
+    // saved, if this is nullptr, it means that the initial state is
+    // the saved state (if m_savedStateIsLost is false) or it means
+    // that there is no saved state (ifm_savedStateIsLost is true)
+    const undo::UndoState* savedState() const {
+      return m_savedState;
+    }
 
     std::string nextUndoLabel() const;
     std::string nextRedoLabel() const;

src/app/script/sprite_class.cpp

@@ -617,6 +617,14 @@ int Sprite_get_filename(lua_State* L)
   return 1;
 }
 
+int Sprite_get_isModified(lua_State* L)
+{
+  auto sprite = get_docobj<Sprite>(L, 1);
+  Doc* doc = static_cast<Doc*>(sprite->document());
+  lua_pushboolean(L, doc->isModified());
+  return 1;
+}
+
 int Sprite_get_width(lua_State* L)
 {
   auto sprite = get_docobj<Sprite>(L, 1);
@@ -854,6 +862,7 @@ const luaL_Reg Sprite_methods[] = {
 
 const Property Sprite_properties[] = {
   { "filename", Sprite_get_filename, Sprite_set_filename },
+  { "isModified", Sprite_get_isModified, nullptr },
   { "width", Sprite_get_width, Sprite_set_width },
   { "height", Sprite_get_height, Sprite_set_height },
   { "colorMode", Sprite_get_colorMode, nullptr },

src/app/transaction.cpp

@@ -45,7 +45,8 @@ Transaction::Transaction(
   m_doc->add_observer(this);
   m_undo = m_doc->undoHistory();
 
-  m_cmds = new CmdTransaction(label);
+  m_cmds = new CmdTransaction(label,
+                              modification == Modification::ModifyDocument);
 
   // Here we are executing an empty CmdTransaction, just to save the
   // SpritePosition. Sub-cmds are executed then one by one, in

tests/scripts/sprite_modified.lua

@@ -0,0 +1,66 @@
+-- Copyright (C) 2022  Igara Studio S.A.
+--
+-- This file is released under the terms of the MIT license.
+-- Read LICENSE.txt for more information.
+
+local fn = '_test_modified.png'
+
+do
+  local doc = Sprite(2, 2)
+
+  -- New sprites are created without modifications (can be closed
+  -- without warning)
+  assert(not doc.isModified)
+
+  doc.width = 3
+  assert(doc.isModified)
+  app.undo()
+  assert(not doc.isModified)
+  app.redo()
+  assert(doc.isModified)
+
+  -- Not modified after it's saved
+  doc:saveAs(fn)
+  assert(not doc.isModified)
+
+  -- Modified if we undo the saved state
+  app.undo()
+  assert(doc.width == 2)
+  assert(doc.isModified)
+  app.redo()
+  assert(doc.width == 3)
+  assert(not doc.isModified)
+
+  -- Selection changes shouldn't change the modified flag
+  app.command.MaskAll()
+  assert(not doc.isModified)
+  app.command.DeselectMask()
+  assert(not doc.isModified)
+  doc:saveAs(fn)
+  assert(not doc.isModified)
+  app.undo()                    -- Undo Deselect
+  assert(not doc.isModified)
+  app.undo()                    -- Undo Select All
+  assert(not doc.isModified)
+  assert(doc.width == 3)
+  app.undo()                    -- Undo size change
+  assert(doc.isModified)
+  assert(doc.width == 2)
+end
+
+do
+  local doc = Sprite{ fromFile=fn }
+
+  -- Loaded sprites are created without modifications (can be closed
+  -- without warning)
+  assert(not doc.isModified)
+
+  app.command.MaskAll()
+  assert(not doc.isModified)
+  doc:saveAs(fn)
+  assert(not doc.isModified)
+  app.undo()
+  assert(not doc.isModified)
+  app.redo()
+  assert(not doc.isModified)
+end