From b7080ae3dc9c7a806fcb17a5205a823ebf972dc2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jean-Micha=C3=ABl=20Celerier?= Date: Tue, 19 Nov 2024 09:56:40 -0500 Subject: [PATCH 1/3] [core] Implement a command transaction system --- src/lib/CMakeLists.txt | 1 + src/lib/core/command/CommandStack.cpp | 15 ++++++- src/lib/core/command/CommandStack.hpp | 39 +++++++++++++++++- src/lib/core/document/DocumentBuilder.cpp | 3 ++ src/lib/score/command/CommandStackFacade.hpp | 7 +++- .../Dispatchers/OngoingCommandDispatcher.hpp | 1 + .../command/Dispatchers/SendStrategy.cpp | 40 ++++++++++++++++++ .../command/Dispatchers/SendStrategy.hpp | 41 ++++++++----------- .../score-lib-process/Effect/EffectLayout.hpp | 1 + .../Process/ExecutionContext.hpp | 4 ++ .../Process/ExecutionSetup.cpp | 17 +++++--- .../Execution/DocumentPlugin.cpp | 30 +++++++++++++- .../Execution/DocumentPlugin.hpp | 2 + .../Execution/ExecutionTick.cpp | 2 +- .../score-plugin-faust/Faust/EffectModel.cpp | 7 ++-- 15 files changed, 170 insertions(+), 40 deletions(-) create mode 100644 src/lib/score/command/Dispatchers/SendStrategy.cpp diff --git a/src/lib/CMakeLists.txt b/src/lib/CMakeLists.txt index b0e00d7e3c..2c95d563f9 100755 --- a/src/lib/CMakeLists.txt +++ b/src/lib/CMakeLists.txt @@ -383,6 +383,7 @@ set(SRCS "${CMAKE_CURRENT_SOURCE_DIR}/score/command/CommandGeneratorMap.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/score/command/PropertyCommand.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/score/command/Validity/ValidityChecker.cpp" +"${CMAKE_CURRENT_SOURCE_DIR}/score/command/Dispatchers/SendStrategy.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/score/model/Component.cpp" "${CMAKE_CURRENT_SOURCE_DIR}/score/model/ColorInterpolator.cpp" diff --git a/src/lib/core/command/CommandStack.cpp b/src/lib/core/command/CommandStack.cpp index d2de37a4e6..30533c816b 100644 --- a/src/lib/core/command/CommandStack.cpp +++ b/src/lib/core/command/CommandStack.cpp @@ -119,6 +119,7 @@ void CommandStack::setIndex(int index) void CommandStack::undoQuiet() { + CommandTransaction t{*this}; updateStack([&]() { auto cmd = m_undoable.pop(); cmd->undo(m_ctx); @@ -131,6 +132,7 @@ void CommandStack::undoQuiet() void CommandStack::redoQuiet() { + CommandTransaction t{*this}; updateStack([&]() { auto cmd = m_redoable.pop(); cmd->redo(m_ctx); @@ -212,12 +214,17 @@ void CommandStack::validateDocument() const m_checker(); } -CommandStackFacade::CommandStackFacade(CommandStack& stack) +CommandTransaction CommandStack::transaction() +{ + return CommandTransaction{*this}; +} + +CommandStackFacade::CommandStackFacade(CommandStack& stack) noexcept : m_stack{stack} { } -const DocumentContext& CommandStackFacade::context() const +const DocumentContext& CommandStackFacade::context() const noexcept { return m_stack.context(); } @@ -242,4 +249,8 @@ void CommandStackFacade::enableActions() const m_stack.enableActions(); } +CommandTransaction CommandStackFacade::transaction() const +{ + return m_stack.transaction(); +} } diff --git a/src/lib/core/command/CommandStack.hpp b/src/lib/core/command/CommandStack.hpp index 9613bd6349..6d1356e46f 100644 --- a/src/lib/core/command/CommandStack.hpp +++ b/src/lib/core/command/CommandStack.hpp @@ -11,7 +11,7 @@ namespace score { class Document; - +struct CommandTransaction; /** * \class score::CommandStack * @@ -123,6 +123,9 @@ class SCORE_LIB_BASE_EXPORT CommandStack final : public QObject void redoQuiet(); W_INVOKABLE(redoQuiet) + void beginTransaction() E_SIGNAL(SCORE_LIB_BASE_EXPORT, beginTransaction) + void endTransaction() E_SIGNAL(SCORE_LIB_BASE_EXPORT, endTransaction) + /** * @brief push Pushes a command on the stack * @param cmd The command @@ -205,14 +208,48 @@ class SCORE_LIB_BASE_EXPORT CommandStack final : public QObject void validateDocument() const; + CommandTransaction transaction(); + private: + friend struct CommandTransaction; QStack m_undoable; QStack m_redoable; int m_savedIndex{}; + int m_inTransaction{}; DocumentValidator m_checker; const score::DocumentContext& m_ctx; }; + +struct CommandTransaction +{ + CommandStack& self; + + explicit CommandTransaction(CommandStack& self) + : self{self} + { + if(self.m_inTransaction == 0) + { + self.beginTransaction(); + } + self.m_inTransaction++; + } + CommandTransaction(const CommandTransaction& other) = delete; + CommandTransaction(CommandTransaction&& other) = delete; + CommandTransaction& operator=(const CommandTransaction& other) = delete; + CommandTransaction& operator=(CommandTransaction&& other) = delete; + + ~CommandTransaction() + { + SCORE_ASSERT(self.m_inTransaction > 0); + + self.m_inTransaction--; + if(self.m_inTransaction == 0) + { + self.endTransaction(); + } + } +}; } W_REGISTER_ARGTYPE(score::Command*) diff --git a/src/lib/core/document/DocumentBuilder.cpp b/src/lib/core/document/DocumentBuilder.cpp index b5695abb16..ac05533225 100644 --- a/src/lib/core/document/DocumentBuilder.cpp +++ b/src/lib/core/document/DocumentBuilder.cpp @@ -192,7 +192,10 @@ Document* DocumentBuilder::restoreDocument( ctx.components, writer, doc->commandStack(), [doc](score::Command* cmd) { try { + qDebug() << ".. replaying: " << cmd->key().toString().c_str() + << cmd->description(); cmd->redo(doc->context()); + qApp->processEvents(); return true; } catch(...) diff --git a/src/lib/score/command/CommandStackFacade.hpp b/src/lib/score/command/CommandStackFacade.hpp index d573394cc8..1f401bebc8 100644 --- a/src/lib/score/command/CommandStackFacade.hpp +++ b/src/lib/score/command/CommandStackFacade.hpp @@ -5,6 +5,7 @@ namespace score { class Command; class CommandStack; +struct CommandTransaction; struct DocumentContext; /** @@ -22,14 +23,16 @@ class SCORE_LIB_BASE_EXPORT CommandStackFacade score::CommandStack& m_stack; public: - explicit CommandStackFacade(score::CommandStack& stack); + explicit CommandStackFacade(score::CommandStack& stack) noexcept; - const score::DocumentContext& context() const; + const score::DocumentContext& context() const noexcept; void push(score::Command* cmd) const; void redoAndPush(score::Command* cmd) const; void disableActions() const; void enableActions() const; + + CommandTransaction transaction() const; }; } diff --git a/src/lib/score/command/Dispatchers/OngoingCommandDispatcher.hpp b/src/lib/score/command/Dispatchers/OngoingCommandDispatcher.hpp index 9ffc49d1c9..50e883eca6 100644 --- a/src/lib/score/command/Dispatchers/OngoingCommandDispatcher.hpp +++ b/src/lib/score/command/Dispatchers/OngoingCommandDispatcher.hpp @@ -1,4 +1,5 @@ #pragma once +#include #include #include #include diff --git a/src/lib/score/command/Dispatchers/SendStrategy.cpp b/src/lib/score/command/Dispatchers/SendStrategy.cpp new file mode 100644 index 0000000000..0fddca3f14 --- /dev/null +++ b/src/lib/score/command/Dispatchers/SendStrategy.cpp @@ -0,0 +1,40 @@ +#include "SendStrategy.hpp" + +#include +#include + +#include + +namespace SendStrategy +{ + +void Simple::send(const score::CommandStackFacade& stack, score::Command* other) +{ + auto trans = stack.context().commandStack.transaction(); + stack.redoAndPush(other); +} + +void UndoRedo::send(const score::CommandStackFacade& stack, score::Command* other) +{ + auto trans = stack.context().commandStack.transaction(); + other->undo(stack.context()); + stack.redoAndPush(other); +} + +void Quiet::send(const score::CommandStackFacade& stack, score::Command* other) +{ + stack.push(other); +} + +} +namespace RedoStrategy +{ +void Redo::redo(const score::DocumentContext& ctx, score::Command& cmd) +{ + auto trans = ctx.commandStack.transaction(); + cmd.redo(ctx); +} + +void Quiet::redo(const score::DocumentContext& ctx, score::Command& cmd) { } + +} diff --git a/src/lib/score/command/Dispatchers/SendStrategy.hpp b/src/lib/score/command/Dispatchers/SendStrategy.hpp index 331da84239..13f74481f1 100644 --- a/src/lib/score/command/Dispatchers/SendStrategy.hpp +++ b/src/lib/score/command/Dispatchers/SendStrategy.hpp @@ -1,45 +1,38 @@ #pragma once -#include +#include +namespace score +{ +class CommandStackFacade; +class Command; +struct DocumentContext; +} namespace SendStrategy { -struct Simple +struct SCORE_LIB_BASE_EXPORT Simple { - static void send(const score::CommandStackFacade& stack, score::Command* other) - { - stack.redoAndPush(other); - } + static void send(const score::CommandStackFacade& stack, score::Command* other); }; -struct Quiet +struct SCORE_LIB_BASE_EXPORT Quiet { - static void send(const score::CommandStackFacade& stack, score::Command* other) - { - stack.push(other); - } + static void send(const score::CommandStackFacade& stack, score::Command* other); }; -struct UndoRedo +struct SCORE_LIB_BASE_EXPORT UndoRedo { - static void send(const score::CommandStackFacade& stack, score::Command* other) - { - other->undo(stack.context()); - stack.redoAndPush(other); - } + static void send(const score::CommandStackFacade& stack, score::Command* other); }; } namespace RedoStrategy { -struct Redo +struct SCORE_LIB_BASE_EXPORT Redo { - static void redo(const score::DocumentContext& ctx, score::Command& cmd) - { - cmd.redo(ctx); - } + static void redo(const score::DocumentContext& ctx, score::Command& cmd); }; -struct Quiet +struct SCORE_LIB_BASE_EXPORT Quiet { - static void redo(const score::DocumentContext& ctx, score::Command& cmd) { } + static void redo(const score::DocumentContext& ctx, score::Command& cmd); }; } diff --git a/src/plugins/score-lib-process/Effect/EffectLayout.hpp b/src/plugins/score-lib-process/Effect/EffectLayout.hpp index f6a1333ed7..ccca17468f 100644 --- a/src/plugins/score-lib-process/Effect/EffectLayout.hpp +++ b/src/plugins/score-lib-process/Effect/EffectLayout.hpp @@ -10,6 +10,7 @@ #include #include +// FIXME put the entirety of this as dynamic behaviour instead namespace Process { diff --git a/src/plugins/score-lib-process/Process/ExecutionContext.hpp b/src/plugins/score-lib-process/Process/ExecutionContext.hpp index 36d92433d6..44b5c7ed87 100644 --- a/src/plugins/score-lib-process/Process/ExecutionContext.hpp +++ b/src/plugins/score-lib-process/Process/ExecutionContext.hpp @@ -96,6 +96,10 @@ struct SCORE_LIB_PROCESS_EXPORT Context const std::shared_ptr& execGraph; const std::shared_ptr& execState; + std::shared_ptr& transaction; + + void execCommand(ExecutionCommand&& cmd); + auto& context() const { return *this; } #if !defined(_MSC_VER) diff --git a/src/plugins/score-lib-process/Process/ExecutionSetup.cpp b/src/plugins/score-lib-process/Process/ExecutionSetup.cpp index 1f41b25381..f86fa9b35a 100644 --- a/src/plugins/score-lib-process/Process/ExecutionSetup.cpp +++ b/src/plugins/score-lib-process/Process/ExecutionSetup.cpp @@ -19,12 +19,19 @@ namespace Execution { +void Context::execCommand(ExecutionCommand&& cmd) +{ + if(transaction) + transaction->push_back(std::move(cmd)); + else + executionQueue.enqueue(std::move(cmd)); +} static auto enqueue_in_context(SetupContext& self) noexcept { return [&self](F&& f) { static_assert(std::is_nothrow_move_constructible_v); - self.context.executionQueue.enqueue(std::move(f)); + self.context.execCommand(std::move(f)); }; } @@ -72,7 +79,7 @@ void SetupContext::on_cableRemoved(const Process::Cable& c) auto it = m_cables.find(c.id()); if(it != m_cables.end()) { - context.executionQueue.enqueue( + context.execCommand( [cable = it->second, graph = context.execGraph] { graph->disconnect(cable); }); } } @@ -135,7 +142,7 @@ void SetupContext::connectCable(Process::Cable& cable) } m_cables[cable.id()] = edge; - context.executionQueue.enqueue([edge, graph = context.execGraph]() mutable { + context.execCommand([edge, graph = context.execGraph]() mutable { graph->connect(std::move(edge)); }); } @@ -428,7 +435,7 @@ void SetupContext::unregister_inlet( if(ossia_port_it != inlets.end()) { std::weak_ptr ws = context.execState; - context.executionQueue.enqueue([ws, ossia_port = ossia_port_it->second.second] { + context.execCommand([ws, ossia_port = ossia_port_it->second.second] { if(auto state = ws.lock()) state->unregister_port(*ossia_port); }); @@ -533,7 +540,7 @@ void SetupContext::unregister_node( { std::weak_ptr wg = context.execGraph; std::weak_ptr ws = context.execState; - context.executionQueue.enqueue([wg, ws, node] { + context.execCommand([wg, ws, node] { if(auto s = ws.lock()) { ossia::for_each_inlet(*node, [&](auto& p) { s->unregister_port(p); }); diff --git a/src/plugins/score-plugin-engine/Execution/DocumentPlugin.cpp b/src/plugins/score-plugin-engine/Execution/DocumentPlugin.cpp index 2b9816087b..2899f65816 100644 --- a/src/plugins/score-plugin-engine/Execution/DocumentPlugin.cpp +++ b/src/plugins/score-plugin-engine/Execution/DocumentPlugin.cpp @@ -50,7 +50,7 @@ DocumentPlugin::ContextData::ContextData(const score::DocumentContext& ctx) , context { {}, ctx, m_created, {}, {}, m_execQueue, m_editionQueue, m_gcQueue, setupContext, - execGraph, execState + execGraph, execState, currentTransaction #if(__cplusplus > 201703L) && !defined(_MSC_VER) , { @@ -91,6 +91,34 @@ DocumentPlugin::DocumentPlugin(const score::DocumentContext& ctx, QObject* paren connect( this, &DocumentPlugin::finished, this, &DocumentPlugin::on_finished, Qt::DirectConnection); + + auto& cstack = ctx.document.commandStack(); + + connect( + &cstack, &score::CommandStack::beginTransaction, this, + [this] { + qDebug("Begin transaction"); + + if(m_ctxData) + { + SCORE_ASSERT(!m_ctxData->currentTransaction); + m_ctxData->currentTransaction + = std::make_shared(m_ctxData->context); + } + }, + Qt::DirectConnection); + + connect( + &cstack, &score::CommandStack::endTransaction, this, + [this] { + qDebug("Submit transaction"); + if(m_ctxData) + { + m_ctxData->currentTransaction->run_all(); + m_ctxData->currentTransaction.reset(); + } + }, + Qt::DirectConnection); } void DocumentPlugin::recreateBase() diff --git a/src/plugins/score-plugin-engine/Execution/DocumentPlugin.hpp b/src/plugins/score-plugin-engine/Execution/DocumentPlugin.hpp index 08272f0c29..e2671fb168 100644 --- a/src/plugins/score-plugin-engine/Execution/DocumentPlugin.hpp +++ b/src/plugins/score-plugin-engine/Execution/DocumentPlugin.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include @@ -67,6 +68,7 @@ class SCORE_PLUGIN_ENGINE_EXPORT DocumentPlugin final : public score::DocumentPl ExecutionCommandQueue m_execQueue{1024}; EditionCommandQueue m_editionQueue{1024}; GCCommandQueue m_gcQueue{1024}; + std::shared_ptr currentTransaction; std::atomic_bool m_created{}; std::shared_ptr execGraph; diff --git a/src/plugins/score-plugin-engine/Execution/ExecutionTick.cpp b/src/plugins/score-plugin-engine/Execution/ExecutionTick.cpp index 3154af810e..1b06442e9e 100644 --- a/src/plugins/score-plugin-engine/Execution/ExecutionTick.cpp +++ b/src/plugins/score-plugin-engine/Execution/ExecutionTick.cpp @@ -3,6 +3,7 @@ #include #include