Maintain hierarchy navigator expansion state between edits.

Prevent the hierarchy navigator from being rebuilt unless there are actual
sheet changes that would cause a change to the tree.

Save the tree expansion state before rebuilding the tree and then restore
the expansion state to the previous state sans edits.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/16635

(cherry picked from commit 797ab998cc)

eeschema/dialogs/dialog_sheet_properties.cpp

@@ -45,10 +45,12 @@
 #include "wx/dcclient.h"
 
 DIALOG_SHEET_PROPERTIES::DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet,
-                                                  bool* aClearAnnotationNewItems ) :
+                                                  bool* aClearAnnotationNewItems,
+                                                  bool* aUpdateHierarchyNavigator ) :
     DIALOG_SHEET_PROPERTIES_BASE( aParent ),
     m_frame( aParent ),
     m_clearAnnotationNewItems( aClearAnnotationNewItems ),
+    m_updateHierarchyNavigator( aUpdateHierarchyNavigator ),
     m_borderWidth( aParent, m_borderWidthLabel, m_borderWidthCtrl, m_borderWidthUnits ),
     m_dummySheet( *aSheet ),
     m_dummySheetNameField( VECTOR2I( -1, -1 ), SHEETNAME, &m_dummySheet )
@@ -335,6 +337,10 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
 
             return false;
         }
+        else if( m_updateHierarchyNavigator )
+        {
+            *m_updateHierarchyNavigator = true;
+        }
 
         if( clearFileName )
             currentScreen->SetFileName( wxEmptyString );
@@ -345,6 +351,9 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataFromWindow()
 
     wxString newSheetname = m_fields->at( SHEETNAME ).GetText();
 
+    if( ( newSheetname != m_sheet->GetName() ) && m_updateHierarchyNavigator )
+        *m_updateHierarchyNavigator = true;
+
     if( newSheetname.IsEmpty() )
         newSheetname = _( "Untitled Sheet" );
 

eeschema/dialogs/dialog_sheet_properties.h

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2009 Wayne Stambaugh <stambaughw@verizon.net>
- * Copyright (C) 2014-2020 KiCad Developers, see AUTHORS.TXT for contributors.
+ * Copyright (C) 2014-2024 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
@@ -38,7 +38,8 @@ class DIALOG_SHEET_PROPERTIES : public DIALOG_SHEET_PROPERTIES_BASE
 {
 public:
     DIALOG_SHEET_PROPERTIES( SCH_EDIT_FRAME* aParent, SCH_SHEET* aSheet,
-                             bool* aClearAnnotationNewItems );
+                             bool* aClearAnnotationNewItems,
+                             bool* aUpdateHierarchyNavigator );
 
     ~DIALOG_SHEET_PROPERTIES() override;
 
@@ -66,6 +67,7 @@ private:
     SCH_EDIT_FRAME*               m_frame;
     SCH_SHEET*                    m_sheet;
     bool*                         m_clearAnnotationNewItems;
+    bool*                         m_updateHierarchyNavigator;
 
     wxSize                        m_size;
     int                           m_delayedFocusRow;

eeschema/sch_edit_frame.h

@@ -574,9 +574,12 @@ public:
      * this sheet need to have their annotation cleared i.e. when an existing item list is used.
      * it can happens when the edited sheet used an existing file, or becomes a new instance
      * of a already existing sheet.
+     * @param aUpdateHierarchyNavigator is an optional flag to indicate the sheet changes require
+     *                                  the hierarchy navigator panel to be updated.
      */
     bool EditSheetProperties( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
-                              bool* aClearAnnotationNewItems );
+                              bool* aClearAnnotationNewItems,
+                              bool* aUpdateHierarchyNavigator = nullptr );
 
     void InitSheet( SCH_SHEET* aSheet, const wxString& aNewFilename );
 

eeschema/schematic_undo_redo.cpp

@@ -269,6 +269,7 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
     std::vector<SCH_ITEM*> bulkRemovedItems;
     std::vector<SCH_ITEM*> bulkChangedItems;
     bool                   dirtyConnectivity = false;
+    bool                   rebuildHierarchyNavigator = false;
     SCH_CLEANUP_FLAGS      connectivityCleanUp = NO_CLEANUP;
 
     // Undo in the reverse order of list creation: (this can allow stacked changes like the
@@ -334,6 +335,8 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
             // If we are removing the current sheet, get out first
             if( eda_item->Type() == SCH_SHEET_T )
             {
+                rebuildHierarchyNavigator = true;
+
                 if( static_cast<SCH_SHEET*>( eda_item )->GetScreen() == GetScreen() )
                     GetToolManager()->PostAction( EE_ACTIONS::leaveSheet );
             }
@@ -345,6 +348,9 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
         }
         else if( status == UNDO_REDO::DELETED )
         {
+            if( eda_item->Type() == SCH_SHEET_T )
+                rebuildHierarchyNavigator = true;
+
             updateConnectivityFlag();
 
             // deleted items are re-inserted on undo
@@ -386,6 +392,20 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
             switch( status )
             {
             case UNDO_REDO::CHANGED:
+                if( schItem->Type() == SCH_SHEET_T )
+                {
+                    const SCH_SHEET* origSheet = static_cast<const SCH_SHEET*>( schItem );
+                    const SCH_SHEET* copySheet = static_cast<const SCH_SHEET*>( schItem );
+
+                    wxCHECK2( origSheet && copySheet, continue );
+
+                    if( ( origSheet->GetName() != copySheet->GetName() )
+                      || ( origSheet->GetFileName() != copySheet->GetFileName() ) )
+                    {
+                        rebuildHierarchyNavigator = true;
+                    }
+                }
+
                 schItem->SwapData( itemCopy );
                 bulkChangedItems.emplace_back( schItem );
 
@@ -450,6 +470,8 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
         if( connectivityCleanUp == GLOBAL_CLEANUP )
         {
             SetSheetNumberAndCount();
+
+            if( rebuildHierarchyNavigator )
                 UpdateHierarchyNavigator();
         }
     }

eeschema/sheet.cpp

@@ -2,7 +2,7 @@
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
  * Copyright (C) 2015 Jean-Pierre Charras, jp.charras at wanadoo.fr
- * Copyright (C) 2004-2022 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2004-2024 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
@@ -589,13 +589,15 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aCurr
 
 
 bool SCH_EDIT_FRAME::EditSheetProperties( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHierarchy,
-                                          bool* aClearAnnotationNewItems )
+                                          bool* aClearAnnotationNewItems,
+                                          bool* aUpdateHierarchyNavigator )
 {
     if( aSheet == nullptr || aHierarchy == nullptr )
         return false;
 
     // Get the new texts
-    DIALOG_SHEET_PROPERTIES dlg( this, aSheet, aClearAnnotationNewItems );
+    DIALOG_SHEET_PROPERTIES dlg( this, aSheet, aClearAnnotationNewItems,
+                                 aUpdateHierarchyNavigator );
 
     if( dlg.ShowModal() == wxID_CANCEL )
         return false;

eeschema/tools/sch_edit_tool.cpp

@@ -1834,6 +1834,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
         SCH_SHEET*     sheet = static_cast<SCH_SHEET*>( curr_item );
         bool           doClearAnnotation;
         bool           doRefresh = false;
+        bool           updateHierarchyNavigator = false;
 
         // Keep track of existing sheet paths. EditSheet() can modify this list.
         // Note that we use the validity checking/repairing version here just to make sure
@@ -1841,7 +1842,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
         SCH_SHEET_LIST originalHierarchy( &m_frame->Schematic().Root(), true );
 
         doRefresh = m_frame->EditSheetProperties( sheet, &m_frame->GetCurrentSheet(),
-                                                  &doClearAnnotation );
+                                                  &doClearAnnotation, &updateHierarchyNavigator );
 
         // If the sheet file is changed and new sheet contents are loaded then we have to
         // clear the annotations on the new content (as it may have been set from some other
@@ -1860,10 +1861,10 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent )
         }
 
         if( doRefresh )
-        {
             m_frame->GetCanvas()->Refresh();
+
+        if( updateHierarchyNavigator )
             m_frame->UpdateHierarchyNavigator();
-        }
 
         break;
     }

eeschema/tools/sch_editor_control.cpp

@@ -1256,16 +1256,6 @@ int SCH_EDITOR_CONTROL::Undo( const TOOL_EVENT& aEvent )
 
     m_frame->PutDataInPreviousState( undo_list );
 
-    // Only rebuild the hierarchy navigator if there are sheet changes.
-    // if( undo_list->ContainsItemType( SCH_SHEET_T ) )
-    // {
-    //     m_frame->SetSheetNumberAndCount();
-    //     m_frame->UpdateHierarchyNavigator();
-    // }
-
-    // m_frame->RecalculateConnections( nullptr, NO_CLEANUP );
-    // m_frame->TestDanglingEnds();
-
     // Now push the old command to the RedoList
     undo_list->ReversePickersListOrder();
     m_frame->PushCommandToRedoList( undo_list );
@@ -1301,16 +1291,6 @@ int SCH_EDITOR_CONTROL::Redo( const TOOL_EVENT& aEvent )
     list->ReversePickersListOrder();
     m_frame->PushCommandToUndoList( list );
 
-    // Only rebuild the hierarchy navigator if there are sheet changes.
-    // if( list->ContainsItemType( SCH_SHEET_T ) )
-    // {
-    //     m_frame->SetSheetNumberAndCount();
-    //     m_frame->UpdateHierarchyNavigator();
-    // }
-
-    // m_frame->RecalculateConnections( nullptr, NO_CLEANUP );
-    // m_frame->TestDanglingEnds();
-
     m_toolMgr->GetTool<EE_SELECTION_TOOL>()->RebuildSelection();
 
     m_frame->GetCanvas()->Refresh();

eeschema/widgets/hierarchy_pane.cpp

@@ -224,6 +224,31 @@ void HIERARCHY_PANE::UpdateHierarchyTree()
         m_events_bound = false;
     }
 
+    std::set<SCH_SHEET_PATH> expandedNodes;
+
+    std::function<void( const wxTreeItemId& )> getExpandedNodes =
+            [&]( const wxTreeItemId& id )
+            {
+                wxCHECK_RET( id.IsOk(), wxT( "Invalid tree item" ) );
+
+                TREE_ITEM_DATA* itemData = static_cast<TREE_ITEM_DATA*>( m_tree->GetItemData( id ) );
+
+                if( m_tree->IsExpanded( id ) )
+                    expandedNodes.emplace( itemData->m_SheetPath );
+
+                wxTreeItemIdValue cookie;
+                wxTreeItemId      child = m_tree->GetFirstChild( id, cookie );
+
+                while( child.IsOk() )
+                {
+                    getExpandedNodes( child );
+                    child = m_tree->GetNextChild( id, cookie );
+                }
+            };
+
+    if( !m_tree->IsEmpty() )
+        getExpandedNodes( m_tree->GetRootItem() );
+
     m_list.clear();
     m_list.push_back( &m_frame->Schematic().Root() );
 
@@ -235,8 +260,35 @@ void HIERARCHY_PANE::UpdateHierarchyTree()
     buildHierarchyTree( &m_list, root );
     UpdateHierarchySelection();
 
-    if( m_tree->ItemHasChildren( root ) )
+    if( !expandedNodes.empty() )
+    {
+        std::function<void( const wxTreeItemId& )> expandNodes =
+                [&]( const wxTreeItemId& id )
+                {
+                    wxCHECK_RET( id.IsOk(), wxT( "Invalid tree item" ) );
+
+                    TREE_ITEM_DATA* itemData =
+                            static_cast<TREE_ITEM_DATA*>( m_tree->GetItemData( id ) );
+
+                    if( expandedNodes.find( itemData->m_SheetPath ) != expandedNodes.end() )
+                        m_tree->Expand( id );
+
+                    wxTreeItemIdValue cookie;
+                    wxTreeItemId      child = m_tree->GetFirstChild( id, cookie );
+
+                    while( child.IsOk() )
+                    {
+                        expandNodes( child );
+                        child = m_tree->GetNextChild( id, cookie );
+                    }
+                };
+
+        expandNodes( m_tree->GetRootItem() );
+    }
+    else if( m_tree->ItemHasChildren( root ) )
+    {
         m_tree->Expand( root );
+    }
 
     if( eventsWereBound )
     {
@@ -282,8 +334,10 @@ void HIERARCHY_PANE::UpdateLabelsHierarchyTree()
             {
                 TREE_ITEM_DATA* itemData = static_cast<TREE_ITEM_DATA*>( m_tree->GetItemData( id ) );
                 SCH_SHEET* sheet = itemData->m_SheetPath.Last();
-                wxString sheetNameLabel = formatPageString( sheet->GetFields()[SHEETNAME].GetShownText( false ),
+                wxString sheetNameLabel =
+                        formatPageString( sheet->GetFields()[SHEETNAME].GetShownText( false ),
                                           itemData->m_SheetPath.GetPageNumber() );
+
                 if( m_tree->GetItemText( id ) != sheetNameLabel )
                     m_tree->SetItemText( id, sheetNameLabel );
             };