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

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 👍

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.

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.

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.

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.

That was driving me crazy - good find @bluenote10 !

Shall I take a look at this one to merge?

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.

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.