From fd0d5fdb382db9f552fde90fc993d5afc245143c Mon Sep 17 00:00:00 2001 From: jean-pierre charras Date: Wed, 8 Jan 2020 15:01:22 +0100 Subject: [PATCH] HIERARCHY_NAVIG_DLG: fix crash when in a subsheet one open the dialog and switch to the paren sheet from the context menu. Mainly, change the way the dialog is managed from the sch_edit_frame: the pointer to find the created dialog is replaced by a call to wxWidows::FindWindowByName(). Using a pointer set and cleared by events is a recipe for crashes. --- eeschema/hierarch.cpp | 17 ++++++++++++++--- eeschema/hierarch.h | 7 ++++++- eeschema/sch_edit_frame.cpp | 36 ++++++++++++++++++------------------ eeschema/sch_edit_frame.h | 15 +++++++-------- 4 files changed, 45 insertions(+), 30 deletions(-) diff --git a/eeschema/hierarch.cpp b/eeschema/hierarch.cpp index 1e84e22eae..a7746e5aee 100644 --- a/eeschema/hierarch.cpp +++ b/eeschema/hierarch.cpp @@ -80,7 +80,7 @@ HIERARCHY_TREE::HIERARCHY_TREE( HIERARCHY_NAVIG_DLG* parent ) : HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) : DIALOG_SHIM( aParent, wxID_ANY, _( "Navigator" ), wxDefaultPosition, wxDefaultSize, - wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER ) + wxDEFAULT_DIALOG_STYLE | wxRESIZE_BORDER, HIERARCHY_NAVIG_DLG_WNAME ) { wxASSERT( dynamic_cast< SCH_EDIT_FRAME* >( aParent ) ); @@ -130,6 +130,9 @@ HIERARCHY_NAVIG_DLG::HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ) : Bind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); // Manage a simple click on a selection, if the selection changes Bind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); + + // Connect close event for the dialog: + this->Connect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnCloseNav ) ); } @@ -138,6 +141,7 @@ HIERARCHY_NAVIG_DLG::~HIERARCHY_NAVIG_DLG() Unbind( wxEVT_TREE_ITEM_ACTIVATED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); Unbind( wxEVT_TREE_SEL_CHANGED, &HIERARCHY_NAVIG_DLG::onSelectSheetPath, this ); m_Tree->Disconnect( wxEVT_CHAR, wxKeyEventHandler( HIERARCHY_TREE::onChar ) ); + this->Disconnect( wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnCloseNav ) ); } @@ -184,12 +188,16 @@ void HIERARCHY_NAVIG_DLG::buildHierarchyTree( SCH_SHEET_PATH* aList, wxTreeItemI void HIERARCHY_NAVIG_DLG::UpdateHierarchyTree() { + Freeze(); + m_currSheet = m_SchFrameEditor->GetCurrentSheet(); wxTreeItemId root = m_Tree->GetRootItem(); m_Tree->DeleteChildren( root ); m_list.clear(); m_list.push_back( g_RootSheet ); buildHierarchyTree( &m_list, &root ); + + Thaw(); } void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event ) @@ -200,15 +208,18 @@ void HIERARCHY_NAVIG_DLG::onSelectSheetPath( wxTreeEvent& event ) wxTreeItemId ItemSel = m_Tree->GetSelection(); m_SchFrameEditor->SetCurrentSheet(( (TreeItemData*) m_Tree->GetItemData( ItemSel ) )->m_SheetPath ); m_SchFrameEditor->DisplayCurrentSheet(); + if( m_SchFrameEditor->GetNavigatorStaysOpen() == false ) Close( true ); } -void HIERARCHY_NAVIG_DLG::OnClose( wxCloseEvent& event ) + +void HIERARCHY_NAVIG_DLG::OnCloseNav( wxCloseEvent& event ) { - m_SchFrameEditor->CloseHierarchyNavigator(); + Destroy(); } + void SCH_EDIT_FRAME::DisplayCurrentSheet() { m_toolManager->RunAction( ACTIONS::cancelInteractive, true ); diff --git a/eeschema/hierarch.h b/eeschema/hierarch.h index dcf751e0a9..d027d2e5a5 100644 --- a/eeschema/hierarch.h +++ b/eeschema/hierarch.h @@ -31,6 +31,9 @@ #include #include +// The window name of the hierarchy navigator, used to find it +#define HIERARCHY_NAVIG_DLG_WNAME "hierarchy_navig_dlg" + class SCH_EDIT_FRAME; class SCH_SHEET_PATH; @@ -65,7 +68,9 @@ public: HIERARCHY_NAVIG_DLG( SCH_EDIT_FRAME* aParent ); ~HIERARCHY_NAVIG_DLG(); - void OnClose( wxCloseEvent& event ); + + void OnCloseNav( wxCloseEvent& event ); + /** * Update the hierarchical tree of the schematic. */ diff --git a/eeschema/sch_edit_frame.cpp b/eeschema/sch_edit_frame.cpp index 7657b67d4e..1fe09b77ed 100644 --- a/eeschema/sch_edit_frame.cpp +++ b/eeschema/sch_edit_frame.cpp @@ -248,7 +248,6 @@ SCH_EDIT_FRAME::SCH_EDIT_FRAME( KIWAY* aKiway, wxWindow* aParent ): m_findReplaceDialog = nullptr; m_findReplaceStatusPopup = nullptr; - m_hierarchyDialog = nullptr; SetForceHVLines( true ); SetSpiceAdjustPassiveValues( false ); @@ -562,10 +561,9 @@ void SCH_EDIT_FRAME::OnCloseWindow( wxCloseEvent& aEvent ) m_findReplaceDialog = nullptr; } - if( m_hierarchyDialog ) + if( FindHierarchyNavigator() ) { - m_hierarchyDialog->Destroy(); - m_hierarchyDialog = nullptr; + FindHierarchyNavigator()->Close( true ); } SCH_SCREENS screens; @@ -676,26 +674,33 @@ wxFindReplaceData* SCH_EDIT_FRAME::GetFindReplaceData() return nullptr; } + +HIERARCHY_NAVIG_DLG* SCH_EDIT_FRAME::FindHierarchyNavigator() +{ + wxWindow* navigator = wxWindow::FindWindowByName( HIERARCHY_NAVIG_DLG_WNAME ); + + return static_cast< HIERARCHY_NAVIG_DLG* >( navigator ); +} + void SCH_EDIT_FRAME::UpdateHierarchyNavigator( bool aForceUpdate ) { - if( aForceUpdate ) { - if( m_hierarchyDialog ) - CloseHierarchyNavigator(); + if( FindHierarchyNavigator() ) + FindHierarchyNavigator()->Close(); - m_hierarchyDialog = new HIERARCHY_NAVIG_DLG( this ); - m_hierarchyDialog->Connect( - wxEVT_CLOSE_WINDOW, wxCloseEventHandler( HIERARCHY_NAVIG_DLG::OnClose ) ); - m_hierarchyDialog->Show( true ); + HIERARCHY_NAVIG_DLG* hierarchyDialog = new HIERARCHY_NAVIG_DLG( this ); + + hierarchyDialog->Show( true ); } else { - if( m_hierarchyDialog ) - m_hierarchyDialog->UpdateHierarchyTree(); + if( FindHierarchyNavigator() ) + FindHierarchyNavigator()->UpdateHierarchyTree(); } } + void SCH_EDIT_FRAME::ShowFindReplaceDialog( bool aReplace ) { if( m_findReplaceStatusPopup ) @@ -748,11 +753,6 @@ void SCH_EDIT_FRAME::OnFindDialogClose() m_findReplaceDialog = nullptr; } -void SCH_EDIT_FRAME::CloseHierarchyNavigator() -{ - m_hierarchyDialog->Destroy(); - m_hierarchyDialog = nullptr; -} void SCH_EDIT_FRAME::OnLoadFile( wxCommandEvent& event ) { diff --git a/eeschema/sch_edit_frame.h b/eeschema/sch_edit_frame.h index 5f31ef314d..741f6bfabe 100644 --- a/eeschema/sch_edit_frame.h +++ b/eeschema/sch_edit_frame.h @@ -139,7 +139,6 @@ private: DIALOG_SCH_FIND* m_findReplaceDialog; STATUS_TEXT_POPUP* m_findReplaceStatusPopup; - HIERARCHY_NAVIG_DLG* m_hierarchyDialog; /// Flag to indicate show hidden pins. bool m_showAllPins; @@ -357,11 +356,16 @@ public: /** * Run the Hierarchy Navigator dialog. - * @param aForceUpdate When true, creates a new dialog. And if a dialog - * already exist, it destroys it first. + * @param aForceUpdate: When true, creates a new dialog. And if a dialog + * already exist, it destroys it first. */ void UpdateHierarchyNavigator( bool aForceUpdate = false ); + /** + * @return a reference to the Hierarchy Navigator dialog if exists, or nullptr. + */ + HIERARCHY_NAVIG_DLG* FindHierarchyNavigator(); + void ShowFindReplaceStatus( const wxString& aMsg ); void ClearFindReplaceStatus(); @@ -375,11 +379,6 @@ public: */ void OnFindDialogClose(); - /** - * Destroy the Hierarchy Navigator dialog. - */ - void CloseHierarchyNavigator(); - /** * Breaks a single segment into two at the specified point. *