Prevent automatic RTree recaching

Calling UpdateItem() may be performed by in a common loop, e.g. for(
SCH_ITEM* item : GetScreen()->Items() ) or similar.  We cannot call
GetScreen()->Update( SCH_ITEM* ) in this routine as it will remove and
re-add the item to the RTree, invalidating iterators.  If needed, the
items need to be cached to an external container before updating

Fixes https://gitlab.com/kicad/code/kicad/issues/9318
This commit is contained in:
Seth Hillbrand 2021-10-05 20:10:20 -07:00
parent 9a8d1246cc
commit 854472d550
14 changed files with 28 additions and 23 deletions

View File

@ -306,7 +306,7 @@ bool SCH_EDIT_FRAME::BreakSegment( SCH_LINE* aSegment, const wxPoint& aPoint,
SaveCopyInUndoList( aScreen, newSegment, UNDO_REDO::NEWITEM, true ); SaveCopyInUndoList( aScreen, newSegment, UNDO_REDO::NEWITEM, true );
SaveCopyInUndoList( aScreen, aSegment, UNDO_REDO::CHANGED, true ); SaveCopyInUndoList( aScreen, aSegment, UNDO_REDO::CHANGED, true );
UpdateItem( aSegment ); UpdateItem( aSegment, false, true );
aSegment->SetEndPoint( aPoint ); aSegment->SetEndPoint( aPoint );
if( aNewSegment ) if( aNewSegment )

View File

@ -542,7 +542,7 @@ void DIALOG_SCH_FIELD_PROPERTIES::UpdateField( SCH_FIELD* aField, SCH_SHEET_PATH
else else
otherUnit->GetField( DATASHEET_FIELD )->SetText( m_text ); otherUnit->GetField( DATASHEET_FIELD )->SetText( m_text );
editFrame->UpdateItem( otherUnit ); editFrame->UpdateItem( otherUnit, false, true );
} }
} }
} }

View File

@ -175,7 +175,7 @@ bool DIALOG_LINE_WIRE_BUS_PROPERTIES::TransferDataFromWindow()
stroke.SetColor( m_colorSwatch->GetSwatchColor() ); stroke.SetColor( m_colorSwatch->GetSwatchColor() );
strokeItem->SetStroke( stroke ); strokeItem->SetStroke( stroke );
m_frame->UpdateItem( strokeItem ); m_frame->UpdateItem( strokeItem, false, true );
} }
m_frame->GetCanvas()->Refresh(); m_frame->GetCanvas()->Refresh();

View File

@ -125,7 +125,7 @@ bool DIALOG_SHEET_PIN_PROPERTIES::TransferDataFromWindow()
auto shape = static_cast<PINSHEETLABEL_SHAPE>( m_choiceConnectionType->GetCurrentSelection() ); auto shape = static_cast<PINSHEETLABEL_SHAPE>( m_choiceConnectionType->GetCurrentSelection() );
m_sheetPin->SetShape( shape ); m_sheetPin->SetShape( shape );
m_frame->UpdateItem( m_sheetPin ); m_frame->UpdateItem( m_sheetPin, false, true );
m_frame->GetCanvas()->Refresh(); m_frame->GetCanvas()->Refresh();
m_frame->OnModify(); m_frame->OnModify();

View File

@ -730,7 +730,7 @@ bool DIALOG_SYMBOL_PROPERTIES::TransferDataFromWindow()
otherUnit->GetField( DATASHEET_FIELD )->SetText( m_fields->at( DATASHEET_FIELD ).GetText() ); otherUnit->GetField( DATASHEET_FIELD )->SetText( m_fields->at( DATASHEET_FIELD ).GetText() );
otherUnit->SetIncludeInBom( !m_cbExcludeFromBom->IsChecked() ); otherUnit->SetIncludeInBom( !m_cbExcludeFromBom->IsChecked() );
otherUnit->SetIncludeOnBoard( !m_cbExcludeFromBoard->IsChecked() ); otherUnit->SetIncludeOnBoard( !m_cbExcludeFromBoard->IsChecked() );
GetParent()->UpdateItem( otherUnit ); GetParent()->UpdateItem( otherUnit, false, true );
} }
} }
} }
@ -748,7 +748,7 @@ bool DIALOG_SYMBOL_PROPERTIES::TransferDataFromWindow()
currentScreen->Append( m_symbol ); currentScreen->Append( m_symbol );
GetParent()->TestDanglingEnds(); GetParent()->TestDanglingEnds();
GetParent()->UpdateItem( m_symbol ); GetParent()->UpdateItem( m_symbol, false, true );
GetParent()->OnModify(); GetParent()->OnModify();
// This must go after OnModify() so that the connectivity graph will have been updated. // This must go after OnModify() so that the connectivity graph will have been updated.

View File

@ -380,7 +380,7 @@ bool DIALOG_TEXT_AND_LABEL_PROPERTIES::TransferDataFromWindow()
} }
} }
m_Parent->UpdateItem( m_CurrentText ); m_Parent->UpdateItem( m_CurrentText, false, true );
m_Parent->GetCanvas()->Refresh(); m_Parent->GetCanvas()->Refresh();
m_Parent->OnModify(); m_Parent->OnModify();

View File

@ -226,7 +226,7 @@ void SCH_EDIT_FRAME::SelectUnit( SCH_SYMBOL* aSymbol, int aUnit )
TestDanglingEnds(); TestDanglingEnds();
UpdateItem( aSymbol ); UpdateItem( aSymbol, false, true );
OnModify(); OnModify();
} }
} }
@ -270,6 +270,6 @@ void SCH_EDIT_FRAME::ConvertPart( SCH_SYMBOL* aSymbol )
if( aSymbol->IsSelected() ) if( aSymbol->IsSelected() )
m_toolManager->RunAction( EE_ACTIONS::addItemToSel, true, aSymbol ); m_toolManager->RunAction( EE_ACTIONS::addItemToSel, true, aSymbol );
UpdateItem( aSymbol ); UpdateItem( aSymbol, false, true );
OnModify(); OnModify();
} }

View File

@ -299,7 +299,7 @@ void SCH_BASE_FRAME::createCanvas()
} }
void SCH_BASE_FRAME::UpdateItem( EDA_ITEM* aItem, bool isAddOrDelete ) void SCH_BASE_FRAME::UpdateItem( EDA_ITEM* aItem, bool isAddOrDelete, bool aUpdateRtree )
{ {
EDA_ITEM* parent = aItem->GetParent(); EDA_ITEM* parent = aItem->GetParent();
@ -321,6 +321,11 @@ void SCH_BASE_FRAME::UpdateItem( EDA_ITEM* aItem, bool isAddOrDelete )
GetCanvas()->GetView()->Update( parent, KIGFX::REPAINT ); GetCanvas()->GetView()->Update( parent, KIGFX::REPAINT );
} }
/**
* Be careful when calling this. Update will invalidate RTree iterators, so you cannot call this
* while doing things like `for( SCH_ITEM* item : screen->Items() )`
*/
if( aUpdateRtree)
GetScreen()->Update( static_cast<SCH_ITEM*>( aItem ) ); GetScreen()->Update( static_cast<SCH_ITEM*>( aItem ) );
// Calling Refresh() here introduces a bi-stable state: when doing operations on a // Calling Refresh() here introduces a bi-stable state: when doing operations on a

View File

@ -213,7 +213,7 @@ public:
/** /**
* Mark an item for refresh. * Mark an item for refresh.
*/ */
void UpdateItem( EDA_ITEM* aItem, bool isAddOrDelete = false ); void UpdateItem( EDA_ITEM* aItem, bool isAddOrDelete = false, bool aUpdateRtree = false );
/** /**
* Mark selected items for refresh. * Mark selected items for refresh.

View File

@ -551,7 +551,7 @@ int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
} }
connections = head->IsConnectable(); connections = head->IsConnectable();
m_frame->UpdateItem( head ); m_frame->UpdateItem( head, false, true );
} }
else else
{ {
@ -627,7 +627,7 @@ int SCH_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
} }
connections |= item->IsConnectable(); connections |= item->IsConnectable();
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
updateItem( item, true ); updateItem( item, true );
} }
@ -760,7 +760,7 @@ int SCH_EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
} }
connections = item->IsConnectable(); connections = item->IsConnectable();
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
} }
else if( selection.GetSize() > 1 ) else if( selection.GetSize() > 1 )
{ {
@ -815,7 +815,7 @@ int SCH_EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
} }
connections |= item->IsConnectable(); connections |= item->IsConnectable();
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
} }
} }
@ -1134,7 +1134,7 @@ void SCH_EDIT_TOOL::editFieldText( SCH_FIELD* aField )
if( m_frame->eeconfig()->m_AutoplaceFields.enable || parentType == SCH_SHEET_T ) if( m_frame->eeconfig()->m_AutoplaceFields.enable || parentType == SCH_SHEET_T )
static_cast<SCH_ITEM*>( aField->GetParent() )->AutoAutoplaceFields( m_frame->GetScreen() ); static_cast<SCH_ITEM*>( aField->GetParent() )->AutoAutoplaceFields( m_frame->GetScreen() );
m_frame->UpdateItem( aField ); m_frame->UpdateItem( aField, false, true );
m_frame->OnModify(); m_frame->OnModify();
// This must go after OnModify() so that the connectivity graph will have been updated. // This must go after OnModify() so that the connectivity graph will have been updated.

View File

@ -544,7 +544,7 @@ int SCH_EDITOR_CONTROL::ReplaceAndFindNext( const TOOL_EVENT& aEvent )
{ {
if( item->Replace( data, sheet ) ) if( item->Replace( data, sheet ) )
{ {
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
m_frame->GetCurrentSheet().UpdateAllScreenReferences(); m_frame->GetCurrentSheet().UpdateAllScreenReferences();
m_frame->OnModify(); m_frame->OnModify();
} }
@ -575,7 +575,7 @@ int SCH_EDITOR_CONTROL::ReplaceAll( const TOOL_EVENT& aEvent )
{ {
if( item->Replace( data, sheet ) ) if( item->Replace( data, sheet ) )
{ {
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
modified = true; modified = true;
} }

View File

@ -705,7 +705,7 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const std::string& aTool, int aType,
m_busUnfold.flipY = flipY; m_busUnfold.flipY = flipY;
m_busUnfold.flipX = flipX; m_busUnfold.flipX = flipX;
m_frame->UpdateItem( entry ); m_frame->UpdateItem( entry, false, true );
m_wires.front()->SetStartPoint( entry->GetEnd() ); m_wires.front()->SetStartPoint( entry->GetEnd() );
} }

View File

@ -167,7 +167,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Rotate( const TOOL_EVENT& aEvent )
{ {
item = static_cast<LIB_ITEM*>( selection.GetItem( ii ) ); item = static_cast<LIB_ITEM*>( selection.GetItem( ii ) );
item->Rotate( rotPoint, ccw ); item->Rotate( rotPoint, ccw );
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
} }
if( item->IsMoving() ) if( item->IsMoving() )
@ -216,7 +216,7 @@ int SYMBOL_EDITOR_EDIT_TOOL::Mirror( const TOOL_EVENT& aEvent )
else else
item->MirrorHorizontal( mirrorPoint ); item->MirrorHorizontal( mirrorPoint );
m_frame->UpdateItem( item ); m_frame->UpdateItem( item, false, true );
} }
m_toolMgr->PostEvent( EVENTS::SelectedItemsModified ); m_toolMgr->PostEvent( EVENTS::SelectedItemsModified );

View File

@ -194,7 +194,7 @@ bool SYMBOL_EDITOR_PIN_TOOL::EditPinProperties( LIB_PIN* aPin )
} }
} }
m_frame->UpdateItem( aPin ); m_frame->UpdateItem( aPin, false, true );
m_frame->OnModify( ); m_frame->OnModify( );
std::vector<MSG_PANEL_ITEM> items; std::vector<MSG_PANEL_ITEM> items;