From 63a0f537d8538d0a3a11b1ad5b98a2a733b675c3 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 5 Oct 2020 13:24:21 +0100 Subject: [PATCH] Performance enhancement: check hierarchy validity only when necessary. --- eeschema/cross-probing.cpp | 2 +- eeschema/dialogs/dialog_sch_sheet_props.cpp | 3 ++ eeschema/plotters/plot_schematic_DXF.cpp | 2 +- eeschema/plotters/plot_schematic_HPGL.cpp | 2 +- eeschema/plotters/plot_schematic_PDF.cpp | 2 +- eeschema/plotters/plot_schematic_PS.cpp | 2 +- eeschema/plotters/plot_schematic_SVG.cpp | 2 +- eeschema/sch_sexpr_plugin.cpp | 2 +- eeschema/sch_sheet_path.cpp | 32 ++++++++++++++------- eeschema/sch_sheet_path.h | 4 +-- eeschema/tools/sch_edit_tool.cpp | 11 ++++--- 11 files changed, 40 insertions(+), 24 deletions(-) diff --git a/eeschema/cross-probing.cpp b/eeschema/cross-probing.cpp index baf15ec56c..7dcae8dff6 100644 --- a/eeschema/cross-probing.cpp +++ b/eeschema/cross-probing.cpp @@ -54,7 +54,7 @@ SCH_ITEM* SCH_EDITOR_CONTROL::FindComponentAndItem( const wxString& aReference, if( !aSearchHierarchy ) sheetList.push_back( m_frame->GetCurrentSheet() ); else - sheetList.BuildSheetList( &m_frame->Schematic().Root() ); + sheetList = m_frame->Schematic().GetSheets(); for( SCH_SHEET_PATH& sheet : sheetList ) { diff --git a/eeschema/dialogs/dialog_sch_sheet_props.cpp b/eeschema/dialogs/dialog_sch_sheet_props.cpp index 5321f90512..5b0296f742 100644 --- a/eeschema/dialogs/dialog_sch_sheet_props.cpp +++ b/eeschema/dialogs/dialog_sch_sheet_props.cpp @@ -270,6 +270,9 @@ bool DIALOG_SCH_SHEET_PROPS::TransferDataFromWindow() { if( !onSheetFilenameChanged( newRelativeFilename ) ) return false; + + // One last validity check (and potential repair) just to be sure to be sure + SCH_SHEET_LIST repairedList( &m_frame->Schematic().Root(), true ); } wxString newSheetname = m_fields->at( SHEETNAME ).GetText(); diff --git a/eeschema/plotters/plot_schematic_DXF.cpp b/eeschema/plotters/plot_schematic_DXF.cpp index ac962891cc..9bc878d098 100644 --- a/eeschema/plotters/plot_schematic_DXF.cpp +++ b/eeschema/plotters/plot_schematic_DXF.cpp @@ -53,7 +53,7 @@ void DIALOG_PLOT_SCHEMATIC::CreateDXFFile( bool aPlotAll, bool aPlotFrameRef, SCH_SHEET_LIST sheetList; if( aPlotAll ) - sheetList.BuildSheetList( &schframe->Schematic().Root() ); + sheetList.BuildSheetList( &schframe->Schematic().Root(), true ); else sheetList.push_back( schframe->GetCurrentSheet() ); diff --git a/eeschema/plotters/plot_schematic_HPGL.cpp b/eeschema/plotters/plot_schematic_HPGL.cpp index 7d15faa925..40b79b9999 100644 --- a/eeschema/plotters/plot_schematic_HPGL.cpp +++ b/eeschema/plotters/plot_schematic_HPGL.cpp @@ -101,7 +101,7 @@ void DIALOG_PLOT_SCHEMATIC::createHPGLFile( bool aPlotAll, bool aPlotFrameRef, SCH_SHEET_LIST sheetList; if( aPlotAll ) - sheetList.BuildSheetList( &m_parent->Schematic().Root() ); + sheetList.BuildSheetList( &m_parent->Schematic().Root(), true ); else sheetList.push_back( m_parent->GetCurrentSheet() ); diff --git a/eeschema/plotters/plot_schematic_PDF.cpp b/eeschema/plotters/plot_schematic_PDF.cpp index f2772af7b7..559340a723 100644 --- a/eeschema/plotters/plot_schematic_PDF.cpp +++ b/eeschema/plotters/plot_schematic_PDF.cpp @@ -56,7 +56,7 @@ void DIALOG_PLOT_SCHEMATIC::createPDFFile( bool aPlotAll, bool aPlotFrameRef, SCH_SHEET_LIST sheetList; if( aPlotAll ) - sheetList.BuildSheetList( &m_parent->Schematic().Root() ); + sheetList.BuildSheetList( &m_parent->Schematic().Root(), true ); else sheetList.push_back( m_parent->GetCurrentSheet() ); diff --git a/eeschema/plotters/plot_schematic_PS.cpp b/eeschema/plotters/plot_schematic_PS.cpp index 23b1f918df..07fcecf4c2 100644 --- a/eeschema/plotters/plot_schematic_PS.cpp +++ b/eeschema/plotters/plot_schematic_PS.cpp @@ -52,7 +52,7 @@ void DIALOG_PLOT_SCHEMATIC::createPSFile( bool aPlotAll, bool aPlotFrameRef, SCH_SHEET_LIST sheetList; if( aPlotAll ) - sheetList.BuildSheetList( &m_parent->Schematic().Root() ); + sheetList.BuildSheetList( &m_parent->Schematic().Root(), true ); else sheetList.push_back( m_parent->GetCurrentSheet() ); diff --git a/eeschema/plotters/plot_schematic_SVG.cpp b/eeschema/plotters/plot_schematic_SVG.cpp index 1a934b3ef2..c84bd12a0d 100644 --- a/eeschema/plotters/plot_schematic_SVG.cpp +++ b/eeschema/plotters/plot_schematic_SVG.cpp @@ -52,7 +52,7 @@ void DIALOG_PLOT_SCHEMATIC::createSVGFile( bool aPrintAll, bool aPrintFrameRef, SCH_SHEET_LIST sheetList; if( aPrintAll ) - sheetList.BuildSheetList( &m_parent->Schematic().Root() ); + sheetList.BuildSheetList( &m_parent->Schematic().Root(), true ); else sheetList.push_back( m_parent->GetCurrentSheet() ); diff --git a/eeschema/sch_sexpr_plugin.cpp b/eeschema/sch_sexpr_plugin.cpp index 03a8f12cf4..b973b73148 100644 --- a/eeschema/sch_sexpr_plugin.cpp +++ b/eeschema/sch_sexpr_plugin.cpp @@ -700,7 +700,7 @@ void SCH_SEXPR_PLUGIN::Format( SCH_SHEET* aSheet ) m_out->Print( 0, "\n" ); m_out->Print( 1, "(symbol_instances\n" ); - SCH_SHEET_LIST sheetPaths( aSheet ); + SCH_SHEET_LIST sheetPaths( aSheet, true ); for( const SCH_SHEET_PATH& sheetPath : sheetPaths ) { diff --git a/eeschema/sch_sheet_path.cpp b/eeschema/sch_sheet_path.cpp index a75bd1ecf9..4c2a9ed36a 100644 --- a/eeschema/sch_sheet_path.cpp +++ b/eeschema/sch_sheet_path.cpp @@ -396,14 +396,14 @@ bool SCH_SHEET_PATH::TestForRecursion( const wxString& aSrcFileName, const wxStr } -SCH_SHEET_LIST::SCH_SHEET_LIST( SCH_SHEET* aSheet ) +SCH_SHEET_LIST::SCH_SHEET_LIST( SCH_SHEET* aSheet, bool aCheckIntegrity ) { if( aSheet != NULL ) - BuildSheetList( aSheet ); + BuildSheetList( aSheet, aCheckIntegrity ); } -void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet ) +void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet, bool aCheckIntegrity ) { wxCHECK_RET( aSheet != NULL, wxT( "Cannot build sheet list from undefined sheet." ) ); @@ -421,25 +421,35 @@ void SCH_SHEET_LIST::BuildSheetList( SCH_SHEET* aSheet ) if( m_currentSheetPath.LastScreen() ) { + wxString parentFileName = aSheet->GetFileName(); std::vector childSheets; m_currentSheetPath.LastScreen()->GetSheets( &childSheets ); for( SCH_ITEM* item : childSheets ) { - auto sheet = static_cast( item ); + SCH_SHEET* sheet = static_cast( item ); - if( !m_currentSheetPath.TestForRecursion( - sheet->GetFileName(), aSheet->GetFileName() ) ) - BuildSheetList( sheet ); + if( aCheckIntegrity ) + { + if( !m_currentSheetPath.TestForRecursion( sheet->GetFileName(), parentFileName ) ) + BuildSheetList( sheet, true ); + else + badSheets.push_back( sheet ); + } else - badSheets.push_back( sheet ); + { + BuildSheetList( sheet, false ); + } } } - for( auto sheet : badSheets ) + if( aCheckIntegrity ) { - m_currentSheetPath.LastScreen()->Remove( sheet ); - m_currentSheetPath.LastScreen()->SetModify(); + for( SCH_SHEET* sheet : badSheets ) + { + m_currentSheetPath.LastScreen()->Remove( sheet ); + m_currentSheetPath.LastScreen()->SetModify(); + } } m_currentSheetPath.pop_back(); diff --git a/eeschema/sch_sheet_path.h b/eeschema/sch_sheet_path.h index 96d6199964..b4575aed92 100644 --- a/eeschema/sch_sheet_path.h +++ b/eeschema/sch_sheet_path.h @@ -343,7 +343,7 @@ public: * * If aSheet == NULL, then this is an empty hierarchy which the user can populate. */ - SCH_SHEET_LIST( SCH_SHEET* aSheet = NULL ); + SCH_SHEET_LIST( SCH_SHEET* aSheet = nullptr, bool aCheckIntegrity = false ); ~SCH_SHEET_LIST() {} @@ -431,7 +431,7 @@ public: * indicating that g_RootSheet should be used. * @throw std::bad_alloc if the memory for the sheet path list could not be allocated. */ - void BuildSheetList( SCH_SHEET* aSheet ); + void BuildSheetList( SCH_SHEET* aSheet, bool aCheckIntegrity ); bool NameExists( const wxString& aSheetName ); diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index d202534b14..21a93a3844 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1503,11 +1503,14 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) SCH_SHEET* sheet = static_cast( item ); bool doClearAnnotation; bool doRefresh = false; - // Keep track of existing sheet paths. EditSheet() can modify this list - SCH_SHEET_LIST initial_sheetpathList = m_frame->Schematic().GetSheets(); - doRefresh = m_frame->EditSheetProperties( - sheet, &m_frame->GetCurrentSheet(), &doClearAnnotation ); + // 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 + // we've got a valid hierarchy to begin with. + SCH_SHEET_LIST initial_sheetpathList( &m_frame->Schematic().Root(), true ); + + doRefresh = m_frame->EditSheetProperties( sheet, &m_frame->GetCurrentSheet(), + &doClearAnnotation ); // 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