diff --git a/eeschema/files-io.cpp b/eeschema/files-io.cpp index 44d3e02d02..b199c628a3 100644 --- a/eeschema/files-io.cpp +++ b/eeschema/files-io.cpp @@ -437,9 +437,6 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in // to the s-expression format. schematic.ReplaceDuplicateTimeStamps(); - schematic.PruneOrphanedSymbolInstances( Prj().GetProjectName(), - Schematic().GetSheets() ); - // Allow the schematic to be saved to new file format without making any edits. OnModify(); } @@ -468,6 +465,8 @@ bool SCH_EDIT_FRAME::OpenProjectFiles( const std::vector& aFileSet, in screen->MigrateSimModels(); } + schematic.PruneOrphanedSymbolInstances( Prj().GetProjectName(), Schematic().GetSheets() ); + Schematic().ConnectionGraph()->Reset(); SetScreen( GetCurrentSheet().LastScreen() ); diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp index 9237e5fd1d..c4579b1331 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp +++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp @@ -785,6 +785,9 @@ void SCH_SEXPR_PLUGIN::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSchema for( size_t i = 0; i < aSymbol->GetInstanceReferences().size(); i++ ) { + // Zero length KIID_PATH objects are not valid and will cause a crash below. + wxCHECK2( aSymbol->GetInstanceReferences()[i].m_Path.size(), continue ); + // If the instance data is part of this design but no longer has an associated sheet // path, don't save it. This prevents large amounts of orphaned instance data for the // current project from accumulating in the schematic files. diff --git a/eeschema/sch_screen.cpp b/eeschema/sch_screen.cpp index e7a5b4a0bc..02e5b39325 100644 --- a/eeschema/sch_screen.cpp +++ b/eeschema/sch_screen.cpp @@ -1630,6 +1630,10 @@ size_t SCH_SCREEN::getLibSymbolNameMatches( const SCH_SYMBOL& aSymbol, void SCH_SCREEN::PruneOrphanedSymbolInstances( const wxString& aProjectName, const SCH_SHEET_LIST& aValidSheetPaths ) { + // The project name cannot be empty. Projects older than 7.0 did not save project names + // when saving instance data. Running this algorithm with an empty project name would + // clobber all instance data for projects other than the current one when a schematic + // file is shared across multiple projects. wxCHECK( !aProjectName.IsEmpty(), /* void */ ); for( SCH_ITEM* item : Items().OfType( SCH_SYMBOL_T ) ) @@ -1638,38 +1642,30 @@ void SCH_SCREEN::PruneOrphanedSymbolInstances( const wxString& aProjectName, wxCHECK2( symbol, continue ); - std::set pathsToPrune; + std::set pathsToPrune; const std::vector instances = symbol->GetInstanceReferences(); for( const SCH_SYMBOL_INSTANCE& instance : instances ) { + // Ignore instance paths from other projects. if( aProjectName != instance.m_ProjectName ) continue; std::optional pathFound = aValidSheetPaths.GetSheetPathByKIIDPath( instance.m_Path ); - // Check for paths from other projects. + // Check for paths that do not exist in the current project and paths that do + // not contain the current symbol. if( !pathFound ) - { - pathsToPrune.emplace( pathFound.value() ); - continue; - } - - // Check for paths that are in that are not valid for this screen. - for( const SCH_SHEET_PATH& path : aValidSheetPaths ) - { - if( path.LastScreen() == this ) - continue; - - pathsToPrune.emplace( path ); - } + pathsToPrune.emplace( instance.m_Path ); + else if( pathFound.value().LastScreen() != this ) + pathsToPrune.emplace( pathFound.value().Path() ); } - for( const SCH_SHEET_PATH& sheetPath : pathsToPrune ) + for( const KIID_PATH& sheetPath : pathsToPrune ) { wxLogTrace( traceSchSheetPaths, wxS( "Pruning project '%s' symbol instance %s." ), - aProjectName, sheetPath.Path().AsString() ); + aProjectName, sheetPath.AsString() ); symbol->RemoveInstance( sheetPath ); } } diff --git a/eeschema/sch_screen.h b/eeschema/sch_screen.h index 2c45cfc6a1..ce21f029fd 100644 --- a/eeschema/sch_screen.h +++ b/eeschema/sch_screen.h @@ -545,6 +545,18 @@ public: */ void MigrateSimModels(); + /** + * Remove all invalid symbol instance data in this screen object for the project defined + * by \a aProjectName and the list of \a aValidSheetPaths. + * + * @warning This method will assert and exit on debug builds when \a aProjectName is empty. + * + * @note This method does not affect instance data for any other projects. + * + * @param aProjectName is the name of the current project. + * @param aValidSheetPaths is the list of valid #SCH_SHEET_PATH objects for the current + * project. + */ void PruneOrphanedSymbolInstances( const wxString& aProjectName, const SCH_SHEET_LIST& aValidSheetPaths ); diff --git a/eeschema/sch_symbol.cpp b/eeschema/sch_symbol.cpp index 8249cae8b8..fbeec12ade 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -641,7 +641,8 @@ void SCH_SYMBOL::RemoveInstance( const KIID_PATH& aInstancePath ) void SCH_SYMBOL::SortInstances( bool (*aSortFunction)( const SCH_SYMBOL_INSTANCE& aLhs, const SCH_SYMBOL_INSTANCE& aRhs ) ) { - std::sort( m_instanceReferences.begin(), m_instanceReferences.end(), aSortFunction ); + if( m_instanceReferences.size() > 1 ) + std::sort( m_instanceReferences.begin(), m_instanceReferences.end(), aSortFunction ); } diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index f56a4e05ae..2624dbcc53 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -1936,6 +1936,14 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) m_frame->GetCurrentSheet().UpdateAllScreenReferences(); + // The copy operation creates instance paths that are not valid for the current project or + // saved as part of another project. Prune them now so they do not accumulate in the saved + // schematic file. + SCH_SCREENS allScreens( m_frame->Schematic().Root() ); + + allScreens.PruneOrphanedSymbolInstances( m_frame->Prj().GetProjectName(), + m_frame->Schematic().GetSheets() ); + // Now clear the previous selection, select the pasted items, and fire up the "move" tool. m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true ); m_toolMgr->RunAction( EE_ACTIONS::addItemsToSel, true, &loadedItems );