← Back to team overview

kicad-developers team mailing list archive

Re: [PATCH] Don't create an extra segment at the end of closed SHAPE_LINE_CHAIN


Hi Orson,

I decided to just fix it in POLYGON_GEOM_MANAGER for now as it is way less
risky.  I was worried we don't have a good way to test all the places that
use SHAPE_LINE_CHAIN and don't want to add risk for RC2.

Maybe we should revisit this after 5.0 release, I think fixing this could
simplify some code in places where SHAPE_LINE_CHAIN is used because there
no longer be this special case to check for.


On Mon, Mar 26, 2018 at 6:58 AM, Maciej Sumiński <maciej.suminski@xxxxxxx>

> Hi Jon,
> If we decide to handle this special case, then we need to account for
> the point count as well. Otherwise it may lead to situations when the
> number of points is higher than the number of segments.
> Please see the attached patch (particularly 0002). I think these changes
> might be committed after a short test period. If we want to go the safe,
> but not so elegant way, then perhaps a simple condition could be added
> to POLYGON_GEOM_MANAGER::IsSelfIntersecting().
> Cheers,
> Orson
> On 03/25/2018 04:07 AM, Jon Evans wrote:
> > Hi all (but especially Orson),
> >
> > I wanted to fix the issue Bernhard raised here:
> > https://bugs.launchpad.net/kicad/+bug/1751654/comments/7
> >
> > I dug in to it a bit and found out
> > that SHAPE_LINE_CHAIN::SelfIntersecting() doesn't work right when
> polygons
> > are actually closed (i.e. the last point is the same as the first) and
> when
> > m_closed is set to true.
> >
> > The attached patch fixes this by only generating a closing segment when
> the
> > last point isn't the same as the first point.  It fixes the issue with
> the
> > self-intersection warning showing up even when you aren't yet
> intersecting
> > (i.e. when the last point is the same as the first), and I didn't notice
> > any obvious other issues, but maybe you can double-check that changing
> the
> > behavior of SegmentCount() won't have any strange side-effects.
> >
> > Thanks,
> > -Jon
> >