From d3f40dde3f3f4aca0a5c4f17b0a3528d67357b17 Mon Sep 17 00:00:00 2001 From: Wayne Stambaugh Date: Mon, 15 Jan 2024 12:15:30 -0500 Subject: [PATCH] Fix assertion when pasting schematic sheets. Pasted sheets do not have assigned SCH_SCREEN objects until after they are loaded. Do not recalculate connectivity unless the SCH_COMMIT object actually has connectable schematic object changes. Fixes https://gitlab.com/kicad/code/kicad/-/issues/16579 --- eeschema/sch_commit.cpp | 8 ++-- eeschema/tools/sch_editor_control.cpp | 69 ++++++++------------------- eeschema/tools/sch_editor_control.h | 6 +-- 3 files changed, 27 insertions(+), 56 deletions(-) diff --git a/eeschema/sch_commit.cpp b/eeschema/sch_commit.cpp index 59a0399e18..2dd69e4877 100644 --- a/eeschema/sch_commit.cpp +++ b/eeschema/sch_commit.cpp @@ -1,7 +1,7 @@ /* * This program source code file is part of KiCad, a free EDA CAD application. * - * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. + * Copyright (C) 2023-2024 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 @@ -209,7 +209,7 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) if( schItem->IsSelected() ) selectedModified = true; - if( !( aCommitFlags & SKIP_CONNECTIVITY ) ) + if( !( aCommitFlags & SKIP_CONNECTIVITY ) && schItem->IsConnectable() ) dirtyConnectivity = true; switch( changeType ) @@ -325,7 +325,9 @@ void SCH_COMMIT::pushSchEdit( const wxString& aMessage, int aCommitFlags ) if( frame ) { frame->SaveCopyInUndoList( undoList, UNDO_REDO::UNSPECIFIED, false, dirtyConnectivity ); - frame->RecalculateConnections( this, NO_CLEANUP ); + + if( dirtyConnectivity ) + frame->RecalculateConnections( this, NO_CLEANUP ); } } diff --git a/eeschema/tools/sch_editor_control.cpp b/eeschema/tools/sch_editor_control.cpp index f7b436a0f7..042e8ed3d6 100644 --- a/eeschema/tools/sch_editor_control.cpp +++ b/eeschema/tools/sch_editor_control.cpp @@ -1513,56 +1513,25 @@ SCH_SHEET_PATH SCH_EDITOR_CONTROL::updatePastedSheet( const SCH_SHEET_PATH& aPas } -void SCH_EDITOR_CONTROL::setPastedSheetInstances( const SCH_SHEET* aPastedSheet ) +void SCH_EDITOR_CONTROL::setPastedSymbolInstances( const SCH_SCREEN* aScreen ) { - wxCHECK( aPastedSheet, /* void */ ); + wxCHECK( aScreen, /* void */ ); - for( const SCH_SHEET_INSTANCE& sheetInstance : aPastedSheet->GetInstances() ) + for( const SCH_ITEM* item : aScreen->Items() ) { - KIID_PATH pathWithSheet = sheetInstance.m_Path; - - pathWithSheet.push_back( aPastedSheet->m_Uuid ); - m_clipboardSheetInstances[pathWithSheet] = sheetInstance; - } - - const SCH_SCREEN* screen = aPastedSheet->GetScreen(); - - wxCHECK( screen, /* void */ ); - - for( SCH_ITEM* item : screen->Items() ) - { - if( item->Type() == SCH_SHEET_T ) + if( item->Type() == SCH_SYMBOL_T ) { - const SCH_SHEET* sheet = static_cast( item ); + const SCH_SYMBOL* symbol = static_cast( item ); - wxCHECK2( sheet, continue ); + wxCHECK2( symbol, continue ); - setPastedSheetInstances( sheet ); - } - } -} - - -void SCH_EDITOR_CONTROL::setPastedSymbolInstances( SCH_SCREENS& aScreenList ) -{ - for( SCH_SCREEN* screen = aScreenList.GetFirst(); screen; screen = aScreenList.GetNext() ) - { - for( const SCH_ITEM* item : screen->Items() ) - { - if( item->Type() == SCH_SYMBOL_T ) + for( const SCH_SYMBOL_INSTANCE& symbolInstance : symbol->GetInstances() ) { - const SCH_SYMBOL* symbol = static_cast( item ); + KIID_PATH pathWithSymbol = symbolInstance.m_Path; - wxCHECK2( symbol, continue ); + pathWithSymbol.push_back( symbol->m_Uuid ); - for( const SCH_SYMBOL_INSTANCE& symbolInstance : symbol->GetInstances() ) - { - KIID_PATH pathWithSymbol = symbolInstance.m_Path; - - pathWithSymbol.push_back( symbol->m_Uuid ); - - m_clipboardSymbolInstances[pathWithSymbol] = symbolInstance; - } + m_clipboardSymbolInstances[pathWithSymbol] = symbolInstance; } } } @@ -1640,15 +1609,11 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) tempScreen->Append( text_item ); } - SCH_SCREENS tempScreens( tempSheet ); - m_pastedSymbols.clear(); - m_clipboardSheetInstances.clear(); m_clipboardSymbolInstances.clear(); - // Save pasted sheet and symbol instances. - setPastedSheetInstances( &tempSheet ); - setPastedSymbolInstances( tempScreens ); + // Save pasted symbol instances in case the user chooses to keep existing symbol annotation. + setPastedSymbolInstances( tempScreen ); tempScreen->MigrateSimModels(); @@ -1867,6 +1832,9 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) m_frame->InitSheet( sheet, sheet->GetFileName() ); } + // Save the symbol instances in case the user chooses to keep the existing + // symbol annotation. + setPastedSymbolInstances( sheet->GetScreen() ); sheetsPasted = true; // Push it to the clipboard path while it still has its old KIID @@ -1960,7 +1928,6 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) } m_frame->SetSheetNumberAndCount(); - m_frame->UpdateHierarchyNavigator(); // Get a version with correct sheet numbers since we've pasted sheets, // we'll need this when annotating next @@ -2124,9 +2091,15 @@ int SCH_EDITOR_CONTROL::Paste( const TOOL_EVENT& aEvent ) } if( m_toolMgr->RunSynchronousAction( EE_ACTIONS::move, &commit ) ) + { + // Pushing the commit will update the connectivity. commit.Push( _( "Paste" ) ); + m_frame->UpdateHierarchyNavigator(); + } else + { commit.Revert(); + } } return 0; diff --git a/eeschema/tools/sch_editor_control.h b/eeschema/tools/sch_editor_control.h index 00e8517a22..bd31d3be97 100644 --- a/eeschema/tools/sch_editor_control.h +++ b/eeschema/tools/sch_editor_control.h @@ -191,8 +191,7 @@ private: SCH_SHEET_LIST* aPastedSheetsSoFar, SCH_REFERENCE_LIST* aPastedSymbolsSoFar ); - void setPastedSheetInstances( const SCH_SHEET* aPastedSheet ); - void setPastedSymbolInstances( SCH_SCREENS& aScreenList ); + void setPastedSymbolInstances( const SCH_SCREEN* aScreen ); /** * Remove all pasted symbol instances that do not belong to the current project. @@ -246,9 +245,6 @@ private: // A map of KIID_PATH --> symbol instances for the clipboard contents. std::map m_clipboardSymbolInstances; - // A map of KIID_PATH --> sheet instances for the clipboard contents. - std::map m_clipboardSheetInstances; - std::set m_pastedSymbols; };