From 79795b97a95d4c56eda8e5ffdf6459244cf9e652 Mon Sep 17 00:00:00 2001 From: David Capello Date: Mon, 3 Jun 2019 22:57:20 -0300 Subject: [PATCH] Fix hang if we press Cmd-S two or more times quickly to launch File > Save With this we avoid to unintentional execute two or more times a command when we are already in a command execution. --- src/app/app_menus.cpp | 4 +- src/app/context.cpp | 52 ++++++++++++++--------- src/app/context.h | 2 +- src/app/modules/gui.cpp | 2 +- src/app/ui/app_menuitem.cpp | 2 +- src/app/ui/editor/drawing_state.cpp | 2 +- src/app/ui/editor/moving_pixels_state.cpp | 2 +- src/app/ui/editor/standby_state.cpp | 4 +- src/app/ui/home_view.cpp | 4 +- src/app/ui/main_window.cpp | 2 +- src/app/ui/palette_popup.cpp | 2 +- src/app/ui/recent_listbox.cpp | 4 +- src/app/ui/status_bar.cpp | 4 +- src/app/ui/timeline/ani_controls.cpp | 2 +- 14 files changed, 49 insertions(+), 39 deletions(-) diff --git a/src/app/app_menus.cpp b/src/app/app_menus.cpp index 15214f478..413ff554b 100644 --- a/src/app/app_menus.cpp +++ b/src/app/app_menus.cpp @@ -730,7 +730,7 @@ void AppMenus::createNativeMenus() about.execute = [native]{ if (can_call_global_shortcut(&native)) { Command* cmd = Commands::instance()->byId(CommandId::About()); - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); } }; about.validate = [native](os::MenuItem* item){ @@ -743,7 +743,7 @@ void AppMenus::createNativeMenus() preferences.execute = [native]{ if (can_call_global_shortcut(&native)) { Command* cmd = Commands::instance()->byId(CommandId::Options()); - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); } }; preferences.validate = [native](os::MenuItem* item){ diff --git a/src/app/context.cpp b/src/app/context.cpp index 322a47201..90a928f8d 100644 --- a/src/app/context.cpp +++ b/src/app/context.cpp @@ -18,7 +18,9 @@ #include "app/console.h" #include "app/doc.h" #include "app/site.h" +#include "base/scoped_value.h" #include "doc/layer.h" +#include "ui/system.h" #include #include @@ -93,24 +95,33 @@ void Context::notifyActiveSiteChanged() notify_observers(&ContextObserver::onActiveSiteChange, site); } -void Context::executeCommand(const char* commandName) +void Context::executeCommandFromMenuOrShortcut(Command* command, const Params& params) { - Command* cmd = Commands::instance()->byId(commandName); - if (cmd) - executeCommand(cmd); - else - throw std::runtime_error("Invalid command name"); + ui::assert_ui_thread(); + + // With this we avoid executing a command when we are inside another + // command (e.g. if we press Cmd-S quickly the program can enter two + // times in the File > Save command and hang). + static Command* executingCommand = nullptr; + if (executingCommand) { // Ignore command execution + LOG(VERBOSE, "CTXT: Ignoring command %s because we are inside %s\n", + command->id().c_str(), executingCommand->id().c_str()); + return; + } + base::ScopedValue commandGuard(executingCommand, + command, nullptr); + + executeCommand(command, params); } void Context::executeCommand(Command* command, const Params& params) { - Console console; - - ASSERT(command != NULL); - if (command == NULL) + ASSERT(command); + if (!command) return; - LOG(VERBOSE) << "CTXT: Executing command " << command->id() << "\n"; + Console console; + LOG(VERBOSE, "CTXT: Executing command %s\n", command->id().c_str()); try { m_flags.update(this); @@ -122,14 +133,14 @@ void Context::executeCommand(Command* command, const Params& params) BeforeCommandExecution(ev); if (ev.isCanceled()) { - LOG(VERBOSE) << "CTXT: Command " << command->id() << " was canceled/simulated.\n"; + LOG(VERBOSE, "CTXT: Command %s was canceled/simulated.\n", command->id().c_str()); } else if (command->isEnabled(this)) { command->execute(this); - LOG(VERBOSE) << "CTXT: Command " << command->id() << " executed successfully\n"; + LOG(VERBOSE, "CTXT: Command %s executed successfully\n", command->id().c_str()); } else { - LOG(VERBOSE) << "CTXT: Command " << command->id() << " is disabled\n"; + LOG(VERBOSE, "CTXT: Command %s is disabled\n", command->id().c_str()); } AfterCommandExecution(ev); @@ -139,20 +150,19 @@ void Context::executeCommand(Command* command, const Params& params) app_rebuild_documents_tabs(); } catch (base::Exception& e) { - LOG(ERROR) << "CTXT: Exception caught executing " << command->id() << " command\n" - << e.what() << "\n"; - + LOG(ERROR, "CTXT: Exception caught executing %s command\n%s\n", + command->id().c_str(), e.what()); Console::showException(e); } catch (std::exception& e) { - LOG(ERROR) << "CTXT: std::exception caught executing " << command->id() << " command\n" - << e.what() << "\n"; - + LOG(ERROR, "CTXT: std::exception caught executing %s command\n%s\n", + command->id().c_str(), e.what()); console.printf("An error ocurred executing the command.\n\nDetails:\n%s", e.what()); } #ifdef NDEBUG catch (...) { - LOG(ERROR) << "CTXT: Unknown exception executing " << command->id() << " command\n"; + LOG(ERROR, "CTXT: Unknown exception executing %s command\n", + command->id().c_str()); console.printf("An unknown error ocurred executing the command.\n" "Please save your work, close the program, try it\n" diff --git a/src/app/context.h b/src/app/context.h index 0fe1f932a..e3c4c0b40 100644 --- a/src/app/context.h +++ b/src/app/context.h @@ -88,7 +88,7 @@ namespace app { bool hasModifiedDocuments() const; void notifyActiveSiteChanged(); - void executeCommand(const char* commandName); + void executeCommandFromMenuOrShortcut(Command* command, const Params& params = Params()); virtual void executeCommand(Command* command, const Params& params = Params()); virtual DocView* getFirstDocView(Doc* document) const { diff --git a/src/app/modules/gui.cpp b/src/app/modules/gui.cpp index 4a07231a7..65676100f 100644 --- a/src/app/modules/gui.cpp +++ b/src/app/modules/gui.cpp @@ -502,7 +502,7 @@ bool CustomizedGuiManager::onProcessMessage(Message* msg) if (getForegroundWindow() == App::instance()->mainWindow()) { // OK, so we can execute the command represented // by the pressed-key in the message... - UIContext::instance()->executeCommand( + UIContext::instance()->executeCommandFromMenuOrShortcut( command, key->params()); return true; } diff --git a/src/app/ui/app_menuitem.cpp b/src/app/ui/app_menuitem.cpp index a220b81cd..fc37da0c8 100644 --- a/src/app/ui/app_menuitem.cpp +++ b/src/app/ui/app_menuitem.cpp @@ -141,7 +141,7 @@ void AppMenuItem::onClick() UIContext* context = UIContext::instance(); if (m_command->isEnabled(context)) { - context->executeCommand(m_command, params); + context->executeCommandFromMenuOrShortcut(m_command, params); } } } diff --git a/src/app/ui/editor/drawing_state.cpp b/src/app/ui/editor/drawing_state.cpp index 1f970fea7..1b027c093 100644 --- a/src/app/ui/editor/drawing_state.cpp +++ b/src/app/ui/editor/drawing_state.cpp @@ -232,7 +232,7 @@ bool DrawingState::onKeyDown(Editor* editor, KeyMessage* msg) ->getCommandFromKeyMessage(msg, &command, ¶ms)) { // We accept zoom commands. if (command->id() == CommandId::Zoom()) { - UIContext::instance()->executeCommand(command, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(command, params); return true; } } diff --git a/src/app/ui/editor/moving_pixels_state.cpp b/src/app/ui/editor/moving_pixels_state.cpp index 3e3c8a97c..0981406c8 100644 --- a/src/app/ui/editor/moving_pixels_state.cpp +++ b/src/app/ui/editor/moving_pixels_state.cpp @@ -436,7 +436,7 @@ bool MovingPixelsState::onKeyDown(Editor* editor, KeyMessage* msg) // The escape key drop pixels and deselect the mask. if (msg->scancode() == kKeyEsc) { // TODO make this key customizable Command* cmd = Commands::instance()->byId(CommandId::DeselectMask()); - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); } return true; diff --git a/src/app/ui/editor/standby_state.cpp b/src/app/ui/editor/standby_state.cpp index 86be71fc0..4305390de 100644 --- a/src/app/ui/editor/standby_state.cpp +++ b/src/app/ui/editor/standby_state.cpp @@ -406,7 +406,7 @@ bool StandbyState::onDoubleClick(Editor* editor, MouseMessage* msg) else if (int(editor->getToolLoopModifiers()) & int(tools::ToolLoopModifiers::kIntersectSelection)) params.set("mode", "intersect"); - UIContext::instance()->executeCommand(selectTileCmd, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(selectTileCmd, params); return true; } // Show slice properties when we double-click it @@ -416,7 +416,7 @@ bool StandbyState::onDoubleClick(Editor* editor, MouseMessage* msg) Command* cmd = Commands::instance()->byId(CommandId::SliceProperties()); Params params; params.set("id", base::convert_to(hit.slice()->id()).c_str()); - UIContext::instance()->executeCommand(cmd, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd, params); return true; } } diff --git a/src/app/ui/home_view.cpp b/src/app/ui/home_view.cpp index 25f252575..1a60ab1ea 100644 --- a/src/app/ui/home_view.cpp +++ b/src/app/ui/home_view.cpp @@ -137,13 +137,13 @@ void HomeView::onWorkspaceViewSelected() void HomeView::onNewFile() { Command* command = Commands::instance()->byId(CommandId::NewFile()); - UIContext::instance()->executeCommand(command); + UIContext::instance()->executeCommandFromMenuOrShortcut(command); } void HomeView::onOpenFile() { Command* command = Commands::instance()->byId(CommandId::OpenFile()); - UIContext::instance()->executeCommand(command); + UIContext::instance()->executeCommandFromMenuOrShortcut(command); } void HomeView::onResize(ui::ResizeEvent& ev) diff --git a/src/app/ui/main_window.cpp b/src/app/ui/main_window.cpp index a05055117..27e0b102f 100644 --- a/src/app/ui/main_window.cpp +++ b/src/app/ui/main_window.cpp @@ -452,7 +452,7 @@ void MainWindow::onTabsContainerDoubleClicked(Tabs* tabs) Doc* oldDoc = UIContext::instance()->activeDocument(); Command* command = Commands::instance()->byId(CommandId::NewFile()); - UIContext::instance()->executeCommand(command); + UIContext::instance()->executeCommandFromMenuOrShortcut(command); Doc* newDoc = UIContext::instance()->activeDocument(); if (newDoc != oldDoc) { diff --git a/src/app/ui/palette_popup.cpp b/src/app/ui/palette_popup.cpp index 771e0d3bf..31e6bb032 100644 --- a/src/app/ui/palette_popup.cpp +++ b/src/app/ui/palette_popup.cpp @@ -111,7 +111,7 @@ void PalettePopup::onLoadPal() SetPaletteCommand* cmd = static_cast( Commands::instance()->byId(CommandId::SetPalette())); cmd->setPalette(palette); - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); m_paletteListBox.requestFocus(); } diff --git a/src/app/ui/recent_listbox.cpp b/src/app/ui/recent_listbox.cpp index 65b0117f8..56014c8ca 100644 --- a/src/app/ui/recent_listbox.cpp +++ b/src/app/ui/recent_listbox.cpp @@ -308,7 +308,7 @@ void RecentFilesListBox::onClick(const std::string& path) Command* command = Commands::instance()->byId(CommandId::OpenFile()); Params params; params.set("filename", path.c_str()); - UIContext::instance()->executeCommand(command, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(command, params); } void RecentFilesListBox::onUpdateRecentListFromUIItems(const base::paths& pinnedPaths, @@ -346,7 +346,7 @@ void RecentFoldersListBox::onClick(const std::string& path) Command* command = Commands::instance()->byId(CommandId::OpenFile()); Params params; params.set("folder", path.c_str()); - UIContext::instance()->executeCommand(command, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(command, params); } void RecentFoldersListBox::onUpdateRecentListFromUIItems(const base::paths& pinnedPaths, diff --git a/src/app/ui/status_bar.cpp b/src/app/ui/status_bar.cpp index a37cc726d..4a1cf4389 100644 --- a/src/app/ui/status_bar.cpp +++ b/src/app/ui/status_bar.cpp @@ -515,7 +515,7 @@ public: Command* cmd = Commands::instance()->byId(CommandId::GotoFrame()); Params params; params.set("frame", text().c_str()); - UIContext::instance()->executeCommand(cmd, params); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd, params); // Select the text again selectAllText(); @@ -813,7 +813,7 @@ void StatusBar::onPixelFormatChanged(DocEvent& ev) void StatusBar::newFrame() { Command* cmd = Commands::instance()->byId(CommandId::NewFrame()); - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); } void StatusBar::onChangeZoom(const render::Zoom& zoom) diff --git a/src/app/ui/timeline/ani_controls.cpp b/src/app/ui/timeline/ani_controls.cpp index 47d0e4578..3a238a7c1 100644 --- a/src/app/ui/timeline/ani_controls.cpp +++ b/src/app/ui/timeline/ani_controls.cpp @@ -83,7 +83,7 @@ void AniControls::onClickButton() Command* cmd = Commands::instance()->byId(getCommandId(item)); if (cmd) { - UIContext::instance()->executeCommand(cmd); + UIContext::instance()->executeCommandFromMenuOrShortcut(cmd); updateUsingEditor(current_editor); } }