From 007906cd168cb62034bddff1fc86a53f34cbf0e7 Mon Sep 17 00:00:00 2001 From: Jeff Young Date: Fri, 8 Apr 2022 16:14:42 +0100 Subject: [PATCH] Fix a degeneracy bug in arc collisions. This also fixes a failure to use the correct effective width for shapes (which might, for instance, inherit their widths from schematic defaults, netclasses, etc.). Fixes https://gitlab.com/kicad/code/kicad/issues/11358 --- common/eda_shape.cpp | 30 +++++++++++++------------- eeschema/sch_shape.h | 8 ++++--- include/eda_shape.h | 5 +++-- libs/kimath/src/geometry/shape_arc.cpp | 2 +- 4 files changed, 24 insertions(+), 21 deletions(-) diff --git a/common/eda_shape.cpp b/common/eda_shape.cpp index 6d8ab6e795..95d1488330 100644 --- a/common/eda_shape.cpp +++ b/common/eda_shape.cpp @@ -1086,16 +1086,16 @@ void EDA_SHAPE::SetPolyPoints( const std::vector& aPoints ) std::vector EDA_SHAPE::MakeEffectiveShapes( bool aEdgeOnly ) const { std::vector effectiveShapes; + int width = GetEffectiveWidth(); switch( m_shape ) { case SHAPE_T::ARC: - effectiveShapes.emplace_back( new SHAPE_ARC( m_arcCenter, m_start, GetArcAngle(), - GetWidth() ) ); + effectiveShapes.emplace_back( new SHAPE_ARC( m_arcCenter, m_start, GetArcAngle(), width ) ); break; case SHAPE_T::SEGMENT: - effectiveShapes.emplace_back( new SHAPE_SEGMENT( m_start, m_end, GetWidth() ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( m_start, m_end, width ) ); break; case SHAPE_T::RECT: @@ -1105,12 +1105,12 @@ std::vector EDA_SHAPE::MakeEffectiveShapes( bool aEdgeOnly ) const if( IsFilled() && !aEdgeOnly ) effectiveShapes.emplace_back( new SHAPE_SIMPLE( pts ) ); - if( GetWidth() > 0 || !IsFilled() || aEdgeOnly ) + if( width > 0 || !IsFilled() || aEdgeOnly ) { - effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[0], pts[1], GetWidth() ) ); - effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[1], pts[2], GetWidth() ) ); - effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[2], pts[3], GetWidth() ) ); - effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[3], pts[0], GetWidth() ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[0], pts[1], width ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[1], pts[2], width ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[2], pts[3], width ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( pts[3], pts[0], width ) ); } } break; @@ -1120,21 +1120,21 @@ std::vector EDA_SHAPE::MakeEffectiveShapes( bool aEdgeOnly ) const if( IsFilled() && !aEdgeOnly ) effectiveShapes.emplace_back( new SHAPE_CIRCLE( getCenter(), GetRadius() ) ); - if( GetWidth() > 0 || !IsFilled() || aEdgeOnly ) - effectiveShapes.emplace_back( new SHAPE_ARC( getCenter(), GetEnd(), ANGLE_360 ) ); + if( width > 0 || !IsFilled() || aEdgeOnly ) + effectiveShapes.emplace_back( new SHAPE_ARC( getCenter(), GetEnd(), ANGLE_360, width ) ); break; } case SHAPE_T::BEZIER: { - std::vector bezierPoints = buildBezierToSegmentsPointsList( GetWidth() ); + std::vector bezierPoints = buildBezierToSegmentsPointsList( width ); VECTOR2I start_pt = bezierPoints[0]; for( unsigned int jj = 1; jj < bezierPoints.size(); jj++ ) { VECTOR2I end_pt = bezierPoints[jj]; - effectiveShapes.emplace_back( new SHAPE_SEGMENT( start_pt, end_pt, GetWidth() ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( start_pt, end_pt, width ) ); start_pt = end_pt; } @@ -1154,10 +1154,10 @@ std::vector EDA_SHAPE::MakeEffectiveShapes( bool aEdgeOnly ) const if( IsFilled() && !aEdgeOnly ) effectiveShapes.emplace_back( new SHAPE_SIMPLE( l ) ); - if( GetWidth() > 0 || !IsFilled() || aEdgeOnly ) + if( width > 0 || !IsFilled() || aEdgeOnly ) { for( int i = 0; i < l.SegmentCount(); i++ ) - effectiveShapes.emplace_back( new SHAPE_SEGMENT( l.Segment( i ), GetWidth() ) ); + effectiveShapes.emplace_back( new SHAPE_SEGMENT( l.Segment( i ), width ) ); } } break; @@ -1194,7 +1194,7 @@ bool EDA_SHAPE::IsPolyShapeValid() const if( GetPolyShape().OutlineCount() == 0 ) return false; - const SHAPE_LINE_CHAIN& outline = ( (SHAPE_POLY_SET&)GetPolyShape() ).Outline( 0 ); + const SHAPE_LINE_CHAIN& outline = static_cast( GetPolyShape() ).Outline( 0 ); return outline.PointCount() > 2; } diff --git a/eeschema/sch_shape.h b/eeschema/sch_shape.h index 9c31787c49..95e5aea251 100644 --- a/eeschema/sch_shape.h +++ b/eeschema/sch_shape.h @@ -58,6 +58,8 @@ public: int GetPenWidth() const override; + int GetEffectiveWidth() const override { return GetPenWidth(); } + bool HasLineStroke() const override { return true; } STROKE_PARAMS GetStroke() const override { return m_stroke; } void SetStroke( const STROKE_PARAMS& aStroke ) override; @@ -77,9 +79,9 @@ public: VECTOR2I GetCenter() const { return getCenter(); } - void BeginEdit( const VECTOR2I& aStartPoint ) { beginEdit( aStartPoint ); } - bool ContinueEdit( const VECTOR2I& aPosition ) { return continueEdit( aPosition ); } - void CalcEdit( const VECTOR2I& aPosition ) { calcEdit( aPosition ); } + void BeginEdit( const VECTOR2I& aStartPoint ) { beginEdit( aStartPoint ); } + bool ContinueEdit( const VECTOR2I& aPosition ) { return continueEdit( aPosition ); } + void CalcEdit( const VECTOR2I& aPosition ) { calcEdit( aPosition ); } void EndEdit() { endEdit(); } void SetEditState( int aState ) { setEditState( aState ); } diff --git a/include/eda_shape.h b/include/eda_shape.h index b080d474eb..2dd7f82655 100644 --- a/include/eda_shape.h +++ b/include/eda_shape.h @@ -104,9 +104,10 @@ public: void SetWidth( int aWidth ) { m_stroke.SetWidth( aWidth ); } int GetWidth() const { return m_stroke.GetWidth(); } + virtual int GetEffectiveWidth() const { return GetWidth(); } - void SetShape( SHAPE_T aShape ) { m_shape = aShape; } - SHAPE_T GetShape() const { return m_shape; } + void SetShape( SHAPE_T aShape ) { m_shape = aShape; } + SHAPE_T GetShape() const { return m_shape; } /** * Return the starting point of the graphic. diff --git a/libs/kimath/src/geometry/shape_arc.cpp b/libs/kimath/src/geometry/shape_arc.cpp index eb8c033c29..000c41a614 100644 --- a/libs/kimath/src/geometry/shape_arc.cpp +++ b/libs/kimath/src/geometry/shape_arc.cpp @@ -369,7 +369,7 @@ bool SHAPE_ARC::IsClockwise() const bool SHAPE_ARC::Collide( const VECTOR2I& aP, int aClearance, int* aActual, VECTOR2I* aLocation ) const { - int minDist = aClearance + m_width / 2; + int minDist = aClearance + std::max( m_width / 2, 1 ); auto bbox = BBox( minDist ); // Fast check using bounding box: