From b97d65e7919e7d37ecc5f8fce54dc2ace66a1b91 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 6 May 2019 16:30:29 +0100 Subject: [PATCH] Fix crash bug when deleting sheet pin and disappearing bug when moving them. Fixes: lp:1827890 * https://bugs.launchpad.net/kicad/+bug/1827890 --- eeschema/bus-wire-junction.cpp | 4 ++-- eeschema/sch_base_frame.cpp | 10 ++++----- eeschema/sch_base_frame.h | 6 +++--- eeschema/sch_painter.cpp | 37 +++++++++++++++----------------- eeschema/tools/sch_edit_tool.cpp | 10 ++++----- eeschema/tools/sch_move_tool.cpp | 14 ++++++------ eeschema/tools/sch_move_tool.h | 2 +- 7 files changed, 39 insertions(+), 44 deletions(-) diff --git a/eeschema/bus-wire-junction.cpp b/eeschema/bus-wire-junction.cpp index 2a1b5e5284..0029ed2b76 100644 --- a/eeschema/bus-wire-junction.cpp +++ b/eeschema/bus-wire-junction.cpp @@ -123,8 +123,8 @@ bool SCH_EDIT_FRAME::TrimWire( const wxPoint& aStart, const wxPoint& aEnd, bool if( IsPointOnSegment( return_line->GetStartPoint(), return_line->GetEndPoint(), aStart ) ) line = return_line; - SaveCopyInUndoList( (SCH_ITEM*)line, UR_DELETED, aAppend ); - RemoveFromScreen( (SCH_ITEM*)line ); + SaveCopyInUndoList( line, UR_DELETED, aAppend ); + RemoveFromScreen( line ); aAppend = true; retval = true; diff --git a/eeschema/sch_base_frame.cpp b/eeschema/sch_base_frame.cpp index d771478430..06ace86eab 100644 --- a/eeschema/sch_base_frame.cpp +++ b/eeschema/sch_base_frame.cpp @@ -609,7 +609,7 @@ void SCH_BASE_FRAME::createCanvas() } -void SCH_BASE_FRAME::RefreshItem( SCH_ITEM* aItem, bool isAddOrDelete ) +void SCH_BASE_FRAME::RefreshItem( EDA_ITEM* aItem, bool isAddOrDelete ) { EDA_ITEM* parent = aItem->GetParent(); @@ -633,14 +633,14 @@ void SCH_BASE_FRAME::RefreshItem( SCH_ITEM* aItem, bool isAddOrDelete ) } -void SCH_BASE_FRAME::AddToScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen ) +void SCH_BASE_FRAME::AddToScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen ) { auto screen = aScreen; if( aScreen == nullptr ) screen = GetScreen(); - screen->Append( aItem ); + screen->Append( (SCH_ITEM*) aItem ); if( screen == GetScreen() ) { @@ -650,7 +650,7 @@ void SCH_BASE_FRAME::AddToScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen ) } -void SCH_BASE_FRAME::RemoveFromScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen ) +void SCH_BASE_FRAME::RemoveFromScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen ) { auto screen = aScreen; @@ -660,7 +660,7 @@ void SCH_BASE_FRAME::RemoveFromScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen ) if( screen == GetScreen() ) GetCanvas()->GetView()->Remove( aItem ); - screen->Remove( aItem ); + screen->Remove( (SCH_ITEM*) aItem ); if( screen == GetScreen() ) RefreshItem( aItem, true ); // handle any additional parent semantics diff --git a/eeschema/sch_base_frame.h b/eeschema/sch_base_frame.h index 8ed802d605..0f8def4006 100644 --- a/eeschema/sch_base_frame.h +++ b/eeschema/sch_base_frame.h @@ -306,18 +306,18 @@ public: * Add an item to the screen (and view) * aScreen is the screen the item is located on, if not the current screen */ - void AddToScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen = nullptr ); + void AddToScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen = nullptr ); /** * Remove an item from the screen (and view) * aScreen is the screen the item is located on, if not the current screen */ - void RemoveFromScreen( SCH_ITEM* aItem, SCH_SCREEN* aScreen = nullptr ); + void RemoveFromScreen( EDA_ITEM* aItem, SCH_SCREEN* aScreen = nullptr ); /** * Mark an item for refresh. */ - void RefreshItem( SCH_ITEM* aItem, bool isAddOrDelete = false ); + void RefreshItem( EDA_ITEM* aItem, bool isAddOrDelete = false ); /** * Mark all items for refresh. diff --git a/eeschema/sch_painter.cpp b/eeschema/sch_painter.cpp index f1452c709d..a0feca7788 100644 --- a/eeschema/sch_painter.cpp +++ b/eeschema/sch_painter.cpp @@ -1320,28 +1320,25 @@ void SCH_PAINTER::draw( SCH_SHEET *aSheet, int aLayer ) { for( auto& sheetPin : aSheet->GetPins() ) { - if( !sheetPin.IsMoving() ) + // For aesthetic reasons, the SHEET_PIN is drawn with a small offset + // of width / 2 + int width = aSheet->GetPenSize(); + wxPoint initial_pos = sheetPin.GetTextPos(); + wxPoint offset_pos = initial_pos; + + switch( sheetPin.GetEdge() ) { - // For aesthetic reasons, the SHEET_PIN is drawn with a small offset - // of width / 2 - int width = aSheet->GetPenSize(); - wxPoint initial_pos = sheetPin.GetTextPos(); - wxPoint offset_pos = initial_pos; - - switch( sheetPin.GetEdge() ) - { - case SCH_SHEET_PIN::SHEET_TOP_SIDE: offset_pos.y -= width / 2; break; - case SCH_SHEET_PIN::SHEET_BOTTOM_SIDE: offset_pos.y += width / 2; break; - case SCH_SHEET_PIN::SHEET_RIGHT_SIDE: offset_pos.x -= width / 2; break; - case SCH_SHEET_PIN::SHEET_LEFT_SIDE: offset_pos.x += width / 2; break; - default: break; - } - - sheetPin.SetTextPos( offset_pos ); - draw( static_cast( &sheetPin ), aLayer ); - m_gal->DrawLine( offset_pos, initial_pos ); - sheetPin.SetTextPos( initial_pos ); + case SCH_SHEET_PIN::SHEET_TOP_SIDE: offset_pos.y -= width / 2; break; + case SCH_SHEET_PIN::SHEET_BOTTOM_SIDE: offset_pos.y += width / 2; break; + case SCH_SHEET_PIN::SHEET_RIGHT_SIDE: offset_pos.x -= width / 2; break; + case SCH_SHEET_PIN::SHEET_LEFT_SIDE: offset_pos.x += width / 2; break; + default: break; } + + sheetPin.SetTextPos( offset_pos ); + draw( static_cast( &sheetPin ), aLayer ); + m_gal->DrawLine( offset_pos, initial_pos ); + sheetPin.SetTextPos( initial_pos ); } } } diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index 26a78ab651..44fcd57bf0 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -926,10 +926,7 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) saveCopyInUndoList( item, UR_DELETED, appendToUndo ); appendToUndo = true; - if( item->Type() == SCH_SHEET_PIN_T ) - static_cast( item->GetParent() )->RemovePin( (SCH_SHEET_PIN*) item ); - else - m_frame->RemoveFromScreen( (SCH_ITEM*) item ); + updateView( item ); SCH_ITEM* sch_item = dynamic_cast( item ); @@ -946,7 +943,10 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) } } - updateView( (SCH_ITEM*) item ); + if( item->Type() == SCH_SHEET_PIN_T ) + static_cast( item->GetParent() )->RemovePin( (SCH_SHEET_PIN*) item ); + else + m_frame->RemoveFromScreen( item ); } } diff --git a/eeschema/tools/sch_move_tool.cpp b/eeschema/tools/sch_move_tool.cpp index afd90d011d..5a674f6b41 100644 --- a/eeschema/tools/sch_move_tool.cpp +++ b/eeschema/tools/sch_move_tool.cpp @@ -262,7 +262,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) // Apply any initial offset in case we're coming from a previous command. // - moveItem( (SCH_ITEM*) item, m_moveOffset, m_frame->GetToolId() == ID_SCH_DRAG ); + moveItem( item, m_moveOffset, m_frame->GetToolId() == ID_SCH_DRAG ); } // Set up the starting position and move/drag offset @@ -274,10 +274,8 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) VECTOR2I delta = m_cursor - selection.GetReferencePoint(); // Drag items to the current cursor position - for( int i = 0; i < selection.GetSize(); ++i ) + for( EDA_ITEM* item : selection ) { - SCH_ITEM* item = static_cast( selection.GetItem( i ) ); - // Don't double move pins, fields, etc. if( item->GetParent() && item->GetParent()->IsSelected() ) continue; @@ -323,7 +321,7 @@ int SCH_MOVE_TOOL::Main( const TOOL_EVENT& aEvent ) if( item->GetParent() && item->GetParent()->IsSelected() ) continue; - moveItem( (SCH_ITEM*) item, delta, m_frame->GetToolId() == ID_SCH_DRAG ); + moveItem( item, delta, m_frame->GetToolId() == ID_SCH_DRAG ); updateView( item ); } @@ -566,7 +564,7 @@ void SCH_MOVE_TOOL::addJunctionsIfNeeded( SELECTION& aSelection, bool* aAppendUn } -void SCH_MOVE_TOOL::moveItem( SCH_ITEM* aItem, VECTOR2I aDelta, bool isDrag ) +void SCH_MOVE_TOOL::moveItem( EDA_ITEM* aItem, VECTOR2I aDelta, bool isDrag ) { switch( aItem->Type() ) { @@ -581,11 +579,11 @@ void SCH_MOVE_TOOL::moveItem( SCH_ITEM* aItem, VECTOR2I aDelta, bool isDrag ) case SCH_PIN_T: case SCH_FIELD_T: - aItem->Move( wxPoint( aDelta.x, -aDelta.y ) ); + static_cast( aItem )->Move( wxPoint( aDelta.x, -aDelta.y ) ); break; default: - aItem->Move( (wxPoint) aDelta ); + static_cast( aItem )->Move( (wxPoint) aDelta ); break; } diff --git a/eeschema/tools/sch_move_tool.h b/eeschema/tools/sch_move_tool.h index 98b7fe1e6c..d0c53245f9 100644 --- a/eeschema/tools/sch_move_tool.h +++ b/eeschema/tools/sch_move_tool.h @@ -56,7 +56,7 @@ public: int Main( const TOOL_EVENT& aEvent ); private: - void moveItem( SCH_ITEM* aItem, VECTOR2I aDelta, bool isDrag ); + void moveItem( EDA_ITEM* aItem, VECTOR2I aDelta, bool isDrag ); ///> Finds additional items for a drag operation. ///> Connected items with no wire are included (as there is no wire to adjust for the drag).