From e4fbd003e09f0acf4c713484baa6f7622e62ae08 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 16 Jun 2019 12:06:49 +0100 Subject: [PATCH] Make m_passEvent event-specific rather than global. If a tool called something like clearSelection while processing a MOUSE_CLICK, the SELECTION_TOOL will pass the clearSelection COMMAND_EVENT because it handles it as a transition, not as an event. Because m_passEvent is effectively global, the tool manager would then interpret that as passing the MOUSE_CLICK and we'd end up processing the click by multiple tools. --- common/tool/tool_manager.cpp | 9 ++------- common/tool/zoom_tool.cpp | 2 +- cvpcb/tools/cvpcb_selection_tool.cpp | 2 +- eeschema/tools/ee_picker_tool.cpp | 2 +- eeschema/tools/ee_point_editor.cpp | 4 +--- eeschema/tools/ee_selection_tool.cpp | 7 +------ gerbview/tools/gerbview_selection_tool.cpp | 2 +- include/tool/tool_event.h | 16 ++++++++++++++++ include/tool/tool_manager.h | 11 ----------- pagelayout_editor/tools/pl_picker_tool.cpp | 4 +--- pagelayout_editor/tools/pl_point_editor.cpp | 4 +--- pagelayout_editor/tools/pl_selection_tool.cpp | 2 +- pcbnew/tools/pcbnew_picker_tool.cpp | 2 +- pcbnew/tools/point_editor.cpp | 4 +--- pcbnew/tools/selection_tool.cpp | 7 +------ 15 files changed, 30 insertions(+), 48 deletions(-) diff --git a/common/tool/tool_manager.cpp b/common/tool/tool_manager.cpp index 54772d12cf..d16160370a 100644 --- a/common/tool/tool_manager.cpp +++ b/common/tool/tool_manager.cpp @@ -195,7 +195,6 @@ TOOL_MANAGER::TOOL_MANAGER() : m_view( NULL ), m_viewControls( NULL ), m_frame( NULL ), - m_passEvent( false ), m_menuActive( false ), m_menuOwner( -1 ), m_activeState( nullptr ) @@ -543,9 +542,6 @@ void TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent ) { if( st->waitEvents.Matches( aEvent ) ) { - // By default only messages are passed further - m_passEvent = ( aEvent.Category() == TC_MESSAGE ); - // got matching event? clear wait list and wake up the coroutine st->wakeupEvent = aEvent; st->pendingWait = false; @@ -560,9 +556,8 @@ void TOOL_MANAGER::dispatchInternal( const TOOL_EVENT& aEvent ) it = finishTool( st ); } - // If the tool did not request to propagate - // the event to other tools, we should stop it now - if( !m_passEvent ) + // If the tool did not request the event be passed to other tools, we're done + if( !aEvent.PassEvent() ) break; } } diff --git a/common/tool/zoom_tool.cpp b/common/tool/zoom_tool.cpp index 9b89681bda..bb1d64e2c1 100644 --- a/common/tool/zoom_tool.cpp +++ b/common/tool/zoom_tool.cpp @@ -60,7 +60,7 @@ int ZOOM_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // Exit zoom tool diff --git a/cvpcb/tools/cvpcb_selection_tool.cpp b/cvpcb/tools/cvpcb_selection_tool.cpp index b7905f3e7e..d041a381bc 100644 --- a/cvpcb/tools/cvpcb_selection_tool.cpp +++ b/cvpcb/tools/cvpcb_selection_tool.cpp @@ -99,7 +99,7 @@ int CVPCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // This tool is supposed to be active forever diff --git a/eeschema/tools/ee_picker_tool.cpp b/eeschema/tools/ee_picker_tool.cpp index 688fabaa23..e58dc8ed52 100644 --- a/eeschema/tools/ee_picker_tool.cpp +++ b/eeschema/tools/ee_picker_tool.cpp @@ -107,7 +107,7 @@ int EE_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) } else { - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } } diff --git a/eeschema/tools/ee_point_editor.cpp b/eeschema/tools/ee_point_editor.cpp index 8b18a17ad3..00b10d9be4 100644 --- a/eeschema/tools/ee_point_editor.cpp +++ b/eeschema/tools/ee_point_editor.cpp @@ -327,9 +327,7 @@ int EE_POINT_EDITOR::Main( const TOOL_EVENT& aEvent ) } else - { - m_toolMgr->PassEvent(); - } + evt->SetPassEvent(); controls->SetAutoPan( inDrag ); controls->CaptureCursor( inDrag ); diff --git a/eeschema/tools/ee_selection_tool.cpp b/eeschema/tools/ee_selection_tool.cpp index ce2927f971..a125c02d78 100644 --- a/eeschema/tools/ee_selection_tool.cpp +++ b/eeschema/tools/ee_selection_tool.cpp @@ -342,11 +342,6 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) // Single click? Select single object if( evt->IsClick( BUT_LEFT ) ) { - // JEY TODO: this is a hack, but I can't figure out why it's needed to - // keep from getting the first click when running the Place Symbol tool. - if( m_frame->GetCurrentToolName() != EE_ACTIONS::selectionTool.GetName() ) - continue; - if( evt->Modifier( MD_CTRL ) && dynamic_cast( m_frame ) ) { m_toolMgr->RunAction( EE_ACTIONS::highlightNet, true ); @@ -461,7 +456,7 @@ int EE_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // This tool is supposed to be active forever diff --git a/gerbview/tools/gerbview_selection_tool.cpp b/gerbview/tools/gerbview_selection_tool.cpp index 6361faab30..8339cb6de0 100644 --- a/gerbview/tools/gerbview_selection_tool.cpp +++ b/gerbview/tools/gerbview_selection_tool.cpp @@ -247,7 +247,7 @@ int GERBVIEW_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // This tool is supposed to be active forever diff --git a/include/tool/tool_event.h b/include/tool/tool_event.h index f313389d94..21a8de42e6 100644 --- a/include/tool/tool_event.h +++ b/include/tool/tool_event.h @@ -186,6 +186,9 @@ public: m_param( aParameter ) { m_hasPosition = ( aCategory == TC_MOUSE || aCategory == TC_COMMAND ); + + // By default only MESSAGEs are passed to multiple recipients + m_passEvent = ( aCategory == TC_MESSAGE ); } TOOL_EVENT( TOOL_EVENT_CATEGORY aCategory, TOOL_ACTIONS aAction, int aExtraParam, @@ -216,6 +219,9 @@ public: m_modifiers = aExtraParam & MD_MODIFIER_MASK; } + // By default only MESSAGEs are passed to multiple recipients + m_passEvent = ( aCategory == TC_MESSAGE ); + m_hasPosition = ( aCategory == TC_MOUSE || aCategory == TC_COMMAND ); } @@ -233,6 +239,9 @@ public: if( aCategory == TC_COMMAND || aCategory == TC_MESSAGE ) m_commandStr = aExtraParam; + // By default only MESSAGEs are passed to multiple recipients + m_passEvent = ( aCategory == TC_MESSAGE ); + m_hasPosition = ( aCategory == TC_MOUSE || aCategory == TC_COMMAND ); } @@ -242,6 +251,12 @@ public: ///> Returns more specific information about the type of an event. TOOL_ACTIONS Action() const { return m_actions; } + ///> These give a tool a method of informing the TOOL_MANAGER that a particular event should + ///> be passed on to subsequent tools on the stack. Defaults to true for TC_MESSAGES; false + ///> for everything else. + bool PassEvent() const { return m_passEvent; } + void SetPassEvent() { m_passEvent = true; } + ///> Returns if it this event has a valid position (true for mouse events and context-menu ///> or hotkey-based command events) bool HasPosition() const { return m_hasPosition; } @@ -456,6 +471,7 @@ private: TOOL_EVENT_CATEGORY m_category; TOOL_ACTIONS m_actions; TOOL_ACTION_SCOPE m_scope; + bool m_passEvent; bool m_hasPosition; ///> Difference between mouse cursor position and diff --git a/include/tool/tool_manager.h b/include/tool/tool_manager.h index 46b520055b..1d248db7b0 100644 --- a/include/tool/tool_manager.h +++ b/include/tool/tool_manager.h @@ -334,14 +334,6 @@ public: void ScheduleContextMenu( TOOL_BASE* aTool, ACTION_MENU* aMenu, CONTEXT_MENU_TRIGGER aTrigger ); - /** - * Allows a tool to pass the already handled event to the next tool on the stack. - */ - void PassEvent() - { - m_passEvent = true; - } - /** * Stores an information to the system clipboard. * @param aText is the information to be stored. @@ -524,9 +516,6 @@ private: /// Queue that stores events to be processed at the end of the event processing cycle. std::list m_eventQueue; - /// Flag saying if the currently processed event should be passed to other tools. - bool m_passEvent; - /// Right click context menu position. VECTOR2D m_menuCursor; diff --git a/pagelayout_editor/tools/pl_picker_tool.cpp b/pagelayout_editor/tools/pl_picker_tool.cpp index 9c31fc4f01..79776c1eff 100644 --- a/pagelayout_editor/tools/pl_picker_tool.cpp +++ b/pagelayout_editor/tools/pl_picker_tool.cpp @@ -134,9 +134,7 @@ int PL_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) // m_menu.ShowContextMenu(); } else - { - m_toolMgr->PassEvent(); - } + evt->SetPassEvent(); } if( m_finalizeHandler ) diff --git a/pagelayout_editor/tools/pl_point_editor.cpp b/pagelayout_editor/tools/pl_point_editor.cpp index c2b32e6475..22753ff51f 100644 --- a/pagelayout_editor/tools/pl_point_editor.cpp +++ b/pagelayout_editor/tools/pl_point_editor.cpp @@ -227,9 +227,7 @@ int PL_POINT_EDITOR::Main( const TOOL_EVENT& aEvent ) } else - { - m_toolMgr->PassEvent(); - } + evt->SetPassEvent(); controls->SetAutoPan( inDrag ); controls->CaptureCursor( inDrag ); diff --git a/pagelayout_editor/tools/pl_selection_tool.cpp b/pagelayout_editor/tools/pl_selection_tool.cpp index afca057af4..655080b9fc 100644 --- a/pagelayout_editor/tools/pl_selection_tool.cpp +++ b/pagelayout_editor/tools/pl_selection_tool.cpp @@ -217,7 +217,7 @@ int PL_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // This tool is supposed to be active forever diff --git a/pcbnew/tools/pcbnew_picker_tool.cpp b/pcbnew/tools/pcbnew_picker_tool.cpp index cbdce04924..7cd02b2cad 100644 --- a/pcbnew/tools/pcbnew_picker_tool.cpp +++ b/pcbnew/tools/pcbnew_picker_tool.cpp @@ -116,7 +116,7 @@ int PCBNEW_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } if( m_finalizeHandler ) diff --git a/pcbnew/tools/point_editor.cpp b/pcbnew/tools/point_editor.cpp index 69ec0984b2..4f04b39d8a 100644 --- a/pcbnew/tools/point_editor.cpp +++ b/pcbnew/tools/point_editor.cpp @@ -408,9 +408,7 @@ int POINT_EDITOR::OnSelectionChange( const TOOL_EVENT& aEvent ) } else - { - m_toolMgr->PassEvent(); - } + evt->SetPassEvent(); } if( m_editPoints ) diff --git a/pcbnew/tools/selection_tool.cpp b/pcbnew/tools/selection_tool.cpp index 67c23ca2bd..093559c046 100644 --- a/pcbnew/tools/selection_tool.cpp +++ b/pcbnew/tools/selection_tool.cpp @@ -278,11 +278,6 @@ int SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) // Single click? Select single object if( evt->IsClick( BUT_LEFT ) ) { - // JEY TODO: this is a hack, but I can't figure out why it's needed to - // keep from getting end-of-segment clicks when running the Draw Lines tool. - if( m_frame->GetToolId() != ID_NO_TOOL_SELECTED ) - continue; - if( evt->Modifier( MD_CTRL ) && !m_editModules ) { m_toolMgr->RunAction( PCB_ACTIONS::highlightNet, true ); @@ -377,7 +372,7 @@ int SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) } else - m_toolMgr->PassEvent(); + evt->SetPassEvent(); } // This tool is supposed to be active forever