Menu

#5 Last segment of an open path is ignored (patch included)

1.0
closed
nobody
None
2016-06-20
2015-09-08
Anonymous
No

Hi Tim,

I noticed that Clipper lib ignored the last segment of an open path in ctDifference operation. This is a sample code I've run.

openPath = [{X: 10, Y: 20}, {X: 10, Y: -20}, {X: -10, Y: -20}, {X: -10, Y: 20}, {X: 10, Y: 20}]
closedPath = [{X: -30, Y: -10}, {X: 40, Y: -10}, {X: 40, Y: 10}, {X: -30, Y: 10}]

solution = new ClipperLib.PolyTree()

c = new ClipperLib.Clipper()
c.AddPath(openPath, ClipperLib.PolyType.ptSubject, false)
c.AddPath(closedPath, ClipperLib.PolyType.ptClip, true)
c.Execute(ClipperLib.ClipType.ctDifference, solution)


The solution does not contain [{X: -10, Y: 20}, {X: 10, Y: 20}].

I've gone through the code and noticed, the last edge is removed if it coincides with the starting point. Although it has to be done for closed paths, I'm curious whether we shouldn't do that for open paths. So I've done following changes (also attached as a patch) and it works as expected. Could you please confirm?

--- clipper.js 2015-09-08 15:01:41.000000000 -0400
+++ clipper_patched.js 2015-09-08 15:01:31.000000000 -0400
@@ -2754,6 +2754,8 @@
{
if (E == E.Next)
break;
+ if (E.Next == eStart && !Closed)
+ break;
if (E == eStart)
eStart = E.Next;
E = this.RemoveEdge(E);

Thanks,

Ruwan

1 Attachments

Discussion

  • Ruwan Janapriya

    Ruwan Janapriya - 2015-09-08

    Sorry, for the formatting Tim, didn't mean those big letters. some how I got logged off, and this was posted as "Anonymous" and couldn't edit now :-).

     
  • Timo Kähkönen

    Timo Kähkönen - 2015-09-08

    Thanks for reporting. This was a bug in 6.1.3a (rev 452) version of original C# Clipper and it is fixed in 6.1.5 (rev 465).

    The fix in C# is this:

        if (E.Curr == E.Next.Curr && (Closed || E.Next != eStart))
        {
          if (E == E.Next) break;
          if (E == eStart) eStart = E.Next;
          E = RemoveEdge(E);
          eLoopStop = E;
          continue;
        }
    

    From: https://sourceforge.net/p/polyclipping/code/465/tree//trunk/C%23/clipper_library/clipper.cs

    This is a fix for JS Clipper 6.1.3.2:

    if (ClipperLib.IntPoint.op_Equality(E.Curr, E.Next.Curr) && (Closed || E.Next != eStart))
    {
        if (E == E.Next) break;
        if (E == eStart) eStart = E.Next;
        E = this.RemoveEdge(E);
        eLoopStop = E;
        continue;
    }
    

    In C# the IntPoints (eg. E.Curr and E.Next.Curr) are structs which are copied by value, not by reference as objects in Javascript.In C# the IntPoint1 == IntPoint2 comparison is made by overloading, but in JS we have to "manually" compare X and Y. Maybe it could be made by comparing references (E.Curr == E.Next.Curr), but I'm not sure if the reference could be broken in some code flow. Edges (E, E.Next, eStart) are internal classes, so they can be compared by reference.

    When I next time make updates, I'll update to the newest version of C# Clipper, so also this bug gets then addressed. Sorry about slow update cycle. I have a bit waited the Float C# Clipper to be released, but it may take time. It would give a huge performance boost in Javascript!

     
  • Ruwan Janapriya

    Ruwan Janapriya - 2015-09-08

    Thanks Timo for the response. After posting this I checked C# lib as well and seen that fix. You might also notice, the same check is done at the bottom of the loop as well.
    E = E.Next;
    if ((E == eLoopStop) || (!Closed && E.Next == eStart)) break;
    }
    Many thanks for porting and maintaining this wonderful lib!! Will patiently wait for the next update.

     
  • Timo Kähkönen

    Timo Kähkönen - 2016-06-20

    This is fixed in 6.2.1.0.

     
  • Timo Kähkönen

    Timo Kähkönen - 2016-06-20
    • status: open --> closed
     

Log in to post a comment.