Re-enforce ordering

Instances, pins and graphics have started to wander around the files.
Sorts instances before writing.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/17737
This commit is contained in:
Seth Hillbrand 2024-04-12 10:05:37 -07:00
parent 7727982478
commit b22fcf70cd
7 changed files with 89 additions and 36 deletions

View File

@ -232,13 +232,14 @@ 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,

View File

@ -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

View File

@ -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() );

View File

@ -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<const LIB_TEXTBOX*>( &aOther );
int result = GetText().CmpNoCase( tmp->GetText() );

View File

@ -789,7 +789,29 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
field.SetText( value );
}
std::vector<SCH_PIN*> pins;
pins.reserve( aSymbol->GetRawPins().size() );
for( const std::unique_ptr<SCH_PIN>& 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() )
{
@ -815,10 +837,21 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
SCH_SHEET_LIST fullHierarchy = aSchematic.GetSheets();
bool project_open = false;
for( size_t i = 0; i < aSymbol->GetInstances().size(); i++ )
std::vector<SCH_SYMBOL_INSTANCE> orderedInstances = aSymbol->GetInstances();
std::sort( orderedInstances.begin(), orderedInstances.end(), []( const SCH_SYMBOL_INSTANCE& a, const SCH_SYMBOL_INSTANCE& b )
{
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( aSymbol->GetInstances()[i].m_Path.size(), continue );
wxCHECK2( inst.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
@ -826,11 +859,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
&& ( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid )
&& !fullHierarchy.GetSheetPathByKIIDPath( aSymbol->GetInstances()[i].m_Path ) )
&& ( inst.m_Path[0] == rootSheetUuid )
&& !fullHierarchy.GetSheetPathByKIIDPath( inst.m_Path ) )
{
if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() )
|| lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) )
if( project_open && ( ( &inst == &orderedInstances.back() )
|| lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) )
{
m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`.
project_open = false;
@ -839,23 +872,23 @@ void SCH_IO_KICAD_SEXPR::saveSymbol( SCH_SYMBOL* aSymbol, const SCHEMATIC& aSche
continue;
}
if( lastProjectUuid != aSymbol->GetInstances()[i].m_Path[0] )
if( lastProjectUuid != inst.m_Path[0] )
{
wxString projectName;
if( aSymbol->GetInstances()[i].m_Path[0] == rootSheetUuid )
if( inst.m_Path[0] == rootSheetUuid )
projectName = aSchematic.Prj().GetProjectName();
else
projectName = aSymbol->GetInstances()[i].m_ProjectName;
projectName = inst.m_ProjectName;
lastProjectUuid = aSymbol->GetInstances()[i].m_Path[0];
lastProjectUuid = inst.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 = aSymbol->GetInstances()[i].m_Path;
KIID_PATH tmp = inst.m_Path;
if( aForClipboard && aRelativePath )
tmp.MakeRelativeTo( aRelativePath->Path() );
@ -865,12 +898,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( aSymbol->GetInstances()[i].m_Reference ).c_str(),
aSymbol->GetInstances()[i].m_Unit );
m_out->Quotew( inst.m_Reference ).c_str(),
inst.m_Unit );
m_out->Print( aNestLevel + 3, ")\n" );
if( project_open && ( ( i + 1 == aSymbol->GetInstances().size() )
|| lastProjectUuid != aSymbol->GetInstances()[i+1].m_Path[0] ) )
if( project_open && ( ( &inst == &orderedInstances.back() )
|| lastProjectUuid != orderedInstances[ii + 1].m_Path[0] ) )
{
m_out->Print( aNestLevel + 2, ")\n" ); // Closes `project`.
project_open = false;

View File

@ -198,6 +198,17 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT
aSymbol->GetFields( fields );
std::sort( fields.begin(), fields.end(),
[]( const LIB_FIELD* a, const LIB_FIELD* b )
{
// Mandatory fields are always first. They are only ones with fixed
// field IDs. The rest are sorted by their comparison operator.
if( a->IsMandatory() || b->IsMandatory() )
return a->GetId() < b->GetId();
return a->Compare( b ) < 0;
} );
for( LIB_FIELD* field : fields )
saveField( field, aFormatter, aNestLevel + 1 );
@ -244,19 +255,16 @@ 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;
};
std::multiset<SCH_ITEM*, decltype( cmp )> save_map( cmp );
std::vector<SCH_ITEM*> save_vec;
save_vec.reserve( unit.m_items.size() );
for( SCH_ITEM* item : unit.m_items )
save_map.insert( item );
save_vec.push_back( item );
for( SCH_ITEM* item : save_map )
std::sort( save_vec.begin(), save_vec.end(), SCH_ITEM::cmp_items());
for( SCH_ITEM* item : save_vec )
saveSymbolDrawItem( item, aFormatter, aNestLevel + 2 );
aFormatter.Print( aNestLevel + 1, ")\n" );
@ -274,6 +282,17 @@ void SCH_IO_KICAD_SEXPR_LIB_CACHE::SaveSymbol( LIB_SYMBOL* aSymbol, OUTPUTFORMAT
aSymbol->GetFields( fields );
std::sort( fields.begin(), fields.end(),
[]( const LIB_FIELD* a, const LIB_FIELD* b )
{
// Mandatory fields are always first. They are only ones with fixed
// field IDs. The rest are sorted by their comparison operator.
if( a->IsMandatory() || b->IsMandatory() )
return a->GetId() < b->GetId();
return a->Compare( b ) < 0;
} );
for( LIB_FIELD* field : fields )
saveField( field, aFormatter, aNestLevel + 1 );

View File

@ -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<SCH_RENDER_SETTINGS*>( aPlotter->RenderSettings() );
}
struct cmp_items
{
bool operator()( const SCH_ITEM* aFirst, const SCH_ITEM* aSecond ) const;
};
void getSymbolEditorMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>& aList );
/**