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
This commit is contained in:
Jeff Young 2020-11-30 19:27:31 +00:00
parent d28fa6c492
commit 3ad5bce67f
2 changed files with 45 additions and 46 deletions

View File

@ -150,10 +150,10 @@ public:
case SCH_LINE_T: case SCH_LINE_T:
{ {
SCH_LINE* line = (SCH_LINE*) aItem; SCH_LINE* line = (SCH_LINE*) aItem;
SCH_LINE* connectedStart = nullptr; std::pair<EDA_ITEM*, int> connectedStart = { nullptr, STARTPOINT };
SCH_LINE* connectedEnd = nullptr; std::pair<EDA_ITEM*, int> 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 ) if( test->GetLayer() != LAYER_NOTES )
continue; continue;
@ -161,28 +161,23 @@ public:
if( test == aItem ) if( test == aItem )
continue; continue;
auto testLine = static_cast<SCH_LINE*>( test ); SCH_LINE* testLine = static_cast<SCH_LINE*>( test );
testLine->ClearFlags( STARTPOINT | ENDPOINT );
if( testLine->GetStartPoint() == line->GetStartPoint() ) if( testLine->GetStartPoint() == line->GetStartPoint() )
{ {
connectedStart = testLine; connectedStart = { testLine, STARTPOINT };
testLine->SetFlags( STARTPOINT );
} }
else if( testLine->GetEndPoint() == line->GetStartPoint() ) else if( testLine->GetEndPoint() == line->GetStartPoint() )
{ {
connectedStart = testLine; connectedStart = { testLine, ENDPOINT };
testLine->SetFlags( ENDPOINT );
} }
else if( testLine->GetStartPoint() == line->GetEndPoint() ) else if( testLine->GetStartPoint() == line->GetEndPoint() )
{ {
connectedEnd = testLine; connectedEnd = { testLine, STARTPOINT };
testLine->SetFlags( STARTPOINT );
} }
else if( testLine->GetEndPoint() == line->GetEndPoint() ) else if( testLine->GetEndPoint() == line->GetEndPoint() )
{ {
connectedEnd = testLine; connectedEnd = { testLine, ENDPOINT };
testLine->SetFlags( ENDPOINT );
} }
} }
@ -612,28 +607,28 @@ void EE_POINT_EDITOR::updateParentItem() const
line->SetStartPoint( (wxPoint) m_editPoints->Point( LINE_START ).GetPosition() ); line->SetStartPoint( (wxPoint) m_editPoints->Point( LINE_START ).GetPosition() );
line->SetEndPoint( (wxPoint) m_editPoints->Point( LINE_END ).GetPosition() ); line->SetEndPoint( (wxPoint) m_editPoints->Point( LINE_END ).GetPosition() );
SCH_LINE* connection = (SCH_LINE*) ( m_editPoints->Point( LINE_START ).GetConnection() ); std::pair<EDA_ITEM*, int> connected = m_editPoints->Point( LINE_START ).GetConnected();
if( connection ) if( connected.first )
{ {
if( connection->HasFlag( STARTPOINT ) ) if( connected.second == STARTPOINT )
connection->SetStartPoint( line->GetPosition() ); static_cast<SCH_LINE*>( connected.first )->SetStartPoint( line->GetPosition() );
else if( connection->HasFlag( ENDPOINT ) ) else if( connected.second == ENDPOINT )
connection->SetEndPoint( line->GetPosition() ); static_cast<SCH_LINE*>( 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 ) ) if( connected.second == STARTPOINT )
connection->SetStartPoint( line->GetEndPoint() ); static_cast<SCH_LINE*>( connected.first )->SetStartPoint( line->GetEndPoint() );
else if( connection->HasFlag( ENDPOINT ) ) else if( connected.second == ENDPOINT )
connection->SetEndPoint( line->GetEndPoint() ); static_cast<SCH_LINE*>( connected.first )->SetEndPoint( line->GetEndPoint() );
getView()->Update( connection, KIGFX::GEOMETRY ); getView()->Update( connected.first, KIGFX::GEOMETRY );
} }
break; break;
@ -882,15 +877,15 @@ void EE_POINT_EDITOR::saveItemsToUndo()
if( m_editPoints->GetParent()->Type() == SCH_LINE_T ) if( m_editPoints->GetParent()->Type() == SCH_LINE_T )
{ {
EDA_ITEM* connection = m_editPoints->Point( LINE_START ).GetConnection(); std::pair<EDA_ITEM*, int> connected = m_editPoints->Point( LINE_START ).GetConnected();
if( connection ) if( connected.first )
saveCopyInUndoList( (SCH_ITEM*) connection, UNDO_REDO::CHANGED, true ); 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 ) if( connected.first )
saveCopyInUndoList( (SCH_ITEM*) connection, UNDO_REDO::CHANGED, true ); saveCopyInUndoList( (SCH_ITEM*) connected.first, UNDO_REDO::CHANGED, true );
} }
} }
} }

View File

@ -51,12 +51,12 @@ public:
* *
* @param aPoint stores coordinates for EDIT_POINT. * @param aPoint stores coordinates for EDIT_POINT.
*/ */
EDIT_POINT( const VECTOR2I& aPoint, EDA_ITEM* aConnection = nullptr ) : EDIT_POINT( const VECTOR2I& aPoint, std::pair<EDA_ITEM*, int> aConnected = { nullptr, 0 } ) :
m_position( aPoint ), m_position( aPoint ),
m_connection( aConnection ), m_isActive( false ),
m_isActive( false ), m_isHover( false ),
m_isHover( false ), m_gridFree( false ),
m_gridFree( false ) m_connected( aConnected )
{ {
} }
@ -73,9 +73,12 @@ public:
return m_position; 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<EDA_ITEM*, int> GetConnected() const
{ {
return m_connection; return m_connected;
} }
/** /**
@ -204,13 +207,14 @@ public:
private: private:
VECTOR2I m_position; // Position of EDIT_POINT 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_isActive; // True if this point is being manipulated
bool m_isHover; // True if this point is being hovered over 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. 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<EDA_ITEM*, int> m_connected;
///> Constraint for the point, NULL if none ///> Constraint for the point, NULL if none
std::shared_ptr<EDIT_CONSTRAINT<EDIT_POINT> > m_constraint; std::shared_ptr<EDIT_CONSTRAINT<EDIT_POINT> > m_constraint;
}; };
@ -387,9 +391,9 @@ public:
* Adds an EDIT_POINT. * Adds an EDIT_POINT.
* @param aPoint are coordinates of the new 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<EDA_ITEM*, int> aConnected = { nullptr, 0 } )
{ {
AddPoint( EDIT_POINT( aPoint, aConnection ) ); AddPoint( EDIT_POINT( aPoint, aConnected ) );
} }
/** /**