From 3257f1a8634ee69fd7bc516c8905c44ffb7f395f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20W=C5=82ostowski?= Date: Thu, 29 Sep 2016 18:59:11 +0200 Subject: [PATCH] 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 | 2 - pcbnew/router/pns_router.h | 14 ----- pcbnew/router/pns_tool_base.cpp | 97 +++++++++------------------------ pcbnew/router/pns_tool_base.h | 3 +- pcbnew/router/router_tool.cpp | 17 ++++-- 5 files changed, 38 insertions(+), 95 deletions(-) diff --git a/pcbnew/router/pns_router.cpp b/pcbnew/router/pns_router.cpp index da30f9ac96..69809699cb 100644 --- a/pcbnew/router/pns_router.cpp +++ b/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; } diff --git a/pcbnew/router/pns_router.h b/pcbnew/router/pns_router.h index 3f732bc9e7..5454243b67 100644 --- a/pcbnew/router/pns_router.h +++ b/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; diff --git a/pcbnew/router/pns_tool_base.cpp b/pcbnew/router/pns_tool_base.cpp index f65649f5e5..083cb7284c 100644 --- a/pcbnew/router/pns_tool_base.cpp +++ b/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_iface = NULL; - m_router = NULL; + m_gridHelper = nullptr; + m_iface = nullptr; + m_router = nullptr; - m_startItem = NULL; + m_startItem = nullptr; m_startLayer = 0; - m_endItem = NULL; + m_endItem = nullptr; - m_frame = NULL; - m_ctls = NULL; - m_board = NULL; - m_gridHelper = NULL; + m_frame = nullptr; + m_ctls = nullptr; + m_board = nullptr; + 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_router->EnableSnapping( snapEnabled ); + m_startItem = pickSingleItem( p ); - if( !snapEnabled && startItem && !startItem->Layers().Overlaps( tl ) ) - startItem = NULL; + if( !snapEnabled && m_startItem && !m_startItem->Layers().Overlaps( tl ) ) + m_startItem = nullptr; - if( startItem && startItem->Net() >= 0 ) - { - 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 ); - } + m_startSnapPoint = snapToItem( snapEnabled, m_startItem, p ); + m_ctls->ForceCursorPosition( true, m_startSnapPoint ); } @@ -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 = cp; + m_endSnapPoint = snapToItem( snapEnabled, nullptr, p ); + 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 nets = m_router->GetCurrentNets(); @@ -294,19 +268,10 @@ void TOOL_BASE::updateEndItem( TOOL_EVENT& aEvent ) break; } - if( endItem ) - { - VECTOR2I cursorPos = snapToItem( endItem, p, dummy ); - m_ctls->ForceCursorPosition( true, cursorPos ); - m_endItem = endItem; - m_endSnapPoint = cursorPos; - } - else - { - m_endItem = NULL; - m_endSnapPoint = cp; - m_ctls->ForceCursorPosition( false ); - } + VECTOR2I cursorPos = snapToItem( snapEnabled, endItem, p ); + m_ctls->ForceCursorPosition( true, cursorPos ); + m_endItem = endItem; + m_endSnapPoint = cursorPos; 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 aP; + return m_gridHelper->Align( aP ); } switch( aItem->Kind() ) { case ITEM::SOLID_T: anchor = static_cast( aItem )->Pos(); - aSplitsSegment = false; break; case ITEM::VIA_T: anchor = static_cast( 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; } diff --git a/pcbnew/router/pns_tool_base.h b/pcbnew/router/pns_tool_base.h index 96ab237939..fcd63043e2 100644 --- a/pcbnew/router/pns_tool_base.h +++ b/pcbnew/router/pns_tool_base.h @@ -59,8 +59,7 @@ public: ROUTER* Router() const; protected: - - const VECTOR2I snapToItem( ITEM* aItem, VECTOR2I aP, bool& aSplitsSegment ); + const VECTOR2I snapToItem( bool aEnabled, ITEM* aItem, VECTOR2I aP); 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 ); diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp index f7382acc78..b373c85d62 100644 --- a/pcbnew/router/router_tool.cpp +++ b/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; } }