From 1411b0917840a120a8119da9ee2f29a25ffa17a2 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Tue, 27 Jun 2023 16:52:50 +0100 Subject: [PATCH] Remove EDIT_TOOL's quasi-global BOARD_COMMIT. It had several encapsulation leakage issues, as well as poorly-defined behaviour of undo for chained-actions (append-to-board, and then rotate while moving, for instance). --- pcbnew/board_commit.cpp | 7 + .../dialogs/dialog_track_via_properties.cpp | 19 +- pcbnew/dialogs/dialog_track_via_properties.h | 6 +- pcbnew/tools/board_inspection_tool.cpp | 10 +- pcbnew/tools/edit_tool.cpp | 304 ++++++++---------- pcbnew/tools/edit_tool.h | 24 +- pcbnew/tools/edit_tool_move_fct.cpp | 172 ++++------ pcbnew/tools/pcb_control.cpp | 49 ++- pcbnew/tools/pcb_control.h | 7 +- 9 files changed, 259 insertions(+), 339 deletions(-) diff --git a/pcbnew/board_commit.cpp b/pcbnew/board_commit.cpp index 0d97f30005..106b47d1be 100644 --- a/pcbnew/board_commit.cpp +++ b/pcbnew/board_commit.cpp @@ -571,6 +571,8 @@ void BOARD_COMMIT::Push( const wxString& aMessage, int aCommitFlags ) wxASSERT( false ); break; } + + boardItem->ClearEditFlags(); } if( bulkAddedItems.size() > 0 ) @@ -767,6 +769,8 @@ void BOARD_COMMIT::Revert() wxASSERT( false ); break; } + + item->ClearEditFlags(); } if( bulkAddedItems.size() > 0 ) @@ -787,6 +791,9 @@ void BOARD_COMMIT::Revert() PCB_SELECTION_TOOL* selTool = m_toolMgr->GetTool(); selTool->RebuildSelection(); + // Property panel needs to know about the reselect + m_toolMgr->PostEvent( EVENTS::SelectedItemsModified ); + clear(); } diff --git a/pcbnew/dialogs/dialog_track_via_properties.cpp b/pcbnew/dialogs/dialog_track_via_properties.cpp index 2bb85fda86..8a04547c60 100644 --- a/pcbnew/dialogs/dialog_track_via_properties.cpp +++ b/pcbnew/dialogs/dialog_track_via_properties.cpp @@ -38,12 +38,10 @@ #include DIALOG_TRACK_VIA_PROPERTIES::DIALOG_TRACK_VIA_PROPERTIES( PCB_BASE_FRAME* aParent, - const PCB_SELECTION& aItems, - BOARD_COMMIT& aCommit ) : + const PCB_SELECTION& aItems ) : DIALOG_TRACK_VIA_PROPERTIES_BASE( aParent ), m_frame( aParent ), m_items( aItems ), - m_commit( aCommit ), m_trackStartX( aParent, m_TrackStartXLabel, m_TrackStartXCtrl, nullptr ), m_trackStartY( aParent, m_TrackStartYLabel, m_TrackStartYCtrl, m_TrackStartYUnit ), m_trackEndX( aParent, m_TrackEndXLabel, m_TrackEndXCtrl, nullptr ), @@ -571,12 +569,13 @@ bool DIALOG_TRACK_VIA_PROPERTIES::TransferDataFromWindow() // We don't bother with updating the nets at this point as it will be useless (any connected // pads will simply drive their existing nets back onto the track segments and vias). - bool changeLock = m_lockedCbox->Get3StateValue() != wxCHK_UNDETERMINED; - bool setLock = m_lockedCbox->Get3StateValue() == wxCHK_CHECKED; + BOARD_COMMIT commit( m_frame ); + bool changeLock = m_lockedCbox->Get3StateValue() != wxCHK_UNDETERMINED; + bool setLock = m_lockedCbox->Get3StateValue() == wxCHK_CHECKED; for( EDA_ITEM* item : m_items ) { - m_commit.Modify( item ); + commit.Modify( item ); switch( item->Type() ) { @@ -755,7 +754,7 @@ bool DIALOG_TRACK_VIA_PROPERTIES::TransferDataFromWindow() } } - m_commit.Push( _( "Edit track/via properties" ) ); + commit.Push( _( "Edit track/via properties" ) ); // Pushing the commit will have updated the connectivity so we can now test to see if we // need to update any pad nets. @@ -795,7 +794,7 @@ bool DIALOG_TRACK_VIA_PROPERTIES::TransferDataFromWindow() { for( EDA_ITEM* item : m_items ) { - m_commit.Modify( item ); + commit.Modify( item ); switch( item->Type() ) { @@ -816,11 +815,11 @@ bool DIALOG_TRACK_VIA_PROPERTIES::TransferDataFromWindow() for( PAD* pad : changingPads ) { - m_commit.Modify( pad ); + commit.Modify( pad ); pad->SetNetCode( newNetCode ); } - m_commit.Push( _( "Updating nets" ) ); + commit.Push( _( "Updating nets" ) ); } return true; diff --git a/pcbnew/dialogs/dialog_track_via_properties.h b/pcbnew/dialogs/dialog_track_via_properties.h index 1583d8a2a8..6cd6eb3d85 100644 --- a/pcbnew/dialogs/dialog_track_via_properties.h +++ b/pcbnew/dialogs/dialog_track_via_properties.h @@ -2,6 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2015 CERN + * Copyright (C) 2015-2023 KiCad Developers, see AUTHORS.txt for contributors. * @author Maciej Suminski * * This program is free software; you can redistribute it and/or @@ -28,15 +29,13 @@ #include class PCB_SELECTION; -class BOARD_COMMIT; class PCB_BASE_FRAME; class PAD; class DIALOG_TRACK_VIA_PROPERTIES : public DIALOG_TRACK_VIA_PROPERTIES_BASE { public: - DIALOG_TRACK_VIA_PROPERTIES( PCB_BASE_FRAME* aParent, const PCB_SELECTION& aItems, - BOARD_COMMIT& aCommit ); + DIALOG_TRACK_VIA_PROPERTIES( PCB_BASE_FRAME* aParent, const PCB_SELECTION& aItems ); ~DIALOG_TRACK_VIA_PROPERTIES(); @@ -63,7 +62,6 @@ private: private: PCB_BASE_FRAME* m_frame; const PCB_SELECTION& m_items; // List of items to be modified. - BOARD_COMMIT& m_commit; // An undo record to add any changes to. UNIT_BINDER m_trackStartX, m_trackStartY; UNIT_BINDER m_trackEndX, m_trackEndY; diff --git a/pcbnew/tools/board_inspection_tool.cpp b/pcbnew/tools/board_inspection_tool.cpp index 3393efd01e..a6306121cb 100644 --- a/pcbnew/tools/board_inspection_tool.cpp +++ b/pcbnew/tools/board_inspection_tool.cpp @@ -1825,15 +1825,9 @@ int BOARD_INSPECTION_TOOL::LocalRatsnestTool( const TOOL_EVENT& aEvent ) int BOARD_INSPECTION_TOOL::UpdateLocalRatsnest( const TOOL_EVENT& aEvent ) { - VECTOR2I delta; + VECTOR2I delta = aEvent.Parameter(); - // If we have passed the simple move vector, we can update without recalculation - if( aEvent.Parameter() ) - { - delta = *aEvent.Parameter(); - delete aEvent.Parameter(); - } - else + if( delta == VECTOR2I() ) { // We can delete the existing map to force a recalculation delete m_dynamicData; diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 8c3dea74f0..eab4bae663 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -82,9 +82,6 @@ void EDIT_TOOL::Reset( RESET_REASON aReason ) m_dragging = false; m_statusPopup = std::make_unique( getEditFrame() ); - - if( aReason != RUN ) - m_commit.reset( new BOARD_COMMIT( this ) ); } @@ -440,7 +437,11 @@ int EDIT_TOOL::DragArcTrack( const TOOL_EVENT& aEvent ) controls->ShowCursor( true ); controls->SetAutoPan( true ); - bool restore_state = false; + BOARD_COMMIT commit( this ); + bool restore_state = false; + + commit.Modify( theArc ); + VECTOR2I arcCenter = theArc->GetCenter(); SEG tanStart = SEG( arcCenter, theArc->GetStart() ).PerpendicularSeg( theArc->GetStart() ); SEG tanEnd = SEG( arcCenter, theArc->GetEnd() ).PerpendicularSeg( theArc->GetEnd() ); @@ -478,42 +479,40 @@ int EDIT_TOOL::DragArcTrack( const TOOL_EVENT& aEvent ) break; } - PCB_TRACK* retval = nullptr; + PCB_TRACK* track = nullptr; if( itemsOnAnchor.size() == 1 && itemsOnAnchor.front()->Type() == PCB_TRACE_T ) { - retval = static_cast( itemsOnAnchor.front() ); - SEG trackSeg( retval->GetStart(), retval->GetEnd() ); + track = static_cast( itemsOnAnchor.front() ); + commit.Modify( track ); + + SEG trackSeg( track->GetStart(), track->GetEnd() ); // Allow deviations in colinearity as defined in ADVANCED_CFG if( trackSeg.Angle( aCollinearSeg ) > maxTangentDeviation ) - retval = nullptr; + track = nullptr; } - if( !retval ) + if( !track ) { - retval = new PCB_TRACK( theArc->GetParent() ); - retval->SetStart( aAnchor ); - retval->SetEnd( aAnchor ); - retval->SetNet( theArc->GetNet() ); - retval->SetLayer( theArc->GetLayer() ); - retval->SetWidth( theArc->GetWidth() ); - retval->SetLocked( theArc->IsLocked() ); - retval->SetFlags( IS_NEW ); - getView()->Add( retval ); + track = new PCB_TRACK( theArc->GetParent() ); + track->SetStart( aAnchor ); + track->SetEnd( aAnchor ); + track->SetNet( theArc->GetNet() ); + track->SetLayer( theArc->GetLayer() ); + track->SetWidth( theArc->GetWidth() ); + track->SetLocked( theArc->IsLocked() ); + track->SetFlags( IS_NEW ); + getView()->Add( track ); + commit.Added( track ); } - return retval; + return track; }; PCB_TRACK* trackOnStart = getUniqueTrackAtAnchorCollinear( theArc->GetStart(), tanStart); PCB_TRACK* trackOnEnd = getUniqueTrackAtAnchorCollinear( theArc->GetEnd(), tanEnd ); - // Make copies of items to be edited - PCB_ARC* theArcCopy = new PCB_ARC( *theArc ); - PCB_TRACK* trackOnStartCopy = new PCB_TRACK( *trackOnStart ); - PCB_TRACK* trackOnEndCopy = new PCB_TRACK( *trackOnEnd ); - if( trackOnStart->GetLength() != 0 ) { tanStart.A = trackOnStart->GetStart(); @@ -622,7 +621,7 @@ int EDIT_TOOL::DragArcTrack( const TOOL_EVENT& aEvent ) VECTOR2I closest = cSegTanStart.NearestPoint( m_cursor ); - for( VECTOR2I candidate : possiblePoints ) + for( const VECTOR2I& candidate : possiblePoints ) { if( ( candidate - m_cursor ).SquaredEuclideanNorm() < ( closest - m_cursor ).SquaredEuclideanNorm() ) @@ -698,56 +697,6 @@ int EDIT_TOOL::DragArcTrack( const TOOL_EVENT& aEvent ) } } - // Ensure we only do one commit operation on each object - auto processTrack = - [&]( PCB_TRACK* aTrack, PCB_TRACK* aTrackCopy, int aMaxLengthIU ) -> bool - { - if( aTrack->IsNew() ) - { - getView()->Remove( aTrack ); - - if( aTrack->GetLength() <= aMaxLengthIU ) - { - aTrack->SetParentGroup( nullptr ); - delete aTrack; - aTrack = nullptr; - - aTrackCopy->SetParentGroup( nullptr ); - delete aTrackCopy; - aTrackCopy = nullptr; - - return false; - } - else - { - m_commit->Add( aTrack ); - - aTrackCopy->SetParentGroup( nullptr ); - delete aTrackCopy; - aTrackCopy = nullptr; - - return true; - } - } - else if( aTrack->GetLength() <= aMaxLengthIU ) - { - aTrack->SwapItemData( aTrackCopy ); //restore the original before notifying COMMIT - m_commit->Remove( aTrack ); - - aTrackCopy->SetParentGroup( nullptr ); - delete aTrackCopy; - aTrackCopy = nullptr; - - return false; - } - else - { - m_commit->Modified( aTrack, aTrackCopy ); - } - - return true; - }; - // Amend the end points of the arc if we delete the joining tracks VECTOR2I newStart = trackOnStart->GetStart(); VECTOR2I newEnd = trackOnEnd->GetStart(); @@ -761,19 +710,26 @@ int EDIT_TOOL::DragArcTrack( const TOOL_EVENT& aEvent ) int maxLengthIU = KiROUND( ADVANCED_CFG::GetCfg().m_MaxTrackLengthToKeep * pcbIUScale.IU_PER_MM ); - if( !processTrack( trackOnStart, trackOnStartCopy, maxLengthIU ) ) + if( trackOnStart->GetLength() <= maxLengthIU ) + { + commit.Remove( trackOnStart ); theArc->SetStart( newStart ); + } - if( !processTrack( trackOnEnd, trackOnEndCopy, maxLengthIU ) ) + if( trackOnEnd->GetLength() <= maxLengthIU ) + { + commit.Remove( trackOnEnd ); theArc->SetEnd( newEnd ); + } - processTrack( theArc, theArcCopy, 0 ); // only delete the arc if start and end points coincide + if( theArc->GetLength() <= 0 ) + commit.Remove( theArc ); // Should we commit? if( restore_state ) - m_commit->Revert(); + commit.Revert(); else - m_commit->Push( _( "Drag Arc Track" ) ); + commit.Push( _( "Drag Arc Track" ) ); return 0; } @@ -795,13 +751,15 @@ int EDIT_TOOL::ChangeTrackWidth( const TOOL_EVENT& aEvent ) }, true /* prompt user regarding locked items */ ); + BOARD_COMMIT commit( this ); + for( EDA_ITEM* item : selection ) { if( item->Type() == PCB_VIA_T ) { PCB_VIA* via = static_cast( item ); - m_commit->Modify( via ); + commit.Modify( via ); int new_width; int new_drill; @@ -828,14 +786,14 @@ int EDIT_TOOL::ChangeTrackWidth( const TOOL_EVENT& aEvent ) wxCHECK( track, 0 ); - m_commit->Modify( track ); + commit.Modify( track ); int new_width = board()->GetDesignSettings().GetCurrentTrackWidth(); track->SetWidth( new_width ); } } - m_commit->Push( _( "Edit track width/via size" ) ); + commit.Push( _( "Edit track width/via size" ) ); if( selection.IsHover() ) { @@ -958,6 +916,7 @@ int EDIT_TOOL::FilletTracks( const TOOL_EVENT& aEvent ) } } + BOARD_COMMIT commit( this ); std::vector itemsToAddToSelection; for( FILLET_OP filletOp : filletOperations ) @@ -1018,11 +977,11 @@ int EDIT_TOOL::FilletTracks( const TOOL_EVENT& aEvent ) tArc->SetWidth( track1->GetWidth() ); tArc->SetNet( track1->GetNet() ); tArc->SetLocked( track1->IsLocked() ); - m_commit->Add( tArc ); + commit.Add( tArc ); itemsToAddToSelection.push_back( tArc ); - m_commit->Modify( track1 ); - m_commit->Modify( track2 ); + commit.Modify( track1 ); + commit.Modify( track2 ); if( filletOp.t1Start ) track1->SetStart( t1newPoint ); @@ -1038,7 +997,7 @@ int EDIT_TOOL::FilletTracks( const TOOL_EVENT& aEvent ) } } - m_commit->Push( _( "Fillet Tracks" ) ); + commit.Push( _( "Fillet Tracks" ) ); //select the newly created arcs for( BOARD_ITEM* item : itemsToAddToSelection ) @@ -1080,7 +1039,7 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) }, true /* prompt user regarding locked items */ ); - std::set lines_to_add; + std::set lines_to_add; std::vector items_to_remove; for( EDA_ITEM* item : selection ) @@ -1159,13 +1118,14 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) return 0; } - bool operationPerformedOnAtLeastOne = false; - bool didOneAttemptFail = false; + BOARD_COMMIT commit( this ); + bool operationPerformedOnAtLeastOne = false; + bool didOneAttemptFail = false; std::vector itemsToAddToSelection; // Only modify one parent in FP editor if( m_isFootprintEditor ) - m_commit->Modify( selection.Front() ); + commit.Modify( selection.Front() ); alg::for_all_pairs( selection.begin(), selection.end(), [&]( EDA_ITEM* a, EDA_ITEM* b ) { @@ -1252,7 +1212,7 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) } else if( !m_isFootprintEditor ) { - m_commit->Modify( line_a ); + commit.Modify( line_a ); } if( lines_to_add.count( line_b ) ) @@ -1262,7 +1222,7 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) } else if( !m_isFootprintEditor ) { - m_commit->Modify( line_b ); + commit.Modify( line_b ); } itemsToAddToSelection.push_back( tArc ); @@ -1277,16 +1237,16 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) } ); - for( auto item : items_to_remove ) + for( PCB_SHAPE* item : items_to_remove ) { - m_commit->Remove( item ); + commit.Remove( item ); m_selectionTool->RemoveItemFromSel( item, true ); } //select the newly created arcs for( BOARD_ITEM* item : itemsToAddToSelection ) { - m_commit->Add( item ); + commit.Add( item ); m_selectionTool->AddItemToSel( item, true ); } @@ -1299,7 +1259,7 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) // Notify other tools of the changes m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); - m_commit->Push( _( "Fillet Lines" ) ); + commit.Push( _( "Fillet Lines" ) ); if( !operationPerformedOnAtLeastOne ) frame()->ShowInfoBarMsg( _( "Unable to fillet the selected lines." ) ); @@ -1321,7 +1281,7 @@ int EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) // Tracks & vias are treated in a special way: if( ( SELECTION_CONDITIONS::OnlyTypes( { PCB_TRACE_T, PCB_ARC_T, PCB_VIA_T } ) )( selection ) ) { - DIALOG_TRACK_VIA_PROPERTIES dlg( editFrame, selection, *m_commit ); + DIALOG_TRACK_VIA_PROPERTIES dlg( editFrame, selection ); dlg.ShowQuasiModal(); // QuasiModal required for NET_SELECTOR } else if( selection.Size() == 1 ) @@ -1379,6 +1339,11 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) } PCB_BASE_EDIT_FRAME* editFrame = getEditFrame(); + BOARD_COMMIT localCommit( this ); + BOARD_COMMIT* commit = dynamic_cast( aEvent.Commit() ); + + if( !commit ) + commit = &localCommit; // Be sure that there is at least one item that we can modify. If nothing was selected before, // try looking for the stuff under mouse cursor (i.e. KiCad old-style hover selection) @@ -1448,31 +1413,27 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) if( !outOfBounds ) { - // When editing footprints, all items have the same parent - if( IsFootprintEditor() ) - m_commit->Modify( selection.Front() ); - for( EDA_ITEM* item : selection ) { - if( !item->IsNew() && !IsFootprintEditor() ) + if( !item->IsNew() && !item->IsMoving() && ( !IsFootprintEditor() || commit->Empty() ) ) { - m_commit->Modify( item ); + commit->Modify( item ); // If rotating a group, record position of all the descendants for undo if( item->Type() == PCB_GROUP_T ) { static_cast( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) - { - m_commit->Modify( bItem ); - }); + { + commit->Modify( bItem ); + }); } } static_cast( item )->Rotate( refPt, rotateAngle ); } - if( !m_dragging ) - m_commit->Push( _( "Rotate" ) ); + if( !localCommit.Empty() ) + localCommit.Push( _( "Rotate" ) ); if( is_hover && !m_dragging ) m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -1480,7 +1441,7 @@ int EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent ) m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); if( m_dragging ) - m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest ); + m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, VECTOR2I() ); } // Restore the old reference so any mouse dragging that occurs doesn't make the selection jump @@ -1590,6 +1551,12 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) return 0; } + BOARD_COMMIT localCommit( this ); + BOARD_COMMIT* commit = dynamic_cast( aEvent.Commit() ); + + if( !commit ) + commit = &localCommit; + PCB_SELECTION& selection = m_selectionTool->RequestSelection( []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) { @@ -1605,10 +1572,6 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) updateModificationPoint( selection ); VECTOR2I mirrorPoint = selection.GetReferencePoint(); - // When editing footprints, all items have the same parent - if( IsFootprintEditor() ) - m_commit->Modify( selection.Front() ); - // Set the mirroring options. // Unfortunately, the mirror function do not have the same parameter for all items // So we need these 2 parameters to avoid mistakes @@ -1626,8 +1589,8 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) if( !item->IsType( MirrorableItems ) ) continue; - if( !item->IsNew() && !IsFootprintEditor() ) - m_commit->Modify( item ); + if( !item->IsNew() && !item->IsMoving() && ( !IsFootprintEditor() || commit->Empty() ) ) + commit->Modify( item ); // modify each object as necessary switch( item->Type() ) @@ -1671,8 +1634,8 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) } } - if( !m_dragging ) - m_commit->Push( _( "Mirror" ) ); + if( !localCommit.Empty() ) + localCommit.Push( _( "Mirror" ) ); if( selection.IsHover() && !m_dragging ) m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -1680,7 +1643,7 @@ int EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent ) m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); if( m_dragging ) - m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest ); + m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, VECTOR2I() ); return 0; } @@ -1694,6 +1657,12 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) return 0; } + BOARD_COMMIT localCommit( this ); + BOARD_COMMIT* commit = dynamic_cast( aEvent.Commit() ); + + if( !commit ) + commit = &localCommit; + PCB_SELECTION& selection = m_selectionTool->RequestSelection( []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) { @@ -1728,28 +1697,26 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) bool leftRight = frame()->GetPcbNewSettings()->m_FlipLeftRight; - // When editing footprints, all items have the same parent - if( IsFootprintEditor() ) - m_commit->Modify( selection.Front() ); - for( EDA_ITEM* item : selection ) { - if( !item->IsNew() && !IsFootprintEditor() ) - m_commit->Modify( item ); - - if( item->Type() == PCB_GROUP_T ) + if( !item->IsNew() && !item->IsMoving() && ( !IsFootprintEditor() || commit->Empty() ) ) { - static_cast( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) - { - m_commit->Modify( bItem ); - }); + commit->Modify( item ); + + if( item->Type() == PCB_GROUP_T ) + { + static_cast( item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) + { + commit->Modify( bItem ); + }); + } } static_cast( item )->Flip( refPt, leftRight ); } - if( !m_dragging ) - m_commit->Push( _( "Change Side / Flip" ) ); + if( !localCommit.Empty() ) + localCommit.Push( _( "Change Side / Flip" ) ); if( selection.IsHover() && !m_dragging ) m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -1757,7 +1724,7 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); if( m_dragging ) - m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest ); + m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, VECTOR2I() ); // Restore the old reference so any mouse dragging that occurs doesn't make the selection jump // to this now invalid reference @@ -1772,6 +1739,8 @@ int EDIT_TOOL::Flip( const TOOL_EVENT& aEvent ) void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) { + BOARD_COMMIT commit( this ); + // As we are about to remove items, they have to be removed from the selection first m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -1783,7 +1752,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) if( parentGroup ) { - m_commit->Modify( parentGroup ); + commit.Modify( parentGroup ); parentGroup->RemoveItem( board_item ); } @@ -1791,7 +1760,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) { case PCB_FIELD_T: wxASSERT( parentFP ); - m_commit->Modify( parentFP ); + commit.Modify( parentFP ); static_cast( board_item )->SetVisible( false ); getView()->Update( board_item ); break; @@ -1802,13 +1771,13 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) case PCB_BITMAP_T: if( parentFP ) { - m_commit->Modify( parentFP ); + commit.Modify( parentFP ); getView()->Remove( board_item ); parentFP->Remove( board_item ); } else { - m_commit->Remove( board_item ); + commit.Remove( board_item ); } break; @@ -1816,7 +1785,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) case PCB_PAD_T: if( IsFootprintEditor() || frame()->GetPcbNewSettings()->m_AllowFreePads ) { - m_commit->Modify( parentFP ); + commit.Modify( parentFP ); getView()->Remove( board_item ); parentFP->Remove( board_item ); } @@ -1826,7 +1795,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) case PCB_ZONE_T: if( parentFP ) { - m_commit->Modify( parentFP ); + commit.Modify( parentFP ); getView()->Remove( board_item ); parentFP->Remove( board_item ); } @@ -1845,7 +1814,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) if( zone->HitTestCutout( curPos, &outlineIdx, &holeIdx ) ) { // Remove the cutout - m_commit->Modify( zone ); + commit.Modify( zone ); zone->RemoveCutout( outlineIdx, holeIdx ); zone->UnFill(); @@ -1863,7 +1832,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) } // Remove the entire zone otherwise - m_commit->Remove( board_item ); + commit.Remove( board_item ); } break; @@ -1880,7 +1849,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) // Just make fields invisible if they happen to be in group. if( bItem->Type() == PCB_FIELD_T ) { - m_commit->Modify( bItem->GetParent() ); + commit.Modify( bItem->GetParent() ); static_cast( board_item )->SetVisible( false ); getView()->Update( board_item ); @@ -1895,13 +1864,13 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) } } - m_commit->Modify( bItem->GetParent() ); + commit.Modify( bItem->GetParent() ); getView()->Remove( bItem ); bItem->GetParent()->Remove( bItem ); } else { - m_commit->Remove( bItem ); + commit.Remove( bItem ); } }; @@ -1915,7 +1884,7 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) } default: - m_commit->Remove( board_item ); + commit.Remove( board_item ); break; } } @@ -1927,9 +1896,9 @@ void EDIT_TOOL::DeleteItems( const PCB_SELECTION& aItems, bool aIsCut ) m_selectionTool->ExitGroup(); if( aIsCut ) - m_commit->Push( _( "Cut" ) ); + commit.Push( _( "Cut" ) ); else - m_commit->Push( _( "Delete" ) ); + commit.Push( _( "Delete" ) ); } @@ -2038,9 +2007,10 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) if( ret == wxID_OK ) { - EDA_ANGLE angle = rotation; - VECTOR2I rp = selection.GetCenter(); - VECTOR2I selCenter( rp.x, rp.y ); + BOARD_COMMIT commit( this ); + EDA_ANGLE angle = rotation; + VECTOR2I rp = selection.GetCenter(); + VECTOR2I selCenter( rp.x, rp.y ); // Make sure the rotation is from the right reference point selCenter += translation; @@ -2050,7 +2020,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) // When editing footprints, all items have the same parent if( IsFootprintEditor() ) - m_commit->Modify( selection.Front() ); + commit.Modify( selection.Front() ); for( EDA_ITEM* selItem : selection ) { @@ -2058,7 +2028,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) if( !item->IsNew() && !IsFootprintEditor() ) { - m_commit->Modify( item ); + commit.Modify( item ); if( item->Type() == PCB_GROUP_T ) { @@ -2066,7 +2036,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) group->RunOnDescendants( [&]( BOARD_ITEM* bItem ) { - m_commit->Modify( bItem ); + commit.Modify( bItem ); }); } } @@ -2094,7 +2064,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) getView()->Update( item ); } - m_commit->Push( _( "Move exact" ) ); + commit.Push( _( "Move exact" ) ); if( selection.IsHover() ) m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -2102,7 +2072,7 @@ int EDIT_TOOL::MoveExact( const TOOL_EVENT& aEvent ) m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); if( m_dragging ) - m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest ); + m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, VECTOR2I() ); } return 0; @@ -2132,6 +2102,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) // we have a selection to work on now, so start the tool process PCB_BASE_EDIT_FRAME* editFrame = getEditFrame(); + BOARD_COMMIT commit( this ); // If the selection was given a hover, we do not keep the selection after completion bool is_hover = selection.IsHover(); @@ -2163,7 +2134,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) } else if( FOOTPRINT* parentFootprint = orig_item->GetParentFootprint() ) { - m_commit->Modify( parentFootprint ); + commit.Modify( parentFootprint ); dupe_item = parentFootprint->DuplicateItem( orig_item, true /* add to parent */ ); } else @@ -2211,7 +2182,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) static_cast( dupe_item )->RunOnDescendants( [&]( BOARD_ITEM* bItem ) { - m_commit->Add( bItem ); + commit.Add( bItem ); }); } @@ -2220,7 +2191,7 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) dupe_item->ClearSelected(); new_items.push_back( dupe_item ); - m_commit->Add( dupe_item ); + commit.Add( dupe_item ); } } @@ -2237,18 +2208,13 @@ int EDIT_TOOL::Duplicate( const TOOL_EVENT& aEvent ) editFrame->DisplayToolMsg( wxString::Format( _( "Duplicated %d item(s)" ), (int) new_items.size() ) ); - // TODO(ISM): This line can't be used to activate the tool until we allow multiple - // activations. - // m_toolMgr->RunAction( PCB_ACTIONS::move ); - // Instead we have to create the event and call the tool's function - // directly - // If items were duplicated, pick them up - // this works well for "dropping" copies around and pushes the commit - TOOL_EVENT evt = PCB_ACTIONS::move.MakeEvent(); - Move( evt ); + if( DoMoveSelection( aEvent, &commit ) ) + commit.Push( _( "Duplicate" ) ); + else + commit.Revert(); - // Deslect the duplicated item if we originally started as a hover selection + // Deselect the duplicated item if we originally started as a hover selection if( is_hover ) m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); } diff --git a/pcbnew/tools/edit_tool.h b/pcbnew/tools/edit_tool.h index 4bbc8bd13d..6c60480694 100644 --- a/pcbnew/tools/edit_tool.h +++ b/pcbnew/tools/edit_tool.h @@ -2,7 +2,7 @@ * This program source code file is part of KiCad, a free EDA CAD application. * * Copyright (C) 2013-2020 CERN - * Copyright (C) 2013-2021 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2013-2023 KiCad Developers, see AUTHORS.txt for contributors. * * @author Maciej Suminski * @author Tomasz Wlostowski @@ -39,12 +39,9 @@ class BOARD_ITEM; class CONNECTIVITY_DATA; class STATUS_TEXT_POPUP; -namespace KIGFX +namespace KIGFX::PREVIEW { - namespace PREVIEW - { - class RULER_ITEM; - } + class RULER_ITEM; } @@ -157,6 +154,8 @@ public: */ int CreateArray( const TOOL_EVENT& aEvent ); + bool DoMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit ); + /** * A selection filter which prunes the selection to contain only items of type #PCB_MODULE_T. */ @@ -169,8 +168,6 @@ public: static void PadFilter( const VECTOR2I&, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ); - BOARD_COMMIT* GetCurrentCommit() const { return m_commit.get(); } - private: ///< Set up handlers for various events. void setTransitions() override; @@ -194,8 +191,6 @@ private: bool invokeInlineRouter( int aDragMode ); bool isRouterActive() const; - int doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommitMessage ); - VECTOR2I getSafeMovement( const VECTOR2I& aMovement, const BOX2I& aSourceBBox, const VECTOR2D& aBBoxOffset ); @@ -206,11 +201,10 @@ private: void rebuildConnectivity(); private: - PCB_SELECTION_TOOL* m_selectionTool; - std::unique_ptr m_commit; - bool m_dragging; // Indicates objects are currently being dragged - VECTOR2I m_cursor; // Last cursor position (so getModificationPoint() - // can avoid changes of edit reference point). + PCB_SELECTION_TOOL* m_selectionTool; + bool m_dragging; // Indicates objects are currently being dragged + VECTOR2I m_cursor; // Last cursor position (so getModificationPoint() + // can avoid changes of edit reference point). std::unique_ptr m_statusPopup; static const unsigned int COORDS_PADDING; // Padding from coordinates limits for this tool diff --git a/pcbnew/tools/edit_tool_move_fct.cpp b/pcbnew/tools/edit_tool_move_fct.cpp index f7b226faab..b04e186b4e 100644 --- a/pcbnew/tools/edit_tool_move_fct.cpp +++ b/pcbnew/tools/edit_tool_move_fct.cpp @@ -24,43 +24,26 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA */ -#include +#include #include #include -#include -#include -#include -#include +#include +#include +#include #include -#include -#include -#include #include #include #include #include #include #include -#include #include -#include -#include -#include -#include -#include -#include #include #include -#include -#include -#include -#include #include #include #include #include -#include -#include #include @@ -84,12 +67,8 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) { BOARD_ITEM* item = aCollector[i]; - switch( item->Type() ) - { - case PCB_TRACE_T: aCollector.Remove( item ); break; - - default: break; - } + if( item->Type() == PCB_TRACE_T ) + aCollector.Remove( item ); } }, true /* prompt user regarding locked items */ ); @@ -97,33 +76,28 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) if( selection.Size() < 2 ) return 0; + BOARD_COMMIT localCommit( this ); + BOARD_COMMIT* commit = dynamic_cast( aEvent.Commit() ); + + if( !commit ) + commit = &localCommit; + std::vector sorted = selection.GetItemsSortedBySelectionOrder(); - // When editing footprints, all items have the same parent - if( IsFootprintEditor() ) + // Save items, so changes can be undone + for( EDA_ITEM* item : selection ) { - m_commit->Modify( selection.Front() ); - } - else - { - // Save items, so changes can be undone - for( EDA_ITEM* item : selection ) + if( !item->IsNew() && !item->IsMoving() && ( !IsFootprintEditor() || commit->Empty() ) ) { - // Don't double move footprint pads, fields, etc. - // - // For PCB_GROUP_T, the parent is the board. - if( item->GetParent() && item->GetParent()->IsSelected() ) - continue; + commit->Modify( item ); - m_commit->Modify( item ); - - // If moving a group, record position of all the descendants for undo + // If swapping a group, record position of all the descendants for undo if( item->Type() == PCB_GROUP_T ) { PCB_GROUP* group = static_cast( item ); group->RunOnDescendants( [&]( BOARD_ITEM* bItem ) { - m_commit->Modify( bItem ); + commit->Modify( bItem ); }); } } @@ -172,8 +146,8 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) } } - if( !m_dragging ) - m_commit->Push( _( "Swap" ) ); + if( !localCommit.Empty() ) + localCommit.Push( _( "Swap" ) ); m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); @@ -183,6 +157,7 @@ int EDIT_TOOL::Swap( const TOOL_EVENT& aEvent ) int EDIT_TOOL::PackAndMoveFootprints( const TOOL_EVENT& aEvent ) { + BOARD_COMMIT commit( this ); PCB_SELECTION& selection = m_selectionTool->RequestSelection( []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) { @@ -213,13 +188,19 @@ int EDIT_TOOL::PackAndMoveFootprints( const TOOL_EVENT& aEvent ) for( FOOTPRINT* item : footprintsToPack ) { - m_commit->Modify( item ); + commit.Modify( item ); + item->SetFlags( IS_MOVING ); footprintsBbox.Merge( item->GetBoundingBox( false, false ) ); } SpreadFootprints( &footprintsToPack, footprintsBbox.Normalize().GetOrigin(), false ); - return doMoveSelection( aEvent, _( "Pack footprints" ) ); + if( DoMoveSelection( aEvent, &commit ) ) + commit.Push( _( "Pack footprints" ) ); + else + commit.Revert(); + + return 0; } @@ -231,7 +212,14 @@ int EDIT_TOOL::Move( const TOOL_EVENT& aEvent ) return 0; } - return doMoveSelection( aEvent, _( "Move" ) ); + BOARD_COMMIT commit( this ); + + if( DoMoveSelection( aEvent, &commit ) ) + commit.Push( _( "Move" ) ); + else + commit.Revert(); + + return 0; } @@ -269,7 +257,7 @@ VECTOR2I EDIT_TOOL::getSafeMovement( const VECTOR2I& aMovement, const BOX2I& aSo } -int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommitMessage ) +bool EDIT_TOOL::DoMoveSelection( const TOOL_EVENT& aEvent, BOARD_COMMIT* aCommit ) { bool moveWithReference = aEvent.IsAction( &PCB_ACTIONS::moveWithReference ); bool moveIndividually = aEvent.IsAction( &PCB_ACTIONS::moveIndividually ); @@ -296,7 +284,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi !m_isFootprintEditor && cfg->m_AllowFreePads ); if( m_dragging || selection.Empty() ) - return 0; + return false; LSET item_layers = selection.GetSelectionLayers(); bool is_hover = selection.IsHover(); // N.B. This must be saved before the second call @@ -318,9 +306,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi } if( selection.Empty() ) - { - return 0; - } + return false; editFrame->PushTool( aEvent ); Activate(); @@ -397,7 +383,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); editFrame->PopTool( aEvent ); - return 0; + return false; } if( moveIndividually ) @@ -432,7 +418,6 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi bool hv45Mode = false; bool eatFirstMouseUp = true; - bool hasRedrawn3D = false; bool allowRedraw3D = cfg->m_Display.m_Live3DRefresh; bool showCourtyardConflicts = !m_isFootprintEditor && cfg->m_ShowCourtyardCollisions; @@ -544,10 +529,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi } if( redraw3D && allowRedraw3D ) - { editFrame->Update3DView( false, true ); - hasRedrawn3D = true; - } if( showCourtyardConflicts && drc_on_move->m_FpInMove.size() ) { @@ -564,23 +546,16 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi m_dragging = true; - // When editing footprints, all items have the same parent - if( IsFootprintEditor() ) + for( EDA_ITEM* item : selection ) { - m_commit->Modify( selection.Front() ); - } - else - { - // Save items, so changes can be undone - for( EDA_ITEM* item : selection ) - { - // Don't double move footprint pads, fields, etc. - // - // For PCB_GROUP_T, the parent is the board. - if( item->GetParent() && item->GetParent()->IsSelected() ) - continue; + if( item->GetParent() && item->GetParent()->IsSelected() ) + continue; - m_commit->Modify( item ); + if( !item->IsNew() && item->IsMoving() + && ( !IsFootprintEditor() || aCommit->Empty() ) ) + { + aCommit->Modify( item ); + item->SetFlags( IS_MOVING ); // If moving a group, record position of all the descendants for undo if( item->Type() == PCB_GROUP_T ) @@ -588,13 +563,13 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi PCB_GROUP* group = static_cast( item ); group->RunOnDescendants( [&]( BOARD_ITEM* bItem ) { - m_commit->Modify( bItem ); + aCommit->Modify( bItem ); + item->SetFlags( IS_MOVING ); }); } } } - editFrame->UndoRedoBlock( true ); m_cursor = controls->GetCursorPosition(); if( selection.HasReferencePoint() ) @@ -666,7 +641,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi statusPopup.Move( wxGetMousePosition() + wxPoint( 20, 20 ) ); - m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, new VECTOR2I( movement ) ); + m_toolMgr->PostAction( PCB_ACTIONS::updateLocalRatsnest, movement ); } else if( evt->IsCancelInteractive() || evt->IsActivate() ) { @@ -676,21 +651,13 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi restore_state = true; // Canceling the tool means that items have to be restored break; // Finish } - else if( evt->IsAction( &ACTIONS::undo ) ) + else if( evt->IsAction( &ACTIONS::undo ) || evt->IsAction( &ACTIONS::doDelete ) ) { restore_state = true; // Perform undo locally break; // Finish } - else if( evt->IsAction( &ACTIONS::doDelete ) || evt->IsAction( &ACTIONS::cut ) ) + else if( evt->IsAction( &ACTIONS::duplicate ) || evt->IsAction( &ACTIONS::cut ) ) { - // Dispatch TOOL_ACTIONs - evt->SetPassEvent(); - break; // finish -- there is no further processing for removed items - } - else if( evt->IsAction( &ACTIONS::duplicate ) ) - { - evt->SetPassEvent(); - break; // finish -- Duplicate tool will start a new Move with the dup'ed items } else if( evt->IsAction( &PCB_ACTIONS::rotateCw ) || evt->IsAction( &PCB_ACTIONS::rotateCcw ) @@ -744,7 +711,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi updateStatusPopup( nextItem, itemIdx + 1, orig_items.size() ); // Pick up new item - m_commit->Modify( nextItem ); + aCommit->Modify( nextItem ); nextItem->SetPosition( controls->GetMousePosition( true ) ); continue; @@ -767,7 +734,7 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi displayConstraintsMessage( hv45Mode ); evt->SetPassEvent( false ); } - else if( ZONE_FILLER_TOOL::IsZoneFillAction( evt ) ) + else if( ZONE_FILLER_TOOL::IsZoneFillAction( evt ) || evt->IsAction( &ACTIONS::redo )) { wxBell(); } @@ -787,47 +754,26 @@ int EDIT_TOOL::doMoveSelection( const TOOL_EVENT& aEvent, const wxString& aCommi controls->SetAutoPan( false ); m_dragging = false; - editFrame->UndoRedoBlock( false ); // Discard reference point when selection is "dropped" onto the board selection.ClearReferencePoint(); // Unselect all items to clear selection flags and then re-select the originally selected - // items (after the potential Revert()). + // items. m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); - // TODO: there's an encapsulation leak here: this commit often has more than just the move - // in it; for instance it might have a paste, append board, etc. as well. - if( restore_state ) + if( !restore_state ) { - m_commit->Revert(); - m_selectionTool->RebuildSelection(); - - // Mainly for point editor, but there might be other clients that need to adjust to - // reverted state. - m_toolMgr->PostEvent( EVENTS::SelectedItemsMoved ); - - // Property panel needs to know about the reselect - m_toolMgr->PostEvent( EVENTS::SelectedItemsModified ); - - if( hasRedrawn3D ) - editFrame->Update3DView( false, true ); - } - else - { - m_commit->Push( aCommitMessage ); - EDA_ITEMS oItems( orig_items.begin(), orig_items.end() ); m_toolMgr->RunAction( PCB_ACTIONS::selectItems, &oItems ); } - m_toolMgr->GetTool()->UpdateStatusBar(); // Remove the dynamic ratsnest from the screen m_toolMgr->RunAction( PCB_ACTIONS::hideLocalRatsnest ); editFrame->PopTool( aEvent ); editFrame->GetCanvas()->SetCurrentCursor( KICURSOR::ARROW ); - return restore_state ? -1 : 0; + return !restore_state; } diff --git a/pcbnew/tools/pcb_control.cpp b/pcbnew/tools/pcb_control.cpp index 60e378cfa0..dbeff7c1c6 100644 --- a/pcbnew/tools/pcb_control.cpp +++ b/pcbnew/tools/pcb_control.cpp @@ -834,7 +834,7 @@ void PCB_CONTROL::pruneItemLayers( std::vector& aItems ) int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) { CLIPBOARD_IO pi; - BOARD_ITEM* clipItem = pi.Parse(); + BOARD_ITEM* clipItem = pi.Parse(); if( !clipItem ) return 0; @@ -871,9 +871,10 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) // The clipboard can contain two different things, an entire kicad_pcb or a single footprint if( isFootprintEditor && ( !board() || !footprint() ) ) - { return 0; - } + + BOARD_COMMIT commit( m_toolMgr ); + bool cancelled = false; switch( clipItem->Type() ) { @@ -942,7 +943,8 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) pruneItemLayers( pastedItems ); - placeBoardItems( pastedItems, true, true, mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); + cancelled = !placeBoardItems( &commit, pastedItems, true, true, + mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); } else { @@ -952,7 +954,8 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) clipFootprint->SetReference( defaultRef ); } - placeBoardItems( clipBoard, true, mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); + cancelled = !placeBoardItems( &commit, clipBoard, true, + mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); } break; @@ -979,7 +982,8 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) pruneItemLayers( pastedItems ); - placeBoardItems( pastedItems, true, true, mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); + cancelled = !placeBoardItems( &commit, pastedItems, true, true, + mode == PASTE_MODE::UNIQUE_ANNOTATIONS ); break; } @@ -988,6 +992,11 @@ int PCB_CONTROL::Paste( const TOOL_EVENT& aEvent ) break; } + if( cancelled ) + commit.Revert(); + else + commit.Push( _( "Paste" ) ); + return 1; } @@ -1013,7 +1022,6 @@ int PCB_CONTROL::AppendBoardFromFile( const TOOL_EVENT& aEvent ) } -// Helper function for PCB_CONTROL::placeBoardItems() template static void moveUnflaggedItems( std::deque& aList, std::vector& aTarget, bool aIsNew ) @@ -1076,7 +1084,8 @@ static void moveUnflaggedItems( ZONES& aList, std::vector& aTarget, -int PCB_CONTROL::placeBoardItems( BOARD* aBoard, bool aAnchorAtOrigin, bool aReannotateDuplicates ) +bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, BOARD* aBoard, bool aAnchorAtOrigin, + bool aReannotateDuplicates ) { // items are new if the current board is not the board source bool isNew = board() != aBoard; @@ -1096,12 +1105,12 @@ int PCB_CONTROL::placeBoardItems( BOARD* aBoard, bool aAnchorAtOrigin, bool aRea pruneItemLayers( items ); - return placeBoardItems( items, isNew, aAnchorAtOrigin, aReannotateDuplicates ); + return placeBoardItems( aCommit, items, isNew, aAnchorAtOrigin, aReannotateDuplicates ); } -int PCB_CONTROL::placeBoardItems( std::vector& aItems, bool aIsNew, - bool aAnchorAtOrigin, bool aReannotateDuplicates ) +bool PCB_CONTROL::placeBoardItems( BOARD_COMMIT* aCommit, std::vector& aItems, + bool aIsNew, bool aAnchorAtOrigin, bool aReannotateDuplicates ) { m_toolMgr->RunAction( PCB_ACTIONS::selectionClear ); @@ -1161,11 +1170,10 @@ int PCB_CONTROL::placeBoardItems( std::vector& aItems, bool aIsNew, for( BOARD_ITEM* item : aItems ) { - // Commit after reannotation if( aIsNew ) - editTool->GetCurrentCommit()->Add( item ); + aCommit->Add( item ); else - editTool->GetCurrentCommit()->Added( item ); + aCommit->Added( item ); } PCB_SELECTION& selection = selectionTool->GetSelection(); @@ -1185,16 +1193,18 @@ int PCB_CONTROL::placeBoardItems( std::vector& aItems, bool aIsNew, getViewControls()->SetCursorPosition( getViewControls()->GetMousePosition(), false ); m_toolMgr->ProcessEvent( EVENTS::SelectedEvent ); - m_toolMgr->RunAction( PCB_ACTIONS::move ); + + return editTool->DoMoveSelection( PCB_ACTIONS::move.MakeEvent(), aCommit ); } - return 0; + return true; } int PCB_CONTROL::AppendBoard( PLUGIN& pi, wxString& fileName ) { PCB_EDIT_FRAME* editFrame = dynamic_cast( m_frame ); + BOARD_COMMIT commit( m_toolMgr ); if( !editFrame ) return 1; @@ -1295,7 +1305,12 @@ int PCB_CONTROL::AppendBoard( PLUGIN& pi, wxString& fileName ) brd->SetEnabledLayers( enabledLayers ); brd->SetVisibleLayers( enabledLayers ); - return placeBoardItems( brd, false, false ); // Do not reannotate duplicates on Append Board + if( placeBoardItems( &commit, brd, false, false /* Don't reannotate dupes on Append Board */ ) ) + commit.Push( _( "Append Board" ) ); + else + commit.Revert(); + + return 0; } diff --git a/pcbnew/tools/pcb_control.h b/pcbnew/tools/pcb_control.h index fb8d7b8d64..df26c1b54e 100644 --- a/pcbnew/tools/pcb_control.h +++ b/pcbnew/tools/pcb_control.h @@ -133,10 +133,11 @@ private: * @param aReannotateDuplicates = true to reannotate any footprints with a designator that already exist in the board. */ - int placeBoardItems( std::vector& aItems, bool aIsNew, bool aAnchorAtOrigin, - bool aReannotateDuplicates ); + bool placeBoardItems( BOARD_COMMIT* aCommit, std::vector& aItems, bool aIsNew, + bool aAnchorAtOrigin, bool aReannotateDuplicates ); - int placeBoardItems( BOARD* aBoard, bool aAnchorAtOrigin, bool aReannotateDuplicates ); + bool placeBoardItems( BOARD_COMMIT* aCommit, BOARD* aBoard, bool aAnchorAtOrigin, + bool aReannotateDuplicates ); ///< Pointer to the currently used edit frame. PCB_BASE_FRAME* m_frame;