From 36f1d023f016f105eaf0eeadc231aa78e5034c31 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Tue, 9 Jul 2019 15:01:43 +0100 Subject: [PATCH] Make sure Push/PopTool() go in the right order. Because the pickers are called with an Activate()/Wait() pair, any usurping tool gets in between the picker handling the cancel (due to activation) and the picker client doing a PopTool(). The new tool is therefore pushed before the old tool is popped. Fixes: lp:1835907 * https://bugs.launchpad.net/kicad/+bug/1835907 --- eeschema/tools/ee_picker_tool.cpp | 2 +- eeschema/tools/lib_edit_tool.cpp | 3 +-- eeschema/tools/sch_edit_tool.cpp | 3 +-- eeschema/tools/sch_editor_control.cpp | 26 ++++++++-------------- pagelayout_editor/tools/pl_edit_tool.cpp | 3 +-- pagelayout_editor/tools/pl_picker_tool.cpp | 1 + pcbnew/tools/edit_tool.cpp | 3 +++ pcbnew/tools/pcb_editor_control.cpp | 9 +++----- pcbnew/tools/pcbnew_control.cpp | 6 ++--- pcbnew/tools/pcbnew_picker_tool.cpp | 1 + pcbnew/tools/position_relative_tool.cpp | 2 ++ 11 files changed, 25 insertions(+), 34 deletions(-) diff --git a/eeschema/tools/ee_picker_tool.cpp b/eeschema/tools/ee_picker_tool.cpp index 87dc7511e2..d68ae09aa0 100644 --- a/eeschema/tools/ee_picker_tool.cpp +++ b/eeschema/tools/ee_picker_tool.cpp @@ -135,7 +135,7 @@ int EE_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) resetPicker(); controls->ForceCursorPosition( false ); - + m_frame->PopTool(); return 0; } diff --git a/eeschema/tools/lib_edit_tool.cpp b/eeschema/tools/lib_edit_tool.cpp index f7dcddc7f8..7e98382aff 100644 --- a/eeschema/tools/lib_edit_tool.cpp +++ b/eeschema/tools/lib_edit_tool.cpp @@ -333,8 +333,7 @@ int LIB_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 19e81a9971..98c96e9e86 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1006,8 +1006,7 @@ int SCH_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index abb37f044d..26a8a33c93 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -476,19 +476,16 @@ static bool probeSimulation( SCH_EDIT_FRAME* aFrame, const VECTOR2D& aPosition ) int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent ) { + EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); + m_frame->PushTool( aEvent.GetCommandStr().get() ); Activate(); - - EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); - assert( picker ); - m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::PROBE ) ); picker->SetClickHandler( std::bind( probeSimulation, m_frame, std::placeholders::_1 ) ); picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } @@ -521,19 +518,16 @@ static bool tuneSimulation( SCH_EDIT_FRAME* aFrame, const VECTOR2D& aPosition ) int SCH_EDITOR_CONTROL::SimTune( const TOOL_EVENT& aEvent ) { + EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); + m_frame->PushTool( aEvent.GetCommandStr().get() ); Activate(); - - EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); - assert( picker ); - m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::TUNE ) ); picker->SetClickHandler( std::bind( tuneSimulation, m_frame, std::placeholders::_1 ) ); picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } #endif /* KICAD_SPICE */ @@ -690,17 +684,15 @@ int SCH_EDITOR_CONTROL::HighlightNetCursor( const TOOL_EVENT& aEvent ) if( !ADVANCED_CFG::GetCfg().m_realTimeConnectivity || !CONNECTION_GRAPH::m_allowRealTime ) m_frame->RecalculateConnections(); + EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); + m_frame->PushTool( aEvent.GetCommandStr().get() ); Activate(); - EE_PICKER_TOOL* picker = m_toolMgr->GetTool(); - wxCHECK( picker, 0 ); - picker->SetClickHandler( std::bind( highlightNet, m_toolMgr, std::placeholders::_1 ) ); picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/pagelayout_editor/tools/pl_edit_tool.cpp b/pagelayout_editor/tools/pl_edit_tool.cpp index 5fd5c9c5ed..e920006380 100644 --- a/pagelayout_editor/tools/pl_edit_tool.cpp +++ b/pagelayout_editor/tools/pl_edit_tool.cpp @@ -399,8 +399,7 @@ int PL_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/pagelayout_editor/tools/pl_picker_tool.cpp b/pagelayout_editor/tools/pl_picker_tool.cpp index 494d4170c1..8e62a655af 100644 --- a/pagelayout_editor/tools/pl_picker_tool.cpp +++ b/pagelayout_editor/tools/pl_picker_tool.cpp @@ -159,6 +159,7 @@ int PL_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) resetPicker(); controls->ForceCursorPosition( false ); + m_frame->PopTool(); return 0; } diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 981e561ed2..8ecd373395 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -1296,7 +1296,9 @@ bool EDIT_TOOL::pickCopyReferencePoint( VECTOR2I& aP ) bool picking = true; bool retVal = true; + frame()->PushTool( _( "Select reference point for the copy..." ) ); statusPopup.SetText( _( "Select reference point for the copy..." ) ); + picker->Activate(); picker->SetClickHandler( [&]( const VECTOR2D& aPoint ) -> bool { @@ -1324,6 +1326,7 @@ bool EDIT_TOOL::pickCopyReferencePoint( VECTOR2I& aP ) } statusPopup.Hide(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return retVal; } diff --git a/pcbnew/tools/pcb_editor_control.cpp b/pcbnew/tools/pcb_editor_control.cpp index cb7dc7265f..78656f2523 100644 --- a/pcbnew/tools/pcb_editor_control.cpp +++ b/pcbnew/tools/pcb_editor_control.cpp @@ -1132,8 +1132,7 @@ int PCB_EDITOR_CONTROL::DrillOrigin( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } @@ -1318,8 +1317,7 @@ int PCB_EDITOR_CONTROL::HighlightNetTool( const TOOL_EVENT& aEvent ) picker->SetLayerSet( LSET::AllCuMask() ); picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } @@ -1399,8 +1397,7 @@ int PCB_EDITOR_CONTROL::LocalRatsnestTool( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/pcbnew/tools/pcbnew_control.cpp b/pcbnew/tools/pcbnew_control.cpp index a452f22d98..5b8c33c192 100644 --- a/pcbnew/tools/pcbnew_control.cpp +++ b/pcbnew/tools/pcbnew_control.cpp @@ -464,8 +464,7 @@ int PCBNEW_CONTROL::GridSetOrigin( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() } return 0; @@ -552,8 +551,7 @@ int PCBNEW_CONTROL::DeleteItemCursor( const TOOL_EVENT& aEvent ) picker->Activate(); Wait(); - - m_frame->PopTool(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() return 0; } diff --git a/pcbnew/tools/pcbnew_picker_tool.cpp b/pcbnew/tools/pcbnew_picker_tool.cpp index 958763a0fc..36f0ab27f0 100644 --- a/pcbnew/tools/pcbnew_picker_tool.cpp +++ b/pcbnew/tools/pcbnew_picker_tool.cpp @@ -147,6 +147,7 @@ int PCBNEW_PICKER_TOOL::Main( const TOOL_EVENT& aEvent ) reset(); controls->ForceCursorPosition( false ); + frame()->PopTool(); return 0; } diff --git a/pcbnew/tools/position_relative_tool.cpp b/pcbnew/tools/position_relative_tool.cpp index cda9116e4b..41e0e25a6f 100644 --- a/pcbnew/tools/position_relative_tool.cpp +++ b/pcbnew/tools/position_relative_tool.cpp @@ -128,6 +128,7 @@ int POSITION_RELATIVE_TOOL::SelectPositionRelativeItem( const TOOL_EVENT& aEvent STATUS_TEXT_POPUP statusPopup( frame() ); bool picking = true; + frame()->PushTool( _( "Select reference item..." ) ); statusPopup.SetText( _( "Select reference item..." ) ); picker->Activate(); @@ -168,6 +169,7 @@ int POSITION_RELATIVE_TOOL::SelectPositionRelativeItem( const TOOL_EVENT& aEvent { statusPopup.Move( wxGetMousePosition() + wxPoint( 20, -50 ) ); Wait(); + // Picker calls PopTool() so that it gets done before activating tool's PushTool() } return 0;