From 3f8cada334e373371a995eaac156b73e9b288ae9 Mon Sep 17 00:00:00 2001
From: Jeff Young <jeff@rokeby.ie>
Date: Thu, 3 Feb 2022 19:38:54 +0000
Subject: [PATCH] Fix some issues with new polygonization of arcs and arc
collision test.
Test should not be testing against the polygonization error; if done
correctly that should all be on the correct side of the shape. Use an
epsilon instead (I chose polygonization error / 10, but the value isn't
terribly important).
Fixes https://gitlab.com/kicad/code/kicad/issues/10724
---
.../src/convert_basic_shapes_to_polygon.cpp | 21 ++++++++++++-------
qa/libs/kimath/geometry/test_shape_arc.cpp | 11 ++++------
2 files changed, 17 insertions(+), 15 deletions(-)
diff --git a/libs/kimath/src/convert_basic_shapes_to_polygon.cpp b/libs/kimath/src/convert_basic_shapes_to_polygon.cpp
index be5fb26f3e..a9eab4ff39 100644
--- a/libs/kimath/src/convert_basic_shapes_to_polygon.cpp
+++ b/libs/kimath/src/convert_basic_shapes_to_polygon.cpp
@@ -583,6 +583,14 @@ void TransformArcToPolygon( SHAPE_POLY_SET& aCornerBuffer, const VECTOR2I& aStar
EDA_ANGLE arc_angle_start = arc.GetStartAngle();
EDA_ANGLE arc_angle = arc.GetCentralAngle();
EDA_ANGLE arc_angle_end = arc_angle_start + arc_angle;
+ EDA_ANGLE sweep = arc_angle < ANGLE_0 ? -ANGLE_180 : ANGLE_180;
+
+ if( arc_angle < ANGLE_0 )
+ {
+ std::swap( arc_angle_start, arc_angle_end );
+ arc = SHAPE_ARC( aEnd, aMid, aStart, aWidth );
+ arc_angle = -arc_angle;
+ }
int radial_offset = arc.GetWidth() / 2;
int arc_outer_radius = arc.GetRadius() + radial_offset;
@@ -596,27 +604,24 @@ void TransformArcToPolygon( SHAPE_POLY_SET& aCornerBuffer, const VECTOR2I& aStar
SHAPE_LINE_CHAIN& outline = polyshape.Outline( 0 );
// Starting end
- ConvertArcToPolyline( outline, arc.GetP0(), radial_offset, -arc_angle_start, ANGLE_180, aError,
- aErrorLoc );
+ ConvertArcToPolyline( outline, arc.GetP0(), radial_offset, arc_angle_start - ANGLE_180,
+ ANGLE_180, aError, aErrorLoc );
// Outside edge
ConvertArcToPolyline( outline, arc.GetCenter(), arc_outer_radius, arc_angle_start, arc_angle,
aError, errorLocOuter );
// Other end
- ConvertArcToPolyline( outline, arc.GetP1(), radial_offset, -arc_angle_end, ANGLE_180, aError,
+ ConvertArcToPolyline( outline, arc.GetP1(), radial_offset, arc_angle_end, ANGLE_180, aError,
aErrorLoc );
// Inside edge
if( arc_inner_radius > 0 )
{
- ConvertArcToPolyline( outline, arc.GetCenter(), arc_inner_radius, arc_angle_end, -arc_angle,
- aError, errorLocInner );
+ ConvertArcToPolyline( outline, arc.GetCenter(), arc_inner_radius, arc_angle_end,
+ -arc_angle, aError, errorLocInner );
}
- // Required. Otherwise Clipper has fits with duplicate points generating winding artefacts.
- polyshape.Simplify( SHAPE_POLY_SET::PM_FAST );
-
aCornerBuffer.Append( polyshape );
}
diff --git a/qa/libs/kimath/geometry/test_shape_arc.cpp b/qa/libs/kimath/geometry/test_shape_arc.cpp
index 32e94b5dcb..161c69a5f3 100644
--- a/qa/libs/kimath/geometry/test_shape_arc.cpp
+++ b/qa/libs/kimath/geometry/test_shape_arc.cpp
@@ -886,7 +886,7 @@ BOOST_AUTO_TEST_CASE( CollideArcToShapeLineChain )
BOOST_AUTO_TEST_CASE( CollideArcToPolygonApproximation )
{
SHAPE_ARC arc( VECTOR2I( 73843527, 74355869 ), VECTOR2I( 71713528, 72965869 ),
- EDA_ANGLE( -76.36664803, DEGREES_T ), 2000000 );
+ EDA_ANGLE( -76.36664803, DEGREES_T ), 1000000 );
// Create a polyset approximation from the arc - error outside (simulating the zone filler)
SHAPE_POLY_SET arcBuffer;
@@ -917,14 +917,11 @@ BOOST_AUTO_TEST_CASE( CollideArcToPolygonApproximation )
int actual = 0;
VECTOR2I location;
+ int epsilon = polygonApproximationError / 10;
- int clearanceReduced = clearance - polygonApproximationError;
+ BOOST_CHECK_EQUAL( zoneFill.Collide( &arc, clearance + epsilon, &actual, &location ), true );
- BOOST_CHECK_EQUAL( zoneFill.Collide( &arc, clearanceReduced, &actual, &location ), false );
-
- BOOST_CHECK_EQUAL( zoneFill.Collide( &arc, clearance * 2, &actual, &location ), true );
-
- BOOST_CHECK( KI_TEST::IsWithin( actual, clearance, polygonApproximationError ) );
+ BOOST_CHECK_EQUAL( zoneFill.Collide( &arc, clearance - epsilon, &actual, &location ), false );
}