From bce1fd337b2d4116273f5d266f195d5ed4c7997d Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Thu, 9 Jun 2022 16:53:24 +0100 Subject: [PATCH] Second tool hotkey accepts action (ie: acts as click). Fixes https://gitlab.com/kicad/code/kicad/issues/11729 --- eeschema/tools/sch_drawing_tools.cpp | 43 +++++++++++++++---- eeschema/tools/sch_line_wire_bus_tool.cpp | 19 +++++--- eeschema/tools/sch_move_tool.cpp | 9 ++++ .../tools/symbol_editor_drawing_tools.cpp | 34 ++++++++++++--- eeschema/tools/symbol_editor_drawing_tools.h | 4 ++ eeschema/tools/symbol_editor_move_tool.cpp | 12 ++++-- 6 files changed, 98 insertions(+), 23 deletions(-) diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index 118245a255..cfc2767f0f 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -214,6 +214,11 @@ int SCH_DRAWING_TOOLS::PlaceSymbol( const TOOL_EVENT& aEvent ) VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !evt->DisableGridSnapping() ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = symbol + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { m_frame->GetInfoBar()->Dismiss(); @@ -228,7 +233,7 @@ int SCH_DRAWING_TOOLS::PlaceSymbol( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( symbol && evt->IsMoveTool() ) { @@ -255,7 +260,7 @@ int SCH_DRAWING_TOOLS::PlaceSymbol( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) ) + else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) || isSyntheticClick ) { if( !symbol ) { @@ -466,6 +471,11 @@ int SCH_DRAWING_TOOLS::PlaceImage( const TOOL_EVENT& aEvent ) setCursor(); VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !evt->DisableGridSnapping() ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = image + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { m_frame->GetInfoBar()->Dismiss(); @@ -486,7 +496,7 @@ int SCH_DRAWING_TOOLS::PlaceImage( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( image && evt->IsMoveTool() ) { @@ -513,7 +523,7 @@ int SCH_DRAWING_TOOLS::PlaceImage( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) ) + else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) || isSyntheticClick ) { if( !image ) { @@ -1146,6 +1156,11 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) cursorPos = grid.BestSnapAnchor( cursorPos, snapLayer, item ); controls->ForceCursorPosition( true, cursorPos ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = item + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { m_frame->GetInfoBar()->Dismiss(); @@ -1160,7 +1175,7 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( item && evt->IsMoveTool() ) { @@ -1190,7 +1205,7 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) ) + else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) || isSyntheticClick ) { // First click creates... if( !item ) @@ -1457,6 +1472,11 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !evt->DisableGridSnapping() ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = item + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { if( item ) @@ -1469,7 +1489,7 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( item && evt->IsMoveTool() ) { @@ -1531,6 +1551,7 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) } else if( item && ( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) + || isSyntheticClick || evt->IsAction( &EE_ACTIONS::finishDrawing ) ) ) { if( evt->IsDblClick( BUT_LEFT ) @@ -1656,6 +1677,11 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent ) VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !evt->DisableGridSnapping() ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = sheet + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { m_frame->GetInfoBar()->Dismiss(); @@ -1670,7 +1696,7 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( sheet && evt->IsMoveTool() ) { @@ -1739,6 +1765,7 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent ) } else if( sheet && ( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) + || isSyntheticClick || evt->IsAction( &EE_ACTIONS::finishSheet ) ) ) { m_view->ClearPreview(); diff --git a/eeschema/tools/sch_line_wire_bus_tool.cpp b/eeschema/tools/sch_line_wire_bus_tool.cpp index 7d6e99729d..533b1a4a65 100644 --- a/eeschema/tools/sch_line_wire_bus_tool.cpp +++ b/eeschema/tools/sch_line_wire_bus_tool.cpp @@ -630,7 +630,12 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, while( TOOL_EVENT* evt = Wait() ) { LINE_MODE currentMode = (LINE_MODE) m_frame->eeconfig()->m_Drawing.line_mode; - bool twoSegments = currentMode != LINE_MODE::LINE_MODE_FREE; + bool twoSegments = currentMode != LINE_MODE::LINE_MODE_FREE; + + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = ( segment || m_busUnfold.in_progress ) + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( aTool ) == 0; setCursor(); grid.SetMask( GRID_HELPER::ALL ); @@ -656,8 +661,7 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, if( currentMode != lastMode ) { // Need to delete extra segment if we have one - if( currentMode == LINE_MODE::LINE_MODE_FREE && m_wires.size() >= 2 - && segment != nullptr ) + if( segment && currentMode == LINE_MODE::LINE_MODE_FREE && m_wires.size() >= 2 ) { m_wires.pop_back(); m_selectionTool->RemoveItemFromSel( segment ); @@ -667,7 +671,7 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, segment->SetEndPoint( cursorPos ); } // Add a segment so we can move orthogonally/45 - else if( lastMode == LINE_MODE::LINE_MODE_FREE && segment ) + else if( segment && lastMode == LINE_MODE::LINE_MODE_FREE ) { segment->SetEndPoint( cursorPos ); @@ -683,7 +687,6 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, lastMode = currentMode; } - //------------------------------------------------------------------------ // Handle cancel: // @@ -701,7 +704,7 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( segment || m_busUnfold.in_progress ) { @@ -744,7 +747,9 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType, //------------------------------------------------------------------------ // Handle click: // - else if( evt->IsClick( BUT_LEFT ) || ( segment && evt->IsDblClick( BUT_LEFT ) ) ) + else if( evt->IsClick( BUT_LEFT ) + || ( segment && evt->IsDblClick( BUT_LEFT ) ) + || isSyntheticClick ) { // First click when unfolding places the label and wire-to-bus entry if( m_busUnfold.in_progress && !m_busUnfold.label_placed ) diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index 259f43dfa7..a1ee6628e2 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -123,6 +123,11 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) m_toolMgr->RunAction( EE_ACTIONS::restartMove ); } } + else + { + // The tool hotkey is interpreted as a click when already dragging/moving + m_toolMgr->RunAction( ACTIONS::cursorClick ); + } return 0; } @@ -387,7 +392,9 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) splitMoves.emplace_back( VECTOR2I( delta.x + m_moveOffset.x, 0 ) ); } else + { splitMoves.emplace_back( VECTOR2I( delta.x, 0 ) ); + } if( alg::signbit( m_moveOffset.y ) != alg::signbit( ( m_moveOffset + delta ).y ) ) { @@ -395,7 +402,9 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) splitMoves.emplace_back( VECTOR2I( 0, delta.y + m_moveOffset.y ) ); } else + { splitMoves.emplace_back( VECTOR2I( 0, delta.y ) ); + } m_moveOffset += delta; diff --git a/eeschema/tools/symbol_editor_drawing_tools.cpp b/eeschema/tools/symbol_editor_drawing_tools.cpp index bc60df506a..2e219054fc 100644 --- a/eeschema/tools/symbol_editor_drawing_tools.cpp +++ b/eeschema/tools/symbol_editor_drawing_tools.cpp @@ -50,7 +50,9 @@ SYMBOL_EDITOR_DRAWING_TOOLS::SYMBOL_EDITOR_DRAWING_TOOLS() : m_lastFillColor( COLOR4D::UNSPECIFIED ), m_lastStroke( 0, PLOT_DASH_TYPE::DEFAULT, COLOR4D::UNSPECIFIED ), m_drawSpecificConvert( true ), - m_drawSpecificUnit( false ) + m_drawSpecificUnit( false ), + m_inDrawShape( false ), + m_inTwoClickPlace( false ) { } @@ -78,6 +80,11 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) auto* settings = Pgm().GetSettingsManager().GetAppSettings(); auto* pinTool = type == LIB_PIN_T ? m_toolMgr->GetTool() : nullptr; + if( m_inTwoClickPlace ) + return 0; + + REENTRANCY_GUARD guard( &m_inTwoClickPlace ); + KIGFX::VIEW_CONTROLS* controls = getViewControls(); EE_GRID_HELPER grid( m_toolMgr ); VECTOR2I cursorPos; @@ -137,6 +144,11 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) cursorPos = grid.Align( controls->GetMousePosition() ); controls->ForceCursorPosition( true, cursorPos ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = item + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { m_frame->GetInfoBar()->Dismiss(); @@ -151,7 +163,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( item && evt->IsMoveTool() ) { @@ -181,7 +193,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) ) + else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) || isSyntheticClick ) { LIB_SYMBOL* symbol = m_frame->GetCurSymbol(); @@ -318,6 +330,11 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) LIB_SHAPE* item = nullptr; bool isTextBox = aEvent.IsAction( &EE_ACTIONS::drawSymbolTextBox ); + if( m_inDrawShape ) + return 0; + + REENTRANCY_GUARD guard( &m_inDrawShape ); + // We might be running as the same shape in another co-routine. Make sure that one // gets whacked. m_toolMgr->DeactivateTool(); @@ -358,6 +375,11 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !evt->DisableGridSnapping() ); + // The tool hotkey is interpreted as a click when drawing + bool isSyntheticClick = item + && evt->IsActivate() && evt->HasPosition() + && evt->GetCommandStr().get().compare( tool ) == 0; + if( evt->IsCancelInteractive() ) { if( item ) @@ -370,7 +392,7 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) break; } } - else if( evt->IsActivate() ) + else if( evt->IsActivate() && !isSyntheticClick ) { if( item ) cleanup(); @@ -434,7 +456,9 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) m_selectionTool->AddItemToSel( item ); } - else if( item && ( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) + else if( item && ( evt->IsClick( BUT_LEFT ) + || evt->IsDblClick( BUT_LEFT ) + || isSyntheticClick || evt->IsAction( &EE_ACTIONS::finishDrawing ) ) ) { if( symbol != m_frame->GetCurSymbol() ) diff --git a/eeschema/tools/symbol_editor_drawing_tools.h b/eeschema/tools/symbol_editor_drawing_tools.h index 27fb4149d0..9ff4cf42a6 100644 --- a/eeschema/tools/symbol_editor_drawing_tools.h +++ b/eeschema/tools/symbol_editor_drawing_tools.h @@ -74,6 +74,10 @@ private: STROKE_PARAMS m_lastStroke; bool m_drawSpecificConvert; bool m_drawSpecificUnit; + + ///< Re-entrancy guards + bool m_inDrawShape; + bool m_inTwoClickPlace; }; #endif /* SYMBOL_EDITOR_DRAWING_TOOLS_H */ diff --git a/eeschema/tools/symbol_editor_move_tool.cpp b/eeschema/tools/symbol_editor_move_tool.cpp index 53ff63af18..33d174d1b0 100644 --- a/eeschema/tools/symbol_editor_move_tool.cpp +++ b/eeschema/tools/symbol_editor_move_tool.cpp @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2019 CERN - * Copyright (C) 2019-2021 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2019-2022 KiCad Developers, see AUTHORS.txt for contributors. * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include "symbol_editor_move_tool.h" @@ -103,9 +102,16 @@ int SYMBOL_EDITOR_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) : m_selectionTool->RequestSelection(); bool unselect = selection.IsHover(); - if( !m_frame->IsSymbolEditable() || selection.Empty() || m_moveInProgress ) + if( !m_frame->IsSymbolEditable() || selection.Empty() ) return 0; + if( m_moveInProgress ) + { + // The tool hotkey is interpreted as a click when already moving + m_toolMgr->RunAction( ACTIONS::cursorClick ); + return 0; + } + std::string tool = aEvent.GetCommandStr().get(); m_frame->PushTool( tool );