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
This commit is contained in:
Jeff Young 2019-07-09 15:01:43 +01:00
parent d7f3a38510
commit 36f1d023f0
11 changed files with 25 additions and 34 deletions

View File

@ -135,7 +135,7 @@ int EE_PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
resetPicker(); resetPicker();
controls->ForceCursorPosition( false ); controls->ForceCursorPosition( false );
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -333,8 +333,7 @@ int LIB_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -1006,8 +1006,7 @@ int SCH_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -476,19 +476,16 @@ static bool probeSimulation( SCH_EDIT_FRAME* aFrame, const VECTOR2D& aPosition )
int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent ) int SCH_EDITOR_CONTROL::SimProbe( const TOOL_EVENT& aEvent )
{ {
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
m_frame->PushTool( aEvent.GetCommandStr().get() ); m_frame->PushTool( aEvent.GetCommandStr().get() );
Activate(); Activate();
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
assert( picker );
m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::PROBE ) ); m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::PROBE ) );
picker->SetClickHandler( std::bind( probeSimulation, m_frame, std::placeholders::_1 ) ); picker->SetClickHandler( std::bind( probeSimulation, m_frame, std::placeholders::_1 ) );
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; 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 ) int SCH_EDITOR_CONTROL::SimTune( const TOOL_EVENT& aEvent )
{ {
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
m_frame->PushTool( aEvent.GetCommandStr().get() ); m_frame->PushTool( aEvent.GetCommandStr().get() );
Activate(); Activate();
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
assert( picker );
m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::TUNE ) ); m_frame->GetCanvas()->SetCursor( SIMULATION_CURSORS::GetCursor( SIMULATION_CURSORS::CURSOR::TUNE ) );
picker->SetClickHandler( std::bind( tuneSimulation, m_frame, std::placeholders::_1 ) ); picker->SetClickHandler( std::bind( tuneSimulation, m_frame, std::placeholders::_1 ) );
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }
#endif /* KICAD_SPICE */ #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 ) if( !ADVANCED_CFG::GetCfg().m_realTimeConnectivity || !CONNECTION_GRAPH::m_allowRealTime )
m_frame->RecalculateConnections(); m_frame->RecalculateConnections();
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
m_frame->PushTool( aEvent.GetCommandStr().get() ); m_frame->PushTool( aEvent.GetCommandStr().get() );
Activate(); Activate();
EE_PICKER_TOOL* picker = m_toolMgr->GetTool<EE_PICKER_TOOL>();
wxCHECK( picker, 0 );
picker->SetClickHandler( std::bind( highlightNet, m_toolMgr, std::placeholders::_1 ) ); picker->SetClickHandler( std::bind( highlightNet, m_toolMgr, std::placeholders::_1 ) );
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -399,8 +399,7 @@ int PL_EDIT_TOOL::DeleteItemCursor( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -159,6 +159,7 @@ int PL_PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
resetPicker(); resetPicker();
controls->ForceCursorPosition( false ); controls->ForceCursorPosition( false );
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -1296,7 +1296,9 @@ bool EDIT_TOOL::pickCopyReferencePoint( VECTOR2I& aP )
bool picking = true; bool picking = true;
bool retVal = true; bool retVal = true;
frame()->PushTool( _( "Select reference point for the copy..." ) );
statusPopup.SetText( _( "Select reference point for the copy..." ) ); statusPopup.SetText( _( "Select reference point for the copy..." ) );
picker->Activate(); picker->Activate();
picker->SetClickHandler( [&]( const VECTOR2D& aPoint ) -> bool picker->SetClickHandler( [&]( const VECTOR2D& aPoint ) -> bool
{ {
@ -1324,6 +1326,7 @@ bool EDIT_TOOL::pickCopyReferencePoint( VECTOR2I& aP )
} }
statusPopup.Hide(); statusPopup.Hide();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
return retVal; return retVal;
} }

View File

@ -1132,8 +1132,7 @@ int PCB_EDITOR_CONTROL::DrillOrigin( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }
@ -1318,8 +1317,7 @@ int PCB_EDITOR_CONTROL::HighlightNetTool( const TOOL_EVENT& aEvent )
picker->SetLayerSet( LSET::AllCuMask() ); picker->SetLayerSet( LSET::AllCuMask() );
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }
@ -1399,8 +1397,7 @@ int PCB_EDITOR_CONTROL::LocalRatsnestTool( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -464,8 +464,7 @@ int PCBNEW_CONTROL::GridSetOrigin( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
} }
return 0; return 0;
@ -552,8 +551,7 @@ int PCBNEW_CONTROL::DeleteItemCursor( const TOOL_EVENT& aEvent )
picker->Activate(); picker->Activate();
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
m_frame->PopTool();
return 0; return 0;
} }

View File

@ -147,6 +147,7 @@ int PCBNEW_PICKER_TOOL::Main( const TOOL_EVENT& aEvent )
reset(); reset();
controls->ForceCursorPosition( false ); controls->ForceCursorPosition( false );
frame()->PopTool();
return 0; return 0;
} }

View File

@ -128,6 +128,7 @@ int POSITION_RELATIVE_TOOL::SelectPositionRelativeItem( const TOOL_EVENT& aEvent
STATUS_TEXT_POPUP statusPopup( frame() ); STATUS_TEXT_POPUP statusPopup( frame() );
bool picking = true; bool picking = true;
frame()->PushTool( _( "Select reference item..." ) );
statusPopup.SetText( _( "Select reference item..." ) ); statusPopup.SetText( _( "Select reference item..." ) );
picker->Activate(); picker->Activate();
@ -168,6 +169,7 @@ int POSITION_RELATIVE_TOOL::SelectPositionRelativeItem( const TOOL_EVENT& aEvent
{ {
statusPopup.Move( wxGetMousePosition() + wxPoint( 20, -50 ) ); statusPopup.Move( wxGetMousePosition() + wxPoint( 20, -50 ) );
Wait(); Wait();
// Picker calls PopTool() so that it gets done before activating tool's PushTool()
} }
return 0; return 0;