From 797ab998cc59d7a41b32f5f6fa3db68b993b51c3 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Sun, 17 Mar 2024 09:02:07 -0400 Subject: [PATCH] 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 --- eeschema/dialogs/dialog_sheet_properties.cpp | 11 +++- eeschema/dialogs/dialog_sheet_properties.h | 6 +- eeschema/sch_edit_frame.h | 5 +- eeschema/schematic_undo_redo.cpp | 24 +++++++- eeschema/sheet.cpp | 8 ++- eeschema/tools/sch_edit_tool.cpp | 7 ++- eeschema/tools/sch_editor_control.cpp | 20 ------- eeschema/widgets/hierarchy_pane.cpp | 60 +++++++++++++++++++- 8 files changed, 107 insertions(+), 34 deletions(-) diff --git a/eeschema/dialogs/dialog_sheet_properties.cpp b/eeschema/dialogs/dialog_sheet_properties.cpp index 03027a84e2..0caa302f35 100644 --- a/eeschema/dialogs/dialog_sheet_properties.cpp +++ b/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" ); diff --git a/eeschema/dialogs/dialog_sheet_properties.h b/eeschema/dialogs/dialog_sheet_properties.h index 4770b24403..192c5a3259 100644 --- a/eeschema/dialogs/dialog_sheet_properties.h +++ b/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 - * 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; diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index bc84a65935..5217b56413 100644 --- a/eeschema/sch_edit_frame.h +++ b/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 ); diff --git a/eeschema/schematic_undo_redo.cpp b/eeschema/schematic_undo_redo.cpp index 55159e9ca6..71c3d779fb 100644 --- a/eeschema/schematic_undo_redo.cpp +++ b/eeschema/schematic_undo_redo.cpp @@ -271,6 +271,7 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList ) std::vector bulkChangedItems; std::set changedTables; 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 @@ -336,6 +337,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( eda_item )->GetScreen() == GetScreen() ) GetToolManager()->PostAction( EE_ACTIONS::leaveSheet ); } @@ -347,6 +350,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 @@ -388,6 +394,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( schItem ); + const SCH_SHEET* copySheet = static_cast( schItem ); + + wxCHECK2( origSheet && copySheet, continue ); + + if( ( origSheet->GetName() != copySheet->GetName() ) + || ( origSheet->GetFileName() != copySheet->GetFileName() ) ) + { + rebuildHierarchyNavigator = true; + } + } + schItem->SwapData( itemCopy ); bulkChangedItems.emplace_back( schItem ); @@ -452,7 +472,9 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList ) if( connectivityCleanUp == GLOBAL_CLEANUP ) { SetSheetNumberAndCount(); - UpdateHierarchyNavigator(); + + if( rebuildHierarchyNavigator ) + UpdateHierarchyNavigator(); } } } diff --git a/eeschema/sheet.cpp b/eeschema/sheet.cpp index b129df68a0..e11bd1da56 100644 --- a/eeschema/sheet.cpp +++ b/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; diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 591b7142ec..5a0d957cd4 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1865,6 +1865,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) SCH_SHEET* sheet = static_cast( 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 @@ -1872,7 +1873,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 @@ -1891,10 +1892,10 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) } if( doRefresh ) - { m_frame->GetCanvas()->Refresh(); + + if( updateHierarchyNavigator ) m_frame->UpdateHierarchyNavigator(); - } break; } diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index cc26d5bf85..1284344346 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -1257,16 +1257,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 ); @@ -1302,16 +1292,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()->RebuildSelection(); m_frame->GetCanvas()->Refresh(); diff --git a/eeschema/widgets/hierarchy_pane.cpp b/eeschema/widgets/hierarchy_pane.cpp index 87c63ff246..76cb1803d8 100644 --- a/eeschema/widgets/hierarchy_pane.cpp +++ b/eeschema/widgets/hierarchy_pane.cpp @@ -228,6 +228,31 @@ void HIERARCHY_PANE::UpdateHierarchyTree() m_events_bound = false; } + std::set expandedNodes; + + std::function getExpandedNodes = + [&]( const wxTreeItemId& id ) + { + wxCHECK_RET( id.IsOk(), wxT( "Invalid tree item" ) ); + + TREE_ITEM_DATA* itemData = static_cast( 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() ); @@ -239,8 +264,35 @@ void HIERARCHY_PANE::UpdateHierarchyTree() buildHierarchyTree( &m_list, root ); UpdateHierarchySelection(); - if( m_tree->ItemHasChildren( root ) ) + if( !expandedNodes.empty() ) + { + std::function expandNodes = + [&]( const wxTreeItemId& id ) + { + wxCHECK_RET( id.IsOk(), wxT( "Invalid tree item" ) ); + + TREE_ITEM_DATA* itemData = + static_cast( 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 ) { @@ -286,8 +338,10 @@ void HIERARCHY_PANE::UpdateLabelsHierarchyTree() { TREE_ITEM_DATA* itemData = static_cast( m_tree->GetItemData( id ) ); SCH_SHEET* sheet = itemData->m_SheetPath.Last(); - wxString sheetNameLabel = formatPageString( sheet->GetFields()[SHEETNAME].GetShownText( false ), - itemData->m_SheetPath.GetPageNumber() ); + wxString sheetNameLabel = + formatPageString( sheet->GetFields()[SHEETNAME].GetShownText( false ), + itemData->m_SheetPath.GetPageNumber() ); + if( m_tree->GetItemText( id ) != sheetNameLabel ) m_tree->SetItemText( id, sheetNameLabel ); };