Restore selection on abort and undo/redo.

Also includes a related bug fix to not create an undo record for
the initial schematic cleanup.

Fixes: lp:1825975
* https://bugs.launchpad.net/kicad/+bug/1825975

eeschema/bus-wire-junction.cpp

@@ -76,7 +76,7 @@ bool SCH_EDIT_FRAME::TestDanglingEnds()
 }
 
 
-bool SCH_EDIT_FRAME::TrimWire( const wxPoint& aStart, const wxPoint& aEnd, bool aAppend )
+bool SCH_EDIT_FRAME::TrimWire( const wxPoint& aStart, const wxPoint& aEnd )
 {
     SCH_LINE* line;
     SCH_ITEM* next_item = NULL;
@@ -113,20 +113,19 @@ bool SCH_EDIT_FRAME::TrimWire( const wxPoint& aStart, const wxPoint& aEnd, bool
         // Step 1: break the segment on one end.  return_line remains line if not broken.
         // Ensure that *line points to the segment containing aEnd
         SCH_LINE* return_line = line;
-        aAppend |= BreakSegment( line, aStart, aAppend, &return_line );
+        BreakSegment( line, aStart, &return_line );
         if( IsPointOnSegment( return_line->GetStartPoint(), return_line->GetEndPoint(), aEnd ) )
             line = return_line;
 
         // Step 2: break the remaining segment.  return_line remains line if not broken.
         // Ensure that *line _also_ contains aStart.  This is our overlapping segment
-        aAppend |= BreakSegment( line, aEnd, aAppend, &return_line );
+        BreakSegment( line, aEnd, &return_line );
         if( IsPointOnSegment( return_line->GetStartPoint(), return_line->GetEndPoint(), aStart ) )
             line = return_line;
 
-        SaveCopyInUndoList( line, UR_DELETED, aAppend );
+        SaveCopyInUndoList( line, UR_DELETED, true );
         RemoveFromScreen( line );
 
-        aAppend = true;
         retval = true;
     }
 
@@ -134,7 +133,7 @@ bool SCH_EDIT_FRAME::TrimWire( const wxPoint& aStart, const wxPoint& aEnd, bool
 }
 
 
-bool SCH_EDIT_FRAME::SchematicCleanUp( bool aUndo, SCH_SCREEN* aScreen )
+bool SCH_EDIT_FRAME::SchematicCleanUp( SCH_SCREEN* aScreen )
 {
     SCH_ITEM*           item = NULL;
     SCH_ITEM*           secondItem = NULL;
@@ -149,7 +148,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aUndo, SCH_SCREEN* aScreen )
         itemList.PushItem( ITEM_PICKER( aItem, UR_DELETED ) );
     };
 
-    BreakSegmentsOnJunctions( true, aScreen );
+    BreakSegmentsOnJunctions( aScreen );
 
     for( item = aScreen->GetDrawItems(); item; item = item->Next() )
     {
@@ -234,7 +233,7 @@ bool SCH_EDIT_FRAME::SchematicCleanUp( bool aUndo, SCH_SCREEN* aScreen )
             RemoveFromScreen( item, aScreen );
     }
 
-    if( itemList.GetCount() && aUndo )
+    if( itemList.GetCount() )
         SaveCopyInUndoList( itemList, UR_DELETED, true );
 
     return itemList.GetCount() > 0;
@@ -249,7 +248,7 @@ bool SCH_EDIT_FRAME::AddMissingJunctions( SCH_SCREEN* aScreen )
     {
         auto junction = new SCH_JUNCTION( aPosition );
         AddToScreen( junction, aScreen );
-        BreakSegments( aPosition, false );
+        BreakSegments( aPosition );
         added = true;
     };
 
@@ -271,10 +270,12 @@ bool SCH_EDIT_FRAME::AddMissingJunctions( SCH_SCREEN* aScreen )
 }
 
 
-void SCH_EDIT_FRAME::NormalizeSchematicOnFirstLoad()
+void SCH_EDIT_FRAME::NormalizeSchematicOnFirstLoad( bool recalculateConnections )
 {
-    BreakSegmentsOnJunctions();
-    SchematicCleanUp();
+    if( recalculateConnections )
+        RecalculateConnections();
+    else
+        SchematicCleanUp();
 
     SCH_SHEET_LIST list( g_RootSheet );
 
@@ -284,8 +285,7 @@ void SCH_EDIT_FRAME::NormalizeSchematicOnFirstLoad()
 
 
 bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint,
-                                   bool aAppend, SCH_LINE** aNewSegment,
-                                   SCH_SCREEN* aScreen )
+                                   SCH_LINE** aNewSegment, SCH_SCREEN* aScreen )
 {
     if( !IsPointOnSegment( aSegment->GetStartPoint(), aSegment->GetEndPoint(), aPoint )
             || aSegment->IsEndPoint( aPoint ) )
@@ -299,7 +299,7 @@ bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint,
     newSegment->SetStartPoint( aPoint );
     AddToScreen( newSegment, aScreen );
 
-    SaveCopyInUndoList( newSegment, UR_NEW, aAppend );
+    SaveCopyInUndoList( newSegment, UR_NEW, true );
     SaveCopyInUndoList( aSegment, UR_CHANGED, true );
 
     RefreshItem( aSegment );
@@ -312,8 +312,10 @@ bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint,
 }
 
 
-bool SCH_EDIT_FRAME::BreakSegments( const wxPoint& aPoint, bool aAppend, SCH_SCREEN* aScreen )
+bool SCH_EDIT_FRAME::BreakSegments( const wxPoint& aPoint, SCH_SCREEN* aScreen )
 {
+    static KICAD_T wiresAndBusses[] = { SCH_LINE_LOCATE_WIRE_T, SCH_LINE_LOCATE_BUS_T, EOT };
+
     if( aScreen == nullptr )
         aScreen = GetScreen();
 
@@ -321,18 +323,15 @@ bool SCH_EDIT_FRAME::BreakSegments( const wxPoint& aPoint, bool aAppend, SCH_SCR
 
     for( SCH_ITEM* segment = aScreen->GetDrawItems(); segment; segment = segment->Next() )
     {
-        if( ( segment->Type() != SCH_LINE_T ) || ( segment->GetLayer() == LAYER_NOTES ) )
-            continue;
-
-        brokenSegments |= BreakSegment( (SCH_LINE*) segment, aPoint,
-                                        aAppend || brokenSegments, NULL, aScreen );
+        if( segment->IsType( wiresAndBusses ) )
+            brokenSegments |= BreakSegment( (SCH_LINE*) segment, aPoint, NULL, aScreen );
     }
 
     return brokenSegments;
 }
 
 
-bool SCH_EDIT_FRAME::BreakSegmentsOnJunctions( bool aAppend, SCH_SCREEN* aScreen )
+bool SCH_EDIT_FRAME::BreakSegmentsOnJunctions( SCH_SCREEN* aScreen )
 {
     if( aScreen == nullptr )
         aScreen = GetScreen();
@@ -345,20 +344,14 @@ bool SCH_EDIT_FRAME::BreakSegmentsOnJunctions( bool aAppend, SCH_SCREEN* aScreen
         {
             SCH_JUNCTION* junction = ( SCH_JUNCTION* ) item;
 
-            if( BreakSegments( junction->GetPosition(), brokenSegments || aAppend, aScreen ) )
-                brokenSegments = true;
+            brokenSegments |= BreakSegments( junction->GetPosition(), aScreen );
         }
-        else
+        else if( item->Type() == SCH_BUS_BUS_ENTRY_T || item->Type() == SCH_BUS_WIRE_ENTRY_T )
         {
-            SCH_BUS_ENTRY_BASE* busEntry = dynamic_cast<SCH_BUS_ENTRY_BASE*>( item );
-            if( busEntry )
-            {
-                if( BreakSegments( busEntry->GetPosition(), brokenSegments || aAppend, aScreen ) )
-                    brokenSegments = true;
+            SCH_BUS_ENTRY_BASE* busEntry = (SCH_BUS_ENTRY_BASE*) item;
 
-                if( BreakSegments( busEntry->m_End(), brokenSegments || aAppend, aScreen ) )
-                    brokenSegments = true;
-            }
+            brokenSegments |= BreakSegments( busEntry->GetPosition(), aScreen );
+            brokenSegments |= BreakSegments( busEntry->m_End(), aScreen );
         }
     }
 
@@ -431,14 +424,13 @@ void SCH_EDIT_FRAME::DeleteJunction( SCH_ITEM* aJunction, bool aAppend )
 }
 
 
-SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( const wxPoint& aPosition, bool aAppend, bool aFinal )
+SCH_JUNCTION* SCH_EDIT_FRAME::AddJunction( const wxPoint& aPos, bool aUndoAppend, bool aFinal )
 {
-    SCH_JUNCTION* junction = new SCH_JUNCTION( aPosition );
-    bool broken_segments = false;
+    SCH_JUNCTION* junction = new SCH_JUNCTION( aPos );
 
     AddToScreen( junction );
-    broken_segments = BreakSegments( aPosition, aAppend );
-    SaveCopyInUndoList( junction, UR_NEW, broken_segments || aAppend );
+    SaveCopyInUndoList( junction, UR_NEW, aUndoAppend );
+    BreakSegments( aPos );
 
     if( aFinal )
     {

eeschema/class_libentry.cpp

@@ -917,12 +917,16 @@ bool LIB_PART::HasConversion() const
     return false;
 }
 
-void LIB_PART::ClearStatus()
+void LIB_PART::ClearTempFlags()
 {
     for( LIB_ITEM& item : m_drawings )
-    {
-        item.m_Flags = 0;
-    }
+        item.ClearTempFlags();
+}
+
+void LIB_PART::ClearEditFlags()
+{
+    for( LIB_ITEM& item : m_drawings )
+        item.ClearEditFlags();
 }
 
 LIB_ITEM* LIB_PART::LocateDrawItem( int aUnit, int aConvert,

eeschema/class_libentry.h

@@ -581,8 +581,8 @@ public:
     /**
      * Clears the status flag all draw objects in this part.
      */
-    void ClearStatus();
-
+    void ClearTempFlags();
+    void ClearEditFlags();
     /**
      * Locate a draw object.
      *

eeschema/files-io.cpp

@@ -370,12 +370,10 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector<wxString>& aFileSet, in
         SetScreen( g_CurrentSheet->LastScreen() );
 
         // Ensure the schematic is fully segmented on first display
-        NormalizeSchematicOnFirstLoad();
-
+        NormalizeSchematicOnFirstLoad( true );
         GetScreen()->ClearUndoORRedoList( GetScreen()->m_UndoList, 1 );
-        GetScreen()->TestDanglingEnds();    // Only perform the dangling end test on root sheet.
 
-        RecalculateConnections(); // Update connectivity graph
+        GetScreen()->TestDanglingEnds();    // Only perform the dangling end test on root sheet.
 
         // Migrate conflicting bus definitions
         // TODO(JE) This should only run once based on schematic file version
@@ -849,7 +847,7 @@ bool SCH_EDIT_FRAME::importFile( const wxString& aFileName, int aFileType )
             schematic.UpdateSymbolLinks();      // Update all symbol library links for all sheets.
 
             // Ensure the schematic is fully segmented on first display
-            NormalizeSchematicOnFirstLoad();
+            NormalizeSchematicOnFirstLoad( false );
 
             GetScreen()->m_Initialized = true;
 

eeschema/lib_draw_item.h

@@ -173,7 +173,7 @@ public:
      */
     virtual void EndEdit( const wxPoint& aPosition )
     {
-        ClearFlags( GetEditFlags() );
+        ClearEditFlags();
     }
 
     /**

eeschema/libedit/libedit_undo_redo.cpp

@@ -32,6 +32,7 @@
 #include <symbol_tree_pane.h>
 #include <tool/tool_manager.h>
 #include <tools/ee_actions.h>
+#include <tools/ee_selection_tool.h>
 
 void LIB_EDIT_FRAME::SaveCopyInUndoList( EDA_ITEM* ItemToCopy, UNDO_REDO_T undoType )
 {
@@ -41,7 +42,8 @@ void LIB_EDIT_FRAME::SaveCopyInUndoList( EDA_ITEM* ItemToCopy, UNDO_REDO_T undoT
     CopyItem = new LIB_PART( * (LIB_PART*) ItemToCopy );
 
     // Clear current flags (which can be temporary set by a current edit command).
-    CopyItem->ClearStatus();
+    CopyItem->ClearTempFlags();
+    CopyItem->ClearEditFlags();
     CopyItem->SetFlags( UR_TRANSIENT );
 
     ITEM_PICKER wrapper( CopyItem, undoType );
@@ -161,6 +163,9 @@ void LIB_EDIT_FRAME::RollbackPartFromUndo()
     part->ClearFlags( UR_TRANSIENT );
     SetCurPart( part );
 
+    EE_SELECTION_TOOL* selTool = m_toolManager->GetTool<EE_SELECTION_TOOL>();
+    selTool->RebuildSelection();
+
     RebuildSymbolUnitsList();
     SetShowDeMorgan( part->HasConversion() );
 

eeschema/sch_edit_frame.cpp

@@ -1254,7 +1254,7 @@ void SCH_EDIT_FRAME::AddItemToScreenAndUndoList( SCH_ITEM* aItem, bool aUndoAppe
         SaveUndoItemInUndoList( undoItem, aUndoAppend );
     }
 
-    aItem->ClearFlags( aItem->GetEditFlags() );
+    aItem->ClearEditFlags();
 
     screen->SetModify();
     RefreshItem( aItem );
@@ -1267,7 +1267,7 @@ void SCH_EDIT_FRAME::AddItemToScreenAndUndoList( SCH_ITEM* aItem, bool aUndoAppe
         for( auto i = pts.begin(); i != pts.end(); i++ )
         {
             for( auto j = i + 1; j != pts.end(); j++ )
-                TrimWire( *i, *j, true );
+                TrimWire( *i, *j );
 
             if( screen->IsJunctionNeeded( *i, true ) )
                 AddJunction( *i, true );
@@ -1320,7 +1320,7 @@ void SCH_EDIT_FRAME::RecalculateConnections( bool aDoCleanup )
     if( aDoCleanup )
     {
         for( const auto& sheet : list )
-            SchematicCleanUp( false, sheet.LastScreen() );
+            SchematicCleanUp( sheet.LastScreen() );
     }
 
     timer.Stop();

eeschema/sch_edit_frame.h

@@ -388,41 +388,41 @@ public:
                                     const wxString& aSearchText );
 
     /**
-     * Breaks a single segment into two at the specified point
+     * Breaks a single segment into two at the specified point.
+     *
+     * NOTE: always appends to the existing undo state.
      *
      * @param aSegment Line segment to break
      * @param aPoint Point at which to break the segment
-     * @param aAppend Add the changes to the previous undo state
      * @param aNewSegment Pointer to the newly created segment (if given and created)
      * @param aScreen is the screen to examine, or nullptr to examine the current screen
      * @return True if any wires or buses were broken.
      */
     bool BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint,
-                       bool aAppend = false, SCH_LINE** aNewSegment = NULL,
-                       SCH_SCREEN* aScreen = nullptr );
+                       SCH_LINE** aNewSegment = NULL, SCH_SCREEN* aScreen = nullptr );
 
     /**
      * Checks every wire and bus for a intersection at \a aPoint and break into two segments
      * at \a aPoint if an intersection is found.
      *
+     * NOTE: always appends to the existing undo state.
+     *
      * @param aPoint Test this point for an intersection.
-     * @param aAppend Add the changes to the previous undo state
      * @param aScreen is the screen to examine, or nullptr to examine the current screen
      * @return True if any wires or buses were broken.
      */
-    bool BreakSegments( const wxPoint& aPoint, bool aAppend = false,
-                        SCH_SCREEN* aScreen = nullptr );
+    bool BreakSegments( const wxPoint& aPoint, SCH_SCREEN* aScreen = nullptr );
 
     /**
      * Tests all junctions and bus entries in the schematic for intersections with wires and
      * buses and breaks any intersections into multiple segments.
      *
-     * @param aAppend Add the changes to the previous undo state
+     * NOTE: always appends to the existing undo state.
+     *
      * @param aScreen is the screen to examine, or nullptr to examine the current screen
      * @return True if any wires or buses were broken.
      */
-    bool BreakSegmentsOnJunctions( bool aAppend = false,
-                                   SCH_SCREEN* aScreen = nullptr );
+    bool BreakSegmentsOnJunctions( SCH_SCREEN* aScreen = nullptr );
 
     /**
      * Test all of the connectable objects in the schematic for unused connection points.
@@ -734,7 +734,7 @@ public:
      */
     bool AskToSaveChanges();
 
-    SCH_JUNCTION* AddJunction( const wxPoint& aPosition, bool aAppendToUndo = false,
+    SCH_JUNCTION* AddJunction( const wxPoint& aPos, bool aAppendToUndo = false,
                                bool aFinal = true );
 
     SCH_TEXT* CreateNewText( int aType );
@@ -743,22 +743,24 @@ public:
      * Performs routine schematic cleaning including breaking wire and buses and
      * deleting identical objects superimposed on top of each other.
      *
-     * @param aAppend The changes to the schematic should be appended to the previous undo
+     * NOTE: always appends to the existing undo state.
+     *
      * @param aScreen is the screen to examine, or nullptr to examine the current screen
      * @return True if any schematic clean up was performed.
      */
-    bool SchematicCleanUp( bool aAppend = false, SCH_SCREEN* aScreen = nullptr );
+    bool SchematicCleanUp( SCH_SCREEN* aScreen = nullptr );
 
     /**
      * If any single wire passes through _both points_, remove the portion between the two points,
      * potentially splitting the wire into two.
      *
+     * NOTE: always appends to the existing undo state.
+     *
      * @param aStart The starting point for trimmming
      * @param aEnd The ending point for trimming
-     * @param aAppend Should the line changes be appended to a previous undo state
      * @return True if any wires were changed by this operation
      */
-    bool TrimWire( const wxPoint& aStart, const wxPoint& aEnd, bool aAppend = true );
+    bool TrimWire( const wxPoint& aStart, const wxPoint& aEnd );
 
     /**
      * Collects a unique list of all possible connection points in the schematic.
@@ -855,7 +857,7 @@ private:
      * Perform all cleanup and normalization steps so that the whole schematic
      * is in a good state.  This should only be called when loading a file.
      */
-    void NormalizeSchematicOnFirstLoad();
+    void NormalizeSchematicOnFirstLoad( bool recalculateConnections );
 
     // Hierarchical Sheet & PinSheet
     void InstallHierarchyFrame( wxCommandEvent& event );

eeschema/sch_field.cpp

@@ -315,7 +315,7 @@ void SCH_FIELD::Place( SCH_EDIT_FRAME* frame, wxDC* DC )
     // save old cmp in undo list
     frame->SaveUndoItemInUndoList( component );
 
-    ClearFlags( GetEditFlags() );
+    ClearEditFlags();
     frame->OnModify();
 }
 

eeschema/schematic_undo_redo.cpp

@@ -36,6 +36,7 @@
 #include <sch_sheet.h>
 #include <sch_bitmap.h>
 #include <sch_view.h>
+#include <tools/ee_selection_tool.h>
 
 /* Functions to undo and redo edit commands.
  *  commands to undo are stored in CurrentScreen->m_UndoList
@@ -265,7 +266,9 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed
         item = (SCH_ITEM*) aList->GetPickedItem( (unsigned) ii );
         alt_item = (SCH_ITEM*) aList->GetPickedItemLink( (unsigned) ii );
 
-        item->ClearFlags();
+        item->SetFlags( aList->GetPickerFlags( (unsigned) ii ) );
+        item->ClearEditFlags();
+        item->ClearTempFlags();
 
         if( status == UR_NEW )
         {
@@ -291,9 +294,7 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed
                 break;
 
             case UR_MOVED:
-                item->SetFlags( aList->GetPickerFlags( (unsigned) ii ) );
                 item->Move( aRedoCommand ? aList->m_TransformPoint : -aList->m_TransformPoint );
-                item->ClearFlags();
                 break;
 
             case UR_MIRRORED_Y:
@@ -334,6 +335,9 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool aRed
         }
     }
 
+    EE_SELECTION_TOOL* selTool = m_toolManager->GetTool<EE_SELECTION_TOOL>();
+    selTool->RebuildSelection();
+
     // Bitmaps are cached in Opengl: clear the cache, because
     // the cache data can be invalid
     GetCanvas()->GetView()->RecacheAllItems();

eeschema/tools/ee_selection_tool.cpp

@@ -786,6 +786,29 @@ int EE_SELECTION_TOOL::ClearSelection( const TOOL_EVENT& aEvent )
 }
 
 
+void EE_SELECTION_TOOL::RebuildSelection()
+{
+    m_selection.Clear();
+
+    EDA_ITEM* start = nullptr;
+
+    if( m_isLibEdit )
+        start = static_cast<LIB_EDIT_FRAME*>( m_frame )->GetCurPart();
+    else
+        start = m_frame->GetScreen()->GetDrawItems();
+
+    INSPECTOR_FUNC inspector = [&] ( EDA_ITEM* item, void* testData )
+    {
+        if( item->IsSelected() )
+            select( item );
+
+        return SEARCH_CONTINUE;
+    };
+
+    EDA_ITEM::IterateForward( start, inspector, nullptr, EE_COLLECTOR::AllItems );
+}
+
+
 int EE_SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent )
 {
     EE_COLLECTOR* collector = aEvent.Parameter<EE_COLLECTOR*>();

eeschema/tools/ee_selection_tool.h

@@ -135,6 +135,12 @@ public:
      */
     int SelectionMenu( const TOOL_EVENT& aEvent );
 
+    /**
+     * Rebuilds the selection from the EDA_ITEMs' selection flags.  Commonly called after
+     * rolling back an undo state to make sure there aren't any stale pointers.
+     */
+    void RebuildSelection();
+
 private:
     /**
      * Function selectMultiple()

eeschema/tools/lib_drawing_tools.cpp

@@ -255,7 +255,7 @@ int LIB_DRAWING_TOOLS::doTwoClickPlace( KICAD_T aType )
                     break;
                 case LIB_TEXT_T:
                     part->AddDrawItem( (LIB_TEXT*) item );
-                    item->ClearFlags( item->GetEditFlags() );
+                    item->ClearEditFlags();
                     break;
                 default:
                     wxFAIL_MSG( "doTwoClickPlace(): unknown type" );

eeschema/tools/lib_move_tool.cpp

@@ -220,8 +220,6 @@ int LIB_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
         //
         else if( TOOL_EVT_UTILS::IsCancelInteractive( evt.get() ) )
         {
-            m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
-
             if( m_moveInProgress )
                 restore_state = true;
 
@@ -297,7 +295,7 @@ int LIB_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
     selection.ClearReferencePoint();
 
     for( auto item : selection )
-        item->ClearFlags( item->GetEditFlags() );
+        item->ClearEditFlags();
 
     if( unselect )
         m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );

eeschema/tools/lib_pin_tool.cpp

@@ -242,7 +242,7 @@ bool LIB_PIN_TOOL::PlacePin( LIB_PIN* aPin )
  */
 LIB_PIN* LIB_PIN_TOOL::CreatePin( const VECTOR2I& aPosition, LIB_PART* aPart )
 {
-    aPart->ClearStatus();
+    aPart->ClearTempFlags();
 
     LIB_PIN* pin = new LIB_PIN( aPart );
 

eeschema/tools/sch_move_tool.cpp

@@ -349,8 +349,6 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
         //
         else if( TOOL_EVT_UTILS::IsCancelInteractive( evt.get() ) )
         {
-            m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
-
             if( m_moveInProgress )
                 restore_state = true;
 
@@ -433,7 +431,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
     selection.ClearReferencePoint();
 
     for( auto item : selection )
-        item->ClearFlags( item->GetEditFlags() );
+        item->ClearEditFlags();
 
     if( unselect )
         m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true );
@@ -446,8 +444,8 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent )
     }
     else
     {
-        addJunctionsIfNeeded( selection, &appendUndo );
-        m_frame->SchematicCleanUp( true );
+        addJunctionsIfNeeded( selection );
+        m_frame->SchematicCleanUp();
         m_frame->TestDanglingEnds();
         m_frame->OnModify();
     }
@@ -552,7 +550,7 @@ void SCH_MOVE_TOOL::getConnectedDragItems( SCH_ITEM* aOriginalItem, wxPoint aPoi
 }
 
 
-void SCH_MOVE_TOOL::addJunctionsIfNeeded( SELECTION& aSelection, bool* aAppendUndo )
+void SCH_MOVE_TOOL::addJunctionsIfNeeded( SELECTION& aSelection )
 {
     std::vector< wxPoint > pts;
     std::vector< wxPoint > connections;
@@ -585,7 +583,7 @@ void SCH_MOVE_TOOL::addJunctionsIfNeeded( SELECTION& aSelection, bool* aAppendUn
             for( auto point = new_pts.begin(); point != new_pts.end(); point++ )
             {
                 for( auto second_point = point + 1; second_point != new_pts.end(); second_point++ )
-                    *aAppendUndo |= m_frame->TrimWire( *point, *second_point, *aAppendUndo );
+                   m_frame->TrimWire( *point, *second_point );
             }
         }
     }
@@ -599,10 +597,7 @@ void SCH_MOVE_TOOL::addJunctionsIfNeeded( SELECTION& aSelection, bool* aAppendUn
     for( auto point : pts )
     {
         if( m_frame->GetScreen()->IsJunctionNeeded( point, true ) )
-        {
-            m_frame->AddJunction( point, aAppendUndo );
-            *aAppendUndo = true;
-        }
+            m_frame->AddJunction( point, true );
     }
 }
 

eeschema/tools/sch_move_tool.h

@@ -65,7 +65,7 @@ private:
 
     ///> Adds junctions if needed to each item in the list after they have been
     ///> moved.
-    void addJunctionsIfNeeded( SELECTION& aSelection, bool* aAppendUndo );
+    void addJunctionsIfNeeded( SELECTION& aSelection );
 
     ///> Returns the right modification point (e.g. for rotation), depending on the number of
     ///> selected items.

eeschema/tools/sch_wire_bus_tool.cpp

@@ -965,7 +965,7 @@ void SCH_WIRE_BUS_TOOL::finishSegments()
     m_frame->SaveCopyInUndoList( itemList, UR_NEW );
 
     // Correct and remove segments that need to be merged.
-    m_frame->SchematicCleanUp( true );
+    m_frame->SchematicCleanUp();
 
     for( auto item = m_frame->GetScreen()->GetDrawItems(); item; item = item->Next() )
     {
@@ -981,7 +981,7 @@ void SCH_WIRE_BUS_TOOL::finishSegments()
         for( auto i = pts.begin(); i != pts.end(); i++ )
         {
             for( auto j = i + 1; j != pts.end(); j++ )
-                m_frame->TrimWire( *i, *j, true );
+                m_frame->TrimWire( *i, *j );
         }
     }
 

include/base_struct.h

@@ -124,8 +124,7 @@ typedef const INSPECTOR_FUNC& INSPECTOR;    /// std::function passed to nested u
 #define CANDIDATE      (1 << 14)   ///< flag indicating that the structure is connected
 #define SKIP_STRUCT    (1 << 15)   ///< flag indicating that the structure should be ignored
 #define DO_NOT_DRAW    (1 << 16)   ///< Used to disable draw function
-#define IS_CANCELLED   (1 << 17)   ///< flag set when edit dialogs are canceled when editing a
-                                   ///< new object
+// empty spot          (1 << 17)
 #define TRACK_LOCKED   (1 << 18)   ///< Pcbnew: track locked: protected from global deletion
 #define TRACK_AR       (1 << 19)   ///< Pcbnew: autorouted track
 #define FLAG1          (1 << 20)   ///< Pcbnew: flag used in local computations
@@ -271,8 +270,12 @@ public:
 
     void ClearTempFlags()
     {
-        ClearFlags( STARTPOINT | ENDPOINT | CANDIDATE |
-                    SKIP_STRUCT | DO_NOT_DRAW | IS_CANCELLED );
+        ClearFlags( STARTPOINT | ENDPOINT | CANDIDATE | IS_LINKED | SKIP_STRUCT | DO_NOT_DRAW );
+    }
+
+    void ClearEditFlags()
+    {
+        ClearFlags( GetEditFlags() );
     }
 
     /**

pcbnew/board_commit.cpp

@@ -26,6 +26,7 @@
 #include <class_module.h>
 #include <pcb_edit_frame.h>
 #include <tool/tool_manager.h>
+#include <tools/selection_tool.h>
 #include <view/view.h>
 #include <board_commit.h>
 #include <tools/pcb_tool.h>
@@ -324,12 +325,6 @@ void BOARD_COMMIT::Revert()
             if( !( changeFlags & CHT_DONE ) )
                 break;
 
-            if( item->Type() == PCB_MODULE_T )
-            {
-                MODULE* newModule = static_cast<MODULE*>( item );
-                newModule->RunOnChildren( std::bind( &EDA_ITEM::ClearFlags, _1, SELECTED ) );
-            }
-
             view->Add( item );
             connectivity->Add( item );
             board->Add( item );
@@ -341,7 +336,6 @@ void BOARD_COMMIT::Revert()
             connectivity->Remove( item );
 
             item->SwapData( copy );
-            item->ClearFlags( SELECTED );
 
             // Update all pads/drawings/texts, as they become invalid
             // for the VIEW after SwapData() called for modules
@@ -366,5 +360,8 @@ void BOARD_COMMIT::Revert()
     if ( !m_editModules )
         connectivity->RecalculateRatsnest();
 
+    SELECTION_TOOL* selTool = m_toolMgr->GetTool<SELECTION_TOOL>();
+    selTool->RebuildSelection();
+
     clear();
 }

pcbnew/tools/edit_tool.cpp

@@ -503,7 +503,7 @@ int EDIT_TOOL::Main( const TOOL_EVENT& aEvent )
         else if( evt->IsCancel() || evt->IsActivate() )
         {
             restore_state = true; // Canceling the tool means that items have to be restored
-            break;          // Finish
+            break;                // Finish
         }
 
         else if( evt->Action() == TA_UNDO_REDO_PRE )
@@ -572,7 +572,7 @@ int EDIT_TOOL::Main( const TOOL_EVENT& aEvent )
     // Discard reference point when selection is "dropped" onto the board (ie: not dragging anymore)
     selection.ClearReferencePoint();
 
-    if( unselect || restore_state )
+    if( unselect )
         m_toolMgr->RunAction( PCB_ACTIONS::selectionClear, true );
 
     if( restore_state )

pcbnew/tools/selection_tool.cpp

@@ -361,12 +361,15 @@ int SELECTION_TOOL::Main( const TOOL_EVENT& aEvent )
             }
         }
 
-        else if( evt->IsCancel() || evt->Action() == TA_UNDO_REDO_PRE )
+        else if( evt->Action() == TA_UNDO_REDO_PRE )
         {
             clearSelection();
+        }
 
-            if( evt->IsCancel() && !m_editModules )
-                m_toolMgr->RunAction( PCB_ACTIONS::clearHighlight, true );
+        else if( evt->IsCancel() )
+        {
+            clearSelection();
+            m_toolMgr->RunAction( PCB_ACTIONS::clearHighlight, true );
         }
 
         else if( evt->Action() == TA_CONTEXT_MENU_CLOSED )
@@ -1399,6 +1402,31 @@ void SELECTION_TOOL::clearSelection()
 }
 
 
+void SELECTION_TOOL::RebuildSelection()
+{
+    m_selection.Clear();
+
+    INSPECTOR_FUNC inspector = [&] ( EDA_ITEM* item, void* testData )
+    {
+        if( item->IsSelected() )
+        {
+            EDA_ITEM* parent = item->GetParent();
+
+            // Flags on module children might be set only because the parent is selected.
+            if( parent && parent->Type() == PCB_MODULE_T && parent->IsSelected() )
+                return SEARCH_CONTINUE;
+
+            highlight( (BOARD_ITEM*) item, SELECTED, m_selection );
+        }
+
+        return SEARCH_CONTINUE;
+    };
+
+    board()->Visit( inspector, nullptr,  m_editModules ? GENERAL_COLLECTOR::ModuleItems
+                                                       : GENERAL_COLLECTOR::AllBoardItems );
+}
+
+
 int SELECTION_TOOL::SelectionMenu( const TOOL_EVENT& aEvent )
 {
     GENERAL_COLLECTOR* collector = aEvent.Parameter<GENERAL_COLLECTOR*>();

pcbnew/tools/selection_tool.h

@@ -125,6 +125,12 @@ public:
     ///> Multiple item unselection event handler
     int UnselectItems( const TOOL_EVENT& aEvent );
 
+    /**
+     * Rebuilds the selection from the EDA_ITEMs' selection flags.  Commonly called after
+     * rolling back an undo state to make sure there aren't any stale pointers.
+     */
+    void RebuildSelection();
+
     /**
      * Function SelectionMenu()
      * Shows a popup menu to trim the COLLECTOR passed as aEvent's parameter down to a single

pcbnew/undo_redo.cpp

@@ -205,7 +205,8 @@ void PCB_BASE_EDIT_FRAME::SaveCopyInUndoList( BOARD_ITEM* aItem, UNDO_REDO_T aCo
 
 
 void PCB_BASE_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsList,
-                                         UNDO_REDO_T aTypeCommand, const wxPoint& aTransformPoint )
+                                              UNDO_REDO_T aTypeCommand,
+                                              const wxPoint& aTransformPoint )
 {
     PICKED_ITEMS_LIST* commandToUndo = new PICKED_ITEMS_LIST();
 
@@ -217,7 +218,7 @@ void PCB_BASE_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsLis
     for( unsigned ii = 0; ii < aItemsList.GetCount(); ii++ )
     {
         ITEM_PICKER curr_picker = aItemsList.GetItemWrapper(ii);
-        BOARD_ITEM* item    = (BOARD_ITEM*) aItemsList.GetPickedItem( ii );
+        BOARD_ITEM* item        = (BOARD_ITEM*) aItemsList.GetPickedItem( ii );
 
         // For items belonging to modules, we need to save state of the parent module
         if( item->Type() == PCB_MODULE_TEXT_T || item->Type() == PCB_MODULE_EDGE_T
@@ -235,7 +236,8 @@ void PCB_BASE_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsLis
 
             for( unsigned j = 0; j < commandToUndo->GetCount(); j++ )
             {
-                if( commandToUndo->GetPickedItem( j ) == item && commandToUndo->GetPickedItemStatus( j ) == UR_CHANGED )
+                if( commandToUndo->GetPickedItem( j ) == item
+                        && commandToUndo->GetPickedItemStatus( j ) == UR_CHANGED )
                 {
                     found = true;
                     break;
@@ -250,15 +252,14 @@ void PCB_BASE_EDIT_FRAME::SaveCopyInUndoList( const PICKED_ITEMS_LIST& aItemsLis
                 clone->SetParent( GetBoard() );
 
                 // Clear current flags (which can be temporary set by a current edit command)
-                for( EDA_ITEM* loc_item = clone->GraphicalItemsList(); loc_item;
-                        loc_item = loc_item->Next() )
-                    loc_item->ClearFlags();
+                for( EDA_ITEM* child = clone->GraphicalItemsList(); child; child = child->Next() )
+                    child->ClearEditFlags();
 
                 for( D_PAD* pad = clone->PadsList(); pad; pad = pad->Next() )
-                    pad->ClearFlags();
+                    pad->ClearEditFlags();
 
-                clone->Reference().ClearFlags();
-                clone->Value().ClearFlags();
+                clone->Reference().ClearEditFlags();
+                clone->Value().ClearEditFlags();
 
                 ITEM_PICKER picker( item, UR_CHANGED );
                 picker.SetLink( clone );
@@ -455,8 +456,6 @@ void PCB_BASE_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool
             }
         }
 
-        item->ClearFlags();
-
         // see if we must rebuild ratsnets and pointers lists
         switch( item->Type() )
         {
@@ -494,18 +493,8 @@ void PCB_BASE_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool
 
             SwapItemData( item, image );
 
-            // Update all pads/drawings/texts, as they become invalid
-            // for the VIEW after SwapData() called for modules
-            if( item->Type() == PCB_MODULE_T )
-            {
-                MODULE* newModule = static_cast<MODULE*>( item );
-                newModule->RunOnChildren( std::bind( &BOARD_ITEM::ClearFlags, _1, EDA_ITEM_ALL_FLAGS ));
-            }
-
             view->Add( item );
             connectivity->Add( item );
-            item->ClearFlags();
-
         }
         break;
 
@@ -581,6 +570,9 @@ void PCB_BASE_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList, bool
         Compile_Ratsnest( NULL, false );
     }
 
+    SELECTION_TOOL* selTool = m_toolManager->GetTool<SELECTION_TOOL>();
+    selTool->RebuildSelection();
+
     GetBoard()->SanitizeNetcodes();
 }