From 3ad5bce67fb69db2dfa5000dde03f56097357404 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 30 Nov 2020 19:27:31 +0000 Subject: [PATCH] Rewrite connected-lines dragger to not use EDA_ITEM flags. We had some spurious bugs where both ends would get dragged to a single point (making them disappear). While I never caught it in the debugger, I'm guessing that the flags weren't getting cleared properly or were getting overwritten or something. Anyway, it now uses std::pair instead. Fixes https://gitlab.com/kicad/code/kicad/issues/6550 Fixes https://gitlab.com/kicad/code/kicad/issues/6431 --- eeschema/tools/ee_point_editor.cpp | 61 ++++++++++++++---------------- include/tool/edit_points.h | 30 ++++++++------- 2 files changed, 45 insertions(+), 46 deletions(-) diff --git a/eeschema/tools/ee_point_editor.cpp b/eeschema/tools/ee_point_editor.cpp index 07b403667d..8a30eedf51 100644 --- a/eeschema/tools/ee_point_editor.cpp +++ b/eeschema/tools/ee_point_editor.cpp @@ -150,10 +150,10 @@ public: case SCH_LINE_T: { SCH_LINE* line = (SCH_LINE*) aItem; - SCH_LINE* connectedStart = nullptr; - SCH_LINE* connectedEnd = nullptr; + std::pair connectedStart = { nullptr, STARTPOINT }; + std::pair connectedEnd = { nullptr, STARTPOINT }; - for( auto test : frame->GetScreen()->Items().OfType( SCH_LINE_T ) ) + for( SCH_ITEM* test : frame->GetScreen()->Items().OfType( SCH_LINE_T ) ) { if( test->GetLayer() != LAYER_NOTES ) continue; @@ -161,28 +161,23 @@ public: if( test == aItem ) continue; - auto testLine = static_cast( test ); - testLine->ClearFlags( STARTPOINT | ENDPOINT ); + SCH_LINE* testLine = static_cast( test ); if( testLine->GetStartPoint() == line->GetStartPoint() ) { - connectedStart = testLine; - testLine->SetFlags( STARTPOINT ); + connectedStart = { testLine, STARTPOINT }; } else if( testLine->GetEndPoint() == line->GetStartPoint() ) { - connectedStart = testLine; - testLine->SetFlags( ENDPOINT ); + connectedStart = { testLine, ENDPOINT }; } else if( testLine->GetStartPoint() == line->GetEndPoint() ) { - connectedEnd = testLine; - testLine->SetFlags( STARTPOINT ); + connectedEnd = { testLine, STARTPOINT }; } else if( testLine->GetEndPoint() == line->GetEndPoint() ) { - connectedEnd = testLine; - testLine->SetFlags( ENDPOINT ); + connectedEnd = { testLine, ENDPOINT }; } } @@ -612,28 +607,28 @@ void EE_POINT_EDITOR::updateParentItem() const line->SetStartPoint( (wxPoint) m_editPoints->Point( LINE_START ).GetPosition() ); line->SetEndPoint( (wxPoint) m_editPoints->Point( LINE_END ).GetPosition() ); - SCH_LINE* connection = (SCH_LINE*) ( m_editPoints->Point( LINE_START ).GetConnection() ); + std::pair connected = m_editPoints->Point( LINE_START ).GetConnected(); - if( connection ) + if( connected.first ) { - if( connection->HasFlag( STARTPOINT ) ) - connection->SetStartPoint( line->GetPosition() ); - else if( connection->HasFlag( ENDPOINT ) ) - connection->SetEndPoint( line->GetPosition() ); + if( connected.second == STARTPOINT ) + static_cast( connected.first )->SetStartPoint( line->GetPosition() ); + else if( connected.second == ENDPOINT ) + static_cast( connected.first )->SetEndPoint( line->GetPosition() ); - getView()->Update( connection, KIGFX::GEOMETRY ); + getView()->Update( connected.first, KIGFX::GEOMETRY ); } - connection = (SCH_LINE*) ( m_editPoints->Point( LINE_END ).GetConnection() ); + connected = m_editPoints->Point( LINE_END ).GetConnected(); - if( connection ) + if( connected.first ) { - if( connection->HasFlag( STARTPOINT ) ) - connection->SetStartPoint( line->GetEndPoint() ); - else if( connection->HasFlag( ENDPOINT ) ) - connection->SetEndPoint( line->GetEndPoint() ); + if( connected.second == STARTPOINT ) + static_cast( connected.first )->SetStartPoint( line->GetEndPoint() ); + else if( connected.second == ENDPOINT ) + static_cast( connected.first )->SetEndPoint( line->GetEndPoint() ); - getView()->Update( connection, KIGFX::GEOMETRY ); + getView()->Update( connected.first, KIGFX::GEOMETRY ); } break; @@ -882,15 +877,15 @@ void EE_POINT_EDITOR::saveItemsToUndo() if( m_editPoints->GetParent()->Type() == SCH_LINE_T ) { - EDA_ITEM* connection = m_editPoints->Point( LINE_START ).GetConnection(); + std::pair connected = m_editPoints->Point( LINE_START ).GetConnected(); - if( connection ) - saveCopyInUndoList( (SCH_ITEM*) connection, UNDO_REDO::CHANGED, true ); + if( connected.first ) + saveCopyInUndoList( (SCH_ITEM*) connected.first, UNDO_REDO::CHANGED, true ); - connection = m_editPoints->Point( LINE_END ).GetConnection(); + connected = m_editPoints->Point( LINE_END ).GetConnected(); - if( connection ) - saveCopyInUndoList( (SCH_ITEM*) connection, UNDO_REDO::CHANGED, true ); + if( connected.first ) + saveCopyInUndoList( (SCH_ITEM*) connected.first, UNDO_REDO::CHANGED, true ); } } } diff --git a/include/tool/edit_points.h b/include/tool/edit_points.h index b5be850400..ba3f4dcd60 100644 --- a/include/tool/edit_points.h +++ b/include/tool/edit_points.h @@ -51,12 +51,12 @@ public: * * @param aPoint stores coordinates for EDIT_POINT. */ - EDIT_POINT( const VECTOR2I& aPoint, EDA_ITEM* aConnection = nullptr ) : - m_position( aPoint ), - m_connection( aConnection ), - m_isActive( false ), - m_isHover( false ), - m_gridFree( false ) + EDIT_POINT( const VECTOR2I& aPoint, std::pair aConnected = { nullptr, 0 } ) : + m_position( aPoint ), + m_isActive( false ), + m_isHover( false ), + m_gridFree( false ), + m_connected( aConnected ) { } @@ -73,9 +73,12 @@ public: return m_position; } - virtual EDA_ITEM* GetConnection() const + /** + * Return a connected item record comprising an EDA_ITEM* and a STARTPOINT/ENDPOINT flag. + */ + virtual std::pair GetConnected() const { - return m_connection; + return m_connected; } /** @@ -204,13 +207,14 @@ public: private: VECTOR2I m_position; // Position of EDIT_POINT - - EDA_ITEM* m_connection; // An optional item to a connected item. Used to mimic - // polyLine behaviour with individual line segments. bool m_isActive; // True if this point is being manipulated bool m_isHover; // True if this point is being hovered over bool m_gridFree; // True if this point should not be snapped to the grid. + ///> An optional connected item record used to mimic polyLine behaviour with individual + /// line segments. + std::pair m_connected; + ///> Constraint for the point, NULL if none std::shared_ptr > m_constraint; }; @@ -387,9 +391,9 @@ public: * Adds an EDIT_POINT. * @param aPoint are coordinates of the new point. */ - void AddPoint( const VECTOR2I& aPoint, EDA_ITEM* aConnection = nullptr ) + void AddPoint( const VECTOR2I& aPoint, std::pair aConnected = { nullptr, 0 } ) { - AddPoint( EDIT_POINT( aPoint, aConnection ) ); + AddPoint( EDIT_POINT( aPoint, aConnected ) ); } /**