haskal
/
kicad
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix crash in incremental update and bus conn 

We store our connectivity dirty flag with the SCH_ITEM but we generate
SCH_CONNECTION based on the SCH_ITEM and the SCH_SHEET_PATH.  For this
reason, we need to avoid clearing the connectivity dirty flag until
we've finished processing all instances of the SCH_ITEM in the graph

This also means that we need to allow getting the SCH_CONNECTION pointer
even when the connectivity is still dirty (getting SCH_CONNECTION
happens based on SCH_ITEM and SCH_SHEET_PATH, not just SCH_ITEM)

(cherry picked from commit 7d12e1c4f5)
This commit is contained in:
Seth Hillbrand 2023-09-19 15:52:26 -07:00
parent a74cffac39
commit 6b43bb8fe3
3 changed files with 105 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

246
eeschema/connection_graph.cpp
View File

@ -64,75 +64,28 @@ static const wxChar DanglingProfileMask[] = wxT( "CONN_PROFILE" );
static const wxChar ConnTrace[] = wxT( "CONN" );
void CONNECTION_SUBGRAPH::RemoveItem( SCH_ITEM* aItem )
{
m_items.erase( aItem );
m_drivers.erase( aItem );
if( aItem == m_driver )
{
m_driver = nullptr;
m_driver_connection = nullptr;
}
if( aItem->Type() == SCH_SHEET_PIN_T )
m_hier_pins.erase( static_cast<SCH_SHEET_PIN*>( aItem ) );
if( aItem->Type() == SCH_HIER_LABEL_T )
m_hier_ports.erase( static_cast<SCH_HIERLABEL*>( aItem ) );
}
bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers )
{
PRIORITY highest_priority = PRIORITY::INVALID;
std::vector<SCH_ITEM*> candidates;
std::vector<SCH_ITEM*> strong_drivers;
m_driver = nullptr;
// Hierarchical labels are lower priority than local labels here,
// because on the first pass we want local labels to drive subgraphs
// so that we can identify same-sheet neighbors and link them together.
// Hierarchical labels will end up overriding the final net name if
// a higher-level sheet has a different name during the hierarchical
// pass.
for( SCH_ITEM* item : m_drivers )
{
PRIORITY item_priority = GetDriverPriority( item );
if( item_priority == PRIORITY::PIN
&& !static_cast<SCH_PIN*>( item )->GetParentSymbol()->IsInNetlist() )
continue;
if( item_priority >= PRIORITY::HIER_LABEL )
strong_drivers.push_back( item );
if( item_priority > highest_priority )
{
candidates.clear();
candidates.push_back( item );
highest_priority = item_priority;
}
else if( !candidates.empty() && ( item_priority == highest_priority ) )
{
candidates.push_back( item );
}
}
if( highest_priority >= PRIORITY::HIER_LABEL )
m_strong_driver = true;
// Power pins are 5, global labels are 6
m_local_driver = ( highest_priority < PRIORITY::POWER_PIN );
if( !candidates.empty() )
{
if( candidates.size() > 1 )
{
if( highest_priority == PRIORITY::SHEET_PIN )
{
// We have multiple options, and they are all hierarchical
// sheet pins. Let's prefer outputs over inputs.
for( SCH_ITEM* c : candidates )
{
SCH_SHEET_PIN* p = static_cast<SCH_SHEET_PIN*>( c );
if( p->GetShape() == LABEL_FLAG_SHAPE::L_OUTPUT )
{
m_driver = c;
break;
}
}
}
else
{
// For all other driver types, sort by quality of name
std::sort( candidates.begin(), candidates.end(),
[&]( SCH_ITEM* a, SCH_ITEM* b ) -> bool
auto candidate_cmp = [&]( SCH_ITEM* a, SCH_ITEM* b ) -> bool
{
// meet irreflexive requirements of std::sort
if( a == b )
@ -172,12 +125,81 @@ bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers )
return true;
else
return a_name < b_name;
} );
};
auto strong_cmp = [this]( SCH_ITEM* a, SCH_ITEM* b ) -> bool
{
const wxString& a_name = GetNameForDriver( a );
const wxString& b_name = GetNameForDriver( b );
return a_name < b_name;
};
PRIORITY highest_priority = PRIORITY::INVALID;
std::set<SCH_ITEM*, decltype( candidate_cmp )> candidates( candidate_cmp );
std::set<SCH_ITEM*, decltype( strong_cmp )> strong_drivers( strong_cmp );
m_driver = nullptr;
// Hierarchical labels are lower priority than local labels here,
// because on the first pass we want local labels to drive subgraphs
// so that we can identify same-sheet neighbors and link them together.
// Hierarchical labels will end up overriding the final net name if
// a higher-level sheet has a different name during the hierarchical
// pass.
for( SCH_ITEM* item : m_drivers )
{
PRIORITY item_priority = GetDriverPriority( item );
if( item_priority == PRIORITY::PIN
&& !static_cast<SCH_PIN*>( item )->GetParentSymbol()->IsInNetlist() )
continue;
if( item_priority >= PRIORITY::HIER_LABEL )
strong_drivers.insert( item );
if( item_priority > highest_priority )
{
candidates.clear();
candidates.insert( item );
highest_priority = item_priority;
}
else if( !candidates.empty() && ( item_priority == highest_priority ) )
{
candidates.insert( item );
}
}
if( highest_priority >= PRIORITY::HIER_LABEL )
m_strong_driver = true;
// Power pins are 5, global labels are 6
m_local_driver = ( highest_priority < PRIORITY::POWER_PIN );
if( !candidates.empty() )
{
if( candidates.size() > 1 )
{
if( highest_priority == PRIORITY::SHEET_PIN )
{
// We have multiple options, and they are all hierarchical
// sheet pins. Let's prefer outputs over inputs.
for( SCH_ITEM* c : candidates )
{
SCH_SHEET_PIN* p = static_cast<SCH_SHEET_PIN*>( c );
if( p->GetShape() == LABEL_FLAG_SHAPE::L_OUTPUT )
{
m_driver = c;
break;
}
}
}
}
if( !m_driver )
m_driver = candidates[0];
m_driver = *candidates.begin();
}
if( strong_drivers.size() > 1 )
@ -185,7 +207,10 @@ bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers )
// Drop weak drivers
if( m_strong_driver )
m_drivers = strong_drivers;
{
m_drivers.clear();
m_drivers.insert( strong_drivers.begin(), strong_drivers.end() );
}
// Cache driver connection
if( m_driver )
@ -200,30 +225,6 @@ bool CONNECTION_SUBGRAPH::ResolveDrivers( bool aCheckMultipleDrivers )
m_driver_connection = nullptr;
}
if( aCheckMultipleDrivers && m_multiple_drivers )
{
// First check if all the candidates are actually the same
bool same = true;
const wxString& first = GetNameForDriver( candidates[0] );
SCH_ITEM* second_item = nullptr;
for( unsigned i = 1; i < candidates.size(); i++ )
{
if( GetNameForDriver( candidates[i] ) != first )
{
second_item = candidates[i];
same = false;
break;
}
}
if( !same )
{
m_first_driver = m_driver;
m_second_driver = second_item;
}
}
return ( m_driver != nullptr );
}
@ -242,11 +243,7 @@ void CONNECTION_SUBGRAPH::getAllConnectedItems( std::set<std::pair<SCH_SHEET_PAT
aSubgraphs.insert( sg->m_absorbed_subgraphs.begin(), sg->m_absorbed_subgraphs.end() );
for( SCH_ITEM* item : sg->m_items )
{
item->Connection( &m_sheet )->Reset();
item->ConnectedItems( m_sheet ).clear();
aItems.emplace( m_sheet, item );
}
for( CONNECTION_SUBGRAPH* child_sg : sg->m_hier_children )
child_sg->getAllConnectedItems( aItems, aSubgraphs );
@ -436,15 +433,15 @@ void CONNECTION_SUBGRAPH::Absorb( CONNECTION_SUBGRAPH* aOther )
void CONNECTION_SUBGRAPH::AddItem( SCH_ITEM* aItem )
{
m_items.push_back( aItem );
m_items.insert( aItem );
if( aItem->Connection( &m_sheet )->IsDriver() )
m_drivers.push_back( aItem );
m_drivers.insert( aItem );
if( aItem->Type() == SCH_SHEET_PIN_T )
m_hier_pins.push_back( static_cast<SCH_SHEET_PIN*>( aItem ) );
m_hier_pins.insert( static_cast<SCH_SHEET_PIN*>( aItem ) );
else if( aItem->Type() == SCH_HIER_LABEL_T )
m_hier_ports.push_back( static_cast<SCH_HIERLABEL*>( aItem ) );
m_hier_ports.insert( static_cast<SCH_HIERLABEL*>( aItem ) );
}
@ -601,6 +598,7 @@ void CONNECTION_GRAPH::Recalculate( const SCH_SHEET_LIST& aSheetList, bool aUnco
PROF_TIMER update_items( "updateItemConnectivity" );
m_sheetList = aSheetList;
std::set<SCH_ITEM*> dirty_items;
for( const SCH_SHEET_PATH& sheet : aSheetList )
{
@ -616,6 +614,7 @@ void CONNECTION_GRAPH::Recalculate( const SCH_SHEET_LIST& aSheetList, bool aUnco
wxLogTrace( ConnTrace, wxT( "Adding item %s to connectivity graph update" ),
item->GetTypeDesc() );
items.push_back( item );
dirty_items.insert( item );
}
// Ensure the hierarchy info stored in SCREENS is built and up to date
// (multi-unit symbols)
@ -648,6 +647,9 @@ void CONNECTION_GRAPH::Recalculate( const SCH_SHEET_LIST& aSheetList, bool aUnco
}
}
for( SCH_ITEM* item : dirty_items )
item->SetConnectivityDirty( false );
if( wxLog::IsAllowedTraceMask( DanglingProfileMask ) )
update_items.Show();
@ -957,8 +959,6 @@ void CONNECTION_GRAPH::updateItemConnectivity( const SCH_SHEET_PATH& aSheet,
for( const VECTOR2I& point : points )
connection_map[ point ].push_back( item );
}
item->SetConnectivityDirty( false );
}
for( const auto& it : connection_map )
@ -1663,10 +1663,16 @@ void CONNECTION_GRAPH::processSubGraphs()
// by the chosen driver of the subgraph, so we need to create a dummy connection
add_connections_to_check( subgraph );
std::set<SCH_CONNECTION*> checked_connections;
for( unsigned i = 0; i < connections_to_check.size(); i++ )
{
auto member = connections_to_check[i];
// Don't check the same connection twice
if( !checked_connections.insert( member.get() ).second )
continue;
if( member->IsBus() )
{
connections_to_check.insert( connections_to_check.end(),
@ -2867,38 +2873,7 @@ int CONNECTION_GRAPH::RunERC()
bool CONNECTION_GRAPH::ercCheckMultipleDrivers( const CONNECTION_SUBGRAPH* aSubgraph )
{
wxCHECK( aSubgraph, false );
/*
* This was changed late in 6.0 to fix https://gitlab.com/kicad/code/kicad/-/issues/9367
* so I'm going to leave the original code in for just a little while. If anyone comes
* across this in 7.0 development (or later), feel free to delete.
*/
#if 0
if( aSubgraph->m_second_driver )
{
SCH_ITEM* primary = aSubgraph->m_first_driver;
SCH_ITEM* secondary = aSubgraph->m_second_driver;
wxPoint pos = primary->Type() == SCH_PIN_T ?
static_cast<SCH_PIN*>( primary )->GetTransformedPosition() :
primary->GetPosition();
const wxString& primaryName = aSubgraph->GetNameForDriver( primary );
const wxString& secondaryName = aSubgraph->GetNameForDriver( secondary );
wxString msg = wxString::Format( _( "Both %s and %s are attached to the same "
"items; %s will be used in the netlist" ),
primaryName, secondaryName, primaryName );
std::shared_ptr<ERC_ITEM> ercItem = ERC_ITEM::Create( ERCE_DRIVER_CONFLICT );
ercItem->SetItems( primary, secondary );
ercItem->SetErrorMessage( msg );
SCH_MARKER* marker = new SCH_MARKER( ercItem, pos );
aSubgraph->m_sheet.LastScreen()->Append( marker );
return false;
}
#else
if( aSubgraph->m_multiple_drivers )
{
for( SCH_ITEM* driver : aSubgraph->m_drivers )
@ -2933,7 +2908,6 @@ bool CONNECTION_GRAPH::ercCheckMultipleDrivers( const CONNECTION_SUBGRAPH* aSubg
}
}
}
#endif
return true;
}

24
eeschema/connection_graph.h
View File

@ -86,9 +86,7 @@ public:
m_bus_entry( nullptr ),
m_driver( nullptr ),
m_driver_connection( nullptr ),
m_hier_parent( nullptr ),
m_first_driver( nullptr ),
m_second_driver( nullptr )
m_hier_parent( nullptr )
{}
~CONNECTION_SUBGRAPH() = default;
@ -130,7 +128,7 @@ public:
void UpdateItemConnections();
/// Provides a read-only reference to the items in the subgraph
const std::vector<SCH_ITEM*>& GetItems() const
const std::set<SCH_ITEM*>& GetItems() const
{
return m_items;
}
@ -162,6 +160,8 @@ public:
return PRIORITY::NONE;
}
void RemoveItem( SCH_ITEM* aItem );
private:
wxString driverName( SCH_ITEM* aItem ) const;
@ -206,9 +206,9 @@ public:
/// Bus entry in graph, if any
SCH_ITEM* m_bus_entry;
std::vector<SCH_ITEM*> m_items;
std::set<SCH_ITEM*> m_items;
std::vector<SCH_ITEM*> m_drivers;
std::set<SCH_ITEM*> m_drivers;
SCH_ITEM* m_driver;
@ -237,10 +237,10 @@ public:
std::unordered_set<CONNECTION_SUBGRAPH*> > m_bus_parents;
// Cache for lookup of any hierarchical (sheet) pins on this subgraph (for referring down)
std::vector<SCH_SHEET_PIN*> m_hier_pins;
std::set<SCH_SHEET_PIN*> m_hier_pins;
// Cache for lookup of any hierarchical ports on this subgraph (for referring up)
std::vector<SCH_HIERLABEL*> m_hier_ports;
std::set<SCH_HIERLABEL*> m_hier_ports;
// If not null, this indicates the subgraph on a higher level sheet that is linked to this one
CONNECTION_SUBGRAPH* m_hier_parent;
@ -251,14 +251,6 @@ public:
/// A cache of escaped netnames from schematic items
mutable std::unordered_map<SCH_ITEM*, wxString> m_driver_name_cache;
/**
* Stores the primary driver for the multiple drivers ERC check. This is the chosen driver
* before subgraphs are absorbed (so m_driver may be different)
*/
SCH_ITEM* m_first_driver;
/// Used for multiple drivers ERC message; stores the second possible driver (or nullptr)
SCH_ITEM* m_second_driver;
};
struct NET_NAME_CODE_CACHE_KEY

10
eeschema/sch_item.cpp
View File

@ -149,9 +149,6 @@ SCH_CONNECTION* SCH_ITEM::Connection( const SCH_SHEET_PATH* aSheet ) const
if( !IsConnectable() )
return nullptr;
wxCHECK_MSG( !IsConnectivityDirty(), nullptr,
wxT( "Shouldn't be asking for connection if connectivity is dirty!" ) );
if( !aSheet )
aSheet = &Schematic()->CurrentSheet();
@ -218,10 +215,11 @@ void SCH_ITEM::AddConnectionTo( const SCH_SHEET_PATH& aSheet, SCH_ITEM* aItem )
SCH_CONNECTION* SCH_ITEM::InitializeConnection( const SCH_SHEET_PATH& aSheet,
CONNECTION_GRAPH* aGraph )
{
SetConnectivityDirty( false );
SCH_CONNECTION* connection = Connection( &aSheet );
// N.B. Do not clear the dirty connectivity flag here because we may need
// to create a connection for a different sheet, and we don't want to
// skip the connection creation because the flag is cleared.
if( connection )
{
connection->Reset();
@ -244,8 +242,6 @@ SCH_CONNECTION* SCH_ITEM::GetOrInitConnection( const SCH_SHEET_PATH& aSheet,
if( !IsConnectable() )
return nullptr;
SetConnectivityDirty( false );
SCH_CONNECTION* connection = Connection( &aSheet );
if( connection )