From 0ead8a14a1ee1f63d451bbc554576585ecfe87dd Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Wed, 14 Feb 2024 17:21:02 +0000
Subject: [PATCH] Handle holes in shapes when creating bounding hulls.
Fixes https://gitlab.com/kicad/code/kicad/-/issues/16347
---
.../dialogs/dialog_rule_area_properties.cpp | 34 ++++++++++---
pcbnew/tools/convert_tool.cpp | 49 +++++++++++--------
2 files changed, 56 insertions(+), 27 deletions(-)
diff --git a/pcbnew/dialogs/dialog_rule_area_properties.cpp b/pcbnew/dialogs/dialog_rule_area_properties.cpp
index 728498633a..915ba0ebb8 100644
--- a/pcbnew/dialogs/dialog_rule_area_properties.cpp
+++ b/pcbnew/dialogs/dialog_rule_area_properties.cpp
@@ -61,7 +61,11 @@ private:
CONVERT_SETTINGS* m_convertSettings;
wxRadioButton* m_rbCenterline;
- wxRadioButton* m_rbEnvelope;
+ wxRadioButton* m_rbBoundingHull;
+ wxStaticText* m_gapLabel;
+ wxTextCtrl* m_gapCtrl;
+ wxStaticText* m_gapUnits;
+ UNIT_BINDER* m_gap;
wxCheckBox* m_cbDeleteOriginals;
};
@@ -83,7 +87,7 @@ DIALOG_RULE_AREA_PROPERTIES::DIALOG_RULE_AREA_PROPERTIES( PCB_BASE_FRAME* aParen
m_outlineHatchPitchCtrl, m_outlineHatchUnits ),
m_convertSettings( aConvertSettings ),
m_rbCenterline( nullptr ),
- m_rbEnvelope( nullptr ),
+ m_rbBoundingHull( nullptr ),
m_cbDeleteOriginals( nullptr )
{
m_parent = aParent;
@@ -101,8 +105,21 @@ DIALOG_RULE_AREA_PROPERTIES::DIALOG_RULE_AREA_PROPERTIES( PCB_BASE_FRAME* aParen
bConvertSizer->Add( m_rbCenterline, 0, wxLEFT|wxRIGHT, 5 );
bConvertSizer->AddSpacer( 2 );
- m_rbEnvelope = new wxRadioButton( this, wxID_ANY, _( "Create bounding hull" ) );
- bConvertSizer->Add( m_rbEnvelope, 0, wxLEFT|wxRIGHT, 5 );
+ m_rbBoundingHull = new wxRadioButton( this, wxID_ANY, _( "Create bounding hull" ) );
+ bConvertSizer->Add( m_rbBoundingHull, 0, wxLEFT|wxRIGHT, 5 );
+
+ m_gapLabel = new wxStaticText( this, wxID_ANY, _( "Gap:" ) );
+ m_gapCtrl = new wxTextCtrl( this, wxID_ANY );
+ m_gapUnits = new wxStaticText( this, wxID_ANY, _( "mm" ) );
+ m_gap = new UNIT_BINDER( aParent, m_gapLabel, m_gapCtrl, m_gapUnits );
+
+ wxBoxSizer* hullParamsSizer = new wxBoxSizer( wxHORIZONTAL );
+ hullParamsSizer->Add( m_gapLabel, 0, wxALIGN_CENTRE_VERTICAL, 5 );
+ hullParamsSizer->Add( m_gapCtrl, 1, wxALIGN_CENTRE_VERTICAL|wxLEFT|wxRIGHT, 3 );
+ hullParamsSizer->Add( m_gapUnits, 0, wxALIGN_CENTRE_VERTICAL, 5 );
+
+ bConvertSizer->AddSpacer( 2 );
+ bConvertSizer->Add( hullParamsSizer, 0, wxLEFT, 26 );
bConvertSizer->AddSpacer( 6 );
m_cbDeleteOriginals = new wxCheckBox( this, wxID_ANY,
@@ -136,7 +153,7 @@ bool DIALOG_RULE_AREA_PROPERTIES::TransferDataToWindow()
if( m_convertSettings )
{
if( m_convertSettings->m_Strategy == BOUNDING_HULL )
- m_rbEnvelope->SetValue( true );
+ m_rbBoundingHull->SetValue( true );
else
m_rbCenterline->SetValue( true );
@@ -201,10 +218,15 @@ bool DIALOG_RULE_AREA_PROPERTIES::TransferDataFromWindow()
{
if( m_convertSettings )
{
- if( m_rbEnvelope->GetValue() )
+ if( m_rbBoundingHull->GetValue() )
+ {
m_convertSettings->m_Strategy = BOUNDING_HULL;
+ m_convertSettings->m_Gap = m_gap->GetIntValue();
+ }
else
+ {
m_convertSettings->m_Strategy = CENTERLINE;
+ }
m_convertSettings->m_DeleteOriginals = m_cbDeleteOriginals->GetValue();
}
diff --git a/pcbnew/tools/convert_tool.cpp b/pcbnew/tools/convert_tool.cpp
index d1e4e9b411..823d828a52 100644
--- a/pcbnew/tools/convert_tool.cpp
+++ b/pcbnew/tools/convert_tool.cpp
@@ -93,7 +93,7 @@ public:
m_rbCenterline->Hide();
}
- m_rbEnvelope = new wxRadioButton( this, wxID_ANY, _( "Create bounding hull" ) );
+ m_rbBoundingHull = new wxRadioButton( this, wxID_ANY, _( "Create bounding hull" ) );
m_gapLabel = new wxStaticText( this, wxID_ANY, _( "Gap:" ) );
m_gapCtrl = new wxTextCtrl( this, wxID_ANY );
@@ -108,7 +108,7 @@ public:
if( aShowBoundingHullOption )
{
topSizer->AddSpacer( 6 );
- topSizer->Add( m_rbEnvelope, 0, wxLEFT|wxRIGHT, 5 );
+ topSizer->Add( m_rbBoundingHull, 0, wxLEFT|wxRIGHT, 5 );
wxBoxSizer* hullParamsSizer = new wxBoxSizer( wxHORIZONTAL );
hullParamsSizer->Add( m_gapLabel, 0, wxALIGN_CENTRE_VERTICAL, 5 );
@@ -126,7 +126,7 @@ public:
}
else
{
- m_rbEnvelope->Hide();
+ m_rbBoundingHull->Hide();
m_gap->Show( false, true );
m_width->Show( false, true );
}
@@ -157,9 +157,9 @@ public:
m_rbCenterline->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED,
wxCommandEventHandler( CONVERT_SETTINGS_DIALOG::onRadioButton ),
nullptr, this );
- m_rbEnvelope->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED,
- wxCommandEventHandler( CONVERT_SETTINGS_DIALOG::onRadioButton ),
- nullptr, this );
+ m_rbBoundingHull->Connect( wxEVT_COMMAND_RADIOBUTTON_SELECTED,
+ wxCommandEventHandler( CONVERT_SETTINGS_DIALOG::onRadioButton ),
+ nullptr, this );
finishDialogSettings();
}
@@ -177,11 +177,11 @@ protected:
{
case COPY_LINEWIDTH: m_rbMimicLineWidth->SetValue( true ); break;
case CENTERLINE: m_rbCenterline->SetValue( true ); break;
- case BOUNDING_HULL: m_rbEnvelope->SetValue( true ); break;
+ case BOUNDING_HULL: m_rbBoundingHull->SetValue( true ); break;
}
- m_gap->Enable( m_rbEnvelope->GetValue() );
- m_width->Enable( m_rbEnvelope->GetValue() );
+ m_gap->Enable( m_rbBoundingHull->GetValue() );
+ m_width->Enable( m_rbBoundingHull->GetValue() );
m_gap->SetValue( m_settings->m_Gap );
m_width->SetValue( m_settings->m_LineWidth );
@@ -191,15 +191,15 @@ protected:
bool TransferDataFromWindow() override
{
- if( m_rbEnvelope->GetValue() )
+ if( m_rbBoundingHull->GetValue() )
m_settings->m_Strategy = BOUNDING_HULL;
else if( m_rbCenterline->GetValue() )
m_settings->m_Strategy = CENTERLINE;
else
m_settings->m_Strategy = COPY_LINEWIDTH;
- m_settings->m_Gap = m_gap->GetValue();
- m_settings->m_LineWidth = m_width->GetValue();
+ m_settings->m_Gap = m_gap->GetIntValue();
+ m_settings->m_LineWidth = m_width->GetIntValue();
m_settings->m_DeleteOriginals = m_cbDeleteOriginals->GetValue();
return true;
@@ -207,8 +207,8 @@ protected:
void onRadioButton( wxCommandEvent& aEvent )
{
- m_gap->Enable( m_rbEnvelope->GetValue() );
- m_width->Enable( m_rbEnvelope->GetValue() );
+ m_gap->Enable( m_rbBoundingHull->GetValue() );
+ m_width->Enable( m_rbBoundingHull->GetValue() );
}
private:
@@ -216,7 +216,7 @@ private:
wxRadioButton* m_rbMimicLineWidth;
wxRadioButton* m_rbCenterline;
- wxRadioButton* m_rbEnvelope;
+ wxRadioButton* m_rbBoundingHull;
wxStaticText* m_gapLabel;
wxTextCtrl* m_gapCtrl;
wxStaticText* m_gapUnits;
@@ -346,16 +346,18 @@ int CONVERT_TOOL::CreatePolys( const TOOL_EVENT& aEvent )
polySet.Append( makePolysFromClosedGraphics( selection.GetItems(), cfg.m_Strategy ) );
- polySet.Append( makePolysFromChainedSegs( selection.GetItems(), cfg.m_Strategy ) );
-
if( cfg.m_Strategy == BOUNDING_HULL )
{
+ polySet.Append( makePolysFromOpenGraphics( selection.GetItems(), cfg.m_Gap ) );
+
polySet.ClearArcs();
polySet.Simplify( SHAPE_POLY_SET::PM_FAST );
polySet.Inflate( cfg.m_Gap, CORNER_STRATEGY::ROUND_ALL_CORNERS, bds.m_MaxError,
ERROR_OUTSIDE );
-
- polySet.Append( makePolysFromOpenGraphics( selection.GetItems(), cfg.m_Gap ) );
+ }
+ else
+ {
+ polySet.Append( makePolysFromChainedSegs( selection.GetItems(), cfg.m_Strategy ) );
}
if( polySet.IsEmpty() )
@@ -836,10 +838,15 @@ SHAPE_POLY_SET CONVERT_TOOL::makePolysFromClosedGraphics( const std::deque<EDA_I
if( !shape->IsClosed() )
continue;
- shape->SetFilled( true );
+ if( aStrategy != BOUNDING_HULL )
+ shape->SetFilled( true );
+
shape->TransformShapeToPolygon( poly, UNDEFINED_LAYER, 0, bds.m_MaxError, ERROR_INSIDE,
aStrategy == COPY_LINEWIDTH || aStrategy == CENTERLINE );
- shape->SetFillMode( wasFilled );
+
+ if( aStrategy != BOUNDING_HULL )
+ shape->SetFillMode( wasFilled );
+
shape->SetFlags( SKIP_STRUCT );
break;
}