From b0a8b502467fdbd91b2704f88c2823fff98aa989 Mon Sep 17 00:00:00 2001 From: David Capello Date: Wed, 11 Sep 2019 19:12:11 -0300 Subject: [PATCH] Use one transaction per document (fix https://community.aseprite.org/t/3851) --- src/app/context.cpp | 13 ------------- src/app/context.h | 6 ------ src/app/doc.cpp | 13 +++++++++++++ src/app/doc.h | 8 ++++++++ src/app/doc_range_ops.cpp | 17 +++++++++-------- src/app/transaction.cpp | 12 ++++++------ src/app/transaction.h | 6 +++++- src/app/tx.h | 27 ++++++++++++++++++++------- 8 files changed, 61 insertions(+), 41 deletions(-) diff --git a/src/app/context.cpp b/src/app/context.cpp index 15f03bc95..5008a3a0f 100644 --- a/src/app/context.cpp +++ b/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) diff --git a/src/app/context.h b/src/app/context.h index d80aead6d..eb39e6d3b 100644 --- a/src/app/context.h +++ b/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 BeforeCommandExecution; obs::signal 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 m_activeSiteHandler; DISABLE_COPYING(Context); diff --git a/src/app/doc.cpp b/src/app/doc.cpp index 65855c6f7..c96a0388b 100644 --- a/src/app/doc.cpp +++ b/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); diff --git a/src/app/doc.h b/src/app/doc.h index edf3aba8a..a8cb03ab6 100644 --- a/src/app/doc.h +++ b/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 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 m_maskBoundaries; diff --git a/src/app/doc_range_ops.cpp b/src/app/doc_range_ops.cpp index 0012a4d12..1665576da 100644 --- a/src/app/doc_range_ops.cpp +++ b/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(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(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 diff --git a/src/app/transaction.cpp b/src/app/transaction.cpp index 4c5d3169c..ef8c0f86b 100644 --- a/src/app/transaction.cpp +++ b/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(); diff --git a/src/app/transaction.h b/src/app/transaction.h index 8f9807195..1cacf40e5 100644 --- a/src/app/transaction.h +++ b/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 diff --git a/src/app/tx.h b/src/app/tx.h index 916efb8ab..9f9c483bf 100644 --- a/src/app/tx.h +++ b/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 + 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 };