From f00dc1befd64092dedf9808482e2d6472ad39bc5 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Mon, 19 Jan 2026 13:51:24 -0800 Subject: [PATCH] Altium Schematic Import: Fix duplicate sheet creation in hierarchical projects When importing Altium projects, sheets were sometimes imported as both top-level sheets and hierarchical subsheets. This happened because the project file lists sheets in arbitrary order. If a parent sheet was processed before its children, the children would be loaded as subsheets during hierarchy descent. Later, when those same files appeared in the project file, duplicate sheets were created. The importer now checks whether a file has already been loaded as a subsheet before creating a new top-level sheet entry. The check includes a case-insensitive fallback search since Altium uses inconsistent casing in its .SchDoc extension. Sheet filenames were also being set to the original Altium paths instead of KiCad project-relative paths. This caused path mismatches in the hierarchy search and left references to non-existent Altium paths in the saved schematic. Filenames now use the KiCad project directory with the .kicad_sch extension. Hierarchical labels on top-level sheets are converted to global labels during import since top-level sheets have no parent sheet to connect them to. Fixes https://gitlab.com/kicad/code/kicad/-/issues/21909 --- eeschema/sch_io/altium/sch_io_altium.cpp | 104 ++++++++++++++++++++--- 1 file changed, 93 insertions(+), 11 deletions(-) diff --git a/eeschema/sch_io/altium/sch_io_altium.cpp b/eeschema/sch_io/altium/sch_io_altium.cpp index bcd79bebbe..e17d23474d 100644 --- a/eeschema/sch_io/altium/sch_io_altium.cpp +++ b/eeschema/sch_io/altium/sch_io_altium.cpp @@ -392,25 +392,61 @@ SCH_SHEET* SCH_IO_ALTIUM::LoadSchematicProject( SCHEMATIC* aSchematic, const std if( !key.starts_with( "sch" ) ) continue; + wxFileName fn( filestring ); + + // Check if this file was already loaded as a subsheet of another sheet. + // This can happen when the project file lists sheets in an order where a parent + // sheet is processed before its subsheets. We need to handle potential case + // differences in filenames (e.g., LVDS.SCHDOC vs LVDS.SchDoc). + SCH_SCREEN* existingScreen = nullptr; + m_rootSheet->SearchHierarchy( fn.GetFullPath(), &existingScreen ); + + // If not found, try case-insensitive search by checking all loaded screens. + // Compare base names only (without extension) since Altium uses .SchDoc/.SCHDOC + // while KiCad uses .kicad_sch + if( !existingScreen ) + { + SCH_SCREENS allScreens( m_rootSheet ); + + for( SCH_SCREEN* screen = allScreens.GetFirst(); screen; screen = allScreens.GetNext() ) + { + wxFileName screenFn( screen->GetFileName() ); + wxFileName checkFn( fn.GetFullPath() ); + + if( screenFn.GetName().IsSameAs( checkFn.GetName(), false ) ) + { + existingScreen = screen; + break; + } + } + } + + if( existingScreen ) + continue; + VECTOR2I pos = VECTOR2I( x * schIUScale.MilsToIU( 1000 ), y * schIUScale.MilsToIU( 1000 ) ); - wxFileName fn( filestring ); wxFileName kicad_fn( fn ); std::unique_ptr sheet = std::make_unique( m_rootSheet, pos ); SCH_SCREEN* screen = new SCH_SCREEN( m_schematic ); sheet->SetScreen( screen ); + // Convert to KiCad project-relative path with .kicad_sch extension kicad_fn.SetExt( FILEEXT::KiCadSchematicFileExtension ); kicad_fn.SetPath( aSchematic->Project().GetProjectPath() ); - sheet->SetFileName( fn.GetFullPath() ); - screen->SetFileName( sheet->GetFileName() ); + + // Sheet uses relative filename, screen uses full path + sheet->SetFileName( kicad_fn.GetFullName() ); + screen->SetFileName( kicad_fn.GetFullPath() ); wxCHECK2( sheet && screen, continue ); wxString pageNo = wxString::Format( wxT( "%d" ), page++ ); m_sheetPath.push_back( sheet.get() ); + + // Parse from the original Altium file location ParseAltiumSch( fn.GetFullPath() ); m_sheetPath.SetPageNumber( pageNo ); @@ -423,7 +459,9 @@ SCH_SHEET* SCH_IO_ALTIUM::LoadSchematicProject( SCHEMATIC* aSchematic, const std sheet->SetParent( m_sheetPath.Last() ); SCH_SHEET* sheetPtr = sheet.release(); currentScreen->Append( sheetPtr ); - sheets[fn.GetFullPath()] = sheetPtr; + + // Use the KiCad path for the map key since screen filenames use KiCad paths + sheets[kicad_fn.GetFullPath()] = sheetPtr; x += 2; @@ -541,6 +579,34 @@ SCH_SHEET* SCH_IO_ALTIUM::LoadSchematicFile( const wxString& aFileName, SCHEMATI if( !topLevelSheets.empty() ) aSchematic->SetTopLevelSheets( topLevelSheets ); + // Convert hierarchical labels to global labels on top-level sheets. + // Top-level sheets have no parent, so hierarchical labels don't make sense. + for( SCH_SHEET* sheet : topLevelSheets ) + { + SCH_SCREEN* screen = sheet->GetScreen(); + + if( !screen ) + continue; + + std::vector hierLabels; + + for( SCH_ITEM* item : screen->Items().OfType( SCH_HIER_LABEL_T ) ) + hierLabels.push_back( static_cast( item ) ); + + for( SCH_HIERLABEL* hierLabel : hierLabels ) + { + SCH_GLOBALLABEL* globalLabel = new SCH_GLOBALLABEL( hierLabel->GetPosition(), + hierLabel->GetText() ); + globalLabel->SetShape( hierLabel->GetShape() ); + globalLabel->SetSpinStyle( hierLabel->GetSpinStyle() ); + globalLabel->GetField( FIELD_T::INTERSHEET_REFS )->SetVisible( false ); + + screen->Remove( hierLabel ); + screen->Append( globalLabel ); + delete hierLabel; + } + } + m_rootSheet = &aSchematic->Root(); } @@ -860,9 +926,7 @@ void SCH_IO_ALTIUM::ParseAltiumSch( const wxString& aFileName ) } catch( const std::exception& exc ) { - wxLogTrace( traceAltiumSch, wxS( "Unhandled exception in Altium schematic parser: %s." ), - exc.what() ); - throw; + THROW_IO_ERROR( wxString::Format( _( "Error parsing Altium schematic: %s" ), exc.what() ) ); } } else // ASCII @@ -933,7 +997,27 @@ void SCH_IO_ALTIUM::ParseAltiumSch( const wxString& aFileName ) SCH_SCREEN* screen = sheet->GetScreen(); if( sheet->GetName().Trim().empty() ) - sheet->SetName( loadAltiumFileName.GetName() ); + { + wxString sheetName = loadAltiumFileName.GetName(); + + std::set sheetNames; + + for( EDA_ITEM* otherItem : currentScreen->Items().OfType( SCH_SHEET_T ) ) + { + SCH_SHEET* otherSheet = static_cast( otherItem ); + sheetNames.insert( otherSheet->GetName() ); + } + + for( int ii = 1; ; ++ii ) + { + if( sheetNames.find( sheetName ) == sheetNames.end() ) + break; + + sheetName = loadAltiumFileName.GetName() + wxString::Format( wxT( "_%d" ), ii ); + } + + sheet->SetName( sheetName ); + } wxCHECK2( screen, continue ); @@ -4826,9 +4910,7 @@ void SCH_IO_ALTIUM::ensureLoadedLibrary( const wxString& aLibraryPath, } catch( const std::exception& exc ) { - wxFAIL_MSG( wxString::Format( wxT( "Unhandled exception in Altium schematic parsers: %s." ), - exc.what() ) ); - throw; + THROW_IO_ERROR( wxString::Format( _( "Error parsing Altium library: %s" ), exc.what() ) ); } }