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

Avoid numerical dereference of selections 

The SELECTION is a std::set.  Numerical dereference of item n is O(n) as
the iterator is not random access.  Therefore, a for loop using
numerical dereference is O(n!) and quickly slows.

We avoid this by storing items to remove separately while iterating and
then removing the items after we complete.

Fixes: lp:1692081
* https://bugs.launchpad.net/kicad/+bug/1692081
This commit is contained in:
Seth Hillbrand 2018-05-22 16:18:31 -07:00
parent 8d52dc9451
commit 8506cdf3ae
2 changed files with 16 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
gerbview/tools/selection_tool.cpp
View File

@ -262,24 +262,25 @@ SELECTION& GERBVIEW_SELECTION_TOOL::GetSelection()
SELECTION& GERBVIEW_SELECTION_TOOL::RequestSelection( int aFlags ) SELECTION& GERBVIEW_SELECTION_TOOL::RequestSelection( int aFlags )
{ {
std::vector<EDA_ITEM*> removed_items;
if( m_selection.Empty() ) if( m_selection.Empty() )
{ {
m_toolMgr->RunAction( GERBVIEW_ACTIONS::selectionCursor, true, 0 ); m_toolMgr->RunAction( GERBVIEW_ACTIONS::selectionCursor, true, 0 );
m_selection.SetIsHover( true ); m_selection.SetIsHover( true );
} }
// Be careful with iterators: items can be removed from list
// that invalidate iterators.
for( unsigned ii = 0; ii < m_selection.GetSize(); ii++ )
{
EDA_ITEM* item = m_selection[ii];
// Be careful with iterators: items can be removed from list that invalidate iterators.
for( auto item : m_selection )
{
if( ( aFlags & SELECTION_EDITABLE ) && item->Type() == PCB_MARKER_T ) if( ( aFlags & SELECTION_EDITABLE ) && item->Type() == PCB_MARKER_T )
{ removed_items.push_back( item );
unselect( static_cast<EDA_ITEM *>( item ) );
}
} }
// Now safely remove the items from the selection
for( auto item : removed_items )
unselect( static_cast<EDA_ITEM *>( item ) );
return m_selection; return m_selection;
} }

16
pcbnew/tools/selection_tool.cpp
View File

@ -370,6 +370,7 @@ SELECTION& SELECTION_TOOL::GetSelection()
SELECTION& SELECTION_TOOL::RequestSelection( int aFlags, CLIENT_SELECTION_FILTER aClientFilter ) SELECTION& SELECTION_TOOL::RequestSelection( int aFlags, CLIENT_SELECTION_FILTER aClientFilter )
{ {
std::vector<EDA_ITEM*> removed_items;
bool selectionEmpty = m_selection.Empty(); bool selectionEmpty = m_selection.Empty();
m_selection.SetIsHover( selectionEmpty ); m_selection.SetIsHover( selectionEmpty );
@ -383,19 +384,16 @@ SELECTION& SELECTION_TOOL::RequestSelection( int aFlags, CLIENT_SELECTION_FILTER
} }
// Be careful with iterators: items can be removed from list that invalidate iterators. // Be careful with iterators: items can be removed from list that invalidate iterators.
for( unsigned ii = 0; ii < m_selection.GetSize(); ii++ ) for( auto item : m_selection )
{ {
EDA_ITEM* item = m_selection[ii];
if( ( aFlags & SELECTION_EDITABLE ) && item->Type() == PCB_MARKER_T ) if( ( aFlags & SELECTION_EDITABLE ) && item->Type() == PCB_MARKER_T )
{ removed_items.push_back( item );
unselect( static_cast<BOARD_ITEM *>( item ) );
// unselect() removed the item from list. Back up to catch the following item.
ii--;
}
} }
// Now safely remove the items from the selection
for( auto item : removed_items )
unselect( static_cast<BOARD_ITEM *>( item ) );
if( aFlags & SELECTION_SANITIZE_PADS ) if( aFlags & SELECTION_SANITIZE_PADS )
SanitizeSelection(); SanitizeSelection();