From 6f389fd320a834c67a017ecaeb1b6838f304c8aa Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 18 Aug 2025 12:43:57 +0100 Subject: [PATCH] Tighten parent/child undo/redo architecture. Always look for pre-existing undo/redo record. Checking for IsNew() is less robust and should be avoided. Also moves the checking to a location where it will be easier to ensure that it's uniform. Push get-undo-level-item processing down a level so it is uniformly called. Make sure tables & labels are uniformly handled. Remove incorrect usage of Get/SetGroupId() for storing lastPin (which we don't use anyway). Lists of deleted and changed items MUST include the screen pointer. An item could be changed on one screen but not on another. Also tightens handling of PCB_NETINFO_T items, which are not in the view. Also fixes a bug where there is no increment parameter if you assign the base increment command to a hotkey. (This was discovered while testing the above changes.) Also fixes a bug where delete during a move in PCB Editor did an undo instead of a delete. (Again, found while testing above.) An experiment was also run to collapse shared parts of SCH_EDIT_FRAME and SYMBOL_EDITOR_FRAME into SCH_BASE_EDIT_FRAME. However, sharing the undo code actually increased complexity, and there was very little else of value in SCH_BASE_EDIT_FRAME (other than the Increment() routines). --- common/commit.cpp | 108 ++++---- common/tool/group_tool.h | 1 - eeschema/sch_commit.cpp | 80 ++---- eeschema/sch_commit.h | 12 +- .../symbol_editor/symbol_editor_undo_redo.cpp | 9 - eeschema/tools/sch_drawing_tools.cpp | 41 ++- eeschema/tools/sch_edit_tool.cpp | 169 +----------- eeschema/tools/sch_edit_tool.h | 22 +- eeschema/tools/sch_move_tool.cpp | 8 +- eeschema/tools/sch_selection_tool.cpp | 5 +- eeschema/tools/sch_tool_base.h | 249 +++++++++++++++++- .../tools/symbol_editor_drawing_tools.cpp | 6 +- eeschema/tools/symbol_editor_drawing_tools.h | 3 - eeschema/tools/symbol_editor_edit_tool.cpp | 194 +------------- eeschema/tools/symbol_editor_edit_tool.h | 9 +- eeschema/tools/symbol_editor_move_tool.cpp | 3 +- include/commit.h | 41 +-- include/tool/tool_event.h | 5 + pcbnew/board_commit.cpp | 205 +++++++------- pcbnew/board_commit.h | 9 +- pcbnew/initpcb.cpp | 6 +- pcbnew/tools/edit_tool.cpp | 66 ++--- pcbnew/tools/edit_tool_move_fct.cpp | 50 ++-- pcbnew/tools/multichannel_tool.cpp | 15 +- pcbnew/tools/pcb_group_tool.cpp | 8 + pcbnew/tools/pcb_group_tool.h | 25 +- pcbnew/tools/pcb_selection_tool.cpp | 151 +++++------ qa/tests/common/test_commit.cpp | 2 +- 28 files changed, 670 insertions(+), 832 deletions(-) diff --git a/common/commit.cpp b/common/commit.cpp index 51c4fa7284..5dc0ffa0b3 100644 --- a/common/commit.cpp +++ b/common/commit.cpp @@ -38,52 +38,63 @@ COMMIT::COMMIT() COMMIT::~COMMIT() { - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& ent : m_entries ) delete ent.m_copy; } -COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen, - RECURSE_MODE aRecurse ) +COMMIT& COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen, RECURSE_MODE aRecurse ) { + int flags = aChangeType & CHT_FLAGS; + int changeType = aChangeType & CHT_TYPE; + EDA_ITEM* undoItem = undoLevelItem( aItem ); + + if( undoItem != aItem ) + changeType = CHT_MODIFY; + // CHT_MODIFY and CHT_DONE are not compatible - wxASSERT( ( aChangeType & ( CHT_MODIFY | CHT_DONE ) ) != ( CHT_MODIFY | CHT_DONE ) ); + if( changeType == CHT_MODIFY ) + wxASSERT( ( flags & CHT_DONE ) == 0 ); - int flag = aChangeType & CHT_FLAGS; - - switch( aChangeType & CHT_TYPE ) + switch( changeType ) { case CHT_ADD: - makeEntry( aItem, CHT_ADD | flag, nullptr, aScreen ); + if( m_addedItems.find( { aItem, aScreen } ) != m_addedItems.end() ) + break; + + makeEntry( aItem, CHT_ADD | flags, nullptr, aScreen ); break; case CHT_REMOVE: - if( m_deletedItems.insert( aItem ).second ) - { - makeEntry( aItem, CHT_REMOVE | flag, makeImage( aItem ), aScreen ); + if( m_deletedItems.find( { aItem, aScreen } ) != m_deletedItems.end() ) + break; - if( EDA_GROUP* parentGroup = aItem->GetParentGroup() ) - Modify( parentGroup->AsEdaItem(), aScreen, RECURSE_MODE::NO_RECURSE ); - } + makeEntry( aItem, CHT_REMOVE | flags, makeImage( aItem ), aScreen ); + + if( EDA_GROUP* parentGroup = aItem->GetParentGroup() ) + Modify( parentGroup->AsEdaItem(), aScreen, RECURSE_MODE::NO_RECURSE ); break; case CHT_MODIFY: - if( EDA_ITEM* parent = parentObject( aItem ) ) - createModified( parent, makeImage( parent ), flag, aScreen ); + if( m_addedItems.find( { aItem, aScreen } ) != m_addedItems.end() ) + break; + if( m_changedItems.find( { undoItem, aScreen } ) != m_changedItems.end() ) + break; + + makeEntry( undoItem, CHT_MODIFY | flags, makeImage( undoItem ), aScreen ); break; default: - wxFAIL; + UNIMPLEMENTED_FOR( undoItem->GetClass() ); } return *this; } -COMMIT& COMMIT::Stage( std::vector &container, CHANGE_TYPE aChangeType, - BASE_SCREEN *aScreen ) +COMMIT& COMMIT::Stage( std::vector &container, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen ) { for( EDA_ITEM* item : container ) Stage( item, aChangeType, aScreen); @@ -121,7 +132,7 @@ COMMIT& COMMIT::Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag, BASE void COMMIT::Unstage( EDA_ITEM* aItem, BASE_SCREEN* aScreen ) { - std::erase_if( m_changes, + std::erase_if( m_entries, [&]( COMMIT_LINE& line ) { if( line.m_item == aItem && line.m_screen == aScreen ) @@ -139,28 +150,22 @@ void COMMIT::Unstage( EDA_ITEM* aItem, BASE_SCREEN* aScreen ) } -int COMMIT::GetStatus( EDA_ITEM* aItem, BASE_SCREEN *aScreen ) +COMMIT& COMMIT::Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy, BASE_SCREEN *aScreen ) { - COMMIT_LINE* entry = findEntry( parentObject( aItem ), aScreen ); + if( undoLevelItem( aItem ) != aItem ) + wxFAIL_MSG( "We've no way to get a copy of the undo level item at this point" ); + else + makeEntry( aItem, CHT_MODIFY, aCopy, aScreen ); - return entry ? entry->m_type : 0; + return *this; } -COMMIT& COMMIT::createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags, - BASE_SCREEN* aScreen ) +int COMMIT::GetStatus( EDA_ITEM* aItem, BASE_SCREEN *aScreen ) { - EDA_ITEM* parent = parentObject( aItem ); + COMMIT_LINE* entry = findEntry( undoLevelItem( aItem ), aScreen ); - if( m_changedItems.find( parent ) != m_changedItems.end() ) - { - delete aCopy; - return *this; // item has been already modified once - } - - makeEntry( parent, CHT_MODIFY | aExtraFlags, aCopy, aScreen ); - - return *this; + return entry ? entry->m_type : 0; } @@ -176,17 +181,24 @@ void COMMIT::makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy, BAS // N.B. Do not throw an assertion for multiple changed items. An item can be changed // multiple times in a single commit such as when importing graphics and grouping them. - m_changedItems.insert( aItem ); - m_changes.push_back( ent ); + switch( aType ) + { + case CHT_ADD: m_addedItems.insert( { aItem, aScreen } ); break; + case CHT_REMOVE: m_deletedItems.insert( { aItem, aScreen } ); break; + case CHT_MODIFY: m_changedItems.insert( { aItem, aScreen } ); break; + default: wxFAIL; break; + } + + m_entries.push_back( ent ); } COMMIT::COMMIT_LINE* COMMIT::findEntry( EDA_ITEM* aItem, BASE_SCREEN *aScreen ) { - for( COMMIT_LINE& change : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - if( change.m_item == aItem && change.m_screen == aScreen ) - return &change; + if( entry.m_item == aItem && entry.m_screen == aScreen ) + return &entry; } return nullptr; @@ -197,10 +209,10 @@ CHANGE_TYPE COMMIT::convert( UNDO_REDO aType ) const { switch( aType ) { - case UNDO_REDO::NEWITEM: return CHT_ADD; - case UNDO_REDO::DELETED: return CHT_REMOVE; - case UNDO_REDO::CHANGED: return CHT_MODIFY; - default: wxASSERT( false ); return CHT_MODIFY; + case UNDO_REDO::NEWITEM: return CHT_ADD; + case UNDO_REDO::DELETED: return CHT_REMOVE; + case UNDO_REDO::CHANGED: return CHT_MODIFY; + default: wxFAIL; return CHT_MODIFY; } } @@ -209,10 +221,10 @@ UNDO_REDO COMMIT::convert( CHANGE_TYPE aType ) const { switch( aType ) { - case CHT_ADD: return UNDO_REDO::NEWITEM; - case CHT_REMOVE: return UNDO_REDO::DELETED; - case CHT_MODIFY: return UNDO_REDO::CHANGED; - default: wxASSERT( false ); return UNDO_REDO::CHANGED; + case CHT_ADD: return UNDO_REDO::NEWITEM; + case CHT_REMOVE: return UNDO_REDO::DELETED; + case CHT_MODIFY: return UNDO_REDO::CHANGED; + default: wxFAIL; return UNDO_REDO::CHANGED; } } diff --git a/common/tool/group_tool.h b/common/tool/group_tool.h index 453d3fffcc..0c33490e1a 100644 --- a/common/tool/group_tool.h +++ b/common/tool/group_tool.h @@ -79,7 +79,6 @@ protected: DIALOG_GROUP_PROPERTIES* m_propertiesDialog = nullptr; SELECTION_TOOL* m_selectionTool = nullptr; std::shared_ptr m_commit; - bool m_isFootprintEditor = false; }; #endif diff --git a/eeschema/sch_commit.cpp b/eeschema/sch_commit.cpp index 17105ee45d..a48372ad5f 100644 --- a/eeschema/sch_commit.cpp +++ b/eeschema/sch_commit.cpp @@ -73,17 +73,6 @@ COMMIT& SCH_COMMIT::Stage( EDA_ITEM *aItem, CHANGE_TYPE aChangeType, BASE_SCREEN { wxCHECK( aItem, *this ); - // If aItem belongs a symbol, sheet or label, the full parent will be saved because undo/redo - // does not handle "sub items" modifications. - if( aItem->Type() != SCH_SHEET_T - && aItem->GetParent() && aItem->GetParent()->IsType( { SCH_SYMBOL_T, LIB_SYMBOL_T, - SCH_SHEET_T, - SCH_LABEL_LOCATE_ANY_T } ) ) - { - aItem = aItem->GetParent(); - aChangeType = CHT_MODIFY; - } - if( aRecurse == RECURSE_MODE::RECURSE ) { if( SCH_GROUP* group = dynamic_cast( aItem ) ) @@ -93,8 +82,7 @@ COMMIT& SCH_COMMIT::Stage( EDA_ITEM *aItem, CHANGE_TYPE aChangeType, BASE_SCREEN } } - // IS_SELECTED flag should not be set on undo items which were added for - // a drag operation. + // IS_SELECTED flag should not be set on undo items which were added for a drag operation. if( aItem->IsSelected() && aItem->HasFlag( SELECTED_BY_DRAG ) ) { aItem->ClearSelected(); @@ -120,18 +108,11 @@ COMMIT& SCH_COMMIT::Stage( std::vector &container, CHANGE_TYPE aChang } -COMMIT& SCH_COMMIT::Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag, - BASE_SCREEN *aScreen ) -{ - return COMMIT::Stage( aItems, aModFlag, aScreen ); -} - - void SCH_COMMIT::pushLibEdit( const wxString& aMessage, int aCommitFlags ) { // Symbol editor just saves copies of the whole symbol, so grab the first and discard the rest - LIB_SYMBOL* symbol = dynamic_cast( m_changes.front().m_item ); - LIB_SYMBOL* copy = dynamic_cast( m_changes.front().m_copy ); + LIB_SYMBOL* symbol = dynamic_cast( m_entries.front().m_item ); + LIB_SYMBOL* copy = dynamic_cast( m_entries.front().m_copy ); if( symbol ) { @@ -220,10 +201,10 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) Modify( enteredGroup ); // Handle wires with Hop Over shapes: - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - SCH_ITEM* schCopyItem = dynamic_cast( ent.m_copy ); - SCH_ITEM* schItem = dynamic_cast( ent.m_item ); + SCH_ITEM* schCopyItem = dynamic_cast( entry.m_copy ); + SCH_ITEM* schItem = dynamic_cast( entry.m_item ); if( schCopyItem && schCopyItem->Type() == SCH_LINE_T ) frame->UpdateHopOveredWires( schCopyItem ); @@ -233,10 +214,10 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) } - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - SCH_ITEM* schItem = dynamic_cast( ent.m_item ); - int changeType = ent.m_type & CHT_TYPE; + SCH_ITEM* schItem = dynamic_cast( entry.m_item ); + int changeType = entry.m_type & CHT_TYPE; wxCHECK2( schItem, continue ); @@ -244,12 +225,12 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) Modify( schItem->GetParentGroup()->AsEdaItem() ); } - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - int changeType = ent.m_type & CHT_TYPE; - int changeFlags = ent.m_type & CHT_FLAGS; - SCH_ITEM* schItem = dynamic_cast( ent.m_item ); - SCH_SCREEN* screen = dynamic_cast( ent.m_screen ); + int changeType = entry.m_type & CHT_TYPE; + int changeFlags = entry.m_type & CHT_FLAGS; + SCH_ITEM* schItem = dynamic_cast( entry.m_item ); + SCH_SCREEN* screen = dynamic_cast( entry.m_screen ); wxCHECK2( schItem, continue ); wxCHECK2( screen, continue ); @@ -311,8 +292,8 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) if( !( aCommitFlags & SKIP_UNDO ) ) { ITEM_PICKER itemWrapper( screen, schItem, UNDO_REDO::DELETED ); - itemWrapper.SetLink( ent.m_copy ); - ent.m_copy = nullptr; // We've transferred ownership to the undo list + itemWrapper.SetLink( entry.m_copy ); + entry.m_copy = nullptr; // We've transferred ownership to the undo list undoList.PushItem( itemWrapper ); } @@ -353,7 +334,7 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) case CHT_MODIFY: { - const SCH_ITEM* itemCopy = static_cast( ent.m_copy ); + const SCH_ITEM* itemCopy = static_cast( entry.m_copy ); SCH_SHEET_PATH currentSheet; if( frame ) @@ -368,8 +349,8 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) if( !( aCommitFlags & SKIP_UNDO ) ) { ITEM_PICKER itemWrapper( screen, schItem, UNDO_REDO::CHANGED ); - itemWrapper.SetLink( ent.m_copy ); - ent.m_copy = nullptr; // We've transferred ownership to the undo list + itemWrapper.SetLink( entry.m_copy ); + entry.m_copy = nullptr; // We've transferred ownership to the undo list undoList.PushItem( itemWrapper ); } @@ -396,8 +377,8 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) } // Delete any copies we still have ownership of - delete ent.m_copy; - ent.m_copy = nullptr; + delete entry.m_copy; + entry.m_copy = nullptr; // Clear all flags but SELECTED and others used to move and rotate commands, // after edition (selected items must keep their selection flag). @@ -484,16 +465,16 @@ void SCH_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) } -EDA_ITEM* SCH_COMMIT::parentObject( EDA_ITEM* aItem ) const +EDA_ITEM* SCH_COMMIT::undoLevelItem( EDA_ITEM* aItem ) const { EDA_ITEM* parent = aItem->GetParent(); - if( parent && ( parent->Type() == SCH_SYMBOL_T || parent->Type() == LIB_SYMBOL_T ) ) - return parent; - if( m_isLibEditor ) return static_cast( m_toolMgr->GetToolHolder() )->GetCurSymbol(); + if( parent && parent->IsType( { SCH_SYMBOL_T, SCH_TABLE_T, SCH_SHEET_T, SCH_LABEL_LOCATE_ANY_T } ) ) + return parent; + return aItem; } @@ -536,7 +517,7 @@ void SCH_COMMIT::revertLibEdit() // Symbol editor just saves copies of the whole symbol, so grab the first and discard the rest SYMBOL_EDIT_FRAME* frame = dynamic_cast( m_toolMgr->GetToolHolder() ); - LIB_SYMBOL* copy = dynamic_cast( m_changes.front().m_copy ); + LIB_SYMBOL* copy = dynamic_cast( m_entries.front().m_copy ); SCH_SELECTION_TOOL* selTool = m_toolMgr->GetTool(); if( frame && copy ) @@ -545,9 +526,6 @@ void SCH_COMMIT::revertLibEdit() m_toolMgr->ResetTools( TOOL_BASE::MODEL_RELOAD ); } - for( size_t ii = 1; ii < m_changes.size(); ++ii ) - delete m_changes[ii].m_copy; - if( selTool ) selTool->RebuildSelection(); @@ -562,7 +540,7 @@ void SCH_COMMIT::Revert() SCH_SELECTION_TOOL* selTool = m_toolMgr->GetTool(); SCH_SHEET_LIST sheets; - if( m_changes.empty() ) + if( m_entries.empty() ) return; if( m_isLibEditor ) @@ -576,7 +554,7 @@ void SCH_COMMIT::Revert() std::vector bulkRemovedItems; std::vector itemsChanged; - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& ent : m_entries ) { int changeType = ent.m_type & CHT_TYPE; int changeFlags = ent.m_type & CHT_FLAGS; @@ -703,9 +681,7 @@ void SCH_COMMIT::Revert() selTool->RebuildSelection(); if( frame ) - { frame->RecalculateConnections( nullptr, NO_CLEANUP ); - } clear(); } diff --git a/eeschema/sch_commit.h b/eeschema/sch_commit.h index daafb19bba..766bcb7009 100644 --- a/eeschema/sch_commit.h +++ b/eeschema/sch_commit.h @@ -23,8 +23,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef SCHEMATIC_COMMIT_H -#define SCHEMATIC_COMMIT_H +#pragma once #include @@ -51,19 +50,16 @@ public: virtual ~SCH_COMMIT(); - virtual void Push( const wxString& aMessage = wxT( "A commit" ), - int aCommitFlags = 0 ) override; + virtual void Push( const wxString& aMessage = wxT( "A commit" ), int aCommitFlags = 0 ) override; virtual void Revert() override; COMMIT& Stage( EDA_ITEM *aItem, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen = nullptr, RECURSE_MODE aRecurse = RECURSE_MODE::NO_RECURSE ) override; COMMIT& Stage( std::vector &container, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen = nullptr ) override; - COMMIT& Stage( const PICKED_ITEMS_LIST &aItems, UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED, - BASE_SCREEN *aScreen = nullptr ) override; private: - EDA_ITEM* parentObject( EDA_ITEM* aItem ) const override; + EDA_ITEM* undoLevelItem( EDA_ITEM* aItem ) const override; EDA_ITEM* makeImage( EDA_ITEM* aItem ) const override; @@ -76,5 +72,3 @@ private: TOOL_MANAGER* m_toolMgr; bool m_isLibEditor; }; - -#endif diff --git a/eeschema/symbol_editor/symbol_editor_undo_redo.cpp b/eeschema/symbol_editor/symbol_editor_undo_redo.cpp index 3ea20179a5..36e0e5eed8 100644 --- a/eeschema/symbol_editor/symbol_editor_undo_redo.cpp +++ b/eeschema/symbol_editor/symbol_editor_undo_redo.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -67,8 +66,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromRedoList() if( GetRedoCommandCount() <= 0 ) return; - auto* drawingTool = GetToolManager()->GetTool(); - // Load the last redo entry PICKED_ITEMS_LIST* redoCommand = PopCommandFromRedoList(); ITEM_PICKER redoWrapper = redoCommand->PopItem(); @@ -77,7 +74,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromRedoList() delete redoCommand; LIB_SYMBOL* symbol = (LIB_SYMBOL*) redoWrapper.GetItem(); - KIID lastPin = redoWrapper.GetGroupId(); UNDO_REDO undoRedoType = redoWrapper.GetStatus(); wxCHECK( symbol, /* void */ ); symbol->ClearFlags( UR_TRANSIENT ); @@ -97,7 +93,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromRedoList() // is now put in undo list and is owned by this list // Just set the current symbol to the symbol which come from the redo list m_symbol = symbol; - drawingTool->SetLastPin( lastPin ); if( undoRedoType == UNDO_REDO::LIB_RENAME ) { @@ -122,8 +117,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromUndoList() if( GetUndoCommandCount() <= 0 ) return; - auto* drawingTool = GetToolManager()->GetTool(); - // Load the last undo entry PICKED_ITEMS_LIST* undoCommand = PopCommandFromUndoList(); wxString description = undoCommand->GetDescription(); @@ -132,7 +125,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromUndoList() delete undoCommand; LIB_SYMBOL* symbol = (LIB_SYMBOL*) undoWrapper.GetItem(); - KIID lastPin = undoWrapper.GetGroupId(); UNDO_REDO undoRedoType = undoWrapper.GetStatus(); wxCHECK( symbol, /* void */ ); symbol->ClearFlags( UR_TRANSIENT ); @@ -152,7 +144,6 @@ void SYMBOL_EDIT_FRAME::GetSymbolFromUndoList() // is now put in redo list and is owned by this list. // Just set the current symbol to the symbol which come from the undo list m_symbol = symbol; - drawingTool->SetLastPin( lastPin ); if( undoRedoType == UNDO_REDO::LIB_RENAME ) { diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index 2181bde5e0..33a1e9c00c 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -51,9 +51,6 @@ #include #include #include -#include -#include -#include #include #include #include @@ -73,7 +70,7 @@ #include #include #include -#include +//#include #include #include @@ -2101,8 +2098,7 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) { m_statusPopup = std::make_unique( m_frame ); m_statusPopup->SetText( _( "No new hierarchical labels found." ) ); - m_statusPopup->Move( KIPLATFORM::UI::GetMousePosition() - + wxPoint( 20, 20 ) ); + m_statusPopup->Move( KIPLATFORM::UI::GetMousePosition() + wxPoint( 20, 20 ) ); m_statusPopup->PopupFor( 2000 ); item = nullptr; @@ -2237,10 +2233,12 @@ int SCH_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) } else if( evt->IsAction( &ACTIONS::increment ) ) { - m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, evt->Parameter() ); + if( evt->HasParameter() ) + m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, evt->Parameter() ); + else + m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, ACTIONS::INCREMENT { 1, 0 } ); } - else if( evt->IsAction( &ACTIONS::duplicate ) - || evt->IsAction( &SCH_ACTIONS::repeatDrawItem ) ) + else if( evt->IsAction( &ACTIONS::duplicate ) || evt->IsAction( &SCH_ACTIONS::repeatDrawItem ) ) { if( item ) { @@ -2442,10 +2440,10 @@ int SCH_DRAWING_TOOLS::DrawShape( const TOOL_EVENT& aEvent ) m_view->ClearPreview(); m_view->AddToPreview( item->Clone() ); } - else if( item && ( evt->IsClick( BUT_LEFT ) - || evt->IsDblClick( BUT_LEFT ) - || isSyntheticClick - || evt->IsAction( &ACTIONS::finishInteractive ) ) ) + else if( item && ( evt->IsClick( BUT_LEFT ) + || evt->IsDblClick( BUT_LEFT ) + || isSyntheticClick + || evt->IsAction( &ACTIONS::finishInteractive ) ) ) { if( evt->IsDblClick( BUT_LEFT ) || evt->IsAction( &ACTIONS::finishInteractive ) @@ -2616,9 +2614,8 @@ int SCH_DRAWING_TOOLS::DrawRuleArea( const TOOL_EVENT& aEvent ) cursorPos = grid.Align( controls->GetMousePosition(), GRID_HELPER_GRIDS::GRID_CONNECTABLE ); controls->ForceCursorPosition( true, cursorPos ); - polyGeomMgr.SetLeaderMode( m_frame->eeconfig()->m_Drawing.line_mode == LINE_MODE_FREE - ? LEADER_MODE::DIRECT - : LEADER_MODE::DEG45 ); + polyGeomMgr.SetLeaderMode( m_frame->eeconfig()->m_Drawing.line_mode == LINE_MODE_FREE ? LEADER_MODE::DIRECT + : LEADER_MODE::DEG45 ); if( evt->IsCancelInteractive() ) { @@ -2663,8 +2660,9 @@ int SCH_DRAWING_TOOLS::DrawRuleArea( const TOOL_EVENT& aEvent ) m_menu->ShowContextMenu( m_selectionTool->GetSelection() ); } // events that lock in nodes - else if( evt->IsClick( BUT_LEFT ) || evt->IsDblClick( BUT_LEFT ) - || evt->IsAction( &SCH_ACTIONS::closeOutline ) ) + else if( evt->IsClick( BUT_LEFT ) + || evt->IsDblClick( BUT_LEFT ) + || evt->IsAction( &SCH_ACTIONS::closeOutline ) ) { // Check if it is double click / closing line (so we have to finish the zone) const bool endPolygon = evt->IsDblClick( BUT_LEFT ) @@ -2692,10 +2690,9 @@ int SCH_DRAWING_TOOLS::DrawRuleArea( const TOOL_EVENT& aEvent ) } } } - else if( started - && ( evt->IsAction( &SCH_ACTIONS::deleteLastPoint ) - || evt->IsAction( &ACTIONS::doDelete ) - || evt->IsAction( &ACTIONS::undo ) ) ) + else if( started && ( evt->IsAction( &SCH_ACTIONS::deleteLastPoint ) + || evt->IsAction( &ACTIONS::doDelete ) + || evt->IsAction( &ACTIONS::undo ) ) ) { if( std::optional last = polyGeomMgr.DeleteLastCorner() ) { diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index a9d2ce411c..94f221f164 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1632,6 +1632,7 @@ int SCH_EDIT_TOOL::RepeatDrawItem( const TOOL_EVENT& aEvent ) static std::vector deletableItems = { + LIB_SYMBOL_T, SCH_MARKER_T, SCH_JUNCTION_T, SCH_LINE_T, @@ -1762,78 +1763,6 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) } -#define HITTEST_THRESHOLD_PIXELS 5 - - -int SCH_EDIT_TOOL::InteractiveDelete( const TOOL_EVENT& aEvent ) -{ - PICKER_TOOL* picker = m_toolMgr->GetTool(); - - m_toolMgr->RunAction( ACTIONS::selectionClear ); - m_pickerItem = nullptr; - - // Deactivate other tools; particularly important if another PICKER is currently running - Activate(); - - picker->SetCursor( KICURSOR::REMOVE ); - picker->SetSnapping( false ); - picker->ClearHandlers(); - - picker->SetClickHandler( - [this]( const VECTOR2D& aPosition ) -> bool - { - if( m_pickerItem ) - { - SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); - selectionTool->UnbrightenItem( m_pickerItem ); - selectionTool->AddItemToSel( m_pickerItem, true /*quiet mode*/ ); - m_toolMgr->RunAction( ACTIONS::doDelete ); - m_pickerItem = nullptr; - } - - return true; - } ); - - picker->SetMotionHandler( - [this]( const VECTOR2D& aPos ) - { - SCH_COLLECTOR collector; - collector.m_Threshold = KiROUND( getView()->ToWorld( HITTEST_THRESHOLD_PIXELS ) ); - collector.Collect( m_frame->GetScreen(), deletableItems, aPos ); - - SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); - selectionTool->GuessSelectionCandidates( collector, aPos ); - - EDA_ITEM* item = collector.GetCount() == 1 ? collector[ 0 ] : nullptr; - - if( m_pickerItem != item ) - { - if( m_pickerItem ) - selectionTool->UnbrightenItem( m_pickerItem ); - - m_pickerItem = item; - - if( m_pickerItem ) - selectionTool->BrightenItem( m_pickerItem ); - } - } ); - - picker->SetFinalizeHandler( - [this]( const int& aFinalState ) - { - if( m_pickerItem ) - m_toolMgr->GetTool()->UnbrightenItem( m_pickerItem ); - - // Wake the selection tool after exiting to ensure the cursor gets updated - m_toolMgr->PostAction( ACTIONS::selectionActivate ); - } ); - - m_toolMgr->RunAction( ACTIONS::pickerTool, &aEvent ); - - return 0; -} - - void SCH_EDIT_TOOL::editFieldText( SCH_FIELD* aField ) { KICAD_T parentType = aField->GetParent() ? aField->GetParent()->Type() : SCHEMATIC_T; @@ -2814,20 +2743,21 @@ int SCH_EDIT_TOOL::ChangeTextType( const TOOL_EVENT& aEvent ) m_frame->RemoveFromScreen( item, m_frame->GetScreen() ); - if( item->IsNew() ) + if( commit->GetStatus( item, m_frame->GetScreen() ) == CHT_ADD ) + { commit->Unstage( item, m_frame->GetScreen() ); + delete item; + } else + { commit->Removed( item, m_frame->GetScreen() ); + } m_frame->AddToScreen( newtext, m_frame->GetScreen() ); commit->Added( newtext, m_frame->GetScreen() ); if( selected ) m_toolMgr->RunAction( ACTIONS::selectItem, newtext ); - - // Otherwise, pointer is owned by the undo stack - if( item->IsNew() ) - delete item; } } @@ -3137,91 +3067,6 @@ int SCH_EDIT_TOOL::EditPageNumber( const TOOL_EVENT& aEvent ) } -int SCH_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) -{ - const ACTIONS::INCREMENT incParam = aEvent.Parameter(); - static const std::vector incrementable = { SCH_LABEL_T, SCH_GLOBAL_LABEL_T, - SCH_HIER_LABEL_T, SCH_TEXT_T }; - SCH_SELECTION& selection = m_selectionTool->RequestSelection( incrementable ); - - if( selection.Empty() ) - return 0; - - KICAD_T type = selection.Front()->Type(); - bool allSameType = true; - - for( EDA_ITEM* item : selection ) - { - if( item->Type() != type ) - { - allSameType = false; - break; - } - } - - // Incrementing multiple types at once seems confusing - // though it would work. - if( !allSameType ) - return 0; - - STRING_INCREMENTER incrementer; - // In schematics, it's probably less common to be operating - // on pin numbers which are usually IOSQXZ-skippy. - incrementer.SetSkipIOSQXZ( false ); - - // If we're coming via another action like 'Move', use that commit - SCH_COMMIT localCommit( m_toolMgr ); - SCH_COMMIT* commit = dynamic_cast( aEvent.Commit() ); - - if( !commit ) - commit = &localCommit; - - const auto modifyItem = - [&]( EDA_ITEM& aItem ) - { - if( aItem.IsNew() ) - m_toolMgr->PostAction( ACTIONS::refreshPreview ); - else - commit->Modify( &aItem, m_frame->GetScreen() ); - }; - - for( EDA_ITEM* item : selection ) - { - switch( item->Type() ) - { - case SCH_LABEL_T: - case SCH_GLOBAL_LABEL_T: - case SCH_HIER_LABEL_T: - case SCH_TEXT_T: - { - SCH_TEXT& label = static_cast( *item ); - - std::optional newLabel = incrementer.Increment( label.GetText(), - incParam.Delta, - incParam.Index ); - - if( newLabel ) - { - modifyItem( label ); - label.SetText( *newLabel ); - } - break; - } - default: - // No increment for other items (yet) - break; - } - } - - commit->Push( _( "Increment" ) ); - - if( selection.IsHover() ) - m_toolMgr->RunAction( ACTIONS::selectionClear ); - - return 0; -} - - int SCH_EDIT_TOOL::DdAppendFile( const TOOL_EVENT& aEvent ) { return m_toolMgr->RunAction( SCH_ACTIONS::importSheet, aEvent.Parameter() ); diff --git a/eeschema/tools/sch_edit_tool.h b/eeschema/tools/sch_edit_tool.h index 9bc241b6d3..21430faf46 100644 --- a/eeschema/tools/sch_edit_tool.h +++ b/eeschema/tools/sch_edit_tool.h @@ -22,22 +22,17 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef KICAD_SCH_EDIT_TOOL_H -#define KICAD_SCH_EDIT_TOOL_H +#pragma once #include #include -class SCH_EDIT_FRAME; -class SCH_SELECTION_TOOL; - - class SCH_EDIT_TOOL : public SCH_TOOL_BASE { public: SCH_EDIT_TOOL(); - ~SCH_EDIT_TOOL() override { } + ~SCH_EDIT_TOOL() = default; static const std::vector RotatableItems; @@ -57,11 +52,6 @@ public: int ChangeBodyStyle( const TOOL_EVENT& aEvent ); int EditPageNumber( const TOOL_EVENT& aEvent ); - /** - * Increment/decrement something about an item. - */ - int Increment( const TOOL_EVENT& aEvent ); - /** * Change a text type to another one. * @@ -84,9 +74,6 @@ public: ///< Delete the selected items, or the item under the cursor. int DoDelete( const TOOL_EVENT& aEvent ); - ///< Run the deletion tool. - int InteractiveDelete( const TOOL_EVENT& aEvent ); - /// Drag and drop int DdAppendFile( const TOOL_EVENT& aEvent ); @@ -102,9 +89,4 @@ private: ///< Set up handlers for various events. void setTransitions() override; - -private: - EDA_ITEM* m_pickerItem; }; - -#endif //KICAD_SCH_EDIT_TOOL_H diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index 64a1a76fe3..6b575329f9 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -936,7 +936,10 @@ bool SCH_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COMMIT* aComm } else if( evt->IsAction( &ACTIONS::increment ) ) { - m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, evt->Parameter() ); + if( evt->HasParameter() ) + m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, evt->Parameter() ); + else + m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, ACTIONS::INCREMENT { 1, 0 } ); } else if( evt->IsAction( &SCH_ACTIONS::toCLabel ) ) { @@ -1461,8 +1464,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_COMMIT* aCommit, SCH_ITEM* aSelec if( aPoint != line->GetStartPoint() && aPoint != line->GetEndPoint() ) { // Split line in half - if( !line->IsNew() ) - aCommit->Modify( line, m_frame->GetScreen() ); + aCommit->Modify( line, m_frame->GetScreen() ); VECTOR2I oldEnd = line->GetEndPoint(); line->SetEndPoint( aPoint ); diff --git a/eeschema/tools/sch_selection_tool.cpp b/eeschema/tools/sch_selection_tool.cpp index 35901fbd71..9838e6f4e5 100644 --- a/eeschema/tools/sch_selection_tool.cpp +++ b/eeschema/tools/sch_selection_tool.cpp @@ -745,10 +745,7 @@ int SCH_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) if( field >= 0 ) { const int delta = evt->Parameter(); - ACTIONS::INCREMENT incParams{ - delta > 0 ? 1 : -1, - field, - }; + ACTIONS::INCREMENT incParams{ delta > 0 ? 1 : -1, field }; m_toolMgr->RunAction( ACTIONS::increment, incParams ); } diff --git a/eeschema/tools/sch_tool_base.h b/eeschema/tools/sch_tool_base.h index a4c548c5e9..e8b58cee7e 100644 --- a/eeschema/tools/sch_tool_base.h +++ b/eeschema/tools/sch_tool_base.h @@ -24,6 +24,7 @@ #pragma once +#include "increment.h" #include #include #include @@ -35,6 +36,9 @@ #include #include #include +#include +#include +#include class SCH_SELECTION; @@ -55,7 +59,8 @@ public: m_frame( nullptr ), m_view( nullptr ), m_selectionTool( nullptr ), - m_isSymbolEditor( false ) + m_isSymbolEditor( false ), + m_pickerItem( nullptr ) {}; ~SCH_TOOL_BASE() override {}; @@ -101,6 +106,247 @@ public: return m_isSymbolEditor; } + int Increment( const TOOL_EVENT& aEvent ) + { + static const std::vector incrementable = { SCH_LABEL_T, + SCH_GLOBAL_LABEL_T, + SCH_HIER_LABEL_T, + SCH_PIN_T, + SCH_TEXT_T }; + + const ACTIONS::INCREMENT param = { 1, 0 }; + + if( aEvent.HasParameter() ) + aEvent.Parameter(); + + SCH_SELECTION& selection = m_selectionTool->RequestSelection( incrementable ); + + if( selection.Empty() ) + return 0; + + KICAD_T type = selection.Front()->Type(); + bool allSameType = true; + + for( EDA_ITEM* item : selection ) + { + if( item->Type() != type ) + { + allSameType = false; + break; + } + } + + // Incrementing multiple types at once seems confusing though it would work. + if( !allSameType ) + return 0; + + const VECTOR2I mousePosition = getViewControls()->GetMousePosition(); + + STRING_INCREMENTER incrementer; + // In schematics, it's probably less common to be operating + // on pin numbers which are usually IOSQXZ-skippy. + incrementer.SetSkipIOSQXZ( m_isSymbolEditor ); + + // If we're coming via another action like 'Move', use that commit + SCH_COMMIT localCommit( m_toolMgr ); + SCH_COMMIT* commit = dynamic_cast( aEvent.Commit() ); + + if( !commit ) + commit = &localCommit; + + const auto modifyItem = + [&]( EDA_ITEM& aItem ) + { + if( aItem.IsNew() ) + m_toolMgr->PostAction( ACTIONS::refreshPreview ); + + commit->Modify( &aItem, m_frame->GetScreen() ); + }; + + for( EDA_ITEM* item : selection ) + { + switch( item->Type() ) + { + case SCH_PIN_T: + { + SCH_PIN& pin = static_cast( *item ); + PIN_LAYOUT_CACHE& layout = pin.GetLayoutCache(); + + bool found = false; + OPT_BOX2I bbox = layout.GetPinNumberBBox(); + + if( bbox && bbox->Contains( mousePosition ) ) + { + std::optional nextNumber = incrementer.Increment( pin.GetNumber(), param.Delta, + param.Index ); + + if( nextNumber ) + { + modifyItem( pin ); + pin.SetNumber( *nextNumber ); + } + + found = true; + } + + if( !found ) + { + bbox = layout.GetPinNameBBox(); + + if( bbox && bbox->Contains( mousePosition ) ) + { + std::optional nextName = incrementer.Increment( pin.GetName(), param.Delta, + param.Index ); + + if( nextName ) + { + modifyItem( pin ); + pin.SetName( *nextName ); + } + + found = true; + } + } + break; + } + + case SCH_LABEL_T: + case SCH_GLOBAL_LABEL_T: + case SCH_HIER_LABEL_T: + case SCH_TEXT_T: + { + SCH_TEXT& label = static_cast( *item ); + + std::optional newLabel = incrementer.Increment( label.GetText(), param.Delta, + param.Index ); + + if( newLabel ) + { + modifyItem( label ); + label.SetText( *newLabel ); + } + + break; + } + + default: + // No increment for other items + break; + } + } + + commit->Push( _( "Increment" ) ); + + if( selection.IsHover() ) + m_toolMgr->RunAction( ACTIONS::selectionClear ); + + return 0; + } + + int InteractiveDelete( const TOOL_EVENT& aEvent ) + { + static std::vector deletableItems = + { + LIB_SYMBOL_T, + SCH_MARKER_T, + SCH_JUNCTION_T, + SCH_LINE_T, + SCH_BUS_BUS_ENTRY_T, + SCH_BUS_WIRE_ENTRY_T, + SCH_SHAPE_T, + SCH_RULE_AREA_T, + SCH_TEXT_T, + SCH_TEXTBOX_T, + SCH_TABLECELL_T, // Clear contents + SCH_TABLE_T, + SCH_LABEL_T, + SCH_GLOBAL_LABEL_T, + SCH_HIER_LABEL_T, + SCH_DIRECTIVE_LABEL_T, + SCH_NO_CONNECT_T, + SCH_SHEET_T, + SCH_SHEET_PIN_T, + SCH_SYMBOL_T, + SCH_FIELD_T, // Will be hidden + SCH_BITMAP_T, + SCH_GROUP_T + }; + + + PICKER_TOOL* picker = m_toolMgr->GetTool(); + + m_toolMgr->RunAction( ACTIONS::selectionClear ); + m_pickerItem = nullptr; + + // Deactivate other tools; particularly important if another PICKER is currently running + Activate(); + + picker->SetCursor( KICURSOR::REMOVE ); + picker->SetSnapping( false ); + picker->ClearHandlers(); + + picker->SetClickHandler( + [this]( const VECTOR2D& aPosition ) -> bool + { + if( m_pickerItem ) + { + SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); + selectionTool->UnbrightenItem( m_pickerItem ); + selectionTool->AddItemToSel( m_pickerItem, true /*quiet mode*/ ); + m_toolMgr->RunAction( ACTIONS::doDelete ); + m_pickerItem = nullptr; + } + + return true; + } ); + + picker->SetMotionHandler( + [this]( const VECTOR2D& aPos ) + { + SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); + SCH_COLLECTOR collector; + + selectionTool->CollectHits( collector, aPos, deletableItems ); + + // Remove unselectable items + for( int i = collector.GetCount() - 1; i >= 0; --i ) + { + if( !selectionTool->Selectable( collector[ i ] ) ) + collector.Remove( i ); + } + + if( collector.GetCount() > 1 ) + selectionTool->GuessSelectionCandidates( collector, aPos ); + + EDA_ITEM* item = collector.GetCount() == 1 ? collector[ 0 ] : nullptr; + + if( m_pickerItem != item ) + { + if( m_pickerItem ) + selectionTool->UnbrightenItem( m_pickerItem ); + + m_pickerItem = item; + + if( m_pickerItem ) + selectionTool->BrightenItem( m_pickerItem ); + } + } ); + + picker->SetFinalizeHandler( + [this]( const int& aFinalState ) + { + if( m_pickerItem ) + m_toolMgr->GetTool()->UnbrightenItem( m_pickerItem ); + + // Wake the selection tool after exiting to ensure the cursor gets updated + m_toolMgr->PostAction( ACTIONS::selectionActivate ); + } ); + + m_toolMgr->RunAction( ACTIONS::pickerTool, &aEvent ); + + return 0; + } + protected: /** * Similar to getView()->Update(), but also updates the SCH_SCREEN's RTree. @@ -155,4 +401,5 @@ protected: KIGFX::SCH_VIEW* m_view; SCH_SELECTION_TOOL* m_selectionTool; bool m_isSymbolEditor; + EDA_ITEM* m_pickerItem; }; diff --git a/eeschema/tools/symbol_editor_drawing_tools.cpp b/eeschema/tools/symbol_editor_drawing_tools.cpp index 42a8136e16..9f75c12c74 100644 --- a/eeschema/tools/symbol_editor_drawing_tools.cpp +++ b/eeschema/tools/symbol_editor_drawing_tools.cpp @@ -323,8 +323,10 @@ int SYMBOL_EDITOR_DRAWING_TOOLS::TwoClickPlace( const TOOL_EVENT& aEvent ) } else if( evt->IsAction( &ACTIONS::increment ) ) { - m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, - evt->Parameter() ); + if( evt->HasParameter() ) + m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, evt->Parameter() ); + else + m_toolMgr->RunSynchronousAction( ACTIONS::increment, &commit, ACTIONS::INCREMENT { 1, 0 } ); } else if( item && ( evt->IsAction( &ACTIONS::refreshPreview ) || evt->IsMotion() ) ) { diff --git a/eeschema/tools/symbol_editor_drawing_tools.h b/eeschema/tools/symbol_editor_drawing_tools.h index 5a909a9854..9f5d4fbd0e 100644 --- a/eeschema/tools/symbol_editor_drawing_tools.h +++ b/eeschema/tools/symbol_editor_drawing_tools.h @@ -65,9 +65,6 @@ public: void SetDrawSpecificUnit( bool aSpecific ) { m_drawSpecificUnit = aSpecific; } bool GetDrawSpecificUnit() const { return m_drawSpecificUnit; } - KIID GetLastPin() { return g_lastPin; } - void SetLastPin( KIID aPin ) { g_lastPin = aPin; } - private: int doDrawShape( const TOOL_EVENT& aEvent, std::optional aDrawingShape ); diff --git a/eeschema/tools/symbol_editor_edit_tool.cpp b/eeschema/tools/symbol_editor_edit_tool.cpp index 6f1e11095a..c653c045ba 100644 --- a/eeschema/tools/symbol_editor_edit_tool.cpp +++ b/eeschema/tools/symbol_editor_edit_tool.cpp @@ -52,8 +52,7 @@ #include SYMBOL_EDITOR_EDIT_TOOL::SYMBOL_EDITOR_EDIT_TOOL() : - SCH_TOOL_BASE( "eeschema.SymbolEditTool" ), - m_pickerItem( nullptr ) + SCH_TOOL_BASE( "eeschema.SymbolEditTool" ) { } @@ -483,86 +482,6 @@ int SYMBOL_EDITOR_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) } -#define HITTEST_THRESHOLD_PIXELS 5 - - -int SYMBOL_EDITOR_EDIT_TOOL::InteractiveDelete( const TOOL_EVENT& aEvent ) -{ - PICKER_TOOL* picker = m_toolMgr->GetTool(); - - m_toolMgr->RunAction( ACTIONS::selectionClear ); - m_pickerItem = nullptr; - - // Deactivate other tools; particularly important if another PICKER is currently running - Activate(); - - picker->SetCursor( KICURSOR::REMOVE ); - picker->SetSnapping( false ); - picker->ClearHandlers(); - - picker->SetClickHandler( - [this]( const VECTOR2D& aPosition ) -> bool - { - if( m_pickerItem ) - { - SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); - selectionTool->UnbrightenItem( m_pickerItem ); - selectionTool->AddItemToSel( m_pickerItem, true /*quiet mode*/ ); - m_toolMgr->RunAction( ACTIONS::doDelete ); - m_pickerItem = nullptr; - } - - return true; - } ); - - picker->SetMotionHandler( - [this]( const VECTOR2D& aPos ) - { - SCH_SELECTION_TOOL* selectionTool = m_toolMgr->GetTool(); - SCH_COLLECTOR collector; - - selectionTool->CollectHits( collector, aPos, nonFields ); - - // Remove unselectable items - for( int i = collector.GetCount() - 1; i >= 0; --i ) - { - if( !selectionTool->Selectable( collector[ i ] ) ) - collector.Remove( i ); - } - - if( collector.GetCount() > 1 ) - selectionTool->GuessSelectionCandidates( collector, aPos ); - - EDA_ITEM* item = collector.GetCount() == 1 ? collector[ 0 ] : nullptr; - - if( m_pickerItem != item ) - { - if( m_pickerItem ) - selectionTool->UnbrightenItem( m_pickerItem ); - - m_pickerItem = item; - - if( m_pickerItem ) - selectionTool->BrightenItem( m_pickerItem ); - } - } ); - - picker->SetFinalizeHandler( - [this]( const int& aFinalState ) - { - if( m_pickerItem ) - m_toolMgr->GetTool()->UnbrightenItem( m_pickerItem ); - - // Wake the selection tool after exiting to ensure the cursor gets updated - m_toolMgr->PostAction( ACTIONS::selectionActivate ); - } ); - - m_toolMgr->RunAction( ACTIONS::pickerTool, &aEvent ); - - return 0; -} - - int SYMBOL_EDITOR_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) { SCH_SELECTION& selection = m_selectionTool->RequestSelection(); @@ -1123,117 +1042,6 @@ int SYMBOL_EDITOR_EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) } -int SYMBOL_EDITOR_EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) -{ - const ACTIONS::INCREMENT incParam = aEvent.Parameter(); - SCH_SELECTION& selection = m_selectionTool->RequestSelection( { SCH_PIN_T, SCH_TEXT_T } ); - - if( selection.Empty() ) - return 0; - - KICAD_T type = selection.Front()->Type(); - bool allSameType = true; - for( EDA_ITEM* item : selection ) - { - if( item->Type() != type ) - { - allSameType = false; - break; - } - } - - // Incrementing multiple types at once seems confusing - // though it would work. - if( !allSameType ) - return 0; - - const VECTOR2I mousePosition = getViewControls()->GetMousePosition(); - - STRING_INCREMENTER incrementer; - incrementer.SetSkipIOSQXZ( true ); - - // If we're coming via another action like 'Move', use that commit - SCH_COMMIT localCommit( m_toolMgr ); - SCH_COMMIT* commit = dynamic_cast( aEvent.Commit() ); - - if( !commit ) - commit = &localCommit; - - const auto modifyItem = [&]( EDA_ITEM& aItem ) - { - if( aItem.IsNew() ) - m_toolMgr->PostAction( ACTIONS::refreshPreview ); - else - commit->Modify( &aItem, m_frame->GetScreen() ); - }; - - for( EDA_ITEM* item : selection ) - { - switch( item->Type() ) - { - case SCH_PIN_T: - { - SCH_PIN& pin = static_cast( *item ); - PIN_LAYOUT_CACHE& layout = pin.GetLayoutCache(); - - bool found = false; - OPT_BOX2I bbox = layout.GetPinNumberBBox(); - - if( bbox && bbox->Contains( mousePosition ) ) - { - std::optional nextNumber = - incrementer.Increment( pin.GetNumber(), incParam.Delta, incParam.Index ); - if( nextNumber ) - { - modifyItem( pin ); - pin.SetNumber( *nextNumber ); - } - found = true; - } - - if( !found ) - { - bbox = layout.GetPinNameBBox(); - - if( bbox && bbox->Contains( mousePosition ) ) - { - std::optional nextName = - incrementer.Increment( pin.GetName(), incParam.Delta, incParam.Index ); - if( nextName ) - { - modifyItem( pin ); - pin.SetName( *nextName ); - } - found = true; - } - } - break; - } - case SCH_TEXT_T: - { - SCH_TEXT& label = static_cast( *item ); - - std::optional newLabel = - incrementer.Increment( label.GetText(), incParam.Delta, incParam.Index ); - if( newLabel ) - { - modifyItem( label ); - label.SetText( *newLabel ); - } - break; - } - default: - // No increment for other items - break; - } - } - - commit->Push( _( "Increment" ) ); - - return 0; -} - - void SYMBOL_EDITOR_EDIT_TOOL::setTransitions() { // clang-format off diff --git a/eeschema/tools/symbol_editor_edit_tool.h b/eeschema/tools/symbol_editor_edit_tool.h index 822196cffa..1369902ab6 100644 --- a/eeschema/tools/symbol_editor_edit_tool.h +++ b/eeschema/tools/symbol_editor_edit_tool.h @@ -36,7 +36,7 @@ class SYMBOL_EDITOR_EDIT_TOOL : public SCH_TOOL_BASE { public: SYMBOL_EDITOR_EDIT_TOOL(); - ~SYMBOL_EDITOR_EDIT_TOOL() override { } + ~SYMBOL_EDITOR_EDIT_TOOL() = default; /// @copydoc TOOL_INTERACTIVE::Init() bool Init() override; @@ -59,16 +59,11 @@ public: int CopyAsText( const TOOL_EVENT& aEvent ); int Paste( const TOOL_EVENT& aEvent ); - int Increment( const TOOL_EVENT& aEvent ); - /** * Delete the selected items, or the item under the cursor. */ int DoDelete( const TOOL_EVENT& aEvent ); - ///< Run the deletion tool. - int InteractiveDelete( const TOOL_EVENT& aEvent ); - private: void editShapeProperties( SCH_SHAPE* aShape ); void editTextProperties( SCH_ITEM* aItem ); @@ -79,6 +74,4 @@ private: ///< Set up handlers for various events. void setTransitions() override; - - EDA_ITEM* m_pickerItem; }; diff --git a/eeschema/tools/symbol_editor_move_tool.cpp b/eeschema/tools/symbol_editor_move_tool.cpp index 5d049a0bb9..f50a927258 100644 --- a/eeschema/tools/symbol_editor_move_tool.cpp +++ b/eeschema/tools/symbol_editor_move_tool.cpp @@ -153,8 +153,7 @@ bool SYMBOL_EDITOR_MOVE_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, SCH_COM VECTOR2I prevPos; VECTOR2I moveOffset; - if( !selection.Front()->IsNew() ) - aCommit->Modify( m_frame->GetCurSymbol(), m_frame->GetScreen() ); + aCommit->Modify( m_frame->GetCurSymbol(), m_frame->GetScreen() ); m_cursor = controls->GetCursorPosition( !aEvent.DisableGridSnapping() ); diff --git a/include/commit.h b/include/commit.h index 74183b31e8..cf9eb3e255 100644 --- a/include/commit.h +++ b/include/commit.h @@ -25,8 +25,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef __COMMIT_H -#define __COMMIT_H +#pragma once #include #include @@ -115,20 +114,7 @@ public: * * @note Requires a copy done before the modification. */ - COMMIT& Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy, BASE_SCREEN *aScreen = nullptr ) - { - return createModified( aItem, aCopy, 0, aScreen ); - } - - template - - COMMIT& StageItems( const Range& aRange, CHANGE_TYPE aChangeType ) - { - for( const auto& item : aRange ) - Stage( item, aChangeType ); - - return *this; - } + COMMIT& Modified( EDA_ITEM* aItem, EDA_ITEM* aCopy, BASE_SCREEN *aScreen = nullptr ); /// Add a change of the item aItem of type aChangeType to the change list. virtual COMMIT& Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen = nullptr, @@ -137,8 +123,7 @@ public: virtual COMMIT& Stage( std::vector& container, CHANGE_TYPE aChangeType, BASE_SCREEN *aScreen = nullptr ); - virtual COMMIT& Stage( const PICKED_ITEMS_LIST& aItems, - UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED, + virtual COMMIT& Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag = UNDO_REDO::UNSPECIFIED, BASE_SCREEN *aScreen = nullptr ); void Unstage( EDA_ITEM* aItem, BASE_SCREEN* aScreen ); @@ -151,13 +136,13 @@ public: bool Empty() const { - return m_changes.empty(); + return m_entries.empty(); } /// Returns status of an item. int GetStatus( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr ); - EDA_ITEM* GetFirst() const { return m_changes.empty() ? nullptr : m_changes[0].m_item; } + EDA_ITEM* GetFirst() const { return m_entries.empty() ? nullptr : m_entries[0].m_item; } protected: struct COMMIT_LINE @@ -165,7 +150,6 @@ protected: EDA_ITEM* m_item; ///< Main item that is added/deleted/modified EDA_ITEM* m_copy; ///< Optional copy of the item CHANGE_TYPE m_type; ///< Modification type - KIID m_parent = NilUuid(); ///< Parent item (primarily for undo of deleted items) BASE_SCREEN* m_screen; }; @@ -174,12 +158,9 @@ protected: { m_changedItems.clear(); m_deletedItems.clear(); - m_changes.clear(); + m_entries.clear(); } - COMMIT& createModified( EDA_ITEM* aItem, EDA_ITEM* aCopy, int aExtraFlags = 0, - BASE_SCREEN *aScreen = nullptr ); - virtual void makeEntry( EDA_ITEM* aItem, CHANGE_TYPE aType, EDA_ITEM* aCopy = nullptr, BASE_SCREEN *aScreen = nullptr ); @@ -190,7 +171,7 @@ protected: */ COMMIT_LINE* findEntry( EDA_ITEM* aItem, BASE_SCREEN *aScreen = nullptr ); - virtual EDA_ITEM* parentObject( EDA_ITEM* aItem ) const = 0; + virtual EDA_ITEM* undoLevelItem( EDA_ITEM* aItem ) const = 0; virtual EDA_ITEM* makeImage( EDA_ITEM* aItem ) const = 0; @@ -198,9 +179,9 @@ protected: UNDO_REDO convert( CHANGE_TYPE aType ) const; protected: - std::set m_changedItems; - std::set m_deletedItems; - std::vector m_changes; + std::set> m_addedItems; + std::set> m_changedItems; + std::set> m_deletedItems; + std::vector m_entries; }; -#endif diff --git a/include/tool/tool_event.h b/include/tool/tool_event.h index 40afefc914..ae49111cca 100644 --- a/include/tool/tool_event.h +++ b/include/tool/tool_event.h @@ -458,6 +458,11 @@ public: */ bool IsSimulator() const; + bool HasParameter() const + { + return m_param.has_value(); + } + /** * Return a parameter assigned to the event. Its meaning depends on the target tool. */ diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp index edba28be60..73eacfac17 100644 --- a/pcbnew/board_commit.cpp +++ b/pcbnew/board_commit.cpp @@ -50,6 +50,7 @@ using namespace std::placeholders; BOARD_COMMIT::BOARD_COMMIT( TOOL_BASE* aTool ) : + COMMIT(), m_toolMgr( aTool->GetManager() ), m_isBoardEditor( false ), m_isFootprintEditor( false ) @@ -63,6 +64,7 @@ BOARD_COMMIT::BOARD_COMMIT( TOOL_BASE* aTool ) : BOARD_COMMIT::BOARD_COMMIT( EDA_DRAW_FRAME* aFrame ) : + COMMIT(), m_toolMgr( aFrame->GetToolManager() ), m_isBoardEditor( aFrame->IsType( FRAME_PCB_EDITOR ) ), m_isFootprintEditor( aFrame->IsType( FRAME_FOOTPRINT_EDITOR ) ) @@ -71,7 +73,8 @@ BOARD_COMMIT::BOARD_COMMIT( EDA_DRAW_FRAME* aFrame ) : BOARD_COMMIT::BOARD_COMMIT( TOOL_MANAGER* aMgr ) : - m_toolMgr( aMgr ), + COMMIT(), + m_toolMgr( aMgr ), m_isBoardEditor( false ), m_isFootprintEditor( false ) { @@ -83,10 +86,12 @@ BOARD_COMMIT::BOARD_COMMIT( TOOL_MANAGER* aMgr ) : m_isFootprintEditor = true; } -BOARD_COMMIT::BOARD_COMMIT( TOOL_MANAGER* aMgr, bool aIsBoardEditor ) : - m_toolMgr( aMgr ), - m_isBoardEditor( aIsBoardEditor ), - m_isFootprintEditor( false ) + +BOARD_COMMIT::BOARD_COMMIT( TOOL_MANAGER* aMgr, bool aIsBoardEditor, bool aIsFootprintEditor ) : + COMMIT(), + m_toolMgr( aMgr ), + m_isBoardEditor( aIsBoardEditor ), + m_isFootprintEditor( aIsFootprintEditor ) { } @@ -97,8 +102,7 @@ BOARD* BOARD_COMMIT::GetBoard() const } -COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen, - RECURSE_MODE aRecurse ) +COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen, RECURSE_MODE aRecurse ) { if( aRecurse == RECURSE_MODE::RECURSE ) { @@ -113,15 +117,13 @@ COMMIT& BOARD_COMMIT::Stage( EDA_ITEM* aItem, CHANGE_TYPE aChangeType, BASE_SCRE } -COMMIT& BOARD_COMMIT::Stage( std::vector& container, CHANGE_TYPE aChangeType, - BASE_SCREEN* aScreen ) +COMMIT& BOARD_COMMIT::Stage( std::vector& container, CHANGE_TYPE aChangeType, BASE_SCREEN* aScreen ) { return COMMIT::Stage( container, aChangeType, aScreen ); } -COMMIT& BOARD_COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag, - BASE_SCREEN* aScreen ) +COMMIT& BOARD_COMMIT::Stage( const PICKED_ITEMS_LIST& aItems, UNDO_REDO aModFlag, BASE_SCREEN* aScreen ) { return COMMIT::Stage( aItems, aModFlag, aScreen ); } @@ -136,7 +138,7 @@ void BOARD_COMMIT::propagateDamage( BOARD_ITEM* aChangedItem, std::vector aStaleZones->push_back( static_cast( aChangedItem ) ); aChangedItem->RunOnChildren( std::bind( &BOARD_COMMIT::propagateDamage, this, _1, aStaleZones, aStaleRuleAreas ), - RECURSE_MODE::NO_RECURSE ); + RECURSE_MODE::NO_RECURSE ); BOARD* board = static_cast( m_toolMgr->GetModel() ); BOX2I damageBBox = aChangedItem->GetBoundingBox(); @@ -163,11 +165,8 @@ void BOARD_COMMIT::propagateDamage( BOARD_ITEM* aChangedItem, std::vector if( zone->GetIsRuleArea() ) continue; - if( ( zone->GetLayerSet() & damageLayers ).any() - && zone->GetBoundingBox().Intersects( damageBBox ) ) - { + if( ( zone->GetLayerSet() & damageLayers ).any() && zone->GetBoundingBox().Intersects( damageBBox ) ) aStaleZones->push_back( zone ); - } } } } @@ -211,9 +210,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) std::vector bulkRemovedItems; std::vector itemsChanged; - if( m_isBoardEditor - && !( aCommitFlags & ZONE_FILL_OP ) - && ( frame && frame->GetPcbNewSettings()->m_AutoRefillZones ) ) + if( m_isBoardEditor && !( aCommitFlags & ZONE_FILL_OP ) + && ( frame && frame->GetPcbNewSettings()->m_AutoRefillZones ) ) { autofillZones = true; staleZones = &staleZonesStorage; @@ -222,12 +220,12 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) zone->CacheBoundingBox(); } - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - if( !ent.m_item || !ent.m_item->IsBOARD_ITEM() ) + if( !entry.m_item || !entry.m_item->IsBOARD_ITEM() ) continue; - BOARD_ITEM* boardItem = static_cast( ent.m_item ); + BOARD_ITEM* boardItem = static_cast( entry.m_item ); if( m_isBoardEditor ) { @@ -297,14 +295,14 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) Modify( enteredGroup ); - for( COMMIT_LINE& ent : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { - if( !ent.m_item || !ent.m_item->IsBOARD_ITEM() ) + if( !entry.m_item || !entry.m_item->IsBOARD_ITEM() ) continue; - BOARD_ITEM* boardItem = static_cast( ent.m_item ); - int changeType = ent.m_type & CHT_TYPE; - int changeFlags = ent.m_type & CHT_FLAGS; + BOARD_ITEM* boardItem = static_cast( entry.m_item ); + int changeType = entry.m_type & CHT_TYPE; + int changeFlags = entry.m_type & CHT_FLAGS; switch( changeType ) { @@ -319,14 +317,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) { if( m_isFootprintEditor ) { - FOOTPRINT* parentFP = board->GetFirstFootprint(); - wxCHECK2_MSG( parentFP, continue, "Commit thinks this is footprint editor, but " - "there is no first footprint!" ); - parentFP->Add( boardItem ); - } - else if( FOOTPRINT* parentFP = boardItem->GetParentFootprint() ) - { - parentFP->Add( boardItem ); + if( FOOTPRINT* parentFP = board->GetFirstFootprint() ) + parentFP->Add( boardItem ); } else { @@ -347,14 +339,13 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) case CHT_REMOVE: { - FOOTPRINT* parentFP = boardItem->GetParentFootprint(); EDA_GROUP* parentGroup = boardItem->GetParentGroup(); if( !( aCommitFlags & SKIP_UNDO ) ) { ITEM_PICKER itemWrapper( nullptr, boardItem, UNDO_REDO::DELETED ); - itemWrapper.SetLink( ent.m_copy ); - ent.m_copy = nullptr; // We've transferred ownership to the undo list + itemWrapper.SetLink( entry.m_copy ); + entry.m_copy = nullptr; // We've transferred ownership to the undo list undoList.PushItem( itemWrapper ); } @@ -369,9 +360,6 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) if( parentGroup && !( parentGroup->AsEdaItem()->GetFlags() & STRUCT_DELETED ) ) parentGroup->RemoveItem( boardItem ); - if( parentFP && !( parentFP->GetFlags() & STRUCT_DELETED ) ) - ent.m_parent = parentFP->m_Uuid; - if( boardItem->Type() != PCB_MARKER_T ) propagateDamage( boardItem, staleZones, staleRuleAreas ); @@ -383,21 +371,21 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) case PCB_TEXT_T: case PCB_PAD_T: - case PCB_SHAPE_T: // a shape (normally not on copper layers) - case PCB_REFERENCE_IMAGE_T: // a bitmap on an associated layer - case PCB_GENERATOR_T: // a generator on a layer - case PCB_TEXTBOX_T: // a line-wrapped (and optionally bordered) text item - case PCB_TABLE_T: // rows and columns of tablecells - case PCB_TRACE_T: // a track segment (segment on a copper layer) - case PCB_ARC_T: // an arced track segment (segment on a copper layer) - case PCB_VIA_T: // a via (like track segment on a copper layer) - case PCB_DIM_ALIGNED_T: // a dimension (graphic item) + case PCB_SHAPE_T: + case PCB_REFERENCE_IMAGE_T: + case PCB_GENERATOR_T: + case PCB_TEXTBOX_T: + case PCB_TABLE_T: + case PCB_TRACE_T: + case PCB_ARC_T: + case PCB_VIA_T: + case PCB_DIM_ALIGNED_T: case PCB_DIM_CENTER_T: case PCB_DIM_RADIAL_T: case PCB_DIM_ORTHOGONAL_T: - case PCB_DIM_LEADER_T: // a leader dimension - case PCB_TARGET_T: // a target (graphic item) - case PCB_MARKER_T: // a marker used to show something + case PCB_DIM_LEADER_T: + case PCB_TARGET_T: + case PCB_MARKER_T: case PCB_ZONE_T: case PCB_FOOTPRINT_T: case PCB_GROUP_T: @@ -406,9 +394,10 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) if( !( changeFlags & CHT_DONE ) ) { - if( parentFP ) + if( m_isFootprintEditor ) { - parentFP->Remove( boardItem ); + if( FOOTPRINT* parentFP = board->GetFirstFootprint() ) + parentFP->Remove( boardItem ); } else { @@ -437,13 +426,13 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) case CHT_MODIFY: { - BOARD_ITEM* boardItemCopy = static_cast( ent.m_copy ); + BOARD_ITEM* boardItemCopy = static_cast( entry.m_copy ); if( !( aCommitFlags & SKIP_UNDO ) ) { ITEM_PICKER itemWrapper( nullptr, boardItem, UNDO_REDO::CHANGED ); - itemWrapper.SetLink( ent.m_copy ); - ent.m_copy = nullptr; // We've transferred ownership to the undo list + itemWrapper.SetLink( entry.m_copy ); + entry.m_copy = nullptr; // We've transferred ownership to the undo list undoList.PushItem( itemWrapper ); } @@ -461,7 +450,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) updateComponentClasses( boardItem ); - if( view ) + if( view && boardItem->Type() != PCB_NETINFO_T ) view->Update( boardItem ); itemsChanged.push_back( boardItem ); @@ -474,8 +463,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) } // Delete any copies we still have ownership of - delete ent.m_copy; - ent.m_copy = nullptr; + delete entry.m_copy; + entry.m_copy = nullptr; boardItem->ClearEditFlags(); boardItem->RunOnChildren( @@ -491,7 +480,7 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) if( m_isBoardEditor ) { - size_t num_changes = m_changes.size(); + size_t originalCount = m_entries.size(); if( aCommitFlags & SKIP_CONNECTIVITY ) { @@ -548,36 +537,36 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) } // Log undo items for any connectivity or teardrop changes - for( size_t i = num_changes; i < m_changes.size(); ++i ) + for( size_t i = originalCount; i < m_entries.size(); ++i ) { - COMMIT_LINE& ent = m_changes[i]; + COMMIT_LINE& entry = m_entries[i]; BOARD_ITEM* boardItem = nullptr; BOARD_ITEM* boardItemCopy = nullptr; - if( ent.m_item && ent.m_item->IsBOARD_ITEM() ) - boardItem = static_cast( ent.m_item ); + if( entry.m_item && entry.m_item->IsBOARD_ITEM() ) + boardItem = static_cast( entry.m_item ); - if( ent.m_copy && ent.m_copy->IsBOARD_ITEM() ) - boardItemCopy = static_cast( ent.m_copy ); + if( entry.m_copy && entry.m_copy->IsBOARD_ITEM() ) + boardItemCopy = static_cast( entry.m_copy ); wxCHECK2( boardItem, continue ); if( !( aCommitFlags & SKIP_UNDO ) ) { - ITEM_PICKER itemWrapper( nullptr, boardItem, convert( ent.m_type & CHT_TYPE ) ); + ITEM_PICKER itemWrapper( nullptr, boardItem, convert( entry.m_type & CHT_TYPE ) ); itemWrapper.SetLink( boardItemCopy ); undoList.PushItem( itemWrapper ); } else { - delete ent.m_copy; + delete entry.m_copy; } - if( view ) + if( view && boardItem->Type() != PCB_NETINFO_T ) { - if( ( ent.m_type & CHT_TYPE ) == CHT_ADD ) + if( ( entry.m_type & CHT_TYPE ) == CHT_ADD ) view->Add( boardItem ); - else if( ( ent.m_type & CHT_TYPE ) == CHT_REMOVE ) + else if( ( entry.m_type & CHT_TYPE ) == CHT_REMOVE ) view->Remove( boardItem ); else view->Update( boardItem ); @@ -640,8 +629,21 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) } -EDA_ITEM* BOARD_COMMIT::parentObject( EDA_ITEM* aItem ) const +EDA_ITEM* BOARD_COMMIT::undoLevelItem( EDA_ITEM* aItem ) const { + // Easiest way to disallow both a parent and one of its children appearing in the list + // is to only ever add the parent when either can be legal (ie: in the board editor). + if( m_isBoardEditor && aItem->IsBOARD_ITEM() ) + { + if( FOOTPRINT* footprint = static_cast( aItem )->GetParentFootprint() ) + return footprint; + } + + EDA_ITEM* parent = aItem->GetParent(); + + if( parent && parent->Type() == PCB_TABLE_T ) + return parent; + return aItem; } @@ -670,20 +672,21 @@ void BOARD_COMMIT::Revert() board->IncrementTimeStamp(); // clear caches - auto updateComponentClasses = [this]( BOARD_ITEM* boardItem ) - { - if( boardItem->Type() != PCB_FOOTPRINT_T ) - return; + auto updateComponentClasses = + [this]( BOARD_ITEM* boardItem ) + { + if( boardItem->Type() != PCB_FOOTPRINT_T ) + return; - FOOTPRINT* footprint = static_cast( boardItem ); - GetBoard()->GetComponentClassManager().RebuildRequiredCaches( footprint ); - }; + FOOTPRINT* footprint = static_cast( boardItem ); + GetBoard()->GetComponentClassManager().RebuildRequiredCaches( footprint ); + }; std::vector bulkAddedItems; std::vector bulkRemovedItems; std::vector itemsChanged; - for( COMMIT_LINE& entry : m_changes ) + for( COMMIT_LINE& entry : m_entries ) { if( !entry.m_item || !entry.m_item->IsBOARD_ITEM() ) continue; @@ -698,11 +701,13 @@ void BOARD_COMMIT::Revert() if( !( changeFlags & CHT_DONE ) ) break; - view->Remove( boardItem ); + if( boardItem->Type() != PCB_NETINFO_T ) + view->Remove( boardItem ); - if( FOOTPRINT* parentFP = boardItem->GetParentFootprint() ) + if( m_isFootprintEditor ) { - parentFP->Remove( boardItem ); + if( FOOTPRINT* parentFP = board->GetFirstFootprint() ) + parentFP->Remove( boardItem ); } else { @@ -717,36 +722,38 @@ void BOARD_COMMIT::Revert() if( !( changeFlags & CHT_DONE ) ) break; - view->Add( boardItem ); + if( boardItem->Type() != PCB_NETINFO_T ) + view->Add( boardItem ); - if( BOARD_ITEM* parent = board->ResolveItem( entry.m_parent, true ) ) + if( m_isFootprintEditor ) { - if( parent->Type() == PCB_FOOTPRINT_T ) - { - static_cast( parent )->Add( boardItem, ADD_MODE::INSERT ); - } - else - { - board->Add( boardItem, ADD_MODE::INSERT ); - bulkAddedItems.push_back( boardItem ); - } + if( FOOTPRINT* parentFP = board->GetFirstFootprint() ) + parentFP->Add( boardItem, ADD_MODE::INSERT ); + } + else + { + board->Add( boardItem, ADD_MODE::INSERT ); + bulkAddedItems.push_back( boardItem ); } updateComponentClasses( boardItem ); - break; } case CHT_MODIFY: { - view->Remove( boardItem ); + if( boardItem->Type() != PCB_NETINFO_T ) + view->Remove( boardItem ); + connectivity->Remove( boardItem ); wxASSERT( entry.m_copy && entry.m_copy->IsBOARD_ITEM() ); BOARD_ITEM* boardItemCopy = static_cast( entry.m_copy ); boardItem->SwapItemData( boardItemCopy ); - view->Add( boardItem ); + if( boardItem->Type() != PCB_NETINFO_T ) + view->Add( boardItem ); + connectivity->Add( boardItem ); itemsChanged.push_back( boardItem ); diff --git a/pcbnew/board_commit.h b/pcbnew/board_commit.h index 72a3621095..6d840f1be1 100644 --- a/pcbnew/board_commit.h +++ b/pcbnew/board_commit.h @@ -23,8 +23,7 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#ifndef BOARD_COMMIT_H -#define BOARD_COMMIT_H +#pragma once #include @@ -51,7 +50,7 @@ public: BOARD_COMMIT( EDA_DRAW_FRAME* aFrame ); BOARD_COMMIT( TOOL_BASE* aTool ); BOARD_COMMIT( TOOL_MANAGER* aMgr ); - BOARD_COMMIT( TOOL_MANAGER* aMgr, bool aIsBoardEditor ); + BOARD_COMMIT( TOOL_MANAGER* aMgr, bool aIsBoardEditor, bool aIsFootprintEditor ); virtual ~BOARD_COMMIT() {} @@ -72,7 +71,7 @@ public: static EDA_ITEM* MakeImage( EDA_ITEM* aItem ); private: - EDA_ITEM* parentObject( EDA_ITEM* aItem ) const override; + EDA_ITEM* undoLevelItem( EDA_ITEM* aItem ) const override; EDA_ITEM* makeImage( EDA_ITEM* aItem ) const override; @@ -84,5 +83,3 @@ private: bool m_isBoardEditor; bool m_isFootprintEditor; }; - -#endif diff --git a/pcbnew/initpcb.cpp b/pcbnew/initpcb.cpp index 6aeff8c45e..6e768c1da8 100644 --- a/pcbnew/initpcb.cpp +++ b/pcbnew/initpcb.cpp @@ -134,6 +134,9 @@ bool FOOTPRINT_EDIT_FRAME::Clear_Pcb( bool doAskAboutUnsavedChanges ) ClearUndoRedoList(); GetScreen()->SetContentModified( false ); + // Clear the view so we don't attempt redraws + GetCanvas()->GetView()->Clear(); + if( !m_isClosing ) { SetBoard( new BOARD ); @@ -169,9 +172,6 @@ bool FOOTPRINT_EDIT_FRAME::Clear_Pcb( bool doAskAboutUnsavedChanges ) { if( m_toolManager ) m_toolManager->ResetTools( TOOL_BASE::MODEL_RELOAD ); - - // Clear the view so we don't attempt redraws - GetCanvas()->GetView()->Clear(); } diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 964417cba9..857710b61f 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -2182,8 +2182,7 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) { for( EDA_ITEM* item : selection ) { - if( !item->IsNew() && !item->IsMoving() ) - commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); + commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); if( item->IsBOARD_ITEM() ) { @@ -2262,6 +2261,7 @@ const std::vector EDIT_TOOL::MirrorableItems = { PCB_GENERATOR_T, }; + int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) { if( isRouterActive() ) @@ -2318,8 +2318,7 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) if( !item->IsType( MirrorableItems ) ) continue; - if( !item->IsNew() && !item->IsMoving() ) - commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); + commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); // modify each object as necessary switch( item->Type() ) @@ -2426,16 +2425,12 @@ int EDIT_TOOL::JustifyText( const TOOL_EVENT& aEvent ) { if( item->Type() == PCB_FIELD_T || item->Type() == PCB_TEXT_T ) { - if( !item->IsNew() && !item->IsMoving() ) - commit->Modify( item ); - + commit->Modify( item ); setJustify( static_cast( item ) ); } else if( item->Type() == PCB_TEXTBOX_T ) { - if( !item->IsNew() && !item->IsMoving() ) - commit->Modify( item ); - + commit->Modify( item ); setJustify( static_cast( item ) ); } } @@ -2524,8 +2519,7 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) BOARD_ITEM* boardItem = static_cast( item ); - if( !boardItem->IsNew() && !boardItem->IsMoving() ) - commit->Modify( boardItem, nullptr, RECURSE_MODE::RECURSE ); + commit->Modify( boardItem, nullptr, RECURSE_MODE::RECURSE ); boardItem->Flip( refPt, flipDirection ); boardItem->Normalize(); @@ -2862,8 +2856,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) BOARD_ITEM* boardItem = static_cast( item ); - if( !boardItem->IsNew() ) - commit.Modify( boardItem, nullptr, RECURSE_MODE::RECURSE ); + commit.Modify( boardItem, nullptr, RECURSE_MODE::RECURSE ); if( !boardItem->GetParent() || !boardItem->GetParent()->IsSelected() ) boardItem->Move( translation ); @@ -3080,20 +3073,20 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) { const auto incrementableFilter = []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) - { - for( int i = aCollector.GetCount() - 1; i >= 0; i-- ) - { - switch( aCollector[i]->Type() ) { - case PCB_PAD_T: - case PCB_TEXT_T: - break; - default: - aCollector.Remove( i ); - break; - } - } - }; + for( int i = aCollector.GetCount() - 1; i >= 0; i-- ) + { + switch( aCollector[i]->Type() ) + { + case PCB_PAD_T: + case PCB_TEXT_T: + break; + default: + aCollector.Remove( i ); + break; + } + } + }; PCB_SELECTION& selection = m_selectionTool->RequestSelection( incrementableFilter, true /* prompt user regarding locked items */ ); @@ -3101,7 +3094,10 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) if( selection.Empty() ) return 0; - const ACTIONS::INCREMENT incParam = aEvent.Parameter(); + ACTIONS::INCREMENT param = { 1, 0 }; + + if( aEvent.HasParameter() ) + param = aEvent.Parameter(); STRING_INCREMENTER incrementer; incrementer.SetSkipIOSQXZ( true ); @@ -3129,14 +3125,11 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) continue; // Increment on the pad numbers - std::optional newNumber = incrementer.Increment( pad.GetNumber(), incParam.Delta, - incParam.Index ); + std::optional newNumber = incrementer.Increment( pad.GetNumber(), param.Delta, param.Index ); if( newNumber ) { - if( !pad.IsNew() ) - commit->Modify( &pad ); - + commit->Modify( &pad ); pad.SetNumber( *newNumber ); } @@ -3146,14 +3139,11 @@ int EDIT_TOOL::Increment( const TOOL_EVENT& aEvent ) { PCB_TEXT& text = static_cast( *item ); - std::optional newText = incrementer.Increment( text.GetText(), incParam.Delta, - incParam.Index ); + std::optional newText = incrementer.Increment( text.GetText(), param.Delta, param.Index ); if( newText ) { - if( !text.IsNew() ) - commit->Modify( &text ); - + commit->Modify( &text ); text.SetText( *newText ); } diff --git a/pcbnew/tools/edit_tool_move_fct.cpp b/pcbnew/tools/edit_tool_move_fct.cpp index b98244fb16..e7824827d6 100644 --- a/pcbnew/tools/edit_tool_move_fct.cpp +++ b/pcbnew/tools/edit_tool_move_fct.cpp @@ -47,8 +47,6 @@ #include #include #include -#include -#include #include #include @@ -92,10 +90,7 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) // Save items, so changes can be undone for( EDA_ITEM* item : selection ) - { - if( !item->IsNew() && !item->IsMoving() ) - commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); - } + commit->Modify( item, nullptr, RECURSE_MODE::RECURSE ); for( size_t i = 0; i < sorted.size() - 1; i++ ) { @@ -474,23 +469,22 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit if( evt->IsMotion() || evt->IsDrag( BUT_LEFT ) ) eatFirstMouseUp = false; - if( evt->IsAction( &PCB_ACTIONS::move ) - || evt->IsMotion() - || evt->IsDrag( BUT_LEFT ) - || evt->IsAction( &ACTIONS::refreshPreview ) - || evt->IsAction( &PCB_ACTIONS::moveWithReference ) - || evt->IsAction( &PCB_ACTIONS::moveIndividually ) ) + if( evt->IsAction( &PCB_ACTIONS::move ) + || evt->IsMotion() + || evt->IsDrag( BUT_LEFT ) + || evt->IsAction( &ACTIONS::refreshPreview ) + || evt->IsAction( &PCB_ACTIONS::moveWithReference ) + || evt->IsAction( &PCB_ACTIONS::moveIndividually ) ) { - if( m_dragging && ( evt->IsMotion() - || evt->IsDrag( BUT_LEFT ) - || evt->IsAction( &ACTIONS::refreshPreview ) ) ) + if( m_dragging && ( evt->IsMotion() + || evt->IsDrag( BUT_LEFT ) + || evt->IsAction( &ACTIONS::refreshPreview ) ) ) { bool redraw3D = false; VECTOR2I mousePos( controls->GetMousePosition() ); - m_cursor = grid.BestSnapAnchor( mousePos, layers, - grid.GetSelectionGrid( selection ), sel_items ); + m_cursor = grid.BestSnapAnchor( mousePos, layers, grid.GetSelectionGrid( selection ), sel_items ); if( controls->GetSettings().m_lastKeyboardCursorPositionValid ) { @@ -498,8 +492,7 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit grid.SetUseGrid( false ); } - m_cursor = grid.BestSnapAnchor( mousePos, layers, - grid.GetSelectionGrid( selection ), sel_items ); + m_cursor = grid.BestSnapAnchor( mousePos, layers, grid.GetSelectionGrid( selection ), sel_items ); if( !selection.HasReferencePoint() ) originalPos = m_cursor; @@ -657,8 +650,7 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit // Use the mouse position over cursor, as otherwise large grids will allow only // snapping to items that are closest to grid points - m_cursor = grid.BestDragOrigin( originalMousePos, sel_items, - grid.GetSelectionGrid( selection ), + m_cursor = grid.BestDragOrigin( originalMousePos, sel_items, grid.GetSelectionGrid( selection ), &m_selectionTool->GetFilter() ); // Set the current cursor position to the first dragged item origin, so the @@ -710,16 +702,22 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit { m_selectionTool->GetToolMenu().ShowContextMenu( selection ); } - else if( evt->IsAction( &ACTIONS::undo ) || evt->IsAction( &ACTIONS::doDelete ) ) + else if( evt->IsAction( &ACTIONS::undo ) ) { restore_state = true; // Perform undo locally break; // Finish } + else if( evt->IsAction( &ACTIONS::doDelete ) ) + { + evt->SetPassEvent(); + // Exit on a delete; there will no longer be anything to drag. + break; + } else if( evt->IsAction( &ACTIONS::duplicate ) || evt->IsAction( &ACTIONS::cut ) ) { wxBell(); } - else if( evt->IsAction( &PCB_ACTIONS::rotateCw ) + else if( evt->IsAction( &PCB_ACTIONS::rotateCw ) || evt->IsAction( &PCB_ACTIONS::rotateCcw ) || evt->IsAction( &PCB_ACTIONS::flip ) || evt->IsAction( &PCB_ACTIONS::mirrorH ) @@ -786,8 +784,10 @@ bool EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit } else if( evt->IsAction( &ACTIONS::increment ) ) { - m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, - evt->Parameter() ); + if( evt->HasParameter() ) + m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, evt->Parameter() ); + else + m_toolMgr->RunSynchronousAction( ACTIONS::increment, aCommit, ACTIONS::INCREMENT { 1, 0 } ); } else if( ZONE_FILLER_TOOL::IsZoneFillAction( evt ) || evt->IsAction( &PCB_ACTIONS::moveExact ) diff --git a/pcbnew/tools/multichannel_tool.cpp b/pcbnew/tools/multichannel_tool.cpp index 34bfa013c1..bf02018643 100644 --- a/pcbnew/tools/multichannel_tool.cpp +++ b/pcbnew/tools/multichannel_tool.cpp @@ -532,13 +532,13 @@ int MULTICHANNEL_TOOL::RepeatLayout( const TOOL_EVENT& aEvent, ZONE* aRefZone ) { int totalCopied = 0; - BOARD_COMMIT commit( GetManager(), true ); + BOARD_COMMIT commit( GetManager(), true, false ); for( auto& targetArea : m_areas.m_compatMap ) { if( !targetArea.second.m_doCopy ) { wxLogTrace( traceMultichannelTool, wxT("skipping copy to RA '%s' (disabled in dialog)\n"), - targetArea.first->m_ruleName ); + targetArea.first->m_ruleName ); continue; } @@ -549,17 +549,14 @@ int MULTICHANNEL_TOOL::RepeatLayout( const TOOL_EVENT& aEvent, ZONE* aRefZone ) m_areas.m_options, targetArea.second.m_affectedItems, targetArea.second.m_groupableItems ) ) { - auto errMsg = wxString::Format( - _( "Copy Rule Area contents failed between rule areas '%s' and '%s'." ), - m_areas.m_refRA->m_area->GetZoneName(), - targetArea.first->m_area->GetZoneName() ); + auto errMsg = wxString::Format( _( "Copy Rule Area contents failed between rule areas '%s' and '%s'." ), + m_areas.m_refRA->m_area->GetZoneName(), + targetArea.first->m_area->GetZoneName() ); commit.Revert(); if( Pgm().IsGUI() ) - { frame()->ShowInfoBarError( errMsg, true ); - } return -1; } @@ -1148,7 +1145,7 @@ int MULTICHANNEL_TOOL::AutogenerateRuleAreas( const TOOL_EVENT& aEvent ) wxLogTrace( traceMultichannelTool, wxT( "%d placement areas found\n" ), (int) m_areas.m_areas.size() ); - BOARD_COMMIT commit( GetManager(), true ); + BOARD_COMMIT commit( GetManager(), true, false ); for( RULE_AREA& ra : m_areas.m_areas ) { diff --git a/pcbnew/tools/pcb_group_tool.cpp b/pcbnew/tools/pcb_group_tool.cpp index 827c6cfde1..103f090f23 100644 --- a/pcbnew/tools/pcb_group_tool.cpp +++ b/pcbnew/tools/pcb_group_tool.cpp @@ -20,6 +20,7 @@ * or you may write to the Free Software Foundation, Inc., * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ + #include #include #include @@ -33,6 +34,13 @@ #include +std::shared_ptr PCB_GROUP_TOOL::createCommit() +{ + return std::make_shared( m_toolMgr, m_frame->IsType( FRAME_PCB_EDITOR ), + m_frame->IsType( FRAME_FOOTPRINT_EDITOR ) ); +} + + int PCB_GROUP_TOOL::PickNewMember( const TOOL_EVENT& aEvent ) { bool isFootprintEditor = m_frame->GetFrameType() == FRAME_FOOTPRINT_EDITOR; diff --git a/pcbnew/tools/pcb_group_tool.h b/pcbnew/tools/pcb_group_tool.h index 0026f42eed..8695b827e5 100644 --- a/pcbnew/tools/pcb_group_tool.h +++ b/pcbnew/tools/pcb_group_tool.h @@ -1,3 +1,26 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright The 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 + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + #include #include #include @@ -14,7 +37,7 @@ public: int Group( const TOOL_EVENT& aEvent ) override; protected: - std::shared_ptr createCommit() override { return std::make_shared( this ); } + std::shared_ptr createCommit() override; EDA_GROUP* getGroupFromItem( EDA_ITEM* aItem ) override { diff --git a/pcbnew/tools/pcb_selection_tool.cpp b/pcbnew/tools/pcb_selection_tool.cpp index 7dea5e7eec..bc384ff639 100644 --- a/pcbnew/tools/pcb_selection_tool.cpp +++ b/pcbnew/tools/pcb_selection_tool.cpp @@ -32,18 +32,8 @@ using namespace std::placeholders; #include #include -#include #include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include #include #include #include @@ -51,7 +41,6 @@ using namespace std::placeholders; #include #include #include -#include #include #include #include @@ -63,9 +52,7 @@ using namespace std::placeholders; #include #include #include -#include #include -#include #include #include #include @@ -208,17 +195,17 @@ bool PCB_SELECTION_TOOL::Init() } // "Cancel" goes at the top of the context menu when a tool is active - menu.AddItem( ACTIONS::cancelInteractive, activeToolCondition, 1 ); - menu.AddItem( ACTIONS::groupEnter, groupEnterCondition, 1 ); - menu.AddItem( ACTIONS::groupLeave, inGroupCondition, 1 ); - menu.AddItem( PCB_ACTIONS::placeLinkedDesignBlock, groupEnterCondition, 1 ); + menu.AddItem( ACTIONS::cancelInteractive, activeToolCondition, 1 ); + menu.AddItem( ACTIONS::groupEnter, groupEnterCondition, 1 ); + menu.AddItem( ACTIONS::groupLeave, inGroupCondition, 1 ); + menu.AddItem( PCB_ACTIONS::placeLinkedDesignBlock, groupEnterCondition, 1 ); menu.AddItem( PCB_ACTIONS::saveToLinkedDesignBlock, groupEnterCondition, 1 ); - menu.AddItem( PCB_ACTIONS::clearHighlight, haveHighlight, 1 ); - menu.AddSeparator( haveHighlight, 1 ); + menu.AddItem( PCB_ACTIONS::clearHighlight, haveHighlight, 1 ); + menu.AddSeparator( haveHighlight, 1 ); - menu.AddItem( ACTIONS::selectColumns, tableCellSelection, 2 ); - menu.AddItem( ACTIONS::selectRows, tableCellSelection, 2 ); - menu.AddItem( ACTIONS::selectTable, tableCellSelection, 2 ); + menu.AddItem( ACTIONS::selectColumns, tableCellSelection, 2 ); + menu.AddItem( ACTIONS::selectRows, tableCellSelection, 2 ); + menu.AddItem( ACTIONS::selectTable, tableCellSelection, 2 ); menu.AddSeparator( 1 ); @@ -266,8 +253,7 @@ void PCB_SELECTION_TOOL::OnIdle( wxIdleEvent& aEvent ) { wxMouseState keyboardState = wxGetMouseState(); - setModifiersState( keyboardState.ShiftDown(), keyboardState.ControlDown(), - keyboardState.AltDown() ); + setModifiersState( keyboardState.ShiftDown(), keyboardState.ControlDown(), keyboardState.AltDown() ); if( m_additive ) m_frame->GetCanvas()->SetCurrentCursor( KICURSOR::ADD ); @@ -293,8 +279,7 @@ int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) trackDragAction = cfg->m_TrackDragAction; // on left click, a selection is made, depending on modifiers ALT, SHIFT, CTRL: - setModifiersState( evt->Modifier( MD_SHIFT ), evt->Modifier( MD_CTRL ), - evt->Modifier( MD_ALT ) ); + setModifiersState( evt->Modifier( MD_SHIFT ), evt->Modifier( MD_CTRL ), evt->Modifier( MD_ALT ) ); PCB_BASE_FRAME* frame = getEditFrame(); bool brd_editor = frame && frame->IsType( FRAME_PCB_EDITOR ); @@ -403,12 +388,9 @@ int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) if( field >= 0 ) { const int delta = evt->Parameter(); - ACTIONS::INCREMENT incParams{ - delta > 0 ? 1 : -1, - field - }; + ACTIONS::INCREMENT params { delta > 0 ? 1 : -1, field }; - m_toolMgr->RunAction( ACTIONS::increment, incParams ); + m_toolMgr->RunAction( ACTIONS::increment, params ); } } else if( evt->IsDrag( BUT_LEFT ) ) @@ -453,10 +435,16 @@ int PCB_SELECTION_TOOL::Main( const TOOL_EVENT& aEvent ) else if( ( hasModifier() || dragAction == MOUSE_DRAG_ACTION::SELECT ) || ( m_selection.Empty() && dragAction != MOUSE_DRAG_ACTION::DRAG_ANY ) ) { - if( m_selectionMode == SELECTION_MODE::INSIDE_RECTANGLE || m_selectionMode == SELECTION_MODE::TOUCHING_RECTANGLE ) + if( m_selectionMode == SELECTION_MODE::INSIDE_RECTANGLE + || m_selectionMode == SELECTION_MODE::TOUCHING_RECTANGLE ) + { SelectRectArea( aEvent ); - else if( m_selectionMode == SELECTION_MODE::INSIDE_LASSO || m_selectionMode == SELECTION_MODE::TOUCHING_LASSO ) + } + else if( m_selectionMode == SELECTION_MODE::INSIDE_LASSO + || m_selectionMode == SELECTION_MODE::TOUCHING_LASSO ) + { SelectPolyArea( aEvent ); + } else { wxASSERT_MSG( false, wxT( "Unknown selection mode" ) ); @@ -771,8 +759,7 @@ bool PCB_SELECTION_TOOL::ctrlClickHighlights() } -bool PCB_SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag, - bool* aSelectionCancelledFlag, +bool PCB_SELECTION_TOOL::selectPoint( const VECTOR2I& aWhere, bool aOnDrag, bool* aSelectionCancelledFlag, CLIENT_SELECTION_FILTER aClientFilter ) { GENERAL_COLLECTORS_GUIDE guide = getCollectorsGuide(); @@ -1081,8 +1068,7 @@ int PCB_SELECTION_TOOL::SelectRectArea( const TOOL_EVENT& aEvent ) area.SetAdditive( m_drag_additive ); area.SetSubtractive( m_drag_subtractive ); area.SetExclusiveOr( false ); - area.SetMode( greedySelection ? SELECTION_MODE::TOUCHING_RECTANGLE - : SELECTION_MODE::INSIDE_RECTANGLE ); + area.SetMode( greedySelection ? SELECTION_MODE::TOUCHING_RECTANGLE : SELECTION_MODE::INSIDE_RECTANGLE ); view->SetVisible( &area, true ); view->Update( &area ); @@ -1163,8 +1149,7 @@ int PCB_SELECTION_TOOL::SelectPolyArea( const TOOL_EVENT& aEvent ) if( getView()->IsMirroredX() && shapeArea != 0 ) isClockwise = !isClockwise; - selectionMode = isClockwise ? SELECTION_MODE::INSIDE_LASSO - : SELECTION_MODE::TOUCHING_LASSO; + selectionMode = isClockwise ? SELECTION_MODE::INSIDE_LASSO : SELECTION_MODE::TOUCHING_LASSO; if( evt->IsCancelInteractive() || evt->IsActivate() ) { @@ -1172,22 +1157,24 @@ int PCB_SELECTION_TOOL::SelectPolyArea( const TOOL_EVENT& aEvent ) evt->SetPassEvent( false ); break; } - else if( evt->IsDrag( BUT_LEFT ) || evt->IsClick( BUT_LEFT ) - || evt->IsAction( &ACTIONS::cursorClick ) ) + else if( evt->IsDrag( BUT_LEFT ) + || evt->IsClick( BUT_LEFT ) + || evt->IsAction( &ACTIONS::cursorClick ) ) { points.Append( evt->Position() ); } - else if( evt->IsDblClick( BUT_LEFT ) || evt->IsAction( &ACTIONS::cursorDblClick ) - || evt->IsAction( &ACTIONS::finishInteractive ) ) + else if( evt->IsDblClick( BUT_LEFT ) + || evt->IsAction( &ACTIONS::cursorDblClick ) + || evt->IsAction( &ACTIONS::finishInteractive ) ) { area.GetPoly().GenerateBBoxCache(); SelectMultiple( area, m_subtractive, m_exclusive_or ); evt->SetPassEvent( false ); break; } - else if( evt->IsAction( &PCB_ACTIONS::deleteLastPoint ) - || evt->IsAction( &ACTIONS::doDelete ) - || evt->IsAction( &ACTIONS::undo ) ) + else if( evt->IsAction( &PCB_ACTIONS::deleteLastPoint ) + || evt->IsAction( &ACTIONS::doDelete ) + || evt->IsAction( &ACTIONS::undo ) ) { if( points.GetPointCount() > 0 ) { @@ -1466,8 +1453,7 @@ int PCB_SELECTION_TOOL::UnselectAll( const TOOL_EVENT& aEvent ) } -void connectedItemFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, - PCB_SELECTION_TOOL* sTool ) +void connectedItemFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) { // Narrow the collection down to a single BOARD_CONNECTED_ITEM for each represented net. // All other items types are removed. @@ -1578,9 +1564,9 @@ int PCB_SELECTION_TOOL::expandConnection( const TOOL_EVENT& aEvent ) for( const EDA_ITEM* item : m_selection.GetItems() ) { - if( item->Type() == PCB_FOOTPRINT_T - || item->Type() == PCB_GENERATOR_T - || ( static_cast( item )->IsConnected() ) ) + if( item->Type() == PCB_FOOTPRINT_T + || item->Type() == PCB_GENERATOR_T + || ( static_cast( item )->IsConnected() ) ) { initialCount++; } @@ -1793,11 +1779,8 @@ void PCB_SELECTION_TOOL::selectAllConnectedTracks( const std::vectorGetStart() != track->GetEnd() - && layerSetCu.Contains( track->GetLayer() ) ) - { + if( track->GetStart() != track->GetEnd() && layerSetCu.Contains( track->GetLayer() ) ) pt_count++; - } } if( pt_count > 2 || gotVia || gotNonStartPad ) @@ -2632,10 +2615,8 @@ void PCB_SELECTION_TOOL::FindItem( BOARD_ITEM* aItem ) if( !screenRect.Contains( aItem->GetBoundingBox() ) ) { - double scaleX = screenSize.x / - static_cast( aItem->GetBoundingBox().GetWidth() ); - double scaleY = screenSize.y / - static_cast( aItem->GetBoundingBox().GetHeight() ); + double scaleX = screenSize.x / static_cast( aItem->GetBoundingBox().GetWidth() ); + double scaleY = screenSize.y / static_cast( aItem->GetBoundingBox().GetHeight() ); scaleX /= marginFactor; scaleY /= marginFactor; @@ -2677,8 +2658,8 @@ static bool itemIsIncludedByFilter( const BOARD_ITEM& aItem, const BOARD& aBoard { const FOOTPRINT& footprint = static_cast( aItem ); - return aFilterOptions.includeModules - && ( aFilterOptions.includeLockedModules || !footprint.IsLocked() ); + return aFilterOptions.includeModules && ( aFilterOptions.includeLockedModules + || !footprint.IsLocked() ); } case PCB_TRACE_T: @@ -3091,12 +3072,13 @@ bool PCB_SELECTION_TOOL::Selectable( const BOARD_ITEM* aItem, bool checkVisibili if( aItem->GetParentGroup() && aItem->GetParentGroup()->AsEdaItem()->Type() == PCB_GENERATOR_T ) return false; - const ZONE* zone = nullptr; - const PCB_VIA* via = nullptr; - const PAD* pad = nullptr; - const PCB_TEXT* text = nullptr; - const PCB_FIELD* field = nullptr; - const PCB_MARKER* marker = nullptr; + const ZONE* zone = nullptr; + const PCB_VIA* via = nullptr; + const PAD* pad = nullptr; + const PCB_TEXT* text = nullptr; + const PCB_FIELD* field = nullptr; + const PCB_MARKER* marker = nullptr; + const PCB_TABLECELL* cell = nullptr; // Most footprint children can only be selected in the footprint editor. if( aItem->GetParentFootprint() && !m_isFootprintEditor && !checkVisibilityOnly ) @@ -3193,27 +3175,35 @@ bool PCB_SELECTION_TOOL::Selectable( const BOARD_ITEM* aItem, bool checkVisibili if( !view()->IsLayerVisible( LAYER_DRAW_BITMAPS ) ) return false; - KI_FALLTHROUGH; + if( !layerVisible( aItem->GetLayer() ) ) + return false; + + break; case PCB_SHAPE_T: if( options.m_FilledShapeOpacity == 0.0 && static_cast( aItem )->IsAnyFill() ) return false; - KI_FALLTHROUGH; - - case PCB_TEXTBOX_T: - case PCB_TABLE_T: - case PCB_TABLECELL_T: if( !layerVisible( aItem->GetLayer() ) ) return false; - if( aItem->Type() == PCB_TABLECELL_T ) - { - const PCB_TABLECELL* cell = static_cast( aItem ); + break; - if( cell->GetRowSpan() == 0 || cell->GetColSpan() == 0 ) - return false; - } + case PCB_TEXTBOX_T: + case PCB_TABLE_T: + if( !layerVisible( aItem->GetLayer() ) ) + return false; + + break; + + case PCB_TABLECELL_T: + cell = static_cast( aItem ); + + if( !layerVisible( aItem->GetLayer() ) ) + return false; + + if( cell->GetRowSpan() == 0 || cell->GetColSpan() == 0 ) + return false; break; @@ -3892,8 +3882,7 @@ void PCB_SELECTION_TOOL::GuessSelectionCandidates( GENERAL_COLLECTOR& aCollector } -void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector, - bool aMultiselect ) const +void PCB_SELECTION_TOOL::FilterCollectorForHierarchy( GENERAL_COLLECTOR& aCollector, bool aMultiselect ) const { std::unordered_set toAdd; diff --git a/qa/tests/common/test_commit.cpp b/qa/tests/common/test_commit.cpp index 6d06cbc881..2fb89ba8bb 100644 --- a/qa/tests/common/test_commit.cpp +++ b/qa/tests/common/test_commit.cpp @@ -41,7 +41,7 @@ public: void Revert() override {} private: - EDA_ITEM* parentObject( EDA_ITEM* aItem ) const override { return aItem; } + EDA_ITEM* undoLevelItem( EDA_ITEM* aItem ) const override { return aItem; } EDA_ITEM* makeImage( EDA_ITEM* aItem ) const override { return aItem->Clone(); } };