From 8e0e9ce7523fa05ff670a56665e7a7c3efb65059 Mon Sep 17 00:00:00 2001 From: John Beard Date: Sat, 1 Jul 2023 00:31:58 +0100 Subject: [PATCH] Generalise fillet tool Describe the actions of the fillet tools is a generic way, so that the same general pattern can be used for other tools that modify shapes on the BOARD. Basically, an "ITEM_MODIFICATION_ROUTINE" is defined, which is configured and called multiple times, calling back to the EDIT_TOOL when it modifies or creates an item. The motivation here is to make it easier to slot in new line-based tools like chamfer, extend and so on without having to redo the complicated item, selection and commit handling each time, and keep the core "routines" simple and decoupled from the EDIT_TOOL's internals. This also resolves #15094 because the new commit handling does the right thing when items were "conjured up" for the fillet (e.g. when a rectangle is decomposed into lines). Fixes: #15094 --- pcbnew/CMakeLists.txt | 1 + pcbnew/tools/edit_tool.cpp | 240 +++++++++------------ pcbnew/tools/item_modification_routine.cpp | 132 ++++++++++++ pcbnew/tools/item_modification_routine.h | 174 +++++++++++++++ 4 files changed, 414 insertions(+), 133 deletions(-) create mode 100644 pcbnew/tools/item_modification_routine.cpp create mode 100644 pcbnew/tools/item_modification_routine.h diff --git a/pcbnew/CMakeLists.txt b/pcbnew/CMakeLists.txt index 4c1a599c5b..51ce9d2e01 100644 --- a/pcbnew/CMakeLists.txt +++ b/pcbnew/CMakeLists.txt @@ -341,6 +341,7 @@ set( PCBNEW_CLASS_SRCS tools/global_edit_tool.cpp tools/group_tool.cpp tools/footprint_editor_control.cpp + tools/item_modification_routine.cpp tools/pad_tool.cpp tools/pcb_control.cpp tools/pcb_picker_tool.cpp diff --git a/pcbnew/tools/edit_tool.cpp b/pcbnew/tools/edit_tool.cpp index 9e77887f4b..8c59f7498b 100644 --- a/pcbnew/tools/edit_tool.cpp +++ b/pcbnew/tools/edit_tool.cpp @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -1016,12 +1017,40 @@ int EDIT_TOOL::FilletTracks( const TOOL_EVENT& aEvent ) return 0; } - -int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) +/** + * Prompt the user for the fillet radius and return it. + * + * @param aFrame + * @param aErrorMsg filled with an error message if the parameter is invalid somehow + * @return std::optional the fillet radius or std::nullopt if no + * valid fillet specified + */ +static std::optional GetFilletParams( PCB_BASE_EDIT_FRAME& aFrame, wxString& aErrorMsg ) { // Store last used fillet radius to allow pressing "enter" if repeat fillet is required static long long filletRadiusIU = 0; + WX_UNIT_ENTRY_DIALOG dia( &aFrame, _( "Enter fillet radius:" ), _( "Fillet Lines" ), + filletRadiusIU ); + + if( dia.ShowModal() == wxID_CANCEL ) + return std::nullopt; + + filletRadiusIU = dia.GetValue(); + + if( filletRadiusIU == 0 ) + { + aErrorMsg = _( "A radius of zero was entered.\n" + "The fillet operation was not performed." ); + return std::nullopt; + } + + return filletRadiusIU; +} + + +int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) +{ PCB_SELECTION& selection = m_selectionTool->RequestSelection( []( const VECTOR2I& aPt, GENERAL_COLLECTOR& aCollector, PCB_SELECTION_TOOL* sTool ) { @@ -1108,168 +1137,113 @@ int EDIT_TOOL::FilletLines( const TOOL_EVENT& aEvent ) return 0; } - WX_UNIT_ENTRY_DIALOG dia( frame(), _( "Enter fillet radius:" ), _( "Fillet Lines" ), - filletRadiusIU ); + BOARD_COMMIT commit{ this }; - if( dia.ShowModal() == wxID_CANCEL ) - return 0; + // List of thing to select at the end of the operation + // (doing it as we go will invalidate the iterator) + std::vector items_to_select_on_success; - filletRadiusIU = dia.GetValue(); - - if( filletRadiusIU == 0 ) + // Handle modifications to existing items by the routine + // How to deal with this depends on whether we're in the footprint editor or not + // and whether the item was conjured up by decomposing a polygon or rectangle + const auto item_modification_handler = [&]( PCB_SHAPE& aItem ) { - frame()->ShowInfoBarMsg( _( "A radius of zero was entered.\n" - "The fillet operation was not performed." ) ); - return 0; + if( !m_isFootprintEditor ) + { + // If the item was "conjured up" it will be added later separately + if( std::find( lines_to_add.begin(), lines_to_add.end(), &aItem ) + == lines_to_add.end() ) + { + commit.Modify( &aItem ); + items_to_select_on_success.push_back( &aItem ); + } + } + }; + + bool any_items_created = false; + const auto item_creation_handler = [&]( std::unique_ptr aItem ) + { + any_items_created = true; + items_to_select_on_success.push_back( aItem.get() ); + commit.Add( aItem.release() ); + }; + + // Construct an appropriate tool + std::unique_ptr pairwise_line_routine; + wxString error_message; + + if( aEvent.IsAction( &PCB_ACTIONS::filletLines ) ) + { + const std::optional filletRadiusIU = GetFilletParams( *frame(), error_message ); + + if( filletRadiusIU.has_value() ) + { + pairwise_line_routine = std::make_unique( + frame()->GetModel(), item_creation_handler, item_modification_handler, + *filletRadiusIU ); + } } - BOARD_COMMIT commit( this ); - bool operationPerformedOnAtLeastOne = false; - bool didOneAttemptFail = false; - std::vector itemsToAddToSelection; + if( !pairwise_line_routine ) + { + // Didn't construct any mofication routine - could be an error or cancellation + if( !error_message.empty() ) + { + frame()->ShowInfoBarMsg( error_message ); + } + + return 0; + } // Only modify one parent in FP editor if( m_isFootprintEditor ) commit.Modify( selection.Front() ); - alg::for_all_pairs( selection.begin(), selection.end(), [&]( EDA_ITEM* a, EDA_ITEM* b ) - { - PCB_SHAPE* line_a = static_cast( a ); - PCB_SHAPE* line_b = static_cast( b ); - - if( line_a->GetLength() == 0.0 || line_b->GetLength() == 0 ) - return; - - SEG seg_a( line_a->GetStart(), line_a->GetEnd() ); - SEG seg_b( line_b->GetStart(), line_b->GetEnd() ); - VECTOR2I* a_pt; - VECTOR2I* b_pt; - - if (seg_a.A == seg_b.A) - { - a_pt = &seg_a.A; - b_pt = &seg_b.A; - } - else if (seg_a.A == seg_b.B) - { - a_pt = &seg_a.A; - b_pt = &seg_b.B; - } - else if (seg_a.B == seg_b.A) - { - a_pt = &seg_a.B; - b_pt = &seg_b.A; - } - else if (seg_a.B == seg_b.B) - { - a_pt = &seg_a.B; - b_pt = &seg_b.B; - } - else - return; - - - SHAPE_ARC sArc( seg_a, seg_b, filletRadiusIU ); - VECTOR2I t1newPoint, t2newPoint; - - auto setIfPointOnSeg = - []( VECTOR2I& aPointToSet, SEG aSegment, VECTOR2I aVecToTest ) - { - VECTOR2I segToVec = aSegment.NearestPoint( aVecToTest ) - aVecToTest; - - // Find out if we are on the segment (minimum precision) - if( segToVec.EuclideanNorm() < SHAPE_ARC::MIN_PRECISION_IU ) + // Apply the tool to every line pair + alg::for_all_pairs( selection.begin(), selection.end(), + [&]( EDA_ITEM* a, EDA_ITEM* b ) { - aPointToSet.x = aVecToTest.x; - aPointToSet.y = aVecToTest.y; - return true; - } + PCB_SHAPE* line_a = static_cast( a ); + PCB_SHAPE* line_b = static_cast( b ); - return false; - }; + pairwise_line_routine->ProcessLinePair( *line_a, *line_b ); + } ); - //Do not draw a fillet if the end points of the arc are not within the track segments - if( !setIfPointOnSeg( t1newPoint, seg_a, sArc.GetP0() ) - && !setIfPointOnSeg( t2newPoint, seg_b, sArc.GetP0() ) ) - { - didOneAttemptFail = true; - return; - } - - if( !setIfPointOnSeg( t1newPoint, seg_a, sArc.GetP1() ) - && !setIfPointOnSeg( t2newPoint, seg_b, sArc.GetP1() ) ) - { - didOneAttemptFail = true; - return; - } - - PCB_SHAPE* tArc = new PCB_SHAPE( frame()->GetModel(), SHAPE_T::ARC ); - - tArc->SetArcGeometry( sArc.GetP0(), sArc.GetArcMid(), sArc.GetP1() ); - tArc->SetWidth( line_a->GetWidth() ); - tArc->SetLayer( line_a->GetLayer() ); - tArc->SetLocked( line_a->IsLocked() ); - - if( lines_to_add.count( line_a ) ) - { - lines_to_add.erase( line_a ); - itemsToAddToSelection.push_back( line_a ); - } - else if( !m_isFootprintEditor ) - { - commit.Modify( line_a ); - } - - if( lines_to_add.count( line_b ) ) - { - lines_to_add.erase( line_b ); - itemsToAddToSelection.push_back( line_b ); - } - else if( !m_isFootprintEditor ) - { - commit.Modify( line_b ); - } - - itemsToAddToSelection.push_back( tArc ); - *a_pt = t1newPoint; - *b_pt = t2newPoint; - line_a->SetStart( seg_a.A ); - line_a->SetEnd( seg_a.B ); - line_b->SetStart( seg_b.A ); - line_b->SetEnd( seg_b.B ); - - operationPerformedOnAtLeastOne = true; - - } ); + // Items created like lines from a rectangle + // Some of these might have been changed by the tool, but we need to + // add *all* of them to the selection and commit them + for( PCB_SHAPE* item : lines_to_add ) + { + m_selectionTool->AddItemToSel( item, true ); + commit.Add( item ); + } + // Remove items like rectangles that we decomposed into lines for( PCB_SHAPE* item : items_to_remove ) { commit.Remove( item ); m_selectionTool->RemoveItemFromSel( item, true ); } - //select the newly created arcs - for( BOARD_ITEM* item : itemsToAddToSelection ) - { - commit.Add( item ); + // Select added and modified items + for( PCB_SHAPE* item : items_to_select_on_success ) m_selectionTool->AddItemToSel( item, true ); - } if( !items_to_remove.empty() ) m_toolMgr->ProcessEvent( EVENTS::UnselectedEvent ); - if( !itemsToAddToSelection.empty() ) + if( !any_items_created ) m_toolMgr->ProcessEvent( EVENTS::SelectedEvent ); // Notify other tools of the changes m_toolMgr->ProcessEvent( EVENTS::SelectedItemsModified ); - commit.Push( _( "Fillet Lines" ) ); + commit.Push( pairwise_line_routine->GetCommitDescription() ); - if( !operationPerformedOnAtLeastOne ) - frame()->ShowInfoBarMsg( _( "Unable to fillet the selected lines." ) ); - else if( didOneAttemptFail ) - frame()->ShowInfoBarMsg( _( "Some of the lines could not be filleted." ) ); + if( pairwise_line_routine->GetSuccesses() == 0 ) + frame()->ShowInfoBarMsg( pairwise_line_routine->GetCompleteFailureMessage() ); + else if( pairwise_line_routine->GetFailures() > 0 ) + frame()->ShowInfoBarMsg( pairwise_line_routine->GetSomeFailuresMessage() ); return 0; } diff --git a/pcbnew/tools/item_modification_routine.cpp b/pcbnew/tools/item_modification_routine.cpp new file mode 100644 index 0000000000..ee90b4015a --- /dev/null +++ b/pcbnew/tools/item_modification_routine.cpp @@ -0,0 +1,132 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#include "item_modification_routine.h" + + +wxString LINE_FILLET_ROUTINE::GetCommitDescription() const +{ + return _( "Fillet Lines" ); +} + +wxString LINE_FILLET_ROUTINE::GetCompleteFailureMessage() const +{ + return _( "Unable to fillet the selected lines." ); +} + +wxString LINE_FILLET_ROUTINE::GetSomeFailuresMessage() const +{ + return _( "Some of the lines could not be filleted." ); +} + +void LINE_FILLET_ROUTINE::ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) +{ + if( aLineA.GetLength() == 0.0 || aLineB.GetLength() == 0.0 ) + return; + + SEG seg_a( aLineA.GetStart(), aLineA.GetEnd() ); + SEG seg_b( aLineB.GetStart(), aLineB.GetEnd() ); + VECTOR2I* a_pt; + VECTOR2I* b_pt; + + if( seg_a.A == seg_b.A ) + { + a_pt = &seg_a.A; + b_pt = &seg_b.A; + } + else if( seg_a.A == seg_b.B ) + { + a_pt = &seg_a.A; + b_pt = &seg_b.B; + } + else if( seg_a.B == seg_b.A ) + { + a_pt = &seg_a.B; + b_pt = &seg_b.A; + } + else if( seg_a.B == seg_b.B ) + { + a_pt = &seg_a.B; + b_pt = &seg_b.B; + } + else + // Nothing to do + return; + + + SHAPE_ARC sArc( seg_a, seg_b, m_filletRadiusIU ); + VECTOR2I t1newPoint, t2newPoint; + + auto setIfPointOnSeg = []( VECTOR2I& aPointToSet, SEG aSegment, VECTOR2I aVecToTest ) + { + VECTOR2I segToVec = aSegment.NearestPoint( aVecToTest ) - aVecToTest; + + // Find out if we are on the segment (minimum precision) + if( segToVec.EuclideanNorm() < SHAPE_ARC::MIN_PRECISION_IU ) + { + aPointToSet.x = aVecToTest.x; + aPointToSet.y = aVecToTest.y; + return true; + } + + return false; + }; + + //Do not draw a fillet if the end points of the arc are not within the track segments + if( !setIfPointOnSeg( t1newPoint, seg_a, sArc.GetP0() ) + && !setIfPointOnSeg( t2newPoint, seg_b, sArc.GetP0() ) ) + { + AddFailure(); + return; + } + + if( !setIfPointOnSeg( t1newPoint, seg_a, sArc.GetP1() ) + && !setIfPointOnSeg( t2newPoint, seg_b, sArc.GetP1() ) ) + { + AddFailure(); + return; + } + + auto tArc = std::make_unique( GetBoard(), SHAPE_T::ARC ); + + tArc->SetArcGeometry( sArc.GetP0(), sArc.GetArcMid(), sArc.GetP1() ); + + // Copy properties from one of the source lines + tArc->SetWidth( aLineA.GetWidth() ); + tArc->SetLayer( aLineA.GetLayer() ); + tArc->SetLocked( aLineA.IsLocked() ); + + MarkItemModified( aLineA ); + MarkItemModified( aLineB ); + + AddNewItem( std::move( tArc ) ); + + *a_pt = t1newPoint; + *b_pt = t2newPoint; + aLineA.SetStart( seg_a.A ); + aLineA.SetEnd( seg_a.B ); + aLineB.SetStart( seg_b.A ); + aLineB.SetEnd( seg_b.B ); + + AddSuccess(); +} \ No newline at end of file diff --git a/pcbnew/tools/item_modification_routine.h b/pcbnew/tools/item_modification_routine.h new file mode 100644 index 0000000000..a1b2369670 --- /dev/null +++ b/pcbnew/tools/item_modification_routine.h @@ -0,0 +1,174 @@ +/* + * This program source code file is part of KiCad, a free EDA CAD application. + * + * Copyright (C) 2023 KiCad Developers, see AUTHORS.txt for contributors. + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * as published by the Free Software Foundation; either version 2 + * of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, you may find one here: + * http://www.gnu.org/licenses/old-licenses/gpl-2.0.html + * or you may search the http://www.gnu.org website for the version 2 license, + * or you may write to the Free Software Foundation, Inc., + * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA + */ + +#ifndef ITEM_MODIFICATION_ROUTINE_H_ +#define ITEM_MODIFICATION_ROUTINE_H_ + +#include +#include +#include +#include + +#include +#include + +/** + * @brief An object that has the ability to modify items on a board + * + * For example, such an object could take pairs of lines and fillet them, + * or produce other modification to items. + * + * Deliberately not called a "tool" to distinguish it from true + * tools that are used interactively by the user. + */ +class ITEM_MODIFICATION_ROUTINE +{ +public: + /* + * Handlers for receiving changes from the tool + * + * These is used to allow the tool's caller to make changes to + * affected board items using extra information that the tool + * does not have access to (e.g. is this an FP editor, was + * the line created from a rectangle and needs to be added, not + * modified, etc). + * + * We can't store them up until the end, because modifications + * need the old state to be known. + * + * @param PCB_SHAPE& the line to modify + * @param bool true if the shape was created, false if it was only modified + */ + using CREATION_HANDLER = std::function )>; + using MODIFICATION_HANDLER = std::function; + + ITEM_MODIFICATION_ROUTINE( BOARD_ITEM* aBoard, CREATION_HANDLER aCreationHandler, + MODIFICATION_HANDLER aModificationHandler ) : + m_board( aBoard ), + m_creationHandler( aCreationHandler ), m_modificationHandler( aModificationHandler ), + m_numSuccesses( 0 ), m_numFailures( 0 ) + { + } + + unsigned GetSuccesses() const { return m_numSuccesses; } + + unsigned GetFailures() const { return m_numFailures; } + + virtual wxString GetCommitDescription() const = 0; + + virtual wxString GetCompleteFailureMessage() const = 0; + virtual wxString GetSomeFailuresMessage() const = 0; + +protected: + /** + * The BOARD used when creating new shapes + */ + BOARD_ITEM* GetBoard() const { return m_board; } + + /** + * Mark that one of the actions succeeded. + */ + void AddSuccess() { ++m_numSuccesses; } + + /** + * Mark that one of the actions failed. + */ + void AddFailure() { ++m_numFailures; } + + /** + * @brief Report that the tools wants to add a new item to the board + * + * @param aItem the new item + */ + void AddNewItem( std::unique_ptr aItem ) { m_creationHandler( std::move( aItem ) ); } + + /** + * @brief Report that the tool has modified an item on the board + * + * @param aItem the modified item + */ + void MarkItemModified( PCB_SHAPE& aItem ) { m_modificationHandler( aItem ); } + +private: + BOARD_ITEM* m_board; + CREATION_HANDLER m_creationHandler; + MODIFICATION_HANDLER m_modificationHandler; + + unsigned m_numSuccesses; + unsigned m_numFailures; +}; + +/** + * A tool that acts on a pair of lines. For example, fillets, chamfers, extensions, etc + */ +class PAIRWISE_LINE_ROUTINE : public ITEM_MODIFICATION_ROUTINE +{ +public: + PAIRWISE_LINE_ROUTINE( BOARD_ITEM* aBoard, CREATION_HANDLER aCreationHandler, + MODIFICATION_HANDLER aModificationHandler ) : + ITEM_MODIFICATION_ROUTINE( aBoard, aCreationHandler, aModificationHandler ) + { + } + + /** + * @brief Perform the action on the pair of lines given + * + * The routine will be called repeatedly with all possible pairs of lines + * in the selection. The tools should handle the ones it's interested in. + * This means that the same line can appear multiple times with different + * partners. + * + * The routine can skip lines that it's not interested in by returning without + * adding to the success or failure count. + * + * @param aLineA the first line + * @param aLineB the second line + * @return did the action succeed + */ + virtual void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) = 0; +}; + +/** + * Pairwise line tool that adds a fillet to the lines. + */ +class LINE_FILLET_ROUTINE : public PAIRWISE_LINE_ROUTINE +{ +public: + LINE_FILLET_ROUTINE( BOARD_ITEM* aBoard, CREATION_HANDLER aCreationHandler, + MODIFICATION_HANDLER aModificationHandler, int filletRadiusIU ) : + PAIRWISE_LINE_ROUTINE( aBoard, aCreationHandler, aModificationHandler ), + m_filletRadiusIU( filletRadiusIU ) + { + } + + wxString GetCommitDescription() const override; + wxString GetCompleteFailureMessage() const override; + wxString GetSomeFailuresMessage() const override; + + void ProcessLinePair( PCB_SHAPE& aLineA, PCB_SHAPE& aLineB ) override; + +private: + int m_filletRadiusIU; +}; + +#endif /* ITEM_MODIFICATION_ROUTINE_H_ */ \ No newline at end of file