From 0ed26e4e339166318cfe0bb2afc2e84633f9bfc7 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Sun, 23 Jun 2019 11:14:26 +0100 Subject: [PATCH] Fix some geometry error in thermal spoke calculations. Also introduces some performance enhancements to make up for the extra time spent in the calculations. --- common/geometry/shape_line_chain.cpp | 13 ++++++-- common/geometry/shape_poly_set.cpp | 44 ++++++++++++---------------- pcbnew/zone_filler.cpp | 23 +++++++-------- 3 files changed, 40 insertions(+), 40 deletions(-) diff --git a/common/geometry/shape_line_chain.cpp b/common/geometry/shape_line_chain.cpp index 6ce8633db3..1b70e12f70 100644 --- a/common/geometry/shape_line_chain.cpp +++ b/common/geometry/shape_line_chain.cpp @@ -368,11 +368,17 @@ bool SHAPE_LINE_CHAIN::PointInside( const VECTOR2I& aPt, int aAccuracy ) const * * Note: slope might be denormal here in the case of a horizontal line but we require our * y to move from above to below the point (or vice versa) + * + * Note: we open-code CPoint() here so that we don't end up calculating the size of the + * vector number-of-points times. This has a non-trivial impact on zone fill times. */ - for( int i = 0; i < PointCount(); i++ ) + const std::vector& points = CPoints(); + int pointCount = points.size(); + + for( int i = 0; i < pointCount; ) { - const auto p1 = CPoint( i ); - const auto p2 = CPoint( i + 1 ); // CPoint wraps, so ignore counts + const auto p1 = points[ i++ ]; + const auto p2 = points[ i == pointCount ? 0 : i ]; const auto diff = p2 - p1; if( diff.y != 0 ) @@ -383,6 +389,7 @@ bool SHAPE_LINE_CHAIN::PointInside( const VECTOR2I& aPt, int aAccuracy ) const inside = !inside; } } + return inside && !PointOnEdge( aPt, aAccuracy ); } diff --git a/common/geometry/shape_poly_set.cpp b/common/geometry/shape_poly_set.cpp index 22a404e5a5..49f378859c 100644 --- a/common/geometry/shape_poly_set.cpp +++ b/common/geometry/shape_poly_set.cpp @@ -626,14 +626,6 @@ void SHAPE_POLY_SET::importTree( PolyTree* tree ) struct FractureEdge { - FractureEdge( bool connected, SHAPE_LINE_CHAIN* owner, int index ) : - m_connected( connected ), - m_next( NULL ) - { - m_p1 = owner->CPoint( index ); - m_p2 = owner->CPoint( index + 1 ); - } - FractureEdge( int y = 0 ) : m_connected( false ), m_next( NULL ) @@ -749,25 +741,27 @@ void SHAPE_POLY_SET::fractureSingle( POLYGON& paths ) int num_unconnected = 0; - for( SHAPE_LINE_CHAIN& path : paths ) + for( const SHAPE_LINE_CHAIN& path : paths ) { - int index = 0; + const std::vector& points = path.CPoints(); + int pointCount = points.size(); FractureEdge* prev = NULL, * first_edge = NULL; int x_min = std::numeric_limits::max(); - for( int i = 0; i < path.PointCount(); i++ ) + for( const VECTOR2I& p : points ) { - const VECTOR2I& p = path.CPoint( i ); - if( p.x < x_min ) x_min = p.x; } - for( int i = 0; i < path.PointCount(); i++ ) + for( int i = 0; i < pointCount; i++ ) { - FractureEdge* fe = new FractureEdge( first, &path, index++ ); + // Do not use path.CPoint() here; open-coding it using the local variables "points" + // and "pointCount" gives a non-trivial performance boost to zone fill times. + FractureEdge* fe = new FractureEdge( first, points[ i ], + points[ i+1 == pointCount ? 0 : i+1 ] ); if( !root ) root = fe; @@ -778,7 +772,7 @@ void SHAPE_POLY_SET::fractureSingle( POLYGON& paths ) if( prev ) prev->m_next = fe; - if( i == path.PointCount() - 1 ) + if( i == pointCount - 1 ) fe->m_next = first_edge; prev = fe; @@ -805,14 +799,14 @@ void SHAPE_POLY_SET::fractureSingle( POLYGON& paths ) FractureEdge* smallestX = NULL; // find the left-most hole edge and merge with the outline - for( FractureEdgeSet::iterator i = border_edges.begin(); i != border_edges.end(); ++i ) + for( FractureEdge* border_edge : border_edges ) { - int xt = (*i)->m_p1.x; + int xt = border_edge->m_p1.x; - if( ( xt < x_min ) && !(*i)->m_connected ) + if( ( xt < x_min ) && !border_edge->m_connected ) { x_min = xt; - smallestX = *i; + smallestX = border_edge; } } @@ -831,10 +825,10 @@ void SHAPE_POLY_SET::fractureSingle( POLYGON& paths ) newPath.Append( e->m_p1 ); - for( FractureEdgeSet::iterator i = edges.begin(); i != edges.end(); ++i ) - delete *i; + for( FractureEdge* edge : edges ) + delete edge; - paths.push_back( newPath ); + paths.push_back( std::move( newPath ) ); } @@ -1445,11 +1439,11 @@ bool SHAPE_POLY_SET::containsSingle( const VECTOR2I& aP, int aSubpolyIndex, bool // Check that the point is not in any of the holes for( int holeIdx = 0; holeIdx < HoleCount( aSubpolyIndex ); holeIdx++ ) { - const SHAPE_LINE_CHAIN hole = CHole( aSubpolyIndex, holeIdx ); + const SHAPE_LINE_CHAIN& hole = CHole( aSubpolyIndex, holeIdx ); // If the point is inside a hole (and not on its edge), // it is outside of the polygon - if( pointInPolygon( aP, hole ) && !hole.PointOnEdge( aP ) ) + if( pointInPolygon( aP, hole ) ) return false; } } diff --git a/pcbnew/zone_filler.cpp b/pcbnew/zone_filler.cpp index fea4f2742f..47ab1e2139 100644 --- a/pcbnew/zone_filler.cpp +++ b/pcbnew/zone_filler.cpp @@ -730,8 +730,8 @@ void ZONE_FILLER::computeRawFilledArea( const ZONE_CONTAINER* aZone, for( SHAPE_LINE_CHAIN& spoke : thermalSpokes ) { - // Add together all spokes which connect to the zone's filled area - if( solidAreas.Contains( spoke.Point( 2 ) ) || solidAreas.Contains( spoke.Point( 3 ) ) ) + // Add together all spokes whose endpoints lie within the zone's filled area + if( solidAreas.Contains( spoke.Point(2) ) && solidAreas.Contains( spoke.Point(3) ) ) amalgamatedSpokes.AddOutline( spoke ); } @@ -859,16 +859,19 @@ void ZONE_FILLER::buildThermalSpokes( const ZONE_CONTAINER* aZone, biggest_clearance = std::max( biggest_clearance, zone_clearance ); zoneBB.Inflate( biggest_clearance ); - int outline_half_thickness = aZone->GetMinThickness() / 2; - int numSegs = std::max( GetArcToSegmentCount( outline_half_thickness, m_high_def, 360.0 ), 6 ); - double arcCorrection = GetCircletoPolyCorrectionFactor( numSegs ); + // half size of the pen used to draw/plot zones outlines + int pen_radius = aZone->GetMinThickness() / 2; // Is a point on the boundary of the polygon inside or outside? This small correction // lets us avoid the question. int boundaryCorrection = KiROUND( IU_PER_MM * 0.04 ); - // half size of the pen used to draw/plot zones outlines - int pen_radius = aZone->GetMinThickness() / 2; + // We'd normally add in an arcCorrection for circles (since a finite number of segments + // is only an approximation of the circle radius). However, boundaryCorrection is already + // twice even our ARC_LOW_DEF error tolerance, so there's little benefit to it (and a small + // but existant performance penalty). + //int numSegs = std::max( GetArcToSegmentCount( pen_raidus, m_high_def, 360.0 ), 6 ); + //double arcCorrection = GetCircletoPolyCorrectionFactor( numSegs ); for( auto module : m_board->Modules() ) { @@ -906,13 +909,9 @@ void ZONE_FILLER::buildThermalSpokes( const ZONE_CONTAINER* aZone, BOX2I reliefBB = pad->GetBoundingBox(); reliefBB.Inflate( thermalReliefGap + pen_radius + boundaryCorrection ); - // This is a CIRCLE pad tweak - // for circle pads, the thermal stubs orientation is 45 deg + // For circle pads, the thermal stubs orientation is 45 deg if( pad->GetShape() == PAD_SHAPE_CIRCLE ) - { - reliefBB.Inflate( ( reliefBB.GetWidth() * arcCorrection ) - reliefBB.GetWidth() ); spokeAngle = s_thermalRot; - } for( int i = 0; i < 4; i++ ) {