From f08cacb30f80ec04b9dc17fb7230f4b668ba3443 Mon Sep 17 00:00:00 2001 From: Seth Hillbrand Date: Fri, 12 Apr 2024 10:34:11 -0700 Subject: [PATCH] Revert "Re-enforce ordering" Unexpected element loss after save. Reverting to fix This reverts commit f2f489034258a85df3de36ce0f482ddd18933a1f. --- eeschema/lib_field.cpp | 3 +- eeschema/lib_pin.cpp | 4 +- eeschema/lib_text.cpp | 4 +- eeschema/lib_textbox.cpp | 4 +- .../sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp | 63 +++++-------------- .../sch_io_kicad_sexpr_lib_cache.cpp | 16 +++-- eeschema/sch_item.h | 10 +-- 7 files changed, 34 insertions(+), 70 deletions(-) diff --git a/eeschema/lib_field.cpp b/eeschema/lib_field.cpp index 5aa8e5c009..b77462920b 100644 --- a/eeschema/lib_field.cpp +++ b/eeschema/lib_field.cpp @@ -232,14 +232,13 @@ void LIB_FIELD::Copy( LIB_FIELD* aTarget ) const int LIB_FIELD::compare( const SCH_ITEM& aOther, int aCompareFlags ) const { + wxASSERT( aOther.Type() == LIB_FIELD_T ); int retv = SCH_ITEM::compare( aOther, aCompareFlags ); if( retv ) return retv; - wxASSERT( aOther.Type() == LIB_FIELD_T ); - const LIB_FIELD* tmp = ( LIB_FIELD* ) &aOther; // Equality test will vary depending whether or not the field is mandatory. Otherwise, diff --git a/eeschema/lib_pin.cpp b/eeschema/lib_pin.cpp index ccde70c7f2..81ed312140 100644 --- a/eeschema/lib_pin.cpp +++ b/eeschema/lib_pin.cpp @@ -995,13 +995,13 @@ EDA_ITEM* LIB_PIN::Clone() const int LIB_PIN::compare( const SCH_ITEM& aOther, int aCompareFlags ) const { + wxASSERT( aOther.Type() == LIB_PIN_T ); + int retv = SCH_ITEM::compare( aOther, aCompareFlags ); if( retv ) return retv; - wxASSERT( aOther.Type() == LIB_PIN_T ); - const LIB_PIN* tmp = (LIB_PIN*) &aOther; // When comparing units, we do not compare the part numbers. If everything else is diff --git a/eeschema/lib_text.cpp b/eeschema/lib_text.cpp index a68d3a58c6..14b9cacd6a 100644 --- a/eeschema/lib_text.cpp +++ b/eeschema/lib_text.cpp @@ -78,13 +78,13 @@ EDA_ITEM* LIB_TEXT::Clone() const int LIB_TEXT::compare( const SCH_ITEM& aOther, int aCompareFlags ) const { + wxASSERT( aOther.Type() == LIB_TEXT_T ); + int retv = SCH_ITEM::compare( aOther, aCompareFlags ); if( retv ) return retv; - wxASSERT( aOther.Type() == LIB_TEXT_T ); - const LIB_TEXT* tmp = ( LIB_TEXT* ) &aOther; int result = GetText().CmpNoCase( tmp->GetText() ); diff --git a/eeschema/lib_textbox.cpp b/eeschema/lib_textbox.cpp index eda96ce34a..57e1a79854 100644 --- a/eeschema/lib_textbox.cpp +++ b/eeschema/lib_textbox.cpp @@ -190,13 +190,13 @@ VECTOR2I LIB_TEXTBOX::GetDrawPos() const int LIB_TEXTBOX::compare( const SCH_ITEM& aOther, int aCompareFlags ) const { + wxASSERT( aOther.Type() == LIB_TEXTBOX_T ); + int retv = SCH_ITEM::compare( aOther, aCompareFlags ); if( retv ) return retv; - wxASSERT( aOther.Type() == LIB_TEXTBOX_T ); - const LIB_TEXTBOX* tmp = static_cast( &aOther ); int result = GetText().CmpNoCase( tmp->GetText() ); diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp index 36e9f03fb0..bd35674edd 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr.cpp @@ -789,29 +789,7 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche field.SetText( value ); } - std::vector pins; - pins.reserve( aSymbol->GetRawPins().size() ); - for( const std::unique_ptr& pin : aSymbol->GetRawPins() ) - pins.push_back( pin.get() ); - - std::sort( pins.begin(), pins.end(), []( const SCH_PIN* a, const SCH_PIN* b ) - { - int cmp = StrNumCmp( a->GetName(), b->GetName() ); - - if( cmp != 0 ) - return cmp < 0; - - cmp = StrNumCmp( a->GetNumber(), b->GetNumber() ); - - if( cmp != 0 ) - return cmp < 0; - - // The UUID is used as a tie breaker to ensure a stable sort. - return a->m_Uuid < b->m_Uuid; - } ); - - for( SCH_PIN* pin : pins ) { if( pin->GetAlt().IsEmpty() ) { @@ -837,21 +815,10 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche SCH_SHEET_LIST fullHierarchy = aSchematic.GetSheets(); bool project_open = false; - std::vector orderedInstances = aSymbol->GetInstances(); - std::sort( orderedInstances.begin(), orderedInstances.end(), []( const SCH_SYMBOL_INSTANCE& a, const SCH_SYMBOL_INSTANCE& b ) + for( size_t i = 0; i < aSymbol->GetInstances().size(); i++ ) { - if( a.m_ProjectName != b.m_ProjectName ) - return a.m_ProjectName < b.m_ProjectName; - - return a.m_Path < b.m_Path; - } ); - - for( size_t ii = 0; ii < orderedInstances.size(); ++ii ) - { - SCH_SYMBOL_INSTANCE& inst = orderedInstances[ii]; - // Zero length KIID_PATH objects are not valid and will cause a crash below. - wxCHECK2( inst.m_Path.size(), continue ); + wxCHECK2( aSymbol->GetInstances()[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 @@ -859,11 +826,11 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche // // Keep all instance data when copying to the clipboard. It may be needed on paste. if( !aForClipboard - && ( inst.m_Path[0] == rootSheetUuid ) - && !fullHierarchy.GetSheetPathByKIIDPath( inst.m_Path ) ) + && ( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid ) + && !fullHierarchy.GetSheetPathByKIIDPath( aSymbol->GetInstances()[i].m_Path ) ) { - if( project_open && ( ( &inst == &orderedInstances.back() ) - || lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) ) + if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() ) + || lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) ) { m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`. project_open = false; @@ -872,23 +839,23 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche continue; } - if( lastProjectUuid != inst.m_Path[0] ) + if( lastProjectUuid != aSymbol->GetInstances()[i].m_Path[0] ) { wxString projectName; - if( inst.m_Path[0] == rootSheetUuid ) + if( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid ) projectName = aSchematic.Prj().GetProjectName(); else - projectName = inst.m_ProjectName; + projectName = aSymbol->GetInstances()[i].m_ProjectName; - lastProjectUuid = inst.m_Path[0]; + lastProjectUuid = aSymbol->GetInstances()[i].m_Path[0]; m_out->Print( aNestLevel + 2, "(project %s\n", m_out->Quotew( projectName ).c_str() ); project_open = true; } wxString path; - KIID_PATH tmp = inst.m_Path; + KIID_PATH tmp = aSymbol->GetInstances()[i].m_Path; if( aForClipboard && aRelativePath ) tmp.MakeRelativeTo( aRelativePath->Path() ); @@ -898,12 +865,12 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche m_out->Print( aNestLevel + 3, "(path %s\n", m_out->Quotew( path ).c_str() ); m_out->Print( aNestLevel + 4, "(reference %s) (unit %d)\n", - m_out->Quotew( inst.m_Reference ).c_str(), - inst.m_Unit ); + m_out->Quotew( aSymbol->GetInstances()[i].m_Reference ).c_str(), + aSymbol->GetInstances()[i].m_Unit ); m_out->Print( aNestLevel + 3, ")\n" ); - if( project_open && ( ( &inst == &orderedInstances.back() ) - || lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) ) + if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() ) + || lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) ) { m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`. project_open = false; diff --git a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.cpp b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.cpp index 76bb4fcdf2..b23b14214a 100644 --- a/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.cpp +++ b/eeschema/sch_io/kicad_sexpr/sch_io_kicad_sexpr_lib_cache.cpp @@ -244,16 +244,14 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT aFormatter.Print( aNestLevel + 2, "(unit_name %s)\n", aFormatter.Quotes( name ).c_str() ); } + // Enforce item ordering + auto cmp = + []( const SCH_ITEM* a, const SCH_ITEM* b ) + { + return *a < *b; + }; - auto set_cmp = []( SCH_ITEM* a, SCH_ITEM* b ) -> bool - { - if( *a == *b ) - return a->m_Uuid < b->m_Uuid; - - return SCH_ITEM::cmp_items()(a, b); - }; - - std::set save_map(set_cmp); + std::multiset save_map( cmp ); for( SCH_ITEM* item : unit.m_items ) save_map.insert( item ); diff --git a/eeschema/sch_item.h b/eeschema/sch_item.h index 4788b58e9b..310b51bdc9 100644 --- a/eeschema/sch_item.h +++ b/eeschema/sch_item.h @@ -651,17 +651,17 @@ public: virtual bool operator<( const SCH_ITEM& aItem ) const; - struct cmp_items - { - bool operator()( const SCH_ITEM* aFirst, const SCH_ITEM* aSecond ) const; - }; - protected: SCH_RENDER_SETTINGS* getRenderSettings( PLOTTER* aPlotter ) const { return static_cast( aPlotter->RenderSettings() ); } + struct cmp_items + { + bool operator()( const SCH_ITEM* aFirst, const SCH_ITEM* aSecond ) const; + }; + void getSymbolEditorMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector& aList ); /**