electron-rare/kicad-source-mirror
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Pcbnew/Allegro: Break out zone merge function.

Browse Source 
This is a tricky function that really needs a test around it.

Also it has several assumptions and traps:

 - It will not work if the zones have multiple polygons in the poly sets
 - It will not work if any polygon has line chains in a different order
 - It is quadratic in number of zones and the compare itself is
   superlinear (Simplify is called, evilly in a const function, which is
   quadratic) and in a cyclic comparison, sort, which is presumably nlog-n
   ish.

However, at least for the quadratic nature, profiling shows this to be
absolutely insignificant even for the VCU118 board.
This commit is contained in:
John Beard
2026-03-07 20:57:28 +08:00
parent 6098e745be
commit d5e129a3a0
8 changed files with 342 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files
common/CMakeLists.txt
+2 -1
View File
@@ -840,7 +840,7 @@ if( KICAD_USE_PCH )
<clipper2/clipper.h>
<eda_draw_frame.h>
<kiid.h>
<pgm_base.h>
<pgm_base.h>
<wx/wx.h>
)
endif()
@@ -949,6 +949,7 @@ set( PCB_COMMON_SRCS
${CMAKE_SOURCE_DIR}/pcbnew/pcb_track.cpp
${CMAKE_SOURCE_DIR}/pcbnew/pcb_generator.cpp
${CMAKE_SOURCE_DIR}/pcbnew/zone.cpp
${CMAKE_SOURCE_DIR}/pcbnew/zone_utils.cpp
${CMAKE_SOURCE_DIR}/pcbnew/collectors.cpp
${CMAKE_SOURCE_DIR}/pcbnew/connectivity/connectivity_algo.cpp
${CMAKE_SOURCE_DIR}/pcbnew/connectivity/connectivity_items.cpp
libs/kimath/include/geometry/shape_utils.h
+8 -1
View File
@@ -81,6 +81,13 @@ std::array<SEG, 4> BoxToSegs( const BOX2I& aBox );
*/
void CollectBoxCorners( const BOX2I& aBox, std::vector<VECTOR2I>& aCorners );
/*
* Get a SHAPE_LINE_CHAIN representing the outline of a box.
*
* The first point and winding direction are not specified.
*/
SHAPE_LINE_CHAIN BoxToLineChain( const BOX2I& aBox );
/**
* Get the segments of a box that are in the given direction.
*
@@ -209,4 +216,4 @@ std::vector<VECTOR2I> MakeRegularPolygonPoints( const VECTOR2I& aCenter, size_t
std::vector<SEG> MakeCrossSegments( const VECTOR2I& aCenter, const VECTOR2I& aSize,
EDA_ANGLE aAngle );
} // namespace KIGEOM
} // namespace KIGEOM
libs/kimath/src/geometry/shape_utils.cpp
+14
View File
@@ -90,6 +90,20 @@ void KIGEOM::CollectBoxCorners( const BOX2I& aBox, std::vector<VECTOR2I>& aCorne
}
SHAPE_LINE_CHAIN KIGEOM::BoxToLineChain( const BOX2I& aBox )
{
SHAPE_LINE_CHAIN result;
result.Append( VECTOR2I{ aBox.GetLeft(), aBox.GetTop() } );
result.Append( VECTOR2I{ aBox.GetRight(), aBox.GetTop() } );
result.Append( VECTOR2I{ aBox.GetRight(), aBox.GetBottom() } );
result.Append( VECTOR2I{ aBox.GetLeft(), aBox.GetBottom() } );
result.SetClosed( true );
return result;
}
std::vector<SEG> KIGEOM::GetSegsInDirection( const BOX2I& aBox, DIRECTION_45::Directions aDir )
{
// clang-format off
pcbnew/pcb_io/allegro/allegro_builder.cpp
+3 -105
View File
@@ -39,6 +39,7 @@
#include <base_units.h>
#include <board_design_settings.h>
#include <convert_basic_shapes_to_polygon.h>
#include <geometry/shape_utils.h>
#include <project/net_settings.h>
#include <footprint.h>
@@ -49,7 +50,7 @@
#include <pcb_shape.h>
#include <pcb_track.h>
#include <zone.h>
#include <convert_basic_shapes_to_polygon.h>
#include <zone_utils.h>
using namespace ALLEGRO;
@@ -4057,109 +4058,6 @@ void BOARD_BUILDER::createBoardText()
}
/**
* Merges zones with identical outlines snd nets on different layers into single multi-layer zones.
*
* The merged zones (and any non-identical zones) are returned, the zones merged from are destroyed,
*/
static std::vector<std::unique_ptr<ZONE>> MergeZones( std::vector<std::unique_ptr<ZONE>> aZones, const BOARD& aBoard )
{
std::vector<std::unique_ptr<ZONE>> deduplicatedZones;
size_t mergedCount = 0;
size_t originalCount = aZones.size();
std::vector<bool> merged( aZones.size(), false );
for( size_t i = 0; i < aZones.size(); i++ )
{
if( merged[i] )
continue;
ZONE* primary = aZones[i].get();
const SHAPE_POLY_SET::POLYGON& primaryPolygon = primary->Outline()->CPolygon( 0 );
LSET layers = primary->GetLayerSet();
std::unordered_map<PCB_LAYER_ID, SHAPE_POLY_SET> mergedFills;
for( size_t j = i + 1; j < aZones.size(); j++ )
{
if( merged[j] )
continue;
ZONE* candidate = aZones[j].get();
if( candidate->GetNetCode() != primary->GetNetCode() )
continue;
const SHAPE_POLY_SET::POLYGON& candidatePolygon = candidate->Outline()->CPolygon( 0 );
if( primaryPolygon.size() != candidatePolygon.size() )
continue;
bool polygonsDiffer = false;
for( size_t lineChainId = 0; lineChainId < primaryPolygon.size(); lineChainId++ )
{
const SHAPE_LINE_CHAIN& primaryChain = primaryPolygon[lineChainId];
const SHAPE_LINE_CHAIN& candidateChain = candidatePolygon[lineChainId];
if( primaryChain.PointCount() != candidateChain.PointCount()
|| primaryChain.BBox() != candidateChain.BBox()
|| !primaryChain.CompareGeometry( candidateChain ) )
{
polygonsDiffer = true;
break;
}
}
if( !polygonsDiffer )
{
for( PCB_LAYER_ID layer : candidate->GetLayerSet() )
{
if( SHAPE_POLY_SET* fill = candidate->GetFill( layer ) )
mergedFills[layer] = *fill;
}
layers |= candidate->GetLayerSet();
merged[j] = true;
mergedCount++;
wxLogTrace( traceAllegroBuilder, " Merging zone on %s into zone on %s (net %d)",
aBoard.GetLayerName( candidate->GetFirstLayer() ),
aBoard.GetLayerName( primary->GetFirstLayer() ), primary->GetNetCode() );
}
}
if( layers != primary->GetLayerSet() )
{
for( PCB_LAYER_ID layer : primary->GetLayerSet() )
{
if( SHAPE_POLY_SET* fill = primary->GetFill( layer ) )
mergedFills[layer] = *fill;
}
primary->SetLayerSet( layers );
for( const auto& [layer, fill] : mergedFills )
primary->SetFilledPolysList( layer, fill );
primary->SetNeedRefill( false );
primary->SetIsFilled( true );
}
// Keep this zone
deduplicatedZones.push_back( std::move( aZones[i] ) );
}
if( mergedCount > 0 )
{
wxLogTrace( traceAllegroBuilder,
" Merged %zu zones into multi-layer zones (%zu zones remain from %zu)",
mergedCount, originalCount - mergedCount, originalCount );
}
return deduplicatedZones;
}
template <std::derived_from<BOARD_ITEM> T>
void BulkAddToBoard( BOARD& aBoard, std::vector<std::unique_ptr<T>>&& aItems )
{
@@ -4262,7 +4160,7 @@ void BOARD_BUILDER::createZones()
// Allegro often defines the same zone outline on multiple copper layers (e.g.
// a ground pour spanning all layers). KiCad represents this as a single zone
// with multiple fill layers.
std::vector<std::unique_ptr<ZONE>> mergedZones = MergeZones( std::move( boundaryZones ), m_board );
std::vector<std::unique_ptr<ZONE>> mergedZones = MergeZonesWithSameOutline( std::move( boundaryZones ) );
int mergedCount = mergedZones.size();
BulkAddToBoard( m_board, std::move( mergedZones ) );
pcbnew/zone_utils.cpp
+148
View File
@@ -0,0 +1,148 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright The 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 3
* 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/gpl-3.0.html
* or you may search the http://www.gnu.org website for the version 3 license,
* or you may write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/
#include "zone_utils.h"
#include <zone.h>
#include <geometry/shape_poly_set.h>
std::vector<std::unique_ptr<ZONE>> MergeZonesWithSameOutline( std::vector<std::unique_ptr<ZONE>>&& aZones )
{
const auto polygonsAreMergeable = []( const SHAPE_POLY_SET::POLYGON& a, const SHAPE_POLY_SET::POLYGON& b ) -> bool
{
if( a.size() != b.size() )
return false;
// NOTE: this assumes the polygons have their line chains in the same order
// But that is not actually required for same geometry (i.e. mergeability)
for( size_t lineChainId = 0; lineChainId < a.size(); lineChainId++ )
{
const SHAPE_LINE_CHAIN& chainA = a[lineChainId];
const SHAPE_LINE_CHAIN& chainB = b[lineChainId];
// Note: this assumes the polygons are either already simplified or that it's
// OK to not merge even if they would be the same after simplification.
if( chainA.PointCount() != chainB.PointCount() || chainA.BBox() != chainB.BBox()
|| !chainA.CompareGeometry( chainB ) )
{
// Different geometry, can't merge
return false;
}
}
return true;
};
const auto zonesAreMergeable = [&]( const ZONE& a, const ZONE& b ) -> bool
{
if( a.GetNetCode() != b.GetNetCode() )
return false;
// Can't merge rule areas with zone fills
if( a.GetIsRuleArea() != b.GetIsRuleArea() )
return false;
const SHAPE_POLY_SET* polySetA = a.Outline();
const SHAPE_POLY_SET* polySetB = b.Outline();
if( polySetA->OutlineCount() != polySetB->OutlineCount() )
return false;
if( polySetA->OutlineCount() == 0 )
{
// both have no outline, so they are the same, but we must not
// derefence them, as they are empty
return true;
}
// REVIEW: this assumes the zones only have a single polygon in the
const SHAPE_POLY_SET::POLYGON& polyA = polySetA->CPolygon( 0 );
const SHAPE_POLY_SET::POLYGON& polyB = polySetB->CPolygon( 0 );
return polygonsAreMergeable( polyA, polyB );
};
std::vector<std::unique_ptr<ZONE>> deduplicatedZones;
size_t mergedCount = 0;
// Map of zone indexes that we have already merged into a prior zone
std::vector<bool> merged( aZones.size(), false );
for( size_t i = 0; i < aZones.size(); i++ )
{
// This one has already been subsumed into a prior zone, so skip it
// and it will be dropped at the end.
if( merged[i] )
continue;
ZONE& primary = *aZones[i];
LSET layers = primary.GetLayerSet();
std::unordered_map<PCB_LAYER_ID, SHAPE_POLY_SET> mergedFills;
for( size_t j = i + 1; j < aZones.size(); j++ )
{
// This zone has already been subsumed by a prior zone, so it
// cannot be merged into another primary
if( merged[j] )
continue;
ZONE& candidate = *aZones[j];
bool canMerge = zonesAreMergeable( primary, candidate );
if( canMerge )
{
for( PCB_LAYER_ID layer : candidate.GetLayerSet() )
{
if( SHAPE_POLY_SET* fill = candidate.GetFill( layer ) )
mergedFills[layer] = *fill;
}
layers |= candidate.GetLayerSet();
merged[j] = true;
mergedCount++;
}
}
if( layers != primary.GetLayerSet() )
{
for( PCB_LAYER_ID layer : primary.GetLayerSet() )
{
if( SHAPE_POLY_SET* fill = primary.GetFill( layer ) )
mergedFills[layer] = *fill;
}
primary.SetLayerSet( layers );
for( const auto& [layer, fill] : mergedFills )
primary.SetFilledPolysList( layer, fill );
primary.SetNeedRefill( false );
primary.SetIsFilled( true );
}
// Keep this zone - it's a primary (may or may not have had other zones merged into it)
deduplicatedZones.push_back( std::move( aZones[i] ) );
}
return deduplicatedZones;
}
pcbnew/zone_utils.h
+39
View File
@@ -0,0 +1,39 @@
/*
* This program source code file is part of KiCad, a free EDA CAD application.
*
* Copyright The 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 3
* 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/gpl-3.0.html
* or you may search the http://www.gnu.org website for the version 3 license,
* or you may write to the Free Software Foundation, Inc.,
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
*/
#pragma once
#include <memory>
#include <vector>
class ZONE;
/**
* Merges zones with identical outlines and nets on different layers into single multi-layer zones.
*
* @param aZones is the zones to merge. Ownership of all the zones is taken (some will be returned)
*
* @return the merged zones. Ownership of all the returned zones is transferred to the caller.
*/
std::vector<std::unique_ptr<ZONE>> MergeZonesWithSameOutline( std::vector<std::unique_ptr<ZONE>>&& aZones );
qa/tests/libs/kimath/geometry/test_shape_line_chain.cpp
+25
View File
@@ -1300,6 +1300,31 @@ BOOST_AUTO_TEST_CASE( CompareGeometry )
}
BOOST_AUTO_TEST_CASE( CompareGeometryReversed )
{
// Square
const std::vector<VECTOR2I> ptsA = {
{ 0, 0 },
{ 100, 0 },
{ 100, 100 },
{ 0, 100 },
};
// Same points, same start, reversed
const std::vector<VECTOR2I> ptsB = {
{ 0, 0 },
{ 0, 100 },
{ 100, 100 },
{ 100, 0 },
};
SHAPE_LINE_CHAIN chainA( ptsA, true );
SHAPE_LINE_CHAIN chainB( ptsB, true );
BOOST_TEST( !chainA.CompareGeometry( chainB, false ) );
BOOST_TEST( chainA.CompareGeometry( chainB, true ) );
}
/**
* Test for issue #22597: Simplify with tolerance should reduce a polygon
* created from a rotated rounded rectangle (many small line segments approximating arcs).
qa/tests/pcbnew/test_zone.cpp
+103
View File
@@ -26,7 +26,9 @@
#include <board.h>
#include <footprint.h>
#include <geometry/shape_utils.h>
#include <zone.h>
#include <zone_utils.h>
struct ZONE_TEST_FIXTURE
@@ -35,6 +37,35 @@ struct ZONE_TEST_FIXTURE
};
static std::unique_ptr<ZONE> CreateSquareZone( BOARD_ITEM_CONTAINER& aParent, BOX2I aBox, PCB_LAYER_ID aLayer )
{
auto zone = std::make_unique<ZONE>( &aParent );
zone->SetLayer( aLayer );
auto outline = std::make_unique<SHAPE_POLY_SET>();
outline->AddOutline( KIGEOM::BoxToLineChain( aBox ) );
zone->SetOutline( outline.release() );
return zone;
}
/**
* Create a similar zone (same outline) on a different layer
*/
static std::unique_ptr<ZONE> CreateSimilarZone( BOARD_ITEM_CONTAINER& aParent, const ZONE& aOther, PCB_LAYER_ID aLayer )
{
auto zone = std::make_unique<ZONE>( &aParent );
zone->SetLayer( aLayer );
std::unique_ptr<SHAPE_POLY_SET> outline = std::make_unique<SHAPE_POLY_SET>( *aOther.Outline() );
zone->SetOutline( outline.release() );
return zone;
}
BOOST_FIXTURE_TEST_SUITE( Zone, ZONE_TEST_FIXTURE )
BOOST_AUTO_TEST_CASE( SingleLayer )
@@ -140,4 +171,76 @@ BOOST_AUTO_TEST_CASE( EmptyZoneGetPosition )
BOOST_TEST( zone.GetPosition() == VECTOR2I( 0, 0 ) );
}
BOOST_AUTO_TEST_CASE( ZoneMergeNull )
{
std::vector<std::unique_ptr<ZONE>> zones;
zones.emplace_back( std::make_unique<ZONE>( &m_board ) );
zones.back()->SetLayer( F_Cu );
zones.emplace_back( std::make_unique<ZONE>( &m_board ) );
zones.back()->SetLayer( F_Cu );
std::vector<std::unique_ptr<ZONE>> merged = MergeZonesWithSameOutline( std::move( zones ) );
// They are the same, so they do merge
BOOST_TEST( merged.size() == 1 );
}
BOOST_AUTO_TEST_CASE( ZoneMergeNonNullNoMerge )
{
std::vector<std::unique_ptr<ZONE>> zones;
zones.emplace_back( CreateSquareZone( m_board, BOX2I( VECTOR2I( 0, 0 ), VECTOR2I( 100, 100 ) ), F_Cu ) );
zones.emplace_back( CreateSquareZone( m_board, BOX2I( VECTOR2I( 200, 200 ), VECTOR2I( 300, 300 ) ), B_Cu ) );
std::vector<std::unique_ptr<ZONE>> merged = MergeZonesWithSameOutline( std::move( zones ) );
// They are different, so they don't merge
BOOST_TEST( merged.size() == 2 );
}
BOOST_AUTO_TEST_CASE( ZoneMergeNonNullMerge )
{
std::vector<std::unique_ptr<ZONE>> zones;
zones.emplace_back( CreateSquareZone( m_board, BOX2I( VECTOR2I( 0, 0 ), VECTOR2I( 100, 100 ) ), F_Cu ) );
zones.emplace_back( CreateSimilarZone( m_board, *zones.back(), B_Cu ) );
std::vector<std::unique_ptr<ZONE>> merged = MergeZonesWithSameOutline( std::move( zones ) );
// They are the same, so they do merge
BOOST_REQUIRE( merged.size() == 1 );
BOOST_TEST( merged[0]->GetLayerSet() == ( LSET{ F_Cu, B_Cu } ) );
BOOST_TEST( merged[0]->GetNumCorners() == 4 );
}
BOOST_AUTO_TEST_CASE( ZoneMergeMergeSameGeomDifferentOrder )
{
std::vector<std::unique_ptr<ZONE>> zones;
zones.emplace_back( CreateSquareZone( m_board, BOX2I( VECTOR2I( 0, 0 ), VECTOR2I( 100, 100 ) ), F_Cu ) );
zones.emplace_back( CreateSimilarZone( m_board, *zones.back(), B_Cu ) );
// Reverse the outline of one of them
// Don't go overboard here - detailed tests of CompareGeometry
// should be in the SHAPE_LINE_CHAIN tests.
auto newPolyB = std::make_unique<SHAPE_POLY_SET>( *zones.back()->Outline() );
newPolyB->Outline( 0 ).Reverse();
zones.back()->SetOutline( newPolyB.release() );
std::vector<std::unique_ptr<ZONE>> merged = MergeZonesWithSameOutline( std::move( zones ) );
// They are the same, so they do merge
BOOST_REQUIRE( merged.size() == 1 );
BOOST_TEST( merged[0]->GetLayerSet() == LSET( { F_Cu, B_Cu } ) );
BOOST_TEST( merged[0]->GetNumCorners() == 4 );
}
BOOST_AUTO_TEST_SUITE_END()