From 30336b2fe3fae5db1fca4c01038e523b1c9f9e34 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Mon, 19 Jun 2023 17:09:03 +0100 Subject: [PATCH] Unify go-back-one-step processing for drawing tools (and router). Also warps mouse on all go-back-one-step operations for better feedback. Fixes https://gitlab.com/kicad/code/kicad/-/issues/14981 Fixes https://gitlab.com/kicad/code/kicad/-/issues/9985 --- common/preview_items/polygon_geom_manager.cpp | 8 ++- eeschema/tools/sch_edit_tool.cpp | 2 - eeschema/tools/sch_line_wire_bus_tool.cpp | 11 ++- include/preview_items/polygon_geom_manager.h | 2 +- pcbnew/pcb_edit_frame.cpp | 5 ++ pcbnew/router/pns_line_placer.cpp | 14 ++-- pcbnew/router/pns_line_placer.h | 2 +- pcbnew/router/pns_placement_algo.h | 2 +- pcbnew/router/pns_router.cpp | 6 +- pcbnew/router/pns_router.h | 2 +- pcbnew/router/router_tool.cpp | 11 ++- pcbnew/tools/drawing_tool.cpp | 72 +++++++++++++------ pcbnew/tools/drawing_tool.h | 3 +- pcbnew/tools/edit_tool.cpp | 6 -- 14 files changed, 93 insertions(+), 53 deletions(-) diff --git a/common/preview_items/polygon_geom_manager.cpp b/common/preview_items/polygon_geom_manager.cpp index 126393586e..545077c9ac 100644 --- a/common/preview_items/polygon_geom_manager.cpp +++ b/common/preview_items/polygon_geom_manager.cpp @@ -129,10 +129,15 @@ bool POLYGON_GEOM_MANAGER::NewPointClosesOutline( const VECTOR2I& aPt ) const } -void POLYGON_GEOM_MANAGER::DeleteLastCorner() +std::optional POLYGON_GEOM_MANAGER::DeleteLastCorner() { + std::optional last; + if( m_lockedPoints.PointCount() > 0 ) + { + last = m_lockedPoints.GetPoint( m_lockedPoints.PointCount() - 1 ); m_lockedPoints.Remove( m_lockedPoints.PointCount() - 1 ); + } // update the new last segment (was previously // locked in), reusing last constraints @@ -140,6 +145,7 @@ void POLYGON_GEOM_MANAGER::DeleteLastCorner() updateTemporaryLines( m_leaderPts.CLastPoint() ); m_client.OnGeometryChange( *this ); + return last; } diff --git a/eeschema/tools/sch_edit_tool.cpp b/eeschema/tools/sch_edit_tool.cpp index d2277930a2..b5ca96325b 100644 --- a/eeschema/tools/sch_edit_tool.cpp +++ b/eeschema/tools/sch_edit_tool.cpp @@ -1428,8 +1428,6 @@ int SCH_EDIT_TOOL::DoDelete( const TOOL_EVENT& aEvent ) m_frame->DeleteJunction( &commit, junction ); } -// m_frame->TestDanglingEnds(); - m_frame->GetCanvas()->Refresh(); commit.Push( _( "Delete" ) ); diff --git a/eeschema/tools/sch_line_wire_bus_tool.cpp b/eeschema/tools/sch_line_wire_bus_tool.cpp index 4719033a69..cfa884c5c6 100644 --- a/eeschema/tools/sch_line_wire_bus_tool.cpp +++ b/eeschema/tools/sch_line_wire_bus_tool.cpp @@ -892,7 +892,9 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const TOOL_EVENT& aTool, int aType, m_view->AddToPreview( wire->Clone() ); } } - else if( evt->IsAction( &EE_ACTIONS::undoLastSegment ) || evt->IsAction( &ACTIONS::undo ) ) + else if( evt->IsAction( &EE_ACTIONS::undoLastSegment ) + || evt->IsAction( &ACTIONS::doDelete ) + || evt->IsAction( &ACTIONS::undo ) ) { if( ( currentMode == LINE_MODE::LINE_MODE_FREE && m_wires.size() > 1 ) || ( LINE_MODE::LINE_MODE_90 && m_wires.size() > 2 ) ) @@ -904,7 +906,8 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const TOOL_EVENT& aTool, int aType, delete segment; segment = m_wires.back(); - segment->SetEndPoint( cursorPos ); + cursorPos = segment->GetEndPoint(); + getViewControls()->WarpMouseCursor( cursorPos, true ); // Find new bend point for current mode if( twoSegments && m_wires.size() >= 2 ) @@ -1001,10 +1004,6 @@ int SCH_LINE_WIRE_BUS_TOOL::doDrawSegments( const TOOL_EVENT& aTool, int aType, wxBell(); } } - else if( evt->IsAction( &ACTIONS::doDelete ) && ( segment || m_busUnfold.in_progress ) ) - { - cleanup(); - } else if( evt->IsAction( &ACTIONS::redo ) ) { wxBell(); diff --git a/include/preview_items/polygon_geom_manager.h b/include/preview_items/polygon_geom_manager.h index e8913f77a4..e5c6724708 100644 --- a/include/preview_items/polygon_geom_manager.h +++ b/include/preview_items/polygon_geom_manager.h @@ -151,7 +151,7 @@ public: /** * Remove the last-added point from the polygon */ - void DeleteLastCorner(); + std::optional DeleteLastCorner(); /* ================================================================= * Interfaces for users of the geometry diff --git a/pcbnew/pcb_edit_frame.cpp b/pcbnew/pcb_edit_frame.cpp index a05bd72327..3c359a7aa9 100644 --- a/pcbnew/pcb_edit_frame.cpp +++ b/pcbnew/pcb_edit_frame.cpp @@ -689,6 +689,11 @@ void PCB_EDIT_FRAME::setupUIConditions() if( drawingTool && drawingTool->GetDrawingMode() != DRAWING_TOOL::MODE::NONE ) return true; + ROUTER_TOOL* routerTool = m_toolManager->GetTool(); + + if( routerTool && routerTool->RoutingInProgress() ) + return true; + return GetUndoCommandCount() > 0; }; diff --git a/pcbnew/router/pns_line_placer.cpp b/pcbnew/router/pns_line_placer.cpp index 007850e714..86e58a5f56 100644 --- a/pcbnew/router/pns_line_placer.cpp +++ b/pcbnew/router/pns_line_placer.cpp @@ -1701,14 +1701,16 @@ bool LINE_PLACER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinis } -bool LINE_PLACER::UnfixRoute() +std::optional LINE_PLACER::UnfixRoute() { - FIXED_TAIL::STAGE st; + FIXED_TAIL::STAGE st; + std::optional ret; if ( !m_fixedTail.PopStage( st ) ) - { - return false; - } + return ret; + + if( m_head.Line().PointCount() ) + ret = m_head.Line().CPoint( 0 ); m_head.Line().Clear(); m_tail.Line().Clear(); @@ -1740,7 +1742,7 @@ bool LINE_PLACER::UnfixRoute() m_lastNode = m_currentNode->Branch(); - return true; + return ret; } diff --git a/pcbnew/router/pns_line_placer.h b/pcbnew/router/pns_line_placer.h index 0fdd67c616..7bc1cc2267 100644 --- a/pcbnew/router/pns_line_placer.h +++ b/pcbnew/router/pns_line_placer.h @@ -108,7 +108,7 @@ public: */ bool FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinish ) override; - bool UnfixRoute() override; + std::optional UnfixRoute() override; bool CommitPlacement() override; diff --git a/pcbnew/router/pns_placement_algo.h b/pcbnew/router/pns_placement_algo.h index f1aafa30f3..2b926e9dc9 100644 --- a/pcbnew/router/pns_placement_algo.h +++ b/pcbnew/router/pns_placement_algo.h @@ -78,7 +78,7 @@ public: */ virtual bool FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinish = false ) = 0; - virtual bool UnfixRoute() { return false; }; + virtual std::optional UnfixRoute() { return std::nullopt; }; virtual bool CommitPlacement() { return false; }; diff --git a/pcbnew/router/pns_router.cpp b/pcbnew/router/pns_router.cpp index d0d99f0a3d..d4d38541ee 100644 --- a/pcbnew/router/pns_router.cpp +++ b/pcbnew/router/pns_router.cpp @@ -893,13 +893,13 @@ bool ROUTER::FixRoute( const VECTOR2I& aP, ITEM* aEndItem, bool aForceFinish ) } -void ROUTER::UndoLastSegment() +std::optional ROUTER::UndoLastSegment() { if( !RoutingInProgress() ) - return; + return std::nullopt; m_logger->Log( LOGGER::EVT_UNFIX ); - m_placer->UnfixRoute(); + return m_placer->UnfixRoute(); } diff --git a/pcbnew/router/pns_router.h b/pcbnew/router/pns_router.h index 1c2b2928a0..595a0c3c20 100644 --- a/pcbnew/router/pns_router.h +++ b/pcbnew/router/pns_router.h @@ -149,7 +149,7 @@ public: bool FixRoute( const VECTOR2I& aP, ITEM* aItem, bool aForceFinish = false ); void BreakSegment( ITEM *aItem, const VECTOR2I& aP ); - void UndoLastSegment(); + std::optional UndoLastSegment(); void CommitRouting(); void GetUpdatedItems( std::vector& aRemoved, std::vector& aAdded, diff --git a/pcbnew/router/router_tool.cpp b/pcbnew/router/router_tool.cpp index 8729fde7b0..7dfed8a7d1 100644 --- a/pcbnew/router/router_tool.cpp +++ b/pcbnew/router/router_tool.cpp @@ -1362,9 +1362,16 @@ void ROUTER_TOOL::performRouting() updateEndItem( *evt ); m_router->Move( m_endSnapPoint, m_endItem ); } - else if( evt->IsAction( &PCB_ACTIONS::routerUndoLastSegment ) ) + else if( evt->IsAction( &PCB_ACTIONS::routerUndoLastSegment ) + || evt->IsAction( &ACTIONS::doDelete ) + || evt->IsAction( &ACTIONS::undo ) ) { - m_router->UndoLastSegment(); + if( std::optional last = m_router->UndoLastSegment() ) + { + getViewControls()->WarpMouseCursor( last.value(), true ); + evt->SetMousePosition( last.value() ); + } + updateEndItem( *evt ); m_router->Move( m_endSnapPoint, m_endItem ); } diff --git a/pcbnew/tools/drawing_tool.cpp b/pcbnew/tools/drawing_tool.cpp index 94c93d14b5..a0d9d6c16a 100644 --- a/pcbnew/tools/drawing_tool.cpp +++ b/pcbnew/tools/drawing_tool.cpp @@ -328,6 +328,7 @@ int DRAWING_TOOL::DrawLine( const TOOL_EVENT& aEvent ) BOARD_COMMIT commit( m_frame ); SCOPED_DRAW_MODE scopedDrawMode( m_mode, MODE::LINE ); std::optional startingPoint; + std::stack committedLines; line->SetShape( SHAPE_T::SEGMENT ); line->SetFlags( IS_NEW ); @@ -338,13 +339,14 @@ int DRAWING_TOOL::DrawLine( const TOOL_EVENT& aEvent ) m_frame->PushTool( aEvent ); Activate(); - while( drawShape( aEvent, &line, startingPoint ) ) + while( drawShape( aEvent, &line, startingPoint, &committedLines ) ) { if( line ) { commit.Add( line ); commit.Push( _( "Draw a line segment" ) ); startingPoint = VECTOR2D( line->GetEnd() ); + committedLines.push( line ); } else { @@ -388,7 +390,7 @@ int DRAWING_TOOL::DrawRectangle( const TOOL_EVENT& aEvent ) m_frame->PushTool( aEvent ); Activate(); - while( drawShape( aEvent, &rect, startingPoint ) ) + while( drawShape( aEvent, &rect, startingPoint, nullptr ) ) { if( rect ) { @@ -449,7 +451,7 @@ int DRAWING_TOOL::DrawCircle( const TOOL_EVENT& aEvent ) m_frame->PushTool( aEvent ); Activate(); - while( drawShape( aEvent, &circle, startingPoint ) ) + while( drawShape( aEvent, &circle, startingPoint, nullptr ) ) { if( circle ) { @@ -1773,7 +1775,8 @@ static void updateSegmentFromGeometryMgr( const KIGFX::PREVIEW::TWO_POINT_GEOMET bool DRAWING_TOOL::drawShape( const TOOL_EVENT& aTool, PCB_SHAPE** aGraphic, - std::optional aStartingPoint ) + std::optional aStartingPoint, + std::stack* aCommittedGraphics ) { SHAPE_T shape = ( *aGraphic )->GetShape(); @@ -1863,7 +1866,7 @@ bool DRAWING_TOOL::drawShape( const TOOL_EVENT& aTool, PCB_SHAPE** aGraphic, COORDS_PADDING ); m_controls->ForceCursorPosition( true, cursorPos ); - if( evt->IsCancelInteractive() || evt->IsAction( &ACTIONS::undo ) ) + if( evt->IsCancelInteractive() ) { cleanup(); @@ -2071,6 +2074,34 @@ bool DRAWING_TOOL::drawShape( const TOOL_EVENT& aTool, PCB_SHAPE** aGraphic, m_view->Update( &preview ); m_view->Update( &twoPointAsst ); } + else if( evt->IsAction( &ACTIONS::undo ) + || evt->IsAction( &PCB_ACTIONS::doDelete ) + || evt->IsAction( &PCB_ACTIONS::deleteLastPoint ) ) + { + if( graphic && !aCommittedGraphics->empty() ) + { + twoPointManager.SetOrigin( aCommittedGraphics->top()->GetStart() ); + twoPointManager.SetEnd( aCommittedGraphics->top()->GetEnd() ); + aCommittedGraphics->pop(); + + getViewControls()->WarpMouseCursor( twoPointManager.GetEnd(), true ); + + if( PICKED_ITEMS_LIST* undo = m_frame->PopCommandFromUndoList() ) + { + m_frame->PutDataInPreviousState( undo ); + m_frame->ClearListAndDeleteItems( undo ); + delete undo; + } + + updateSegmentFromGeometryMgr( twoPointManager, graphic ); + m_view->Update( &preview ); + m_view->Update( &twoPointAsst ); + } + else if( graphic ) + { + cleanup(); + } + } else if( graphic && evt->IsAction( &PCB_ACTIONS::incWidth ) ) { m_stroke.SetWidth( m_stroke.GetWidth() + WIDTH_STEP ); @@ -2568,13 +2599,9 @@ int DRAWING_TOOL::DrawZone( const TOOL_EVENT& aEvent ) polyGeomMgr.SetLeaderMode( Is45Limited() ? POLYGON_GEOM_MANAGER::LEADER_MODE::DEG45 : POLYGON_GEOM_MANAGER::LEADER_MODE::DIRECT ); - if( evt->IsCancelInteractive() || evt->IsAction( &ACTIONS::undo ) ) + if( evt->IsCancelInteractive() ) { - if( polyGeomMgr.PolygonPointCount() >= 2 && evt->IsAction( &ACTIONS::undo ) ) - { - polyGeomMgr.DeleteLastCorner(); - } - else if( polyGeomMgr.IsPolygonInProgress() ) + if( polyGeomMgr.IsPolygonInProgress() ) { cleanup(); } @@ -2658,19 +2685,20 @@ int DRAWING_TOOL::DrawZone( const TOOL_EVENT& aEvent ) } } } - else if( evt->IsAction( &PCB_ACTIONS::deleteLastPoint ) ) + else if( evt->IsAction( &PCB_ACTIONS::deleteLastPoint ) + || evt->IsAction( &ACTIONS::doDelete ) + || evt->IsAction( &ACTIONS::undo ) ) { - polyGeomMgr.DeleteLastCorner(); - - if( !polyGeomMgr.IsPolygonInProgress() ) + if( std::optional last = polyGeomMgr.DeleteLastCorner() ) { - // report finished as an empty shape - polyGeomMgr.SetFinished(); - - // start again - started = false; - m_controls->SetAutoPan( false ); - m_controls->CaptureCursor( false ); + cursorPos = last.value(); + getViewControls()->WarpMouseCursor( cursorPos, true ); + m_controls->ForceCursorPosition( true, cursorPos ); + polyGeomMgr.SetCursorPosition( cursorPos ); + } + else if( polyGeomMgr.IsPolygonInProgress() ) + { + cleanup(); } } else if( polyGeomMgr.IsPolygonInProgress() diff --git a/pcbnew/tools/drawing_tool.h b/pcbnew/tools/drawing_tool.h index 95eb7cb761..628505d181 100644 --- a/pcbnew/tools/drawing_tool.h +++ b/pcbnew/tools/drawing_tool.h @@ -230,7 +230,8 @@ private: * the same point. */ bool drawShape( const TOOL_EVENT& aTool, PCB_SHAPE** aGraphic, - std::optional aStartingPoint ); + std::optional aStartingPoint, + std::stack* aCommittedGraphics ); /** * Start drawing an arc. diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 568db7ae84..67c475559e 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -1960,12 +1960,6 @@ int EDIT_TOOL::Remove( const TOOL_EVENT& aEvent ) { PCB_BASE_EDIT_FRAME* editFrame = getEditFrame(); - if( isRouterActive() ) - { - m_toolMgr->RunAction( PCB_ACTIONS::routerUndoLastSegment, true ); - return 0; - } - editFrame->PushTool( aEvent ); std::vector lockedItems;