From 90659088592cc80d5a6d753bc403382f922a4109 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 30 Oct 2020 15:15:20 +0000 Subject: [PATCH] Update SCH_SCREEN's RTree when moving items. Or when modifying geometric shape/properties. Fixes https://gitlab.com/kicad/code/kicad/issues/5922 --- eeschema/tools/ee_point_editor.cpp | 10 ++++----- eeschema/tools/ee_point_editor.h | 2 +- eeschema/tools/ee_tool_base.h | 35 ++++++++++++++++++++++++------ eeschema/tools/lib_edit_tool.cpp | 6 ++--- eeschema/tools/lib_move_tool.cpp | 4 ++-- eeschema/tools/sch_edit_tool.cpp | 11 ++++------ eeschema/tools/sch_move_tool.cpp | 29 +++++-------------------- 7 files changed, 49 insertions(+), 48 deletions(-) diff --git a/eeschema/tools/ee_point_editor.cpp b/eeschema/tools/ee_point_editor.cpp index b04db8d1f2..803db95267 100644 --- a/eeschema/tools/ee_point_editor.cpp +++ b/eeschema/tools/ee_point_editor.cpp @@ -321,7 +321,7 @@ int EE_POINT_EDITOR::Main( const TOOL_EVENT& aEvent ) m_editedPoint->SetPosition( controls->GetCursorPosition( snap ) ); - updateItem(); + updateParentItem(); updatePoints(); } @@ -464,7 +464,7 @@ static void pinEditedCorner( int aEditedPointIndex, int minWidth, int minHeight, } -void EE_POINT_EDITOR::updateItem() const +void EE_POINT_EDITOR::updateParentItem() const { EDA_ITEM* item = m_editPoints->GetParent(); @@ -637,7 +637,7 @@ void EE_POINT_EDITOR::updateItem() const break; } - updateView( item ); + updateItem( item, true ); m_frame->SetMsgPanel( item ); } @@ -831,7 +831,7 @@ int EE_POINT_EDITOR::addCorner( const TOOL_EVENT& aEvent ) VECTOR2I cursorPos = getViewControls()->GetCursorPosition( !aEvent.Modifier( MD_ALT ) ); polyLine->AddCorner( mapCoords( cursorPos ) ); - updateView( polyLine ); + updateItem( polyLine, true ); updatePoints(); return 0; @@ -850,7 +850,7 @@ int EE_POINT_EDITOR::removeCorner( const TOOL_EVENT& aEvent ) polyLine->RemoveCorner( getEditedPointIndex() ); - updateView( polyLine ); + updateItem( polyLine, true ); updatePoints(); return 0; diff --git a/eeschema/tools/ee_point_editor.h b/eeschema/tools/ee_point_editor.h index fb59a6e8c0..a9c50bd3af 100644 --- a/eeschema/tools/ee_point_editor.h +++ b/eeschema/tools/ee_point_editor.h @@ -61,7 +61,7 @@ public: private: ///> Updates item's points with edit points. - void updateItem() const; + void updateParentItem() const; ///> Updates edit points with item's points. void updatePoints(); diff --git a/eeschema/tools/ee_tool_base.h b/eeschema/tools/ee_tool_base.h index 1e5c457c9c..9b584aaefb 100644 --- a/eeschema/tools/ee_tool_base.h +++ b/eeschema/tools/ee_tool_base.h @@ -100,17 +100,38 @@ public: } protected: - ///> Similar to getView()->Update(), but handles items that are redrawn by their parents. - void updateView( EDA_ITEM* aItem ) const + /** + * Similar to getView()->Update(), but handles items that are redrawn by their parents + * and updating the SCH_SCREEN's RTree. + */ + void updateItem( EDA_ITEM* aItem, bool aUpdateRTree ) const { - KICAD_T itemType = aItem->Type(); - - if( itemType == SCH_PIN_T || itemType == SCH_FIELD_T || itemType == SCH_SHEET_PIN_T ) + switch( aItem->Type() ) + { + case SCH_SHEET_PIN_T: + getView()->Update( aItem ); getView()->Update( aItem->GetParent() ); - getView()->Update( aItem ); - } + // Moving sheet pins does not change the BBox. + break; + case SCH_PIN_T: + case SCH_FIELD_T: + getView()->Update( aItem ); + getView()->Update( aItem->GetParent() ); + + if( aUpdateRTree ) + m_frame->GetScreen()->Update( static_cast( aItem->GetParent() ) ); + + break; + + default: + getView()->Update( aItem ); + + if( aUpdateRTree ) + m_frame->GetScreen()->Update( static_cast( aItem ) ); + } + } ///> Similar to m_frame->SaveCopyInUndoList(), but handles items that are owned by their ///> parents. diff --git a/eeschema/tools/lib_edit_tool.cpp b/eeschema/tools/lib_edit_tool.cpp index 1d28b6a5ad..7f16e2ff30 100644 --- a/eeschema/tools/lib_edit_tool.cpp +++ b/eeschema/tools/lib_edit_tool.cpp @@ -453,7 +453,7 @@ void LIB_EDIT_TOOL::editGraphicProperties( LIB_ITEM* aItem ) else aItem->SetUnit( m_frame->GetUnit() ); - updateView( aItem ); + updateItem( aItem, true ); m_frame->GetCanvas()->Refresh(); m_frame->OnModify( ); @@ -477,7 +477,7 @@ void LIB_EDIT_TOOL::editTextProperties( LIB_ITEM* aItem ) if( dlg.ShowModal() != wxID_OK ) return; - updateView( aItem ); + updateItem( aItem, true ); m_frame->GetCanvas()->Refresh(); m_frame->OnModify( ); } @@ -524,7 +524,7 @@ void LIB_EDIT_TOOL::editFieldProperties( LIB_FIELD* aField ) } else { - updateView( aField ); + updateItem( aField, true ); m_frame->GetCanvas()->Refresh(); m_frame->OnModify(); m_frame->DisplayCmpDoc(); diff --git a/eeschema/tools/lib_move_tool.cpp b/eeschema/tools/lib_move_tool.cpp index bf580ab28c..ef06e6d079 100644 --- a/eeschema/tools/lib_move_tool.cpp +++ b/eeschema/tools/lib_move_tool.cpp @@ -151,7 +151,7 @@ int LIB_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) for( EDA_ITEM* item : selection ) { moveItem( item, delta ); - updateView( item ); + updateItem( item, false ); } m_anchorPos = m_cursor; @@ -191,7 +191,7 @@ int LIB_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) for( EDA_ITEM* item : selection ) { moveItem( item, delta ); - updateView( item ); + updateItem( item, false ); } m_toolMgr->PostEvent( EVENTS::SelectedItemsMoved ); diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 22dedf050b..670bf4d029 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1055,7 +1055,7 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) saveCopyInUndoList( item, UNDO_REDO::DELETED, appendToUndo ); appendToUndo = true; - updateView( sch_item ); + updateItem( sch_item, false ); if( sch_item->Type() == SCH_SHEET_PIN_T ) { @@ -1282,8 +1282,7 @@ int SCH_EDIT_TOOL::AutoplaceFields( const TOOL_EVENT& aEvent ) sheet->AutoplaceFields( m_frame->GetScreen(), /* aManual */ true ); } - m_frame->GetScreen()->Update( item ); - updateView( item ); + updateItem( item, true ); m_frame->OnModify(); if( selection.IsHover() ) @@ -1593,8 +1592,7 @@ int SCH_EDIT_TOOL::Properties( const TOOL_EVENT& aEvent ) wxFAIL_MSG( wxString( "Cannot edit schematic item type " ) + item->GetClass() ); } - m_frame->GetScreen()->Update( item ); - updateView( item ); + updateItem( item, true ); if( selection.IsHover() ) m_toolMgr->RunAction( EE_ACTIONS::clearSelection, true ); @@ -1741,8 +1739,7 @@ int SCH_EDIT_TOOL::CleanupSheetPins( const TOOL_EVENT& aEvent ) sheet->CleanupSheet(); - m_frame->GetScreen()->Update( sheet ); - updateView( sheet ); + updateItem( sheet, true ); m_frame->OnModify(); if( selection.IsHover() ) diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index 6f87507ce9..627b83b839 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -298,7 +298,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) continue; moveItem( item, delta ); - updateView( item ); + updateItem( item, false ); } m_anchorPos = m_cursor; @@ -355,7 +355,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) continue; moveItem( item, delta ); - updateView( item ); + updateItem( item, false ); } m_toolMgr->PostEvent( EVENTS::SelectedItemsMoved ); @@ -456,26 +456,9 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) } else { - // Moving items changes the RTree box bounds. + // One last update after exiting loop (for slower stuff, such as updating SCREEN's RTree). for( auto item : selection ) - { - switch( item->Type() ) - { - // Moving sheet pins does not change the BBox. - case SCH_SHEET_PIN_T: - break; - - // Moving fields should update the associated component - case SCH_FIELD_T: - if( item->GetParent() ) - m_frame->GetScreen()->Update( static_cast( item->GetParent() ) ); - - break; - - default: - m_frame->GetScreen()->Update( static_cast( item ) ); - } - } + updateItem( item, true ); // If we move items away from a junction, we _may_ want to add a junction there // to denote the state. @@ -802,7 +785,7 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) append_undo = true; moveItem( dritem, gridpt ); - updateView( dritem ); + updateItem( dritem, true ); } } @@ -845,7 +828,7 @@ int SCH_MOVE_TOOL::AlignElements( const TOOL_EVENT& aEvent ) append_undo = true; moveItem( dritem, most_common ); - updateView( dritem ); + updateItem( dritem, true ); } } }