Fix several issues with multilayer zones

- Properly refill if layer set is modified
- Fix some threading issues with island removal
- Fix copy constructor

Fixes https://gitlab.com/kicad/code/kicad/-/issues/4765
This commit is contained in:
Jon Evans 2020-06-30 22:21:59 -04:00
parent e320a3f112
commit f8bfb2bc16
8 changed files with 140 additions and 105 deletions

View File

@ -87,6 +87,10 @@ ZONE_CONTAINER& ZONE_CONTAINER::operator=( const ZONE_CONTAINER& aOther )
m_Poly = new SHAPE_POLY_SET( *aOther.m_Poly ); m_Poly = new SHAPE_POLY_SET( *aOther.m_Poly );
m_isKeepout = aOther.m_isKeepout; m_isKeepout = aOther.m_isKeepout;
SetLayerSet( aOther.GetLayerSet() );
m_zoneName = aOther.m_zoneName;
m_IsFilled = aOther.m_IsFilled;
m_CornerSelection = nullptr; // for corner moving, corner index to (null if no selection) m_CornerSelection = nullptr; // for corner moving, corner index to (null if no selection)
m_ZoneClearance = aOther.m_ZoneClearance; // clearance value m_ZoneClearance = aOther.m_ZoneClearance; // clearance value
m_ZoneMinThickness = aOther.m_ZoneMinThickness; m_ZoneMinThickness = aOther.m_ZoneMinThickness;
@ -99,11 +103,14 @@ ZONE_CONTAINER& ZONE_CONTAINER::operator=( const ZONE_CONTAINER& aOther )
SetHatchPitch( aOther.GetHatchPitch() ); SetHatchPitch( aOther.GetHatchPitch() );
m_HatchLines = aOther.m_HatchLines; // copy vector <SEG> m_HatchLines = aOther.m_HatchLines; // copy vector <SEG>
m_FilledPolysList = aOther.m_FilledPolysList; for( PCB_LAYER_ID layer : aOther.GetLayerSet().Seq() )
m_RawPolysList = aOther.m_RawPolysList; {
m_filledPolysHash = aOther.m_filledPolysHash; m_FilledPolysList[layer] = aOther.m_FilledPolysList.at( layer );
m_FillSegmList = aOther.m_FillSegmList; // vector <> copy m_RawPolysList[layer] = aOther.m_RawPolysList.at( layer );
m_insulatedIslands = aOther.m_insulatedIslands; m_filledPolysHash[layer] = aOther.m_filledPolysHash.at( layer );
m_FillSegmList[layer] = aOther.m_FillSegmList.at( layer ); // vector <> copy
m_insulatedIslands[layer] = aOther.m_insulatedIslands.at( layer );
}
m_HatchFillTypeThickness = aOther.m_HatchFillTypeThickness; m_HatchFillTypeThickness = aOther.m_HatchFillTypeThickness;
m_HatchFillTypeGap = aOther.m_HatchFillTypeGap; m_HatchFillTypeGap = aOther.m_HatchFillTypeGap;
@ -111,8 +118,6 @@ ZONE_CONTAINER& ZONE_CONTAINER::operator=( const ZONE_CONTAINER& aOther )
m_HatchFillTypeSmoothingLevel = aOther.m_HatchFillTypeSmoothingLevel; m_HatchFillTypeSmoothingLevel = aOther.m_HatchFillTypeSmoothingLevel;
m_HatchFillTypeSmoothingValue = aOther.m_HatchFillTypeSmoothingValue; m_HatchFillTypeSmoothingValue = aOther.m_HatchFillTypeSmoothingValue;
SetLayerSet( aOther.GetLayerSet() );
return *this; return *this;
} }
@ -137,6 +142,8 @@ void ZONE_CONTAINER::initDataFromSrcInCopyCtor( const ZONE_CONTAINER& aZone )
m_isKeepout = aZone.m_isKeepout; m_isKeepout = aZone.m_isKeepout;
SetLayerSet( aZone.GetLayerSet() ); SetLayerSet( aZone.GetLayerSet() );
m_zoneName = aZone.m_zoneName;
m_Poly = new SHAPE_POLY_SET( *aZone.m_Poly ); m_Poly = new SHAPE_POLY_SET( *aZone.m_Poly );
// For corner moving, corner index to drag, or nullptr if no selection // For corner moving, corner index to drag, or nullptr if no selection
@ -151,11 +158,15 @@ void ZONE_CONTAINER::initDataFromSrcInCopyCtor( const ZONE_CONTAINER& aZone )
m_PadConnection = aZone.m_PadConnection; m_PadConnection = aZone.m_PadConnection;
m_ThermalReliefGap = aZone.m_ThermalReliefGap; m_ThermalReliefGap = aZone.m_ThermalReliefGap;
m_ThermalReliefCopperBridge = aZone.m_ThermalReliefCopperBridge; m_ThermalReliefCopperBridge = aZone.m_ThermalReliefCopperBridge;
m_FilledPolysList = aZone.m_FilledPolysList;
m_RawPolysList = aZone.m_RawPolysList; for( PCB_LAYER_ID layer : aZone.GetLayerSet().Seq() )
m_filledPolysHash = aZone.m_filledPolysHash; {
m_FillSegmList = aZone.m_FillSegmList; // vector <> copy m_FilledPolysList[layer] = aZone.m_FilledPolysList.at( layer );
m_insulatedIslands = aZone.m_insulatedIslands; m_RawPolysList[layer] = aZone.m_RawPolysList.at( layer );
m_filledPolysHash[layer] = aZone.m_filledPolysHash.at( layer );
m_FillSegmList[layer] = aZone.m_FillSegmList.at( layer ); // vector <> copy
m_insulatedIslands[layer] = aZone.m_insulatedIslands.at( layer );
}
m_doNotAllowCopperPour = aZone.m_doNotAllowCopperPour; m_doNotAllowCopperPour = aZone.m_doNotAllowCopperPour;
m_doNotAllowVias = aZone.m_doNotAllowVias; m_doNotAllowVias = aZone.m_doNotAllowVias;
@ -267,6 +278,12 @@ void ZONE_CONTAINER::SetLayerSet( LSET aLayerSet )
UnFill(); UnFill();
m_FillSegmList.clear();
m_FilledPolysList.clear();
m_RawPolysList.clear();
m_filledPolysHash.clear();
m_insulatedIslands.clear();
for( PCB_LAYER_ID layer : aLayerSet.Seq() ) for( PCB_LAYER_ID layer : aLayerSet.Seq() )
{ {
m_FillSegmList[layer] = {}; m_FillSegmList[layer] = {};

View File

@ -30,6 +30,7 @@
#define CLASS_ZONE_H_ #define CLASS_ZONE_H_
#include <mutex>
#include <vector> #include <vector>
#include <gr_basic.h> #include <gr_basic.h>
#include <class_board_item.h> #include <class_board_item.h>
@ -176,6 +177,11 @@ public:
return m_area; return m_area;
} }
std::mutex& GetLock()
{
return m_lock;
}
bool IsFilled() const { return m_IsFilled; } bool IsFilled() const { return m_IsFilled; }
void SetIsFilled( bool isFilled ) { m_IsFilled = isFilled; } void SetIsFilled( bool isFilled ) { m_IsFilled = isFilled; }
@ -929,6 +935,9 @@ protected:
bool m_hv45; // constrain edges to horizontal, vertical or 45º bool m_hv45; // constrain edges to horizontal, vertical or 45º
double m_area; // The filled zone area double m_area; // The filled zone area
/// Lock used for multi-threaded filling on multi-layer zones
std::mutex m_lock;
}; };

View File

@ -552,38 +552,33 @@ void CN_CONNECTIVITY_ALGO::FindIsolatedCopperIslands( ZONE_CONTAINER* aZone,
void CN_CONNECTIVITY_ALGO::FindIsolatedCopperIslands( std::vector<CN_ZONE_ISOLATED_ISLAND_LIST>& aZones ) void CN_CONNECTIVITY_ALGO::FindIsolatedCopperIslands( std::vector<CN_ZONE_ISOLATED_ISLAND_LIST>& aZones )
{ {
for ( auto& z : aZones ) for( auto& z : aZones )
Remove( z.m_zone );
for ( auto& z : aZones )
{ {
for( PCB_LAYER_ID layer : z.m_zone->GetLayerSet().Seq() ) Remove( z.m_zone );
{ Add( z.m_zone );
if( !z.m_zone->GetFilledPolysList( layer ).IsEmpty() )
{
Add( z.m_zone );
break;
}
}
} }
m_connClusters = SearchClusters( CSM_CONNECTIVITY_CHECK ); m_connClusters = SearchClusters( CSM_CONNECTIVITY_CHECK );
for ( auto& zone : aZones ) for( auto& zone : aZones )
{ {
PCB_LAYER_ID layer = zone.m_layer; for( PCB_LAYER_ID layer : zone.m_zone->GetLayerSet().Seq() )
if( zone.m_zone->GetFilledPolysList( layer ).IsEmpty() )
continue;
for( const auto& cluster : m_connClusters )
{ {
if( cluster->Contains( zone.m_zone ) && cluster->IsOrphaned() ) if( zone.m_zone->GetFilledPolysList( layer ).IsEmpty() )
continue;
for( const auto& cluster : m_connClusters )
{ {
for( auto z : *cluster ) if( cluster->Contains( zone.m_zone ) && cluster->IsOrphaned() )
{ {
if( z->Parent() == zone.m_zone && z->Layer() == layer ) for( auto z : *cluster )
zone.m_islands.push_back( static_cast<CN_ZONE*>( z )->SubpolyIndex() ); {
if( z->Parent() == zone.m_zone && z->Layer() == layer )
{
zone.m_islands[layer].push_back(
static_cast<CN_ZONE*>( z )->SubpolyIndex() );
}
}
} }
} }
} }

View File

@ -61,21 +61,17 @@ struct CN_DISJOINT_NET_ENTRY
}; };
/** /**
* A structure used for filling a copper zone on one layer. * A structure used for calculating isolated islands on a given zone across all its layers
* Multilayer zones will have one of these for each active layer.
*/ */
struct CN_ZONE_ISOLATED_ISLAND_LIST struct CN_ZONE_ISOLATED_ISLAND_LIST
{ {
CN_ZONE_ISOLATED_ISLAND_LIST( ZONE_CONTAINER* aZone, PCB_LAYER_ID aLayer ) : CN_ZONE_ISOLATED_ISLAND_LIST( ZONE_CONTAINER* aZone ) :
m_zone( aZone ), m_zone( aZone )
m_layer( aLayer )
{} {}
ZONE_CONTAINER* m_zone; ZONE_CONTAINER* m_zone;
PCB_LAYER_ID m_layer; std::map<PCB_LAYER_ID, std::vector<int>> m_islands;
std::vector<int> m_islands;
}; };
struct RN_DYNAMIC_LINE struct RN_DYNAMIC_LINE

View File

@ -100,7 +100,9 @@ void ZONE_FILLER::InstallNewProgressReporter( wxWindow* aParent, const wxString&
bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck ) bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck )
{ {
std::vector<CN_ZONE_ISOLATED_ISLAND_LIST> toFill; std::vector<std::pair<ZONE_CONTAINER*, PCB_LAYER_ID>> toFill;
std::vector<CN_ZONE_ISOLATED_ISLAND_LIST> islandsList;
auto connectivity = m_board->GetConnectivity(); auto connectivity = m_board->GetConnectivity();
bool filledPolyWithOutline = not m_board->GetDesignSettings().m_ZoneUseNoOutlineInFill; bool filledPolyWithOutline = not m_board->GetDesignSettings().m_ZoneUseNoOutlineInFill;
@ -112,7 +114,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
if( m_progressReporter ) if( m_progressReporter )
{ {
m_progressReporter->Report( aCheck ? _( "Checking zone fills..." ) : _( "Building zone fills..." ) ); m_progressReporter->Report( aCheck ? _( "Checking zone fills..." ) : _( "Building zone fills..." ) );
m_progressReporter->SetMaxProgress( toFill.size() ); m_progressReporter->SetMaxProgress( aZones.size() );
} }
// The board outlines is used to clip solid areas inside the board (when outlines are valid) // The board outlines is used to clip solid areas inside the board (when outlines are valid)
@ -145,9 +147,11 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
zone->BuildHashValue( layer ); zone->BuildHashValue( layer );
// Add the zone to the list of zones to test or refill // Add the zone to the list of zones to test or refill
toFill.emplace_back( CN_ZONE_ISOLATED_ISLAND_LIST( zone, layer ) ); toFill.emplace_back( std::make_pair( zone, layer ) );
} }
islandsList.emplace_back( CN_ZONE_ISOLATED_ISLAND_LIST( zone ) );
// Remove existing fill first to prevent drawing invalid polygons // Remove existing fill first to prevent drawing invalid polygons
// on some platforms // on some platforms
zone->UnFill(); zone->UnFill();
@ -164,14 +168,16 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
for( size_t i = nextItem++; i < toFill.size(); i = nextItem++ ) for( size_t i = nextItem++; i < toFill.size(); i = nextItem++ )
{ {
PCB_LAYER_ID layer = toFill[i].m_layer; PCB_LAYER_ID layer = toFill[i].second;
ZONE_CONTAINER* zone = toFill[i].m_zone; ZONE_CONTAINER* zone = toFill[i].first;
zone->SetFilledPolysUseThickness( filledPolyWithOutline ); zone->SetFilledPolysUseThickness( filledPolyWithOutline );
SHAPE_POLY_SET rawPolys, finalPolys; SHAPE_POLY_SET rawPolys, finalPolys;
fillSingleZone( zone, layer, rawPolys, finalPolys ); fillSingleZone( zone, layer, rawPolys, finalPolys );
std::unique_lock<std::mutex> zoneLock( zone->GetLock() );
zone->SetRawPolysList( layer, rawPolys ); zone->SetRawPolysList( layer, rawPolys );
zone->SetFilledPolysList( layer, finalPolys ); zone->SetFilledPolysList( layer, finalPolys );
zone->SetIsFilled( true ); zone->SetIsFilled( true );
@ -215,68 +221,70 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
} }
connectivity->SetProgressReporter( m_progressReporter ); connectivity->SetProgressReporter( m_progressReporter );
connectivity->FindIsolatedCopperIslands( toFill ); connectivity->FindIsolatedCopperIslands( islandsList );
// Now remove insulated copper islands and islands outside the board edge // Now remove insulated copper islands and islands outside the board edge
bool outOfDate = false; bool outOfDate = false;
for( auto& zone : toFill ) for( auto& zone : islandsList )
{ {
std::sort( zone.m_islands.begin(), zone.m_islands.end(), std::greater<int>() ); for( PCB_LAYER_ID layer : zone.m_zone->GetLayerSet().Seq() )
SHAPE_POLY_SET poly = zone.m_zone->GetFilledPolysList( zone.m_layer );
long long int minArea = zone.m_zone->GetMinIslandArea();
ISLAND_REMOVAL_MODE mode = zone.m_zone->GetIslandRemovalMode();
// 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 )
{ {
// solid areas outside the board cutouts are also removed, because they are usually if( !zone.m_islands.count( layer ) )
// insulated islands continue;
for( auto idx : zone.m_islands )
{
#if 0 // for tests only
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;
#endif
if( mode == ISLAND_REMOVAL_MODE::ALWAYS std::vector<int>& islands = zone.m_islands.at( layer );
|| ( mode == ISLAND_REMOVAL_MODE::AREA
&& poly.Outline( idx ).Area() < minArea ) std::sort( islands.begin(), islands.end(), std::greater<int>() );
|| !m_boardOutline.Contains( poly.Polygon( idx ).front().CPoint( 0 ) ) ) SHAPE_POLY_SET poly = zone.m_zone->GetFilledPolysList( layer );
poly.DeletePolygon( idx );
else long long int minArea = zone.m_zone->GetMinIslandArea();
zone.m_zone->SetIsIsland( zone.m_layer, idx ); ISLAND_REMOVAL_MODE mode = zone.m_zone->GetIslandRemovalMode();
}
} // Remove solid areas outside the board cutouts and the insulated islands
// Zones with no net can have areas outside the board cutouts. // only zones with net code > 0 can have insulated islands by definition
// By definition, Zones with no net have no isolated island if( zone.m_zone->GetNetCode() > 0 )
// (in fact all filled areas are isolated islands)
// but they can have some areas outside the board cutouts.
// A filled area outside the board cutouts has all points outside cutouts,
// so we only need to check one point for each filled polygon.
// Note also non copper zones are already clipped
else if( m_brdOutlinesValid && zone.m_zone->IsOnCopperLayer() )
{
for( int idx = 0; idx < poly.OutlineCount(); )
{ {
if( poly.Polygon( idx ).empty() // solid areas outside the board cutouts are also removed, because they are usually
|| !m_boardOutline.Contains( poly.Polygon( idx ).front().CPoint( 0 ) ) ) // insulated islands
for( auto idx : islands )
{ {
poly.DeletePolygon( idx ); 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
zone.m_zone->SetIsIsland( layer, idx );
} }
else
idx++;
} }
// Zones with no net can have areas outside the board cutouts.
// By definition, Zones with no net have no isolated island
// (in fact all filled areas are isolated islands)
// but they can have some areas outside the board cutouts.
// A filled area outside the board cutouts has all points outside cutouts,
// so we only need to check one point for each filled polygon.
// Note also non copper zones are already clipped
else if( m_brdOutlinesValid && zone.m_zone->IsOnCopperLayer() )
{
for( int idx = 0; idx < poly.OutlineCount(); )
{
if( poly.Polygon( idx ).empty()
|| !m_boardOutline.Contains( poly.Polygon( idx ).front().CPoint( 0 ) ) )
{
poly.DeletePolygon( idx );
}
else
idx++;
}
}
zone.m_zone->SetFilledPolysList( layer, poly );
zone.m_zone->CalculateFilledArea();
if( aCheck && zone.m_zone->GetHashValue( layer ) != poly.GetHash() )
outOfDate = true;
} }
zone.m_zone->SetFilledPolysList( zone.m_layer, poly );
zone.m_zone->CalculateFilledArea();
if( aCheck && zone.m_zone->GetHashValue( zone.m_layer ) != poly.GetHash() )
outOfDate = true;
} }
if( aCheck && outOfDate ) if( aCheck && outOfDate )
@ -312,9 +320,9 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
{ {
size_t num = 0; size_t num = 0;
for( size_t i = nextItem++; i < toFill.size(); i = nextItem++ ) for( size_t i = nextItem++; i < islandsList.size(); i = nextItem++ )
{ {
toFill[i].m_zone->CacheTriangulation(); islandsList[i].m_zone->CacheTriangulation();
num++; num++;
if( m_progressReporter ) if( m_progressReporter )
@ -361,7 +369,7 @@ bool ZONE_FILLER::Fill( const std::vector<ZONE_CONTAINER*>& aZones, bool aCheck
else else
{ {
for( auto& i : toFill ) for( auto& i : toFill )
connectivity->Update( i.m_zone ); connectivity->Update( i.first );
connectivity->RecalculateRatsnest(); connectivity->RecalculateRatsnest();
} }

View File

@ -129,7 +129,8 @@ void PCB_EDIT_FRAME::Edit_Zone_Params( ZONE_CONTAINER* aZone )
continue; continue;
} }
if( zone->IsFilled() ) // aZone won't be filled if the layer set was modified, but it needs to be updated
if( zone->IsFilled() || zone == aZone )
zones_to_refill.push_back( zone ); zones_to_refill.push_back( zone );
} }

View File

@ -62,7 +62,7 @@
bool ZONE_CONTAINER::IsSame( const ZONE_CONTAINER& aZoneToCompare ) bool ZONE_CONTAINER::IsSame( const ZONE_CONTAINER& aZoneToCompare )
{ {
// compare basic parameters: // compare basic parameters:
if( GetLayer() != aZoneToCompare.GetLayer() ) if( GetLayerSet() != aZoneToCompare.GetLayerSet() )
return false; return false;
if( GetNetCode() != aZoneToCompare.GetNetCode() ) if( GetNetCode() != aZoneToCompare.GetNetCode() )
@ -111,6 +111,15 @@ bool ZONE_CONTAINER::IsSame( const ZONE_CONTAINER& aZoneToCompare )
if( m_ThermalReliefCopperBridge != aZoneToCompare.m_ThermalReliefCopperBridge ) if( m_ThermalReliefCopperBridge != aZoneToCompare.m_ThermalReliefCopperBridge )
return false; return false;
if( m_zoneName != aZoneToCompare.m_zoneName )
return false;
if( m_islandRemovalMode != aZoneToCompare.m_islandRemovalMode )
return false;
if( m_minIslandArea != aZoneToCompare.m_minIslandArea )
return false;
// Compare outlines // Compare outlines
wxASSERT( m_Poly ); // m_Poly == NULL Should never happen wxASSERT( m_Poly ); // m_Poly == NULL Should never happen
@ -149,7 +158,7 @@ int SaveCopyOfZones( PICKED_ITEMS_LIST& aPickList, BOARD* aPcb, int aNetCode, LA
if( aNetCode >= 0 && aNetCode != zone->GetNetCode() ) if( aNetCode >= 0 && aNetCode != zone->GetNetCode() )
continue; continue;
if( aLayer >= 0 && aLayer != zone->GetLayer() ) if( aLayer >= 0 && !zone->GetLayerSet().test( aLayer ) )
continue; continue;
ZONE_CONTAINER* zoneDup = new ZONE_CONTAINER( *zone ); ZONE_CONTAINER* zoneDup = new ZONE_CONTAINER( *zone );

View File

@ -127,7 +127,7 @@ bool BOARD::CombineAllAreasInNet( PICKED_ITEMS_LIST* aDeletedList, int aNetCode,
if( curr_area->GetIsKeepout() != area2->GetIsKeepout() ) if( curr_area->GetIsKeepout() != area2->GetIsKeepout() )
continue; continue;
if( curr_area->GetLayer() != area2->GetLayer() ) if( curr_area->GetLayerSet() != area2->GetLayerSet() )
continue; continue;
BOX2I b2 = area2->Outline()->BBox(); BOX2I b2 = area2->Outline()->BBox();