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
This commit is contained in:
Wayne Stambaugh 2024-03-17 09:02:07 -04:00
parent d317b8da74
commit 797ab998cc
8 changed files with 107 additions and 34 deletions

View File

@ -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" );

View File

@ -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;

View File

@ -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 );

View File

@ -271,6 +271,7 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
std::vector<SCH_ITEM*> bulkChangedItems;
std::set<SCH_TABLE*> 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<SCH_SHEET*>( 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<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 );
@ -452,7 +472,9 @@ void SCH_EDIT_FRAME::PutDataInPreviousState( PICKED_ITEMS_LIST* aList )
if( connectivityCleanUp == GLOBAL_CLEANUP )
{
SetSheetNumberAndCount();
UpdateHierarchyNavigator();
if( rebuildHierarchyNavigator )
UpdateHierarchyNavigator();
}
}
}

View File

@ -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;

View File

@ -1865,6 +1865,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
@ -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;
}

View File

@ -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<EE_SELECTION_TOOL>()->RebuildSelection();
m_frame->GetCanvas()->Refresh();

View File

@ -228,6 +228,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() );
@ -239,8 +264,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 )
{
@ -286,8 +338,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 ),
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 );
};