From 4585b5e7e557cf359911081b916b083bbe463135 Mon Sep 17 00:00:00 2001 From: David Capello Date: Fri, 27 Oct 2017 09:25:50 -0300 Subject: [PATCH] Fix possibilities of random crashes using filters w/Undo History window visible Same problem as in 86a6462d7b82608cc14a8417bbdb42e2a4e3c467 --- .../commands/filters/filter_manager_impl.cpp | 25 +++++++++++-------- .../commands/filters/filter_manager_impl.h | 7 ++++-- src/app/commands/filters/filter_worker.cpp | 4 ++- src/app/transaction.h | 7 +++++- 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/src/app/commands/filters/filter_manager_impl.cpp b/src/app/commands/filters/filter_manager_impl.cpp index 83eb7c7c6..4000fad3b 100644 --- a/src/app/commands/filters/filter_manager_impl.cpp +++ b/src/app/commands/filters/filter_manager_impl.cpp @@ -183,7 +183,7 @@ bool FilterManagerImpl::applyStep() return true; } -void FilterManagerImpl::apply(Transaction& transaction) +void FilterManagerImpl::apply() { bool cancelled = false; @@ -203,7 +203,7 @@ void FilterManagerImpl::apply(Transaction& transaction) if (algorithm::shrink_bounds2(m_src.get(), m_dst.get(), m_bounds, output)) { if (m_cel->layer()->isBackground()) { - transaction.execute( + m_transaction->execute( new cmd::CopyRegion( m_cel->image(), m_dst.get(), @@ -212,7 +212,7 @@ void FilterManagerImpl::apply(Transaction& transaction) } else { // Patch "m_cel" - transaction.execute( + m_transaction->execute( new cmd::PatchCel( m_cel, m_dst.get(), gfx::Region(output), @@ -239,7 +239,7 @@ void FilterManagerImpl::applyToTarget() // Initialize writting operation ContextReader reader(m_context); ContextWriter writer(reader); - Transaction transaction(writer.context(), m_filter->getName(), ModifyDocument); + m_transaction.reset(new Transaction(writer.context(), m_filter->getName(), ModifyDocument)); m_progressBase = 0.0f; m_progressWidth = 1.0f / images.size(); @@ -250,7 +250,7 @@ void FilterManagerImpl::applyToTarget() if (paletteChange) { Palette newPalette = *getNewPalette(); restoreSpritePalette(); - transaction.execute( + m_transaction->execute( new cmd::SetPalette(m_site.sprite(), m_site.frame(), &newPalette)); } @@ -264,7 +264,7 @@ void FilterManagerImpl::applyToTarget() // Avoid applying the filter two times to the same image if (visited.find(image->id()) == visited.end()) { visited.insert(image->id()); - applyToCel(transaction, it->cel()); + applyToCel(it->cel()); } // Is there a delegate to know if the process was cancelled by the user? @@ -275,12 +275,17 @@ void FilterManagerImpl::applyToTarget() m_progressBase += m_progressWidth; } - transaction.commit(); - // Reset m_oldPalette to avoid restoring the color palette m_oldPalette.reset(nullptr); } +void FilterManagerImpl::commitTransaction() +{ + // This must be executed in the main UI thread. + // Check Transaction::commit() comments. + m_transaction->commit(); +} + void FilterManagerImpl::flush() { int h = m_row - m_nextRowToFlush; @@ -395,10 +400,10 @@ void FilterManagerImpl::init(Cel* cel) m_target &= ~TARGET_ALPHA_CHANNEL; } -void FilterManagerImpl::applyToCel(Transaction& transaction, Cel* cel) +void FilterManagerImpl::applyToCel(Cel* cel) { init(cel); - apply(transaction); + apply(); } bool FilterManagerImpl::updateBounds(doc::Mask* mask) diff --git a/src/app/commands/filters/filter_manager_impl.h b/src/app/commands/filters/filter_manager_impl.h index f381c3d9a..6ad5c0c00 100644 --- a/src/app/commands/filters/filter_manager_impl.h +++ b/src/app/commands/filters/filter_manager_impl.h @@ -19,6 +19,7 @@ #include "gfx/rect.h" #include +#include namespace doc { class Cel; @@ -82,6 +83,7 @@ namespace app { void end(); bool applyStep(); void applyToTarget(); + void commitTransaction(); app::Document* document(); doc::Sprite* sprite() { return m_site.sprite(); } @@ -114,8 +116,8 @@ namespace app { private: void init(doc::Cel* cel); - void apply(Transaction& transaction); - void applyToCel(Transaction& transaction, doc::Cel* cel); + void apply(); + void applyToCel(doc::Cel* cel); bool updateBounds(doc::Mask* mask); bool paletteHasChanged(); void restoreSpritePalette(); @@ -136,6 +138,7 @@ namespace app { Target m_targetOrig; // Original targets Target m_target; // Filtered targets base::UniquePtr m_oldPalette; + std::unique_ptr m_transaction; // Hooks float m_progressBase; diff --git a/src/app/commands/filters/filter_worker.cpp b/src/app/commands/filters/filter_worker.cpp index b6aa093dc..ba67cbb02 100644 --- a/src/app/commands/filters/filter_worker.cpp +++ b/src/app/commands/filters/filter_worker.cpp @@ -104,7 +104,9 @@ void FilterWorker::run() { scoped_lock lock(m_mutex); - if (!m_done) + if (m_done) + m_filterMgr->commitTransaction(); + else m_cancelled = true; } diff --git a/src/app/transaction.h b/src/app/transaction.h index f12a63032..65e906bfc 100644 --- a/src/app/transaction.h +++ b/src/app/transaction.h @@ -1,5 +1,5 @@ // Aseprite -// Copyright (C) 2001-2015 David Capello +// Copyright (C) 2001-2017 David Capello // // This program is distributed under the terms of // the End-User License Agreement for Aseprite. @@ -51,6 +51,11 @@ namespace app { // If you don't use this routine, all the changes will be discarded // (if the sprite's undo was enabled when the Transaction was // created). + // + // WARNING: This must be called from the main UI thread, because + // it will generate a DocumentUndo::add() which triggers a + // DocumentUndoObserver::onAddUndoState() notification, which + // updates the Undo History window UI. void commit(); void execute(Cmd* cmd);