Add testcases from https://github.com/21re/rust-geo-booleanop and fix fatal2 test

[stefannikolei]

No description provided.

[stefannikolei]

@JimBobSquarePants i will have a look if I can fix more testcases over the weekend.

[JimBobSquarePants]

@stefannikolei Thanks mate. I've pushed some significant refactoring's to the code to pull it closer to the JS version but I'm not confident in my NextPos(...) implementation, I tried to port the JS and broke everything so left it as-is despite knowing it was different 🤣 If there's a bug, that's where it is.

… I’ve updated that method. Bug must be elsewhere

[JimBobSquarePants]

It's precision problem. I'll have to introduce a Vertex type that does double precision.

[JimBobSquarePants]

I've added my new Vertex type, it should perform well as I've backed it with intrinsics.

I'm currently testing fatal2.geojson which appears to initially queue already but yields 1 less (314 vs 315) sorted events than the JavaScript. reference, my assumption is that there might be differences in the intersection counting code.

[stefannikolei]

@JimBobSquarePants Yeah I also looked into that. But for me it seems, that the problem is a precision problem.

This comparison failed when it shouldn't

if ((operation == BooleanOperation.Intersection && sweepEvent.Point.X > minMaxX) ||
(operation == BooleanOperation.Difference && sweepEvent.Point.X > subjectMaxX))
{
return ConnectEdges(sortedEvents, comparer);
}

I saw, that after the Intersection Code ran, that the Vertexes in the events differed from the JS version

[JimBobSquarePants]

Ah, that might be why the bound box dimensions are calculated in the loop. We include all the segments, they only include the ones that pass when processing the polygon.

I'll make the changes now in main.....

Done.

[stefannikolei]

What do you think about the testdata that can not be deserialized by geojson?

[stefannikolei]

Should we copy geojson into the testcode and remove the check of the 4 coordinates?

[JimBobSquarePants]

What do you think about the testdata that can not be deserialized by geojson?

I was hoping the library I’m using to deserialise them had a less strict mode.

[stefannikolei]

I think we are running behind ghosts.

For me the problem is in the precision of double. When the testdata ist loaded we already get other data

We have following point in the testdata:

When we load it into the vertex we get this:

The data is already "wrong" when it is deserialized because it is a double. In my eyes we should operate with decimal to overcome this issue.

[JimBobSquarePants]

That’s interesting. JavaScript Number types are double precision floating point values the same as our double.

[stefannikolei]

I debugged the fatal2 to a point where in the js two points differed. And in this implementation were exactly the same...

Set a breakpoint here with sortedEvents.Count == 173.

When you get back to the beginning of the while you have in JS: 125 entries in the event queue and in C# 123

[stefannikolei]

I fixed the fatal2 test

Removing the SnapToSegment calls fixed it.

I also added some calls to Vertex.Cross instead of multiplying it manually

[JimBobSquarePants]

I fixed the fatal2 test

Removing the SnapToSegment calls fixed it.

I also added some calls to Vertex.Cross instead of multiplying it manually

First thing that popped into my head when I woke up this morning was SnapToSegment!!

Do all the tests you've added so far pass now or are there any I should look at?

[stefannikolei]

23 are still failing.

Some have the deserialization problem.

Some have a precision problem.

I have not looked further.

[stefannikolei]

I have sorted the test files, those who fail are now in a separate directory. we now have 25 green

[JimBobSquarePants]

I wonder if we should merge what we have so far?

[stefannikolei]

I wonder if we should merge what we have so far?

I would say so. Makes it easier to verify what fixes what 😅

Should I add the build scripts from the ImageSharp repo?

[JimBobSquarePants]

Just for tests, no packages yet until the kinks are ironed out and I’ve added some benchmarking to compare to Clipper2