diff --git a/common/kiid.cpp b/common/kiid.cpp index a9a4c63f9a..d48fe1d2d7 100644 --- a/common/kiid.cpp +++ b/common/kiid.cpp @@ -3,7 +3,7 @@ * * Copyright (C) 2020 Ian McInerney * Copyright (C) 2007-2014 Jean-Pierre Charras, jp.charras at wanadoo.fr - * Copyright (C) 1992-2021 KiCad Developers, see AUTHORS.TXT for contributors. + * Copyright (C) 1992-2022 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 @@ -335,6 +335,27 @@ bool KIID_PATH::MakeRelativeTo( const KIID_PATH& aPath ) } +bool KIID_PATH::EndsWith( const KIID_PATH& aPath ) const +{ + if( aPath.size() > size() ) + return false; // this path can not end aPath + + KIID_PATH copyThis = *this; + KIID_PATH copyThat = aPath; + + while( !copyThat.empty() ) + { + if( *std::prev( copyThis.end() ) != *std::prev( copyThat.end() ) ) + return false; + + copyThis.pop_back(); + copyThat.pop_back(); + } + + return true; +} + + wxString KIID_PATH::AsString() const { wxString path; diff --git a/eeschema/dialogs/dialog_sheet_properties.cpp b/eeschema/dialogs/dialog_sheet_properties.cpp index 99b7def317..2488b07d6c 100644 --- a/eeschema/dialogs/dialog_sheet_properties.cpp +++ b/eeschema/dialogs/dialog_sheet_properties.cpp @@ -179,9 +179,21 @@ bool DIALOG_SHEET_PROPERTIES::TransferDataToWindow() instance.push_back( m_sheet ); - wxString nextPageNumber = instance.GetPageNumber(); + wxString pageNumber; - m_pageNumberTextCtrl->ChangeValue( nextPageNumber ); + if( m_sheet->IsNew() ) + { + // Don't try to be too clever when assigning the next availabe page number. Just use + // the number of sheets plus one. + pageNumber.Printf( wxT( "%d" ), static_cast( hierarchy.size() ) + 1 ); + instance.SetPageNumber( pageNumber ); + } + else + { + pageNumber = instance.GetPageNumber(); + } + + m_pageNumberTextCtrl->ChangeValue( pageNumber ); return true; } @@ -420,10 +432,7 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena } SCHEMATIC& schematic = m_frame->Schematic(); - SCH_SCREEN* rootScreen = schematic.RootScreen(); SCH_SHEET_LIST fullHierarchy = schematic.GetFullHierarchy(); - std::vector sheetInstances = fullHierarchy.GetSheetInstances(); - std::vector symbolInstances = rootScreen->GetSymbolInstances(); wxFileName screenFileName( sheetFileName ); wxFileName tmp( sheetFileName ); @@ -615,7 +624,6 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena // It's safe to set the sheet screen now. m_sheet->SetScreen( useScreen ); - // currentSheet.LastScreen()->Append( m_sheet ); } else if( loadFromFile ) { @@ -646,11 +654,6 @@ bool DIALOG_SHEET_PROPERTIES::onSheetFilenameChanged( const wxString& aNewFilena if( restoreSheet ) currentSheet.LastScreen()->Append( m_sheet ); - - // The full hierarchy needs to be reloaded due to the addition of a new sheet. - fullHierarchy = schematic.GetFullHierarchy(); - fullHierarchy.UpdateSheetInstanceData( sheetInstances ); - fullHierarchy.UpdateSymbolInstanceData( symbolInstances ); } if( m_clearAnnotationNewItems ) diff --git a/eeschema/sch_plugins/eagle/sch_eagle_plugin.cpp b/eeschema/sch_plugins/eagle/sch_eagle_plugin.cpp index f4994c40f0..de421e4a0c 100644 --- a/eeschema/sch_plugins/eagle/sch_eagle_plugin.cpp +++ b/eeschema/sch_plugins/eagle/sch_eagle_plugin.cpp @@ -708,7 +708,6 @@ void SCH_EAGLE_PLUGIN::loadSchematic( wxXmlNode* aSchematicNode ) m_sheetPath.push_back( m_rootSheet ); SCH_SHEET_PATH rootPath; - m_rootSheet->AddInstance( m_sheetPath ); rootPath.SetPageNumber( wxT( "1" ) ); int sheetCount = countChildren( sheetNode->GetParent(), wxT( "sheet" ) ); diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp index b7e8c424fd..6f873461e4 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp +++ b/eeschema/sch_plugins/kicad/sch_sexpr_parser.cpp @@ -3157,7 +3157,7 @@ SCH_SHEET* SCH_SEXPR_PARSER::parseSheet() } } - sheet->SetInstances( instances ); + sheet->setInstances( instances ); break; } diff --git a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp index db6de68937..1354bd21d0 100644 --- a/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp +++ b/eeschema/sch_plugins/kicad/sch_sexpr_plugin.cpp @@ -1015,7 +1015,7 @@ void SCH_SEXPR_PLUGIN::saveSheet( SCH_SHEET* aSheet, int aNestLevel ) m_out->Print( aNestLevel + 1, ")\n" ); // Closes pin token. } - // We all sheet instances here except the root sheet instance. + // Save all sheet instances here except the root sheet instance. std::vector< SCH_SHEET_INSTANCE > sheetInstances = aSheet->GetInstances(); auto it = sheetInstances.begin(); @@ -1045,12 +1045,12 @@ void SCH_SEXPR_PLUGIN::saveSheet( SCH_SHEET* aSheet, int aNestLevel ) // // Keep all instance data when copying to the clipboard. It may be needed on paste. if( ( sheetInstances[i].m_Path[0] == rootSheetUuid ) - && !fullHierarchy.GetSheetPathByKIIDPath( sheetInstances[i].m_Path ) ) + && !fullHierarchy.GetSheetPathByKIIDPath( sheetInstances[i].m_Path, false ) ) { if( project_open && ( ( i + 1 == sheetInstances.size() ) || lastProjectUuid != sheetInstances[i+1].m_Path[0] ) ) { - m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`. + m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project` token. project_open = false; } @@ -1081,12 +1081,12 @@ void SCH_SEXPR_PLUGIN::saveSheet( SCH_SHEET* aSheet, int aNestLevel ) if( project_open && ( ( i + 1 == sheetInstances.size() ) || lastProjectUuid != sheetInstances[i+1].m_Path[0] ) ) { - m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`. + m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project` token. project_open = false; } } - m_out->Print( aNestLevel + 1, ")\n" ); // Closes `instances`. + m_out->Print( aNestLevel + 1, ")\n" ); // Closes `instances` token. } m_out->Print( aNestLevel, ")\n" ); // Closes sheet token. diff --git a/eeschema/sch_sheet.cpp b/eeschema/sch_sheet.cpp index 6d20450211..396893ce70 100644 --- a/eeschema/sch_sheet.cpp +++ b/eeschema/sch_sheet.cpp @@ -1202,7 +1202,7 @@ bool SCH_SHEET::operator <( const SCH_ITEM& aItem ) const } -bool SCH_SHEET::AddInstance( const SCH_SHEET_PATH& aSheetPath ) +bool SCH_SHEET::addInstance( const SCH_SHEET_PATH& aSheetPath ) { wxCHECK( aSheetPath.IsFullPath(), false ); wxCHECK( !aSheetPath.Last() || ( aSheetPath.Last()->m_Uuid != m_Uuid ), false ); @@ -1228,6 +1228,30 @@ bool SCH_SHEET::AddInstance( const SCH_SHEET_PATH& aSheetPath ) } +bool SCH_SHEET::getInstance( SCH_SHEET_INSTANCE& aInstance, const KIID_PATH& aSheetPath, + bool aTestFromEnd ) const +{ + for( const SCH_SHEET_INSTANCE& instance : m_instances ) + { + if( !aTestFromEnd ) + { + if( instance.m_Path == aSheetPath ) + { + aInstance = instance; + return true; + } + } + else if( instance.m_Path.EndsWith( aSheetPath ) ) + { + aInstance = instance; + return true; + } + } + + return false; +} + + bool SCH_SHEET::HasRootInstance() const { for( const SCH_SHEET_INSTANCE& instance : m_instances ) diff --git a/eeschema/sch_sheet.h b/eeschema/sch_sheet.h index 1cb784ff1c..5a79bd28c3 100644 --- a/eeschema/sch_sheet.h +++ b/eeschema/sch_sheet.h @@ -29,6 +29,8 @@ class KIID_PATH; class SCH_SCREEN; +class SCH_SEXPR_PARSER; +class SCH_SHEET_LIST; class SCH_SHEET_PIN; class SCH_SHEET_PATH; class EDA_DRAW_FRAME; @@ -378,28 +380,6 @@ public: */ const std::vector& GetInstances() const { return m_instances; } - void SetInstances( const std::vector& aInstances ) - { - m_instances = aInstances; - } - - /** - * Add a new instance \a aSheetPath to the instance list. - * - * If \a aSheetPath does not already exist, it is added to the list. If already exists - * in the list, do nothing. Sheet instances allow for the sharing in complex hierarchies - * which allows for per instance data such as page number for sheets to stored. - * - * @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the - * #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path - * will raise an assertion on debug builds and silently fail and return false - * on release builds. - * - * @param[in] aInstance is the #SCH_SHEET_PATH of the sheet instance to the instance list. - * @return false if the instance already exists, true if the instance was added. - */ - bool AddInstance( const SCH_SHEET_PATH& aInstance ); - /** * Check to see if this sheet has a root sheet instance. * @@ -436,6 +416,29 @@ public: protected: friend SCH_SHEET_PATH; + friend SCH_SEXPR_PARSER; + + void setInstances( const std::vector& aInstances ) + { + m_instances = aInstances; + } + + /** + * Add a new instance \a aSheetPath to the instance list. + * + * If \a aSheetPath does not already exist, it is added to the list. If already exists + * in the list, do nothing. Sheet instances allow for the sharing in complex hierarchies + * which allows for per instance data such as page number for sheets to stored. + * + * @warning The #SCH_SHEET_PATH object must be a full hierarchical path which means the + * #SCH_SHEET object at index 0 must be the root sheet. A partial sheet path + * will raise an assertion on debug builds and silently fail and return false + * on release builds. + * + * @param[in] aInstance is the #SCH_SHEET_PATH of the sheet instance to the instance list. + * @return false if the instance already exists, true if the instance was added. + */ + bool addInstance( const SCH_SHEET_PATH& aInstance ); /** * Return the sheet page number for \a aInstance. @@ -462,6 +465,9 @@ protected: */ void setPageNumber( const SCH_SHEET_PATH& aInstance, const wxString& aPageNumber ); + bool getInstance( SCH_SHEET_INSTANCE& aInstance, const KIID_PATH& aSheetPath, + bool aTestFromEnd = false ) const; + /** * Renumber the sheet pins in the sheet. * @@ -481,6 +487,7 @@ private: bool doIsConnected( const VECTOR2I& aPosition ) const override; friend class SCH_SHEET_PIN; + friend class SCH_SHEET_LIST; private: SCH_SCREEN* m_screen; // Screen that contains the physical data for the diff --git a/eeschema/sch_sheet_path.cpp b/eeschema/sch_sheet_path.cpp index b6bb0cb697..7fcb0f7c2f 100644 --- a/eeschema/sch_sheet_path.cpp +++ b/eeschema/sch_sheet_path.cpp @@ -538,13 +538,19 @@ void SCH_SHEET_PATH::SetPageNumber( const wxString& aPageNumber ) tmpPath.pop_back(); - sheet->AddInstance( tmpPath ); + sheet->addInstance( tmpPath ); sheet->setPageNumber( tmpPath, aPageNumber ); } void SCH_SHEET_PATH::AddNewSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPath ) { + SCH_SHEET_PATH newSheetPath( aPrefixSheetPath ); + SCH_SHEET_PATH currentSheetPath( *this ); + + // Prefix the new hierarchical path. + newSheetPath = newSheetPath + currentSheetPath; + for( SCH_ITEM* item : LastScreen()->Items().OfType( SCH_SYMBOL_T ) ) { SCH_SYMBOL* symbol = static_cast( item ); @@ -552,17 +558,8 @@ void SCH_SHEET_PATH::AddNewSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPa wxCHECK2( symbol, continue ); SYMBOL_INSTANCE_REFERENCE newSymbolInstance; - SCH_SHEET_PATH newSheetPath( aPrefixSheetPath ); - SCH_SHEET_PATH currentSheetPath( *this ); - // Remove the root sheet from this path. - currentSheetPath.m_sheets.erase( currentSheetPath.m_sheets.begin() ); - - // Prefix the new hierarchical path. - newSheetPath = newSheetPath + currentSheetPath; - newSymbolInstance.m_Path = newSheetPath.Path(); - - if( symbol->GetInstance( newSymbolInstance, Path() ) ) + if( symbol->GetInstance( newSymbolInstance, Path(), true ) ) { // Use an existing symbol instance for this path if it exists. newSymbolInstance.m_Path = newSheetPath.Path(); @@ -570,25 +567,18 @@ void SCH_SHEET_PATH::AddNewSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPa } else if( !symbol->GetInstanceReferences().empty() ) { - // Use the first symbol instance if any symbol exists. + // Use the first symbol instance if any symbol instance data exists. newSymbolInstance = symbol->GetInstanceReferences()[0]; newSymbolInstance.m_Path = newSheetPath.Path(); symbol->AddHierarchicalReference( newSymbolInstance ); } - else if( symbol->GetLibSymbolRef() ) - { - // Fall back to library symbol reference prefix, value, and footprint fields and - // set the unit to 1 if the library symbol is valid. - newSymbolInstance.m_Reference = symbol->GetLibSymbolRef()->GetReferenceField().GetText(); - newSymbolInstance.m_Reference += wxT( "?" ); - newSymbolInstance.m_Unit = 1; - symbol->AddHierarchicalReference( newSymbolInstance ); - } else { - // All hope is lost so set the reference to 'U' and the unit to 1. - newSymbolInstance.m_Reference += wxT( "U?" ); - newSymbolInstance.m_Unit = 1; + // Fall back to the last saved symbol field and unit settings if there is no + // instance data. + newSymbolInstance.m_Path = newSheetPath.Path(); + newSymbolInstance.m_Reference = symbol->GetField( REFERENCE_FIELD )->GetText(); + newSymbolInstance.m_Unit = symbol->GetUnit(); symbol->AddHierarchicalReference( newSymbolInstance ); } } @@ -613,39 +603,6 @@ void SCH_SHEET_PATH::RemoveSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPa } -int SCH_SHEET_PATH::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPath, - int aNextVirtualPageNumber ) -{ - int nextVirtualPageNumber = aNextVirtualPageNumber; - - for( SCH_ITEM* item : LastScreen()->Items().OfType( SCH_SHEET_T ) ) - { - SCH_SHEET* sheet = static_cast( item ); - - wxCHECK2( sheet, continue ); - - SCH_SHEET_PATH newSheetPath( aPrefixSheetPath ); - SCH_SHEET_PATH currentSheetPath( *this ); - - // Remove the root sheet from current sheet path. - currentSheetPath.m_sheets.erase( currentSheetPath.m_sheets.begin() ); - - // Prefix the new hierarchical path. - newSheetPath = newSheetPath + currentSheetPath; - - wxString pageNumber; - - pageNumber.Printf( wxT( "%d" ), nextVirtualPageNumber ); - sheet->AddInstance( newSheetPath ); - newSheetPath.SetVirtualPageNumber( nextVirtualPageNumber ); - newSheetPath.SetPageNumber( pageNumber ); - nextVirtualPageNumber += 1; - } - - return nextVirtualPageNumber; -} - - void SCH_SHEET_PATH::MakeFilePathRelativeToParentSheet() { wxCHECK( m_sheets.size() > 1, /* void */ ); @@ -988,11 +945,17 @@ void SCH_SHEET_LIST::GetSheetsWithinPath( SCH_SHEET_PATHS& aSheets, } -std::optional SCH_SHEET_LIST::GetSheetPathByKIIDPath( const KIID_PATH& aPath ) const +std::optional SCH_SHEET_LIST::GetSheetPathByKIIDPath( const KIID_PATH& aPath, + bool aIncludeLastSheet ) const { for( const SCH_SHEET_PATH& sheet : *this ) { - if( sheet.Path() == aPath ) + KIID_PATH testPath = sheet.Path(); + + if( !aIncludeLastSheet ) + testPath.pop_back(); + + if( testPath == aPath ) return SCH_SHEET_PATH( sheet ); } @@ -1248,11 +1211,66 @@ void SCH_SHEET_LIST::RemoveSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPa void SCH_SHEET_LIST::AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPath, int aLastVirtualPageNumber ) { - int nextVirtualPageNumber = aLastVirtualPageNumber + 1; + wxString pageNumber; + int lastUsedPageNumber = 1; + int nextVirtualPageNumber = aLastVirtualPageNumber; + + // Fetch the list of page numbers already in use. + std::vector< wxString > usedPageNumbers; + + if( aPrefixSheetPath.size() ) + { + SCH_SHEET_LIST prefixHierarchy( aPrefixSheetPath.at( 0 ) ); + + for( const SCH_SHEET_PATH& path : prefixHierarchy ) + { + pageNumber = path.GetPageNumber(); + + if( !pageNumber.IsEmpty() ) + usedPageNumbers.emplace_back( pageNumber ); + } + } for( SCH_SHEET_PATH& sheetPath : *this ) - nextVirtualPageNumber = sheetPath.AddNewSheetInstances( aPrefixSheetPath, - nextVirtualPageNumber ); + { + SCH_SHEET_PATH tmp( sheetPath ); + SCH_SHEET_PATH newSheetPath( aPrefixSheetPath ); + + // Prefix the new hierarchical path. + newSheetPath = newSheetPath + sheetPath; + + // Sheets cannot have themselves in the path. + tmp.pop_back(); + + SCH_SHEET* sheet = sheetPath.Last(); + + wxCHECK2( sheet, continue ); + + nextVirtualPageNumber += 1; + + SCH_SHEET_INSTANCE instance; + + if( sheet->getInstance( instance, tmp.Path(), true ) + && !instance.m_PageNumber.IsEmpty() ) + { + newSheetPath.SetPageNumber( instance.m_PageNumber ); + usedPageNumbers.push_back( instance.m_PageNumber ); + } + else + { + // Generate the next available page number. + do + { + pageNumber.Printf( wxT( "%d" ), lastUsedPageNumber ); + lastUsedPageNumber += 1; + } while( std::find( usedPageNumbers.begin(), usedPageNumbers.end(), pageNumber ) != + usedPageNumbers.end() ); + + newSheetPath.SetVirtualPageNumber( nextVirtualPageNumber ); + newSheetPath.SetPageNumber( pageNumber ); + usedPageNumbers.push_back( pageNumber ); + } + } } diff --git a/eeschema/sch_sheet_path.h b/eeschema/sch_sheet_path.h index 5b47990ef5..af43cdfd50 100644 --- a/eeschema/sch_sheet_path.h +++ b/eeschema/sch_sheet_path.h @@ -400,8 +400,6 @@ public: void RemoveSymbolInstances( const SCH_SHEET_PATH& aPrefixSheetPath ); - int AddNewSheetInstances( const SCH_SHEET_PATH& aPrefixSheetPath, int aNextVirtualPageNumber ); - bool operator==( const SCH_SHEET_PATH& d1 ) const; bool operator!=( const SCH_SHEET_PATH& d1 ) const { return !( *this == d1 ) ; } @@ -542,7 +540,8 @@ public: * * @param aPath The KIID_PATH to search for. */ - std::optional GetSheetPathByKIIDPath( const KIID_PATH& aPath ) const; + std::optional GetSheetPathByKIIDPath( const KIID_PATH& aPath, + bool aIncludeLastSheet = true ) const; /** diff --git a/eeschema/sch_symbol.cpp b/eeschema/sch_symbol.cpp index 9a3ea91bd2..1d21ba1bed 100644 --- a/eeschema/sch_symbol.cpp +++ b/eeschema/sch_symbol.cpp @@ -477,11 +477,19 @@ void SCH_SYMBOL::Print( const RENDER_SETTINGS* aSettings, const VECTOR2I& aOffse bool SCH_SYMBOL::GetInstance( SYMBOL_INSTANCE_REFERENCE& aInstance, - const KIID_PATH& aSheetPath ) const + const KIID_PATH& aSheetPath, bool aTestFromEnd ) const { for( const SYMBOL_INSTANCE_REFERENCE& instance : m_instanceReferences ) { - if( instance.m_Path == aSheetPath ) + if( !aTestFromEnd ) + { + if( instance.m_Path == aSheetPath ) + { + aInstance = instance; + return true; + } + } + else if( instance.m_Path.EndsWith( aSheetPath ) ) { aInstance = instance; return true; diff --git a/eeschema/sch_symbol.h b/eeschema/sch_symbol.h index ad673c11cc..201d996a4e 100644 --- a/eeschema/sch_symbol.h +++ b/eeschema/sch_symbol.h @@ -143,7 +143,7 @@ public: } bool GetInstance( SYMBOL_INSTANCE_REFERENCE& aInstance, - const KIID_PATH& aSheetPath ) const; + const KIID_PATH& aSheetPath, bool aTestFromEnd = false ) const; void RemoveInstance( const SCH_SHEET_PATH& aInstancePath ); diff --git a/eeschema/sheet.cpp b/eeschema/sheet.cpp index c7e1e4d48e..8c35b61965 100644 --- a/eeschema/sheet.cpp +++ b/eeschema/sheet.cpp @@ -112,11 +112,12 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier bool libTableChanged = false; SCH_IO_MGR::SCH_FILE_T schFileType = SCH_IO_MGR::GuessPluginTypeFromSchPath( aFileName ); SCH_PLUGIN::SCH_PLUGIN_RELEASER pi( SCH_IO_MGR::FindPlugin( schFileType ) ); - std::unique_ptr< SCH_SHEET> newSheet = std::make_unique( &Schematic() ); + std::unique_ptr< SCH_SHEET> tmpSheet = std::make_unique( &Schematic() ); - // This will cause the sheet UUID to be set to the loaded schematic UUID. This is required - // to ensure all of the sheet paths in any subsheets are correctly generated. - const_cast( newSheet->m_Uuid ) = KIID( 0 ); + // This will cause the sheet UUID to be set to the UUID of the aSheet argument. This is + // required to ensure all of the sheet paths in any sub-sheets are correctly generated when + // using the temporary SCH_SHEET object that the file is loaded into.. + const_cast( tmpSheet->m_Uuid ) = aSheet->m_Uuid; wxFileName fileName( aFileName ); @@ -132,12 +133,12 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier { if( aSheet->GetScreen() != nullptr ) { - newSheet.reset( pi->Load( fullFilename, &Schematic() ) ); + tmpSheet.reset( pi->Load( fullFilename, &Schematic() ) ); } else { - newSheet->SetFileName( fullFilename ); - pi->Load( fullFilename, &Schematic(), newSheet.get() ); + tmpSheet->SetFileName( fullFilename ); + pi->Load( fullFilename, &Schematic(), tmpSheet.get() ); } if( !pi->GetError().IsEmpty() ) @@ -168,9 +169,9 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier // If the loaded schematic is in a different folder from the current project and // it contains hierarchical sheets, the hierarchical sheet paths need to be updated. - if( fileName.GetPathWithSep() != Prj().GetProjectPath() && newSheet->CountSheets() ) + if( fileName.GetPathWithSep() != Prj().GetProjectPath() && tmpSheet->CountSheets() ) { - SCH_SHEET_LIST loadedSheets( newSheet.get() ); + SCH_SHEET_LIST loadedSheets( tmpSheet.get() ); for( const SCH_SHEET_PATH& sheetPath : loadedSheets ) { @@ -208,12 +209,12 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier } } - SCH_SHEET_LIST sheetHierarchy( newSheet.get() ); // This is the hierarchy of the loaded file. + SCH_SHEET_LIST sheetHierarchy( tmpSheet.get() ); // This is the hierarchy of the loaded file. SCH_SHEET_LIST hierarchy = Schematic().GetSheets(); // This is the schematic sheet hierarchy. // Make sure any new sheet changes do not cause any recursion issues. - if( CheckSheetForRecursion( newSheet.get(), aHierarchy ) - || checkForNoFullyDefinedLibIds( newSheet.get() ) ) + if( CheckSheetForRecursion( tmpSheet.get(), aHierarchy ) + || checkForNoFullyDefinedLibIds( tmpSheet.get() ) ) { return false; } @@ -222,7 +223,7 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier // be broken symbol library links. wxArrayString names; wxArrayString newLibNames; - SCH_SCREENS newScreens( newSheet.get() ); // All screens associated with the import. + SCH_SCREENS newScreens( tmpSheet.get() ); // All screens associated with the import. SCH_SCREENS prjScreens( &Schematic().Root() ); newScreens.GetLibNicknames( names ); @@ -234,8 +235,8 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier // sheet so loading a hierarchical sheet that is not the root sheet will have no symbol // instance data. Give the user a chance to go back and save the project that contains this // hierarchical sheet so the symbol instance data will be correct on load. - if( ( newSheet->GetScreen()->GetFileFormatVersionAtLoad() < 20221002 ) - && newSheet->GetScreen()->GetSymbolInstances().empty() ) + if( ( tmpSheet->GetScreen()->GetFileFormatVersionAtLoad() < 20221002 ) + && tmpSheet->GetScreen()->GetSymbolInstances().empty() ) { msg = _( "There hierarchical sheets in the loaded schematic file from an older " "file version resulting in missing symbol instance data. This will " @@ -482,7 +483,7 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier } } - SCH_SCREEN* newScreen = newSheet->GetScreen(); + SCH_SCREEN* newScreen = tmpSheet->GetScreen(); wxCHECK_MSG( newScreen, false, "No screen defined for sheet." ); if( libTableChanged ) @@ -516,8 +517,8 @@ bool SCH_EDIT_FRAME::LoadSheetFromFile( SCH_SHEET* aSheet, SCH_SHEET_PATH* aHier else aSheet->GetScreen()->Append( newScreen ); - SCH_SCREENS allScreens( Schematic().Root() ); - allScreens.ReplaceDuplicateTimeStamps(); + SCH_SCREENS allLoadedScreens( aSheet ); + allLoadedScreens.ReplaceDuplicateTimeStamps(); return true; } diff --git a/eeschema/tools/sch_drawing_tools.cpp b/eeschema/tools/sch_drawing_tools.cpp index d5a8890ade..15c9a9dd65 100644 --- a/eeschema/tools/sch_drawing_tools.cpp +++ b/eeschema/tools/sch_drawing_tools.cpp @@ -1834,23 +1834,6 @@ int SCH_DRAWING_TOOLS::DrawSheet( const TOOL_EVENT& aEvent ) getViewControls()->SetAutoPan( false ); getViewControls()->CaptureCursor( false ); - // Find the list of paths in the hierarchy that refer to the destination sheet where - // the new sheet will be drawn - SCH_SCREEN* currentScreen = m_frame->GetCurrentSheet().LastScreen(); - SCH_SHEET_LIST hierarchy = m_frame->Schematic().GetSheets(); - SCH_SHEET_LIST instances = hierarchy.FindAllSheetsForScreen( currentScreen ); - instances.SortByPageNumbers(); - - int pageNum = static_cast( hierarchy.size() ) + 1; - - // Set a page number for all the instances of the new sheet in the hierarchy - for( SCH_SHEET_PATH& instance : instances ) - { - SCH_SHEET_PATH sheetPath = instance; - sheetPath.push_back( sheet ); - instance.SetPageNumber( wxString::Format( "%d", pageNum++ ) ); - } - if( m_frame->EditSheetProperties( static_cast( sheet ), &m_frame->GetCurrentSheet(), nullptr ) ) { diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index b4ce17099e..f91bcf845e 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1673,7 +1673,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) // 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 ); + SCH_SHEET_LIST originalHierarchy( &m_frame->Schematic().Root(), true ); doRefresh = m_frame->EditSheetProperties( sheet, &m_frame->GetCurrentSheet(), &doClearAnnotation ); @@ -1684,8 +1684,10 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) if( doClearAnnotation ) { SCH_SCREENS screensList( &m_frame->Schematic().Root() ); + // We clear annotation of new sheet paths here: - screensList.ClearAnnotationOfNewSheetPaths( initial_sheetpathList ); + screensList.ClearAnnotationOfNewSheetPaths( originalHierarchy ); + // Clear annotation of g_CurrentSheet itself, because its sheetpath is not a new // path, but symbols managed by its sheet path must have their annotation cleared // because they are new: diff --git a/include/kiid.h b/include/kiid.h index 3989d96593..1dcfe4cb92 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-2020 KiCad Developers, see AUTHORS.TXT for contributors. + * Copyright (C) 1992-2022 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 @@ -147,6 +147,17 @@ public: bool MakeRelativeTo( const KIID_PATH& aPath ); + /** + * Test if \a aPath from the last path towards the first path. + * + * This is useful for testing for existing schematic symbol and sheet instances when + * copying or adding a new sheet that is lower in the hierarchy than the current path. + * + * @param aPath is the path to compare this path against. + * @return true if this path ends with \a aPath or false if it does not. + */ + bool EndsWith( const KIID_PATH& aPath ) const; + wxString AsString() const; bool operator==( KIID_PATH const& rhs ) const diff --git a/qa/unittests/common/test_kiid.cpp b/qa/unittests/common/test_kiid.cpp index 6dac2d0c44..96b054ad1c 100644 --- a/qa/unittests/common/test_kiid.cpp +++ b/qa/unittests/common/test_kiid.cpp @@ -49,4 +49,25 @@ BOOST_AUTO_TEST_CASE( Seeding ) } +BOOST_AUTO_TEST_CASE( KiidPathTest ) +{ + KIID a, b, c, d; + + KIID_PATH path1; + KIID_PATH path2; + + path1.push_back( a ); + path1.push_back( b ); + path1.push_back( c ); + path1.push_back( d ); + + path2.push_back( b ); + path2.push_back( c ); + path2.push_back( d ); + + BOOST_CHECK( path1.EndsWith( path2 ) == true ); + BOOST_CHECK( path2.EndsWith( path1 ) == false ); +} + + BOOST_AUTO_TEST_SUITE_END()