Pcbnew: polygon booleans - try subtraction in both orders if null result

While there is technically some kinds of defined order (last selected item is
'A' in the 'A - B (- C...)' operation, it's easy to drag select and then
get a null result.

So if this happens to a non-commutative operation, try again in the
reverse order. In the usual case (2 items), this will get what the user,
presumably, wants. If there are more than 2, it will either work, or the
user can undo and try merging the "B" operand before subtracting, or do
them one at a time (at which point the order-reverse logic will help
again).

Thanks to Kliment for an this usability suggestion!
This commit is contained in:
John Beard 2025-07-13 18:15:43 +08:00
parent c594e1da5c
commit 8d07eb7f24
2 changed files with 64 additions and 20 deletions

View File

@ -1890,29 +1890,62 @@ int EDIT_TOOL::BooleanPolygons( const TOOL_EVENT& aEvent )
// Construct an appropriate routine
std::unique_ptr<POLYGON_BOOLEAN_ROUTINE> boolean_routine;
const auto create_routine = [&]() -> std::unique_ptr<POLYGON_BOOLEAN_ROUTINE>
{
// (Re-)construct the boolean routine based on the action
// This is done here so that we can re-init the routine if we need to
// go again in the reverse order.
BOARD_ITEM_CONTAINER* const model = frame()->GetModel();
wxCHECK( model, nullptr );
if( aEvent.IsAction( &PCB_ACTIONS::mergePolygons ) )
{
boolean_routine = std::make_unique<POLYGON_MERGE_ROUTINE>( frame()->GetModel(), change_handler );
return std::make_unique<POLYGON_MERGE_ROUTINE>( model, change_handler );
}
else if( aEvent.IsAction( &PCB_ACTIONS::subtractPolygons ) )
{
boolean_routine = std::make_unique<POLYGON_SUBTRACT_ROUTINE>( frame()->GetModel(), change_handler );
return std::make_unique<POLYGON_SUBTRACT_ROUTINE>( model, change_handler );
}
else if( aEvent.IsAction( &PCB_ACTIONS::intersectPolygons ) )
{
boolean_routine = std::make_unique<POLYGON_INTERSECT_ROUTINE>( frame()->GetModel(), change_handler );
}
else
{
wxASSERT_MSG( false, "Could not find a polygon routine for this action" );
return 0;
return std::make_unique<POLYGON_INTERSECT_ROUTINE>( model, change_handler );
}
return nullptr;
};
const auto run_routine = [&]()
{
// Perform the operation on each polygon
for( PCB_SHAPE* shape : items_to_process )
boolean_routine->ProcessShape( *shape );
boolean_routine->Finalize();
};
boolean_routine = create_routine();
wxCHECK_MSG( boolean_routine, 0, "Could not find a polygon routine for this action" );
// First run the routine and see what we get
run_routine();
// If we are doing a non-commutative operation (e.g. subtract), and we just got null,
// assume the user meant go in the opposite order (this is probably mainly useful
// with a selection size of 2 items)
if( !boolean_routine->IsCommutative() && items_to_select_on_success.empty() )
{
// Clear the commit and the selection
commit.Revert();
items_to_select_on_success.clear();
// Reverse the order of the shapes and try again
std::reverse( items_to_process.begin(), items_to_process.end() );
// Run the routine again
boolean_routine = create_routine();
run_routine();
}
// Select new items
for( BOARD_ITEM* item : items_to_select_on_success )

View File

@ -343,6 +343,11 @@ public:
{
}
/**
* @brief False if the order of the polygons matters
*/
virtual bool IsCommutative() const = 0;
void ProcessShape( PCB_SHAPE& aPcbShape );
/**
@ -380,6 +385,8 @@ public:
{
}
bool IsCommutative() const override { return true; }
wxString GetCommitDescription() const override;
std::optional<wxString> GetStatusMessage() const override;
@ -396,6 +403,8 @@ public:
{
}
bool IsCommutative() const override { return false; }
wxString GetCommitDescription() const override;
std::optional<wxString> GetStatusMessage() const override;
@ -412,6 +421,8 @@ public:
{
}
bool IsCommutative() const override { return true; }
wxString GetCommitDescription() const override;
std::optional<wxString> GetStatusMessage() const override;