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

Rework ruleAreas to avoid stale pointers.

Browse Source 
1) UpdateRuleAreasInScreens() is going to reset the rule
area caches on all SCH_RULE_AREAs.  This means that every
SCH_ITEM in the screen that *had* a rule area cache is going
to get it cleared.  So we might as well just up-front clear
the caches of all SCH_ITEMs, avoiding the need to use the
(possibly stale) pointers inside SCH_RULE_AREA::m_items.

2) UpdateRuleAreasInScreens() also sets the repaint flag on
all SCH_DIRECTIVE_LABELs, so there's no need to worry about
them elsewhere.

3) SCH_RULE_AREA::m_prev_items could also contain stale
pointers.  We avoid this one by converting it to KIIDs.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/21120
This commit is contained in:
Jeff Young
2025-06-15 13:35:41 +01:00
parent 95000ce62f
commit 98dc94f8f2
3 changed files with 61 additions and 101 deletions
Download Patch File Download Diff File Expand all files Collapse all files
eeschema/sch_rule_area.cpp
+32 -68
View File
@@ -198,21 +198,22 @@ wxString SCH_RULE_AREA::GetItemDescription( UNITS_PROVIDER* aUnitsProvider, bool
}
void SCH_RULE_AREA::ResetCaches( KIGFX::SCH_VIEW* view )
void SCH_RULE_AREA::resetCaches()
{
// Save the current state
m_prev_items = m_items;
m_prev_directives = m_directives;
m_prev_items = m_itemIDs;
m_prev_directives = m_directiveIDs;
// Reset the rule area
clearContainedItems();
clearDirectives( view );
// Do NOT assume these pointers are valid.
m_items.clear();
m_itemIDs.clear();
m_directives.clear();
m_directiveIDs.clear();
}
void SCH_RULE_AREA::RefreshContainedItemsAndDirectives(
SCH_SCREEN* screen, KIGFX::SCH_VIEW* view,
std::vector<std::pair<SCH_RULE_AREA*, SCH_SCREEN*>>& forceUpdateRuleAreas )
void SCH_RULE_AREA::RefreshContainedItemsAndDirectives( SCH_SCREEN* screen )
{
EE_RTREE& items = screen->Items();
const BOX2I boundingBox = GetBoundingBox();
@@ -228,13 +229,9 @@ void SCH_RULE_AREA::RefreshContainedItemsAndDirectives(
assert( labelConnectionPoints.size() == 1 );
if( GetPolyShape().CollideEdge( labelConnectionPoints[0], nullptr, 5 ) )
addDirective( label, view );
addDirective( label );
}
// If directives have changed, we need to force an update of the contained items connectivity
if( m_directives != m_prev_directives )
forceUpdateRuleAreas.push_back( { this, screen } );
// Next find any connectable items which lie within the rule area
EE_RTREE::EE_TYPE ruleAreaItems = items.Overlapping( boundingBox );
@@ -280,17 +277,6 @@ void SCH_RULE_AREA::RefreshContainedItemsAndDirectives(
}
std::unordered_set<SCH_ITEM*> SCH_RULE_AREA::GetPastAndPresentContainedItems() const
{
std::unordered_set<SCH_ITEM*> items = m_items;
for( SCH_ITEM* item : m_prev_items )
items.insert( item );
return items;
}
std::vector<std::pair<SCH_RULE_AREA*, SCH_SCREEN*>>
SCH_RULE_AREA::UpdateRuleAreasInScreens( std::unordered_set<SCH_SCREEN*>& screens,
KIGFX::SCH_VIEW* view )
@@ -301,17 +287,26 @@ SCH_RULE_AREA::UpdateRuleAreasInScreens( std::unordered_set<SCH_SCREEN*>& screen
{
// First reset all item caches - must be done first to ensure two rule areas
// on the same item don't overwrite each other's caches
for( SCH_ITEM* ruleAreaAsItem : screen->Items().OfType( SCH_RULE_AREA_T ) )
for( SCH_ITEM* item : screen->Items() )
{
SCH_RULE_AREA* ruleArea = static_cast<SCH_RULE_AREA*>( ruleAreaAsItem );
ruleArea->ResetCaches( view );
if( item->Type() == SCH_RULE_AREA_T )
static_cast<SCH_RULE_AREA*>( item )->resetCaches();
if( item->Type() == SCH_DIRECTIVE_LABEL_T && view )
view->Update( item, KIGFX::REPAINT );
item->ClearRuleAreasCache();
}
// Secondly refresh the contained items
for( SCH_ITEM* ruleAreaAsItem : screen->Items().OfType( SCH_RULE_AREA_T ) )
{
SCH_RULE_AREA* ruleArea = static_cast<SCH_RULE_AREA*>( ruleAreaAsItem );
ruleArea->RefreshContainedItemsAndDirectives( screen, view, forceUpdateRuleAreas );
ruleArea->RefreshContainedItemsAndDirectives( screen );
if( ruleArea->m_directiveIDs != ruleArea->m_prev_directives )
forceUpdateRuleAreas.push_back( { ruleArea, screen } );
}
}
@@ -331,6 +326,12 @@ const std::unordered_set<SCH_DIRECTIVE_LABEL*>& SCH_RULE_AREA::GetDirectives() c
}
const std::unordered_set<KIID>& SCH_RULE_AREA::GetPastContainedItems() const
{
return m_prev_items;
}
const std::vector<std::pair<wxString, SCH_ITEM*>> SCH_RULE_AREA::GetResolvedNetclasses() const
{
std::vector<std::pair<wxString, SCH_ITEM*>> resolvedNetclasses;
@@ -362,19 +363,6 @@ const std::vector<std::pair<wxString, SCH_ITEM*>> SCH_RULE_AREA::GetResolvedNetc
}
void SCH_RULE_AREA::ResetDirectivesAndItems( KIGFX::SCH_VIEW* view )
{
for( SCH_DIRECTIVE_LABEL* label : m_directives )
{
label->ClearConnectedRuleAreas();
view->Update( label, KIGFX::REPAINT );
}
for( SCH_ITEM* item : m_items )
item->ClearRuleAreasCache();
}
void SCH_RULE_AREA::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>& aList )
{
aList.emplace_back( _( "Rule Area" ), wxEmptyString );
@@ -396,27 +384,11 @@ void SCH_RULE_AREA::GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PAN
}
void SCH_RULE_AREA::addDirective( SCH_DIRECTIVE_LABEL* label, KIGFX::SCH_VIEW* view )
void SCH_RULE_AREA::addDirective( SCH_DIRECTIVE_LABEL* label )
{
label->AddConnectedRuleArea( this );
m_directives.insert( label );
if( view )
view->Update( label, KIGFX::REPAINT );
}
void SCH_RULE_AREA::clearDirectives( KIGFX::SCH_VIEW* view )
{
for( SCH_DIRECTIVE_LABEL* label : m_directives )
{
label->ClearConnectedRuleAreas();
if( view )
view->Update( label, KIGFX::REPAINT );
}
m_directives.clear();
m_directiveIDs.insert( label->m_Uuid );
}
@@ -424,15 +396,7 @@ void SCH_RULE_AREA::addContainedItem( SCH_ITEM* item )
{
item->AddRuleAreaToCache( this );
m_items.insert( item );
}
void SCH_RULE_AREA::clearContainedItems()
{
for( SCH_ITEM* item : m_items )
item->ClearRuleAreasCache();
m_items.clear();
m_itemIDs.insert( item->m_Uuid );
}
eeschema/sch_rule_area.h
+10 -20
View File
@@ -95,16 +95,8 @@ public:
wxString GetItemDescription( UNITS_PROVIDER* aUnitsProvider, bool aFull ) const override;
/// Reset all item and directive caches, saving the current state first.
void ResetCaches( KIGFX::SCH_VIEW* view );
/// Refresh the list of items which this rule area affects.
void RefreshContainedItemsAndDirectives(
SCH_SCREEN* screen, KIGFX::SCH_VIEW* view,
std::vector<std::pair<SCH_RULE_AREA*, SCH_SCREEN*>>& forceUpdateRuleAreas );
/// Fetch all items which were, or are, within the rule area.
std::unordered_set<SCH_ITEM*> GetPastAndPresentContainedItems() const;
void RefreshContainedItemsAndDirectives( SCH_SCREEN* screen );
/// Update all rule area connectvity / caches in the given sheet paths.
///
@@ -115,6 +107,8 @@ public:
/// Return a set of all items contained within the rule area.
const std::unordered_set<SCH_ITEM*>& GetContainedItems() const;
const std::unordered_set<KIID>& GetPastContainedItems() const;
/// Return the set of all directive labels attached to the rule area border.
const std::unordered_set<SCH_DIRECTIVE_LABEL*>& GetDirectives() const;
@@ -123,24 +117,18 @@ public:
/// @return The resolved netclass (if any), and the SCH_ITEM providing the declaration.
const std::vector<std::pair<wxString, SCH_ITEM*>> GetResolvedNetclasses() const;
/// Clear and resets items and directives attached to this rule area.
void ResetDirectivesAndItems( KIGFX::SCH_VIEW* view );
/// Get the message panel info for the rule area.
void GetMsgPanelInfo( EDA_DRAW_FRAME* aFrame, std::vector<MSG_PANEL_ITEM>& aList ) override;
protected:
/// Add a directive label which applies to items within ths rule area.
void addDirective( SCH_DIRECTIVE_LABEL* label, KIGFX::SCH_VIEW* view );
/// Clear the list of directives.
void clearDirectives( KIGFX::SCH_VIEW* view );
void addDirective( SCH_DIRECTIVE_LABEL* label );
/// Add an item to the list of items which this rule area affects.
void addContainedItem( SCH_ITEM* item );
/// Clear the list of items which this rule area affects.
void clearContainedItems();
/// Reset all item and directive caches, saving the current state first.
void resetCaches();
protected:
bool m_excludedFromSim;
@@ -150,15 +138,17 @@ protected:
/// All #SCH_ITEM objects currently contained or intersecting the rule area.
std::unordered_set<SCH_ITEM*> m_items;
std::unordered_set<KIID> m_itemIDs;
/// All #SCH_DIRECTIVE_LABEL objectss attached to the rule area border.
std::unordered_set<SCH_DIRECTIVE_LABEL*> m_directives;
std::unordered_set<KIID> m_directiveIDs;
/// All #SCH_ITEM objectss contained or intersecting the rule area in the previous update.
std::unordered_set<SCH_ITEM*> m_prev_items;
std::unordered_set<KIID> m_prev_items;
/// All SCH_DIRECTIVE_LABEL objects attached to the rule area border in the previous update.
std::unordered_set<SCH_DIRECTIVE_LABEL*> m_prev_directives;
std::unordered_set<KIID> m_prev_directives;
};
#endif
eeschema/schematic.cpp
+19 -13
View File
@@ -1382,10 +1382,6 @@ void SCHEMATIC::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FLAGS a
if( item->Type() == SCH_RULE_AREA_T )
{
SCH_RULE_AREA* ruleArea = static_cast<SCH_RULE_AREA*>( item );
// Clear item and directive associations for this rule area
ruleArea->ResetDirectivesAndItems( aSchView );
changed_rule_areas.insert( { ruleArea, screen } );
}
else if( item->IsConnectable() )
@@ -1408,11 +1404,24 @@ void SCHEMATIC::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FLAGS a
// If a SCH_RULE_AREA was changed, we need to add all past and present contained items to
// update their connectivity
for( const std::pair<SCH_RULE_AREA*, SCH_SCREEN*>& changedRuleArea : changed_rule_areas )
{
for( SCH_ITEM* containedItem : changedRuleArea.first->GetPastAndPresentContainedItems() )
addItemToChangeSet( { containedItem, nullptr, changedRuleArea.second } );
}
std::map<KIID, EDA_ITEM*> itemMap;
list.FillItemMap( itemMap );
auto addPastAndPresentContainedItems =
[&]( SCH_RULE_AREA* changedRuleArea, SCH_SCREEN* screen )
{
for( const KIID& pastItem : changedRuleArea->GetPastContainedItems() )
{
if( itemMap.contains( pastItem ) )
addItemToChangeSet( { static_cast<SCH_ITEM*>( itemMap[pastItem] ), nullptr, screen } );
}
for( SCH_ITEM* containedItem : changedRuleArea->GetContainedItems() )
addItemToChangeSet( { containedItem, nullptr, screen } );
};
for( const auto& [changedRuleArea, screen] : changed_rule_areas )
addPastAndPresentContainedItems( changedRuleArea, screen );
// Add all changed items, and associated items, to the change set
for( CHANGED_ITEM& changed_item_data : changed_connectable_items )
@@ -1436,10 +1445,7 @@ void SCHEMATIC::RecalculateConnections( SCH_COMMIT* aCommit, SCH_CLEANUP_FLAGS a
std::vector<SHAPE*> borderShapes = ruleArea->MakeEffectiveShapes( true );
if( ruleArea->GetPolyShape().CollideEdge( labelConnectionPoints[0], nullptr, 5 ) )
{
for( SCH_ITEM* containedItem : ruleArea->GetPastAndPresentContainedItems() )
addItemToChangeSet( { containedItem, nullptr, changed_item_data.screen } );
}
addPastAndPresentContainedItems( ruleArea, changed_item_data.screen );
}
}
}