Use one transaction per document (fix https://community.aseprite.org/t/3851)

src/app/context.cpp

@@ -30,7 +30,6 @@ namespace app {
 Context::Context()
   : m_docs(this)
   , m_lastSelectedDoc(nullptr)
-  , m_transaction(nullptr)
 {
   m_docs.add_observer(this);
 }
@@ -228,18 +227,6 @@ void Context::onSetSelectedColors(const doc::PalettePicks& picks)
     activeSiteHandler()->setSelectedColorsInDoc(m_lastSelectedDoc, picks);
 }
 
-void Context::setTransaction(Transaction* transaction)
-{
-  if (transaction) {
-    ASSERT(!m_transaction);
-    m_transaction = transaction;
-  }
-  else {
-    ASSERT(m_transaction);
-    m_transaction = nullptr;
-  }
-}
-
 ActiveSiteHandler* Context::activeSiteHandler() const
 {
   if (!m_activeSiteHandler)

src/app/context.h

@@ -33,7 +33,6 @@ namespace app {
   class Command;
   class Doc;
   class DocView;
-  class Transaction;
 
   class CommandPreconditionException : public base::Exception {
   public:
@@ -97,10 +96,6 @@ namespace app {
       return nullptr;
     }
 
-    // Sets active/running transaction.
-    void setTransaction(Transaction* transaction);
-    Transaction* transaction() { return m_transaction; }
-
     obs::signal<void (CommandExecutionEvent&)> BeforeCommandExecution;
     obs::signal<void (CommandExecutionEvent&)> AfterCommandExecution;
 
@@ -124,7 +119,6 @@ namespace app {
     Docs m_docs;
     ContextFlags m_flags;       // Last updated flags.
     Doc* m_lastSelectedDoc;
-    Transaction* m_transaction;
     mutable std::unique_ptr<ActiveSiteHandler> m_activeSiteHandler;
 
     DISABLE_COPYING(Context);

src/app/doc.cpp

@@ -48,6 +48,7 @@ Doc::Doc(Sprite* sprite)
   : m_ctx(nullptr)
   , m_flags(kMaskVisible)
   , m_undo(new DocUndo)
+  , m_transaction(nullptr)
   // Information about the file format used to load/save this document
   , m_format_options(nullptr)
   // Mask
@@ -88,6 +89,18 @@ void Doc::setContext(Context* ctx)
   onContextChanged();
 }
 
+void Doc::setTransaction(Transaction* transaction)
+{
+  if (transaction) {
+    ASSERT(!m_transaction);
+    m_transaction = transaction;
+  }
+  else {
+    ASSERT(m_transaction);
+    m_transaction = nullptr;
+  }
+}
+
 DocApi Doc::getApi(Transaction& transaction)
 {
   return DocApi(this, transaction);

src/app/doc.h

@@ -72,6 +72,10 @@ namespace app {
     Context* context() const { return m_ctx; }
     void setContext(Context* ctx);
 
+    // Sets active/running transaction.
+    void setTransaction(Transaction* transaction);
+    Transaction* transaction() { return m_transaction; }
+
     // Returns a high-level API: observable and undoable methods.
     DocApi getApi(Transaction& transaction);
 
@@ -207,6 +211,10 @@ namespace app {
     // Undo and redo information about the document.
     std::unique_ptr<DocUndo> m_undo;
 
+    // Current transaction for this document (when this is commit(), a
+    // new undo command is added to m_undo).
+    Transaction* m_transaction;
+
     // Selected mask region boundaries
     std::unique_ptr<doc::MaskBoundaries> m_maskBoundaries;
 

src/app/doc_range_ops.cpp

@@ -19,6 +19,7 @@
 #include "app/doc_api.h"
 #include "app/doc_range.h"
 #include "app/transaction.h"
+#include "app/tx.h"
 #include "doc/layer.h"
 #include "doc/sprite.h"
 
@@ -331,8 +332,8 @@ static DocRange drop_range_op(
     const app::Context* context = static_cast<app::Context*>(doc->context());
     const ContextReader reader(context);
     ContextWriter writer(reader, 500);
-    Transaction transaction(writer.context(), undoLabel, ModifyDocument);
-    DocApi api = doc->getApi(transaction);
+    Tx tx(writer.context(), undoLabel, ModifyDocument);
+    DocApi api = doc->getApi(tx);
 
     // TODO Try to add the range with just one call to DocApi
     // methods, to avoid generating a lot of cmd::SetCelFrame (see
@@ -455,9 +456,9 @@ static DocRange drop_range_op(
     }
 
     if (resultRange.type() != DocRange::kNone)
-      transaction.setNewDocRange(resultRange);
+      tx.setNewDocRange(resultRange);
 
-    transaction.commit();
+    tx.commit();
   }
 
   return resultRange;
@@ -488,8 +489,8 @@ void reverse_frames(Doc* doc, const DocRange& range)
   const app::Context* context = static_cast<app::Context*>(doc->context());
   const ContextReader reader(context);
   ContextWriter writer(reader, 500);
-  Transaction transaction(writer.context(), "Reverse Frames");
-  DocApi api = doc->getApi(transaction);
+  Tx tx(writer.context(), "Reverse Frames");
+  DocApi api = doc->getApi(tx);
   Sprite* sprite = doc->sprite();
   LayerList layers;
   frame_t frameBegin, frameEnd;
@@ -544,8 +545,8 @@ void reverse_frames(Doc* doc, const DocRange& range)
     }
   }
 
-  transaction.setNewDocRange(range);
-  transaction.commit();
+  tx.setNewDocRange(range);
+  tx.commit();
 }
 
 } // namespace app

src/app/transaction.cpp

@@ -24,9 +24,13 @@ namespace app {
 
 using namespace doc;
 
-Transaction::Transaction(Context* ctx, const std::string& label, Modification modification)
+Transaction::Transaction(
+  Context* ctx,
+  Doc* doc,
+  const std::string& label,
+  Modification modification)
   : m_ctx(ctx)
-  , m_doc(nullptr)
+  , m_doc(doc)
   , m_undo(nullptr)
   , m_cmds(nullptr)
   , m_changes(Changes::kNone)
@@ -36,10 +40,6 @@ Transaction::Transaction(Context* ctx, const std::string& label, Modification mo
            modification == ModifyDocument ? "modifies document":
                                             "doesn't modify document");
 
-  m_doc = m_ctx->activeDocument();
-  if (!m_doc)
-    throw std::runtime_error("No active document to execute a transaction");
-
   m_doc->add_observer(this);
   m_undo = m_doc->undoHistory();
 

src/app/transaction.h

@@ -46,7 +46,11 @@ namespace app {
     // Starts a undoable sequence of operations in a transaction that
     // can be committed or rollbacked.  All the operations will be
     // grouped in the sprite's undo as an atomic operation.
-    Transaction(Context* ctx, const std::string& label, Modification mod = ModifyDocument);
+    Transaction(
+      Context* ctx,
+      Doc* doc,
+      const std::string& label,
+      Modification mod = ModifyDocument);
     virtual ~Transaction();
 
     // Can be used to change the new document range resulting from

src/app/tx.h

@@ -11,22 +11,31 @@
 
 #include "app/app.h"
 #include "app/context.h"
+#include "app/doc.h"
 #include "app/transaction.h"
 
+#include <stdexcept>
+
 namespace app {
 
   // Wrapper to create a new transaction or get the current
   // transaction in the context.
   class Tx {
   public:
-    Tx(Context* ctx, const std::string& label = "Transaction", Modification mod = ModifyDocument)
-      : m_ctx(ctx) {
-      m_transaction = m_ctx->transaction();
+    Tx(Context* ctx,
+       const std::string& label = "Transaction",
+       Modification mod = ModifyDocument)
+    {
+      m_doc = ctx->activeDocument();
+      if (!m_doc)
+        throw std::runtime_error("No active document to execute a transaction");
+
+      m_transaction = m_doc->transaction();
       if (m_transaction)
         m_owner = false;
       else {
-        m_transaction = new Transaction(m_ctx, label, mod);
-        m_ctx->setTransaction(m_transaction);
+        m_transaction = new Transaction(ctx, m_doc, label, mod);
+        m_doc->setTransaction(m_transaction);
         m_owner = true;
       }
     }
@@ -38,7 +47,7 @@ namespace app {
 
     ~Tx() {
       if (m_owner) {
-        m_ctx->setTransaction(nullptr);
+        m_doc->setTransaction(nullptr);
         delete m_transaction;
       }
     }
@@ -48,6 +57,10 @@ namespace app {
         m_transaction->commit();
     }
 
+    void setNewDocRange(const DocRange& range) {
+      m_transaction->setNewDocRange(range);
+    }
+
     void rollbackAndStartAgain() {
       m_transaction->rollbackAndStartAgain();
     }
@@ -61,7 +74,7 @@ namespace app {
     }
 
   private:
-    Context* m_ctx;
+    Doc* m_doc;
     Transaction* m_transaction;
     bool m_owner;               // Owner of the transaction
   };