From cf4cbea8a19de9e325ad595ed2a4f409aabffd3e Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 22 Mar 2021 12:51:32 +0000 Subject: [PATCH] Use a more robust storage of items added to drag. We kept having various issues of the pointer list going stale. This will prevent all permutations of that. Also while I was in there I fixed the double-move thing (see bug number). Fixes https://gitlab.com/kicad/code/kicad/issues/7910 --- eeschema/tools/ee_selection_tool.cpp | 16 ++++++++- eeschema/tools/ee_selection_tool.h | 6 ++++ eeschema/tools/sch_move_tool.cpp | 52 +++++++++++++++++----------- eeschema/tools/sch_move_tool.h | 4 +-- 4 files changed, 54 insertions(+), 24 deletions(-) diff --git a/eeschema/tools/ee_selection_tool.cpp b/eeschema/tools/ee_selection_tool.cpp index c674696f2f..e6c369af2d 100644 --- a/eeschema/tools/ee_selection_tool.cpp +++ b/eeschema/tools/ee_selection_tool.cpp @@ -24,6 +24,7 @@ #include #include +#include #include #include #include @@ -51,7 +52,6 @@ #include #include - SELECTION_CONDITION EE_CONDITIONS::SingleSymbol = []( const SELECTION& aSel ) { if( aSel.GetSize() == 1 ) @@ -1483,6 +1483,20 @@ void EE_SELECTION_TOOL::RemoveItemsFromSel( EDA_ITEMS* aList, bool aQuietMode ) } +void EE_SELECTION_TOOL::RemoveItemsFromSel( std::vector* aList, bool aQuietMode ) +{ + EDA_ITEMS removeItems; + + for( EDA_ITEM* item : m_selection ) + { + if( alg::contains( *aList, item->m_Uuid ) ) + removeItems.push_back( item ); + } + + RemoveItemsFromSel( &removeItems, aQuietMode ); +} + + void EE_SELECTION_TOOL::BrightenItem( EDA_ITEM* aItem ) { highlight( aItem, BRIGHTENED ); diff --git a/eeschema/tools/ee_selection_tool.h b/eeschema/tools/ee_selection_tool.h index 3ab4f3169b..b263e50ce6 100644 --- a/eeschema/tools/ee_selection_tool.h +++ b/eeschema/tools/ee_selection_tool.h @@ -114,6 +114,12 @@ public: int RemoveItemsFromSel( const TOOL_EVENT& aEvent ); void RemoveItemsFromSel( EDA_ITEMS* aList, bool aQuietMode = false ); + /** + * A safer version of RemoveItemsFromSel( EDA_ITEMS ) which doesn't require the items to + * still exist. + */ + void RemoveItemsFromSel( std::vector* aList, bool aQuietMode = false ); + void BrightenItem( EDA_ITEM* aItem ); void UnbrightenItem( EDA_ITEM* aItem ); diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index 85abb16c25..f68549f9a6 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -47,7 +47,7 @@ SCH_MOVE_TOOL::SCH_MOVE_TOOL() : EE_TOOL_BASE( "eeschema.InteractiveMove" ), m_moveInProgress( false ), - m_isDragOperation( false ), + m_isDrag( false ), m_moveOffset( 0, 0 ) { } @@ -93,35 +93,39 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) EESCHEMA_SETTINGS* cfg = Pgm().GetSettingsManager().GetAppSettings(); KIGFX::VIEW_CONTROLS* controls = getViewControls(); EE_GRID_HELPER grid( m_toolMgr ); + bool wasDragging = m_moveInProgress && m_isDrag; m_anchorPos.reset(); if( aEvent.IsAction( &EE_ACTIONS::move ) ) - m_isDragOperation = false; + m_isDrag = false; else if( aEvent.IsAction( &EE_ACTIONS::drag ) ) - m_isDragOperation = true; + m_isDrag = true; else if( aEvent.IsAction( &EE_ACTIONS::moveActivate ) ) - m_isDragOperation = !cfg->m_Input.drag_is_move; + m_isDrag = !cfg->m_Input.drag_is_move; else return 0; if( m_moveInProgress ) { - auto sel = m_selectionTool->GetSelection().Front(); - - if( sel && !sel->IsNew() ) + if( m_isDrag != wasDragging ) { - // User must have switched from move to drag or vice-versa. Reset the selected - // items so we can start again with the current m_isDragOperation. - m_frame->RollbackSchematicFromUndo(); - m_selectionTool->RemoveItemsFromSel( &m_dragAdditions, QUIET_MODE ); - m_anchorPos = m_cursor - m_moveOffset; - m_moveInProgress = false; - controls->SetAutoPan( false ); + auto sel = m_selectionTool->GetSelection().Front(); - // And give it a kick so it doesn't have to wait for the first mouse movement to - // refresh. - m_toolMgr->RunAction( EE_ACTIONS::restartMove ); + if( sel && !sel->IsNew() ) + { + // Reset the selected items so we can start again with the current m_isDrag + // state. + m_frame->RollbackSchematicFromUndo(); + m_selectionTool->RemoveItemsFromSel( &m_dragAdditions, QUIET_MODE ); + m_anchorPos = m_cursor - m_moveOffset; + m_moveInProgress = false; + controls->SetAutoPan( false ); + + // And give it a kick so it doesn't have to wait for the first mouse movement + // to refresh. + m_toolMgr->RunAction( EE_ACTIONS::restartMove ); + } } return 0; @@ -196,8 +200,10 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) it->SetFlags( STARTPOINT | ENDPOINT ); } - if( m_isDragOperation ) + if( m_isDrag ) { + EDA_ITEMS connectedDragItems; + // Add connections to the selection for a drag. // for( EDA_ITEM* edaItem : selection ) @@ -211,10 +217,14 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) connections = item->GetConnectionPoints(); for( wxPoint point : connections ) - getConnectedDragItems( item, point, m_dragAdditions ); + getConnectedDragItems( item, point, connectedDragItems ); } - m_selectionTool->AddItemsToSel( &m_dragAdditions, QUIET_MODE ); + for( EDA_ITEM* item : connectedDragItems ) + { + m_dragAdditions.push_back( item->m_Uuid ); + m_selectionTool->AddItemToSel( item, QUIET_MODE ); + } } else { @@ -246,7 +256,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) if( item->IsNew() ) { - if( item->HasFlag( TEMP_SELECTED ) && m_isDragOperation ) + if( item->HasFlag( TEMP_SELECTED ) && m_isDrag ) { // Item was added in getConnectedDragItems saveCopyInUndoList( (SCH_ITEM*) item, UNDO_REDO::NEWITEM, appendUndo ); diff --git a/eeschema/tools/sch_move_tool.h b/eeschema/tools/sch_move_tool.h index 729e309322..1079eb2cc7 100644 --- a/eeschema/tools/sch_move_tool.h +++ b/eeschema/tools/sch_move_tool.h @@ -76,10 +76,10 @@ private: private: ///< Flag determining if anything is being dragged right now bool m_moveInProgress; - bool m_isDragOperation; + bool m_isDrag; ///< Items (such as wires) which were added to the selection for a drag - EDA_ITEMS m_dragAdditions; + std::vector m_dragAdditions; ///< Used for chaining commands VECTOR2I m_moveOffset;