From 44f3cdb31e1eacec244b70110e27144683f97014 Mon Sep 17 00:00:00 2001 From: Jon Evans Date: Mon, 29 Jun 2020 12:16:37 -0400 Subject: [PATCH] Fix a few issues with zone islands --- common/base_units.cpp | 45 ++++++++++++++++++++++++++++------ common/widgets/unit_binder.cpp | 10 +++++--- include/base_units.h | 10 ++++---- pcbnew/kicad_plugin.cpp | 2 +- pcbnew/pcb_parser.cpp | 5 +++- pcbnew/plot_board_layers.cpp | 4 +-- pcbnew/zone_filler.cpp | 15 +++++++----- 7 files changed, 64 insertions(+), 27 deletions(-) diff --git a/common/base_units.cpp b/common/base_units.cpp index 3ee7bd2dca..9bb4d7b268 100644 --- a/common/base_units.cpp +++ b/common/base_units.cpp @@ -230,9 +230,25 @@ void StripTrailingZeros( wxString& aStringValue, unsigned aTrailingZeroAllowed ) * otherwise the actual value is rounded when read from dialog and converted * in internal units, and therefore modified. */ -wxString StringFromValue( EDA_UNITS aUnits, double aValue, bool aAddUnitSymbol, bool aUseMils ) +wxString StringFromValue( EDA_UNITS aUnits, double aValue, bool aAddUnitSymbol, bool aUseMils, + EDA_DATA_TYPE aType ) { - double value_to_print = To_User_Unit( aUnits, aValue, aUseMils ); + double value_to_print = aValue; + + switch( aType ) + { + case EDA_DATA_TYPE::VOLUME: + value_to_print = To_User_Unit( aUnits, value_to_print, aUseMils ); + KI_FALLTHROUGH; + + case EDA_DATA_TYPE::AREA: + value_to_print = To_User_Unit( aUnits, value_to_print, aUseMils ); + KI_FALLTHROUGH; + + case EDA_DATA_TYPE::DISTANCE: + value_to_print = To_User_Unit( aUnits, value_to_print, aUseMils ); + } + #if defined( EESCHEMA ) wxString stringValue = wxString::Format( wxT( "%.3f" ), value_to_print ); @@ -327,9 +343,9 @@ double From_User_Unit( EDA_UNITS aUnits, double aValue, bool aUseMils ) } -double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils ) +double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils, + EDA_DATA_TYPE aType ) { - double value; double dtmp = 0; // Acquire the 'right' decimal point separator @@ -395,9 +411,21 @@ double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool } } - value = From_User_Unit( aUnits, dtmp, aUseMils ); + switch( aType ) + { + case EDA_DATA_TYPE::VOLUME: + dtmp = From_User_Unit( aUnits, dtmp, aUseMils ); + KI_FALLTHROUGH; - return value; + case EDA_DATA_TYPE::AREA: + dtmp = From_User_Unit( aUnits, dtmp, aUseMils ); + KI_FALLTHROUGH; + + case EDA_DATA_TYPE::DISTANCE: + dtmp = From_User_Unit( aUnits, dtmp, aUseMils ); + } + + return dtmp; } @@ -440,9 +468,10 @@ void FetchUnitsFromString( const wxString& aTextValue, EDA_UNITS& aUnits, bool& } -long long int ValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils ) +long long int ValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils, + EDA_DATA_TYPE aType ) { - double value = DoubleValueFromString( aUnits, aTextValue, aUseMils ); + double value = DoubleValueFromString( aUnits, aTextValue, aUseMils, aType ); return KiROUND( value ); } diff --git a/common/widgets/unit_binder.cpp b/common/widgets/unit_binder.cpp index 4d6d114b7a..5ed16b60f5 100644 --- a/common/widgets/unit_binder.cpp +++ b/common/widgets/unit_binder.cpp @@ -187,6 +187,8 @@ bool UNIT_BINDER::Validate( double aMin, double aMax, EDA_UNITS aUnits, bool aUs return true; } + // TODO: Validate() does not currently support m_dataType being anything other than DISTANCE + if( GetValue() < From_User_Unit( aUnits, aMin, aUseMils ) ) { m_errorMessage = wxString::Format( _( "%s must be at least %s." ), @@ -219,13 +221,13 @@ bool UNIT_BINDER::Validate( double aMin, double aMax, EDA_UNITS aUnits, bool aUs void UNIT_BINDER::SetValue( int aValue ) { - SetValue( StringFromValue( m_units, aValue, false, m_useMils ) ); + SetValue( StringFromValue( m_units, aValue, false, m_useMils, m_dataType ) ); } void UNIT_BINDER::SetDoubleValue( double aValue ) { - SetValue( StringFromValue( m_units, aValue, false, m_useMils ) ); + SetValue( StringFromValue( m_units, aValue, false, m_useMils, m_dataType ) ); } @@ -287,7 +289,7 @@ long long int UNIT_BINDER::GetValue() else return 0; - return ValueFromString( m_units, value, m_useMils ); + return ValueFromString( m_units, value, m_useMils, m_dataType ); } @@ -309,7 +311,7 @@ double UNIT_BINDER::GetDoubleValue() else return 0.0; - return DoubleValueFromString( m_units, value, m_useMils ); + return DoubleValueFromString( m_units, value, m_useMils, m_dataType ); } diff --git a/include/base_units.h b/include/base_units.h index 2da18b3c58..408957bef9 100644 --- a/include/base_units.h +++ b/include/base_units.h @@ -150,7 +150,7 @@ wxString MessageTextFromValue( EDA_UNITS aUnits, long long int aValue, bool aUse * @return A wxString object containing value and optionally the symbol unit (like 2.000 mm) */ wxString StringFromValue( EDA_UNITS aUnit, double aValue, bool aAddUnitSymbol = false, - bool aUseMils = false ); + bool aUseMils = false, EDA_DATA_TYPE aType = EDA_DATA_TYPE::DISTANCE ); /** * Return in internal units the value "val" given in a real unit @@ -167,8 +167,8 @@ double From_User_Unit( EDA_UNITS aUnit, double aValue, bool aUseMils = false ); * @param aUseMils Indicates mils should be used for imperial units (inches). * @return A double representing that value in internal units */ -double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, - bool aUseMils = false ); +double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils = false, + EDA_DATA_TYPE aType = EDA_DATA_TYPE::DISTANCE ); /** * Function ValueFromString @@ -179,8 +179,8 @@ double DoubleValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, * @param aUseMils Indicates mils should be used for imperial units (inches). * @return The string from Value, according to units (inch, mm ...) for display, */ -long long int ValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, - bool aUseMils = false ); +long long int ValueFromString( EDA_UNITS aUnits, const wxString& aTextValue, bool aUseMils = false, + EDA_DATA_TYPE aType = EDA_DATA_TYPE::DISTANCE ); /** * Function FetchUnitsFromString diff --git a/pcbnew/kicad_plugin.cpp b/pcbnew/kicad_plugin.cpp index 913022fbdf..bc374edb8d 100644 --- a/pcbnew/kicad_plugin.cpp +++ b/pcbnew/kicad_plugin.cpp @@ -1905,7 +1905,7 @@ void PCB_IO::format( ZONE_CONTAINER* aZone, int aNestLevel ) const { m_out->Print( 0, " (island_removal_mode %d) (island_area_min %s)", static_cast( aZone->GetIslandRemovalMode() ), - FormatInternalUnits( aZone->GetMinIslandArea() ).c_str() ); + FormatInternalUnits( aZone->GetMinIslandArea() / IU_PER_MM ).c_str() ); } if( aZone->GetFillMode() == ZONE_FILL_MODE::HATCH_PATTERN ) diff --git a/pcbnew/pcb_parser.cpp b/pcbnew/pcb_parser.cpp index 88e80fa4bd..e1049d5142 100644 --- a/pcbnew/pcb_parser.cpp +++ b/pcbnew/pcb_parser.cpp @@ -3981,9 +3981,12 @@ ZONE_CONTAINER* PCB_PARSER::parseZONE_CONTAINER( BOARD_ITEM_CONTAINER* aParent ) break; case T_island_area_min: - zone->SetMinIslandArea( parseBoardUnits( T_island_area_min ) ); + { + int area = parseBoardUnits( T_island_area_min ); + zone->SetMinIslandArea( area * IU_PER_MM ); NeedRIGHT(); break; + } default: Expecting( "mode, arc_segments, thermal_gap, thermal_bridge_width, " diff --git a/pcbnew/plot_board_layers.cpp b/pcbnew/plot_board_layers.cpp index c00f0289ab..130cfb8165 100644 --- a/pcbnew/plot_board_layers.cpp +++ b/pcbnew/plot_board_layers.cpp @@ -500,7 +500,7 @@ void PlotStandardLayer( BOARD *aBoard, PLOTTER* aPlotter, // aggregateArea will be simplified and fractured // (Long calculation time) - for( int i = 0; i < aggregateArea.OutlineCount(); i++ ) + for( int i = aggregateArea.OutlineCount() - 1; i >= 0; i-- ) { if( zone->IsIsland( layer, i ) ) { @@ -539,7 +539,7 @@ void PlotStandardLayer( BOARD *aBoard, PLOTTER* aPlotter, SHAPE_POLY_SET candidateArea = candidate->GetFilledPolysList( layer ); - for( int i = 0; i < candidateArea.OutlineCount(); i++ ) + for( int i = candidateArea.OutlineCount() - 1; i >= 0; i-- ) { if( candidate->IsIsland( layer, i ) ) { diff --git a/pcbnew/zone_filler.cpp b/pcbnew/zone_filler.cpp index 337e80fa70..8d02bfcdb7 100644 --- a/pcbnew/zone_filler.cpp +++ b/pcbnew/zone_filler.cpp @@ -230,17 +230,20 @@ bool ZONE_FILLER::Fill( const std::vector& aZones, bool aCheck // Remove solid areas outside the board cutouts and the insulated islands // only zones with net code > 0 can have insulated islands by definition - if( zone.m_zone->GetNetCode() > 0 && mode != ISLAND_REMOVAL_MODE::NEVER ) + if( zone.m_zone->GetNetCode() > 0 ) { - // Area threshold is stored 1-D in internal units - if( mode == ISLAND_REMOVAL_MODE::AREA ) - minArea *= minArea; - // solid areas outside the board cutouts are also removed, because they are usually // insulated islands for( auto idx : zone.m_islands ) { - if( mode == ISLAND_REMOVAL_MODE::ALWAYS || poly.Outline( idx ).Area() < minArea + double metricMin = minArea * ( MM_PER_IU * MM_PER_IU ); + double metricArea = poly.Outline( idx ).Area() * ( MM_PER_IU * MM_PER_IU ); + + std::cout << ( metricArea < metricMin ) << std::endl; + + if( mode == ISLAND_REMOVAL_MODE::ALWAYS + || ( mode == ISLAND_REMOVAL_MODE::AREA + && poly.Outline( idx ).Area() < minArea ) || !m_boardOutline.Contains( poly.Polygon( idx ).front().CPoint( 0 ) ) ) poly.DeletePolygon( idx ); else