Fix compare segments endpoint touching

[bluenote10]

This fixes a bug in compare_segments related to start/left points falling exactly onto the other segment. In this case the proper solution is to use the end/right points for the ordering.

In case it helps, here is what I used to debug #110:

When inserting the "highlighted" segment, its neighbors are the ones labeled with "next" and "prev". Normally "next" should always be the one above. If a start/left point falls exactly on top of a segment the existing implementation would always report the segment is "above" even though it can be "below". In the follow-up this leads to wrongly filled itOut/otherInOut/inResult fields.

The behavior can also be verified in test case #110 by multiplying all y values by -1 -- in this case the old logic happened to be right.

I think the right solution is to determine the case when the start/left point falls onto the other segment and use the right point to determine the order in this case. This is also how it is done in polybooljs.

Fixes:

• Wrong result for two simple polygons #110
• Simple intersection fails #97

Other tested issues:

• Fixes the main issue of Overlapping edges still a problem #35 but not the diff result problem mentioned as well.

[rowanwins]

WIll try and do a proper review this evening @bluenote10 but it looks relatively straight forward - and bonus marks for including new tests!

[rowanwins]

With the revised changes to the demo site in #119 this no longer seems to fix #110 , sorry @bluenote10 :(

97 does seem to work though 👍

[bluenote10]

I've done all debugging on nodejs, and the tests also succeed in nodejs. Do you mean that after we fix #118, it doesn't work in the demo site, and we have effectively reached a point where the browser and the (node based) test suit behave differently?

If #97 works, but #110 doesn't I think I know what is happening: This PR fixes the issue that occurs if signedArea returns exactly 0 for the left segment point. In #97, there is a horizontal line, so the other endpoint can perfectly fall onto that segment. In #110 its a "diagonal" segment, so falling onto the segment is more challenging numerically. In nodejs, orient2d returns a perfect zero, which makes it the same case. If in the browser orient2d is a tiny non-zero, it most likely turns into the problem described in #114 (inconsistency between compare segment and intersection computation).

Maybe it's worth checking if robust-predicates is supposed to work the same in the browser and in node.

[rowanwins]

In nodejs, orient2d returns a perfect zero, which makes it the same case. If in the browser orient2d is a tiny non-zero

Are you able to create a gist or code snippet @bluenote10 which shows the coords you use to generate the 0 score in nodejs? I'll then run some browser comparisons and can see if I can make things a bit more cross-platform compatible.

[bluenote10]

The test case for #110 is already part of the PR, see the main.test('issue #110', (t) => { ... } for a reproducible snippet. I could later also add some details from the debugger output if it's helpful.

[bluenote10]

Ah, now I got it! The issue110.geojson file that has been committed in the repo is confusing: It is missing the last coordinates in the polygon specification. For some reason the visualization doesn't have a problem with that, but the boolean operations do. When I merge your #119 branch into this branch, and add the closing point in issue110.geojson the demo is now working as well:

We should probably follow up on the idea of #67 -- this had already saved me a lot of time, it is very confusing that omitting the last coordinate is allowed sometimes, not other times.

[rowanwins]

That was driving me crazy - good find @bluenote10 !

[w8r]

Shall I take a look at this one to merge?

[bluenote10]

I think it would make more sense to first merge #113

This PR only fixes a small problem of compare_segments (fixes #110), but further fixes are required for #114. I've solved #114 in the linked PR on Rust side, but even that leaves a few open issues. I'm currently working on fixing the remaining issues around compare segment, and I would ping you / update this PR as soon as I have found a solution.

[dbuenzli]

I'm not sure whether this can be solved by solely at the segment comparison level. Just to give a simpler example, here's a union failing:

with triangles coordinates (-1, 1) (1, -1) (1, 1) (green) and (0, 0) (0, 1) (1, 0) (purple). As @bluenote10 notes this happens when there's a single divide here. In my test case this happens on the point (0,0) here and we get the segment (0,0)-(1,0) processed before (0,0)-(1,-1) which breaks the algorithm.

The logic of possible_intersection in the reference implementation is wrong. There is also this dodgy bit in divide_segment that mutates se.left which could be in your sweep line status and thus break the invariant that you only have left endpoints in it.

I haven't fully cooked up a fix for now. But I think the right way of solving this is to be much more careful in possible_intersection. This entails that on a divide that does so on a single segment you should thoroughly check the priority queue order of the new segments events against the one that was not divided. If there is a misordering, remove the event that was not divided from the status line and re-add it to the priority queue.

Besides I suspect that if you start mutating se.left you should likely also remove it from the status line (it's also strange not to have the same test for the other new event r) and re-add it to the priority queue. So that your left endpoints are always processed before your right ones as it should be.