pns: fixed inline drag grid snapping & undo-related assertion failure

Fixes: lp:1628697
* https://bugs.launchpad.net/kicad/+bug/1628697

pcbnew/router/pns_router.cpp

@@ -77,8 +77,6 @@ ROUTER::ROUTER()
     m_iterLimit = 0;
     m_showInterSteps = false;
     m_snapshotIter = 0;
-    m_view = nullptr;
-    m_snappingEnabled  = false;
     m_violation = false;
 }
 

pcbnew/router/pns_router.h

@@ -194,16 +194,6 @@ public:
         m_settings = aSettings;
     }
 
-    void EnableSnapping( bool aEnable )
-    {
-        m_snappingEnabled = aEnable;
-    }
-
-    bool SnappingEnabled() const
-    {
-        return m_snappingEnabled;
-    }
-
     SIZES_SETTINGS& Sizes()
     {
         return m_sizes;
@@ -262,10 +252,6 @@ private:
     int m_iterLimit;
     bool m_showInterSteps;
     int m_snapshotIter;
-
-    KIGFX::VIEW* m_view;
-
-    bool m_snappingEnabled;
     bool m_violation;
 
     ROUTING_SETTINGS m_settings;

pcbnew/router/pns_tool_base.cpp

@@ -69,19 +69,19 @@ TOOL_ACTION TOOL_BASE::ACT_RouterOptions( "pcbnew.InteractiveRouter.RouterOption
 TOOL_BASE::TOOL_BASE( const std::string& aToolName ) :
     TOOL_INTERACTIVE( aToolName )
 {
-    m_gridHelper = NULL;
+    m_gridHelper = nullptr;
-    m_iface = NULL;
+    m_iface = nullptr;
-    m_router = NULL;
+    m_router = nullptr;
 
-    m_startItem = NULL;
+    m_startItem = nullptr;
     m_startLayer = 0;
 
-    m_endItem = NULL;
+    m_endItem = nullptr;
 
-    m_frame = NULL;
+    m_frame = nullptr;
-    m_ctls = NULL;
+    m_ctls = nullptr;
-    m_board = NULL;
+    m_board = nullptr;
-    m_gridHelper = NULL;
+    m_gridHelper = nullptr;
 }
 
 
@@ -212,7 +212,6 @@ void TOOL_BASE::updateStartItem( TOOL_EVENT& aEvent )
     VECTOR2I cp = m_ctls->GetCursorPosition();
     VECTOR2I p;
 
-    ITEM* startItem = NULL;
     bool snapEnabled = true;
 
     if( aEvent.IsMotion() || aEvent.IsClick() )
@@ -225,36 +224,13 @@ void TOOL_BASE::updateStartItem( TOOL_EVENT& aEvent )
         p = cp;
     }
 
-    startItem = pickSingleItem( p );
+    m_startItem = pickSingleItem( p );
-    m_router->EnableSnapping( snapEnabled );
 
-    if( !snapEnabled && startItem && !startItem->Layers().Overlaps( tl ) )
+    if( !snapEnabled && m_startItem && !m_startItem->Layers().Overlaps( tl ) )
-        startItem = NULL;
+        m_startItem = nullptr;
 
-    if( startItem && startItem->Net() >= 0 )
+    m_startSnapPoint = snapToItem( snapEnabled, m_startItem, p );
-    {
+    m_ctls->ForceCursorPosition( true, m_startSnapPoint );
-        bool dummy;
-        VECTOR2I psnap = snapToItem( startItem, p, dummy );
-
-        if( snapEnabled )
-        {
-            m_startSnapPoint = psnap;
-            m_ctls->ForceCursorPosition( true, psnap );
-        }
-        else
-        {
-            m_startSnapPoint = cp;
-            m_ctls->ForceCursorPosition( false );
-        }
-
-        m_startItem = startItem;
-    }
-    else
-    {
-        m_startItem = NULL;
-        m_startSnapPoint = cp;
-        m_ctls->ForceCursorPosition( false );
-    }
 }
 
 
@@ -266,23 +242,21 @@ void TOOL_BASE::updateEndItem( TOOL_EVENT& aEvent )
     int layer;
     bool snapEnabled = !aEvent.Modifier( MD_SHIFT );
 
-    m_router->EnableSnapping( snapEnabled );
-
     if( m_router->GetCurrentNets().empty() || m_router->GetCurrentNets().front() < 0 )
     {
-        m_endItem = NULL;
+        m_endSnapPoint = snapToItem( snapEnabled, nullptr, p );
-        m_endSnapPoint = cp;
+        m_ctls->ForceCursorPosition( true, m_endSnapPoint );
+        m_endItem = nullptr;
+
         return;
     }
 
-    bool dummy;
-
     if( m_router->IsPlacingVia() )
         layer = -1;
     else
         layer = m_router->GetCurrentLayer();
 
-    ITEM* endItem = NULL;
+    ITEM* endItem = nullptr;
 
     std::vector<int> nets = m_router->GetCurrentNets();
 
@@ -294,19 +268,10 @@ void TOOL_BASE::updateEndItem( TOOL_EVENT& aEvent )
             break;
     }
 
-    if( endItem )
+    VECTOR2I cursorPos = snapToItem( snapEnabled, endItem, p );
-    {
+    m_ctls->ForceCursorPosition( true, cursorPos );
-        VECTOR2I cursorPos = snapToItem( endItem, p, dummy );
+    m_endItem = endItem;
-        m_ctls->ForceCursorPosition( true, cursorPos );
+    m_endSnapPoint = cursorPos;
-        m_endItem = endItem;
-        m_endSnapPoint = cursorPos;
-    }
-    else
-    {
-        m_endItem = NULL;
-        m_endSnapPoint = cp;
-        m_ctls->ForceCursorPosition( false );
-    }
 
     if( m_endItem )
     {
@@ -345,26 +310,23 @@ ROUTER *TOOL_BASE::Router() const
 }
 
 
-const VECTOR2I TOOL_BASE::snapToItem( ITEM* aItem, VECTOR2I aP, bool& aSplitsSegment )
+const VECTOR2I TOOL_BASE::snapToItem( bool aEnabled, ITEM* aItem, VECTOR2I aP)
 {
     VECTOR2I anchor;
 
-    if( !aItem )
+    if( !aItem || !aEnabled )
     {
-        aSplitsSegment = false;
+        return m_gridHelper->Align( aP );
-        return aP;
     }
 
     switch( aItem->Kind() )
     {
     case ITEM::SOLID_T:
         anchor = static_cast<SOLID*>( aItem )->Pos();
-        aSplitsSegment = false;
         break;
 
     case ITEM::VIA_T:
         anchor = static_cast<VIA*>( aItem )->Pos();
-        aSplitsSegment = false;
         break;
 
     case ITEM::SEGMENT_T:
@@ -373,20 +335,13 @@ const VECTOR2I TOOL_BASE::snapToItem( ITEM* aItem, VECTOR2I aP, bool& aSplitsSeg
         const SEG& s = seg->Seg();
         int w = seg->Width();
 
-        aSplitsSegment = false;
 
         if( ( aP - s.A ).EuclideanNorm() < w / 2 )
             anchor = s.A;
         else if( ( aP - s.B ).EuclideanNorm() < w / 2 )
             anchor = s.B;
         else
-        {
-            anchor = s.NearestPoint( aP );
-            aSplitsSegment = true;
-
             anchor = m_gridHelper->AlignToSegment( aP, s );
-            aSplitsSegment = ( anchor != s.A && anchor != s.B );
-        }
 
         break;
     }

pcbnew/router/pns_tool_base.h

@@ -59,8 +59,7 @@ public:
     ROUTER* Router() const;
 
 protected:
-
+    const VECTOR2I snapToItem( bool aEnabled, ITEM* aItem, VECTOR2I aP);
-    const VECTOR2I snapToItem( ITEM* aItem, VECTOR2I aP, bool& aSplitsSegment );
     virtual ITEM* pickSingleItem( const VECTOR2I& aWhere, int aNet = -1, int aLayer = -1 );
     virtual void highlightNet( bool aEnabled, int aNetcode = -1 );
     virtual void updateStartItem( TOOL_EVENT& aEvent );

pcbnew/router/router_tool.cpp

@@ -487,7 +487,6 @@ bool ROUTER_TOOL::prepareInteractive()
 {
     int routingLayer = getStartLayer( m_startItem );
     m_frame->SetActiveLayer( ToLAYER_ID( routingLayer ) );
-    m_frame->UndoRedoBlock( true );
 
     // fixme: switch on invisible layer
 
@@ -524,7 +523,7 @@ bool ROUTER_TOOL::prepareInteractive()
     m_endItem = NULL;
     m_endSnapPoint = m_startSnapPoint;
 
-    m_frame->UndoRedoBlock( false );
+    m_frame->UndoRedoBlock( true );
 
     return true;
 }
@@ -536,6 +535,7 @@ bool ROUTER_TOOL::finishInteractive()
 
     m_ctls->SetAutoPan( false );
     m_ctls->ForceCursorPosition( false );
+    m_frame->UndoRedoBlock( false );
     highlightNet( false );
 
     return true;
@@ -670,7 +670,6 @@ int ROUTER_TOOL::mainLoop( PNS::ROUTER_MODE aMode )
 
     m_router->SetMode( aMode );
 
-    m_ctls->SetSnapping( true );
     m_ctls->ShowCursor( true );
 
     m_startSnapPoint = getViewControls()->GetCursorPosition();
@@ -760,6 +759,8 @@ void ROUTER_TOOL::performDragging()
 
     ctls->SetAutoPan( true );
 
+    m_frame->UndoRedoBlock( true );
+
     while( OPT_TOOL_EVENT evt = Wait() )
     {
         ctls->ForceCursorPosition( false );
@@ -785,6 +786,7 @@ void ROUTER_TOOL::performDragging()
 
     m_startItem = NULL;
 
+    m_frame->UndoRedoBlock( false );
     ctls->SetAutoPan( false );
     ctls->ForceCursorPosition( false );
     highlightNet( false );
@@ -813,6 +815,8 @@ int ROUTER_TOOL::InlineDrag( const TOOL_EVENT& aEvent )
 
     VECTOR2I p0 = ctls->GetCursorPosition();
 
+    printf("StartDrag : %p\n", m_startItem );
+
     bool dragStarted = m_router->StartDragging( p0, m_startItem );
 
     if( !dragStarted )
@@ -824,7 +828,6 @@ int ROUTER_TOOL::InlineDrag( const TOOL_EVENT& aEvent )
 
     while( OPT_TOOL_EVENT evt = Wait() )
     {
-        p0 = ctls->GetCursorPosition();
 
         if( evt->IsCancel() )
         {
@@ -832,11 +835,13 @@ int ROUTER_TOOL::InlineDrag( const TOOL_EVENT& aEvent )
         }
         else if( evt->IsMotion() || evt->IsDrag( BUT_LEFT ) )
         {
-            m_router->Move( p0, NULL );
+            updateEndItem( *evt );
+            m_router->Move( m_endSnapPoint, m_endItem );
         }
         else if( evt->IsMouseUp( BUT_LEFT ) || evt->IsClick( BUT_LEFT ) )
         {
-            m_router->FixRoute( p0, NULL );
+            updateEndItem( *evt );
+            m_router->FixRoute( m_endSnapPoint, m_endItem );
             break;
         }
     }