From 98dc21360a9a304965c1f4578f525f90e8d49aca Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Mon, 11 Dec 2023 08:29:10 -0500 Subject: [PATCH] Fix crash on save after pasting symbols in schematic editor. This was caused by pasting zero length instance paths which are not valid. Fixes https://gitlab.com/kicad/code/kicad/-/issues/16300 (cherry picked from commit 484491aaea47eaee293cbe90b9dfb588fe2f6941) --- eeschema/tools/sch_editor_control.cpp | 62 +++++++++++++++++++-------- eeschema/tools/sch_editor_control.h | 12 ++++++ include/kiid.h | 16 ++++++- 3 files changed, 70 insertions(+), 20 deletions(-) diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index 166f6fb0d2..c35c7a5586 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -1392,21 +1392,19 @@ void SCH_EDITOR_CONTROL::updatePastedSymbol( SCH_SYMBOL* aSymbol, SCH_SCREEN* aP const KIID_PATH& aClipPath, bool aForceKeepAnnotations ) { - wxCHECK( aSymbol && aPasteScreen, /* void */ ); + wxCHECK( m_frame && aSymbol && aPasteScreen, /* void */ ); - KIID_PATH clipItemPath = aClipPath; + SCH_SYMBOL_INSTANCE instance; + KIID_PATH pasteLookupPath = aClipPath; - wxString reference, value, footprint; - int unit; + m_pastedSymbols.insert( aSymbol ); - clipItemPath.push_back( aSymbol->m_Uuid ); + // The pasted symbol look up paths include the symbol UUID. + pasteLookupPath.push_back( aSymbol->m_Uuid ); - if( m_clipboardSymbolInstances.count( clipItemPath ) > 0 ) + if( m_clipboardSymbolInstances.count( pasteLookupPath ) > 0 ) { - SCH_SYMBOL_INSTANCE instance = m_clipboardSymbolInstances.at( clipItemPath ); - - unit = instance.m_Unit; - reference = instance.m_Reference; + instance = m_clipboardSymbolInstances.at( pasteLookupPath ); } else { @@ -1417,20 +1415,20 @@ void SCH_EDITOR_CONTROL::updatePastedSymbol( SCH_SYMBOL* aSymbol, SCH_SCREEN* aP // Pasted from notepad or an older instance of eeschema. Use the values in the fields // instead. - reference = aSymbol->GetField( REFERENCE_FIELD )->GetText(); - value = aSymbol->GetField( VALUE_FIELD )->GetText(); - footprint = aSymbol->GetField( FOOTPRINT_FIELD )->GetText(); - unit = aSymbol->GetUnit(); + instance.m_Reference = aSymbol->GetField( REFERENCE_FIELD )->GetText(); + instance.m_Unit = aSymbol->GetUnit(); } - if( aForceKeepAnnotations && !reference.IsEmpty() ) - aSymbol->SetRef( &aPastePath, reference ); - else + instance.m_Path = aPastePath.Path() + aClipPath; + instance.m_ProjectName = m_frame->Prj().GetProjectName(); + + aSymbol->AddHierarchicalReference( instance ); + + if( !aForceKeepAnnotations ) aSymbol->ClearAnnotation( &aPastePath, false ); // We might clear annotations but always leave the original unit number from the paste. - aSymbol->SetUnitSelection( &aPastePath, unit ); - aSymbol->SetUnit( unit ); + aSymbol->SetUnit( instance.m_Unit ); } @@ -1536,6 +1534,29 @@ void SCH_EDITOR_CONTROL::setPastedSymbolInstances( SCH_SCREENS& aScreenList ) } +void SCH_EDITOR_CONTROL::prunePastedSymbolInstances() +{ + wxCHECK( m_frame, /* void */ ); + + for( SCH_SYMBOL* symbol : m_pastedSymbols ) + { + wxCHECK2( symbol, continue ); + + std::vector instancePathsToRemove; + + for( const SCH_SYMBOL_INSTANCE& instance : symbol->GetInstanceReferences() ) + { + if( ( instance.m_ProjectName != m_frame->Prj().GetProjectName() ) + || instance.m_Path.empty() ) + instancePathsToRemove.emplace_back( instance.m_Path ); + } + + for( const KIID_PATH& path : instancePathsToRemove ) + symbol->RemoveInstance( path ); + } +} + + int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) { wxTextEntry* textEntry = dynamic_cast( wxWindow::FindFocus() ); @@ -1587,6 +1608,7 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) SCH_SCREENS tempScreens( tempSheet ); + m_pastedSymbols.clear(); m_clipboardSheetInstances.clear(); m_clipboardSymbolInstances.clear(); @@ -1938,6 +1960,8 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) m_frame->GetCurrentSheet().UpdateAllScreenReferences(); + prunePastedSymbolInstances(); + // 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. diff --git a/eeschema/tools/sch_editor_control.h b/eeschema/tools/sch_editor_control.h index 979b5e581d..2797353a9f 100644 --- a/eeschema/tools/sch_editor_control.h +++ b/eeschema/tools/sch_editor_control.h @@ -187,6 +187,16 @@ private: void setPastedSheetInstances( const SCH_SHEET* aPastedSheet ); void setPastedSymbolInstances( SCH_SCREENS& aScreenList ); + /** + * Remove all pasted symbol instances that do not belong to the current project. + * + * @warning This should **only** be called when cleaning up after a paste. Otherwise it + * could clobber symbol instances for schematics shared across projects. Use + * SCH_SCREENS::PruneOrphanedSymbolInstances() to clean up invalid instance for + * the current project. + */ + void prunePastedSymbolInstances(); + /** * Read the footprint info from each line in the stuff file by reference designator. * @@ -231,6 +241,8 @@ private: // A map of KIID_PATH --> sheet instances for the clipboard contents. std::map m_clipboardSheetInstances; + + std::set m_pastedSymbols; }; diff --git a/include/kiid.h b/include/kiid.h index 1dcfe4cb92..3ba73f36af 100644 --- a/include/kiid.h +++ b/include/kiid.h @@ -3,7 +3,7 @@ * * Copyright (C) 2020 Ian McInerney * Copyright (C) 2007-2014 Jean-Pierre Charras, jp.charras at wanadoo.fr - * Copyright (C) 1992-2022 KiCad Developers, see AUTHORS.TXT for contributors. + * Copyright (C) 1992-2023 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 @@ -190,6 +190,20 @@ public: return false; } + + KIID_PATH& operator+=( const KIID_PATH& aRhs ) + { + for( const KIID& kiid : aRhs ) + emplace_back( kiid ); + + return *this; + } + + friend KIID_PATH operator+( KIID_PATH aLhs, const KIID_PATH& aRhs ) + { + aLhs += aRhs; + return aLhs; + } }; /**