Fixes point not detected in polygon

[MJohnson459]

This fixes some cases where a path was not being found. Specifically when a point was close to the edge it was detected as an Edge and not as part of the polygon. This test was quick heavy handed which caused a lot of false positives.

Most of the failures in mentioned in #45 were caused by this.

[mockersf]

This value of epsilon currently set works well with the example meshes, but will fail with meshes at a smaller scale, which is what you're getting.

I think changing the value has some small perf impact. Ideally instead of it being a const, it should be possible to set it like delta as the correct value will depend on the mesh.

A value of delta / 10 would be a good default, in your case that would be 1e-3, is that small enough for your mesh?

[MJohnson459]

It will be a bigger change but I can make it. I assumed the EPSILON value was based on the precision of the type rather than for performance reasons. The cpp implementation uses 1e-8.

I assume the performance implication is just less points being classified as edges and an edge uses less expansions?

For the change, should I just change fn side(self, line: (Vec2, Vec2)) -> EdgeSide { to take the epsilon value and pass it through from where we have the delta? It has 22 references but those are mostly test and debug.

[mockersf]

I assume the performance implication is just less points being classified as edges and an edge uses less expansions?

I think so

Could be worth running the benches on another computer than mine, could you try running them on yours? cargo bench on main then on your PR

I assumed the EPSILON value was based on the precision of the type rather than for performance reasons. The cpp implementation uses 1e-8.

Maybe https://doc.rust-lang.org/std/primitive.f32.html#associatedconstant.EPSILON would actually make sense instead of another "magic" value

[MJohnson459]

I ran the benchmarks with 1e-6 as a baseline. Changing it back to 1e-2 most of the benches were within error except for these two which "regressed" (most likely just noise).

is in mesh Vec2(135.0, 360.0)
                        time:   [120.07 ns 120.98 ns 121.96 ns]
                        change: [+1.3456% +2.5911% +3.8709%] (p = 0.00 < 0.05)
                        Performance has regressed.
...
is not in mesh Vec2(726.0, 470.0)
                        time:   [369.50 ns 369.86 ns 370.20 ns]
                        change: [+1.2981% +1.9975% +2.7468%] (p = 0.00 < 0.05)
                        Performance has regressed.

Changing it to f32::EPSILON again most were the same but these were changed

triangulation arena overlapping
                        time:   [77.848 µs 77.879 µs 77.913 µs]
                        change: [+1.4502% +1.5352% +1.6193%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

triangulation square    time:   [11.151 µs 11.157 µs 11.163 µs]
                        change: [+1.2152% +1.3350% +1.4361%] (p = 0.00 < 0.05)
                        Performance has regressed.
Found 7 outliers among 100 measurements (7.00%)
  3 (3.00%) low mild
  2 (2.00%) high mild
  2 (2.00%) high severe

triangulation square overlapping
                        time:   [33.152 µs 33.164 µs 33.175 µs]
                        change: [+1.0208% +1.2747% +1.4472%] (p = 0.00 < 0.05)
                        Performance has regressed.

Edit: Sorry posted too soon

[MJohnson459]

Ok, I did a clean bench with 1e-2 as the base and another as f32::EPSILON. It didn't find any regressions so I think any changes in performance are just noise from running the test (positive and negative).

[MJohnson459]

Ok, I think this is probably the best I can get it outside being variable but that will have its own issues. f32::EPSILON is the "machine epsilon" which means it is the smallest unit that can be represented by a f32. I tested with this and it "works" in that it always gives the correct left or right result. However it also never returns an EDGE unless the line is very specific as in the tests.

To account for this, I added a more complicated line to test against. This failed for the f32::EPSILON as well as 1e-6. This makes sense as each calculation in the pipeline multiplies the error. 1e-5 does work but I am not sure how much room that leaves for user calculation on the values. 1e-4 seems the "safe" option and should be accurate for all float operations.

For reference, the default in the cpp implementation is 1e-8 and the EPSILON for an f64 is 1e-16 so plenty of margin there.

[mockersf]

thanks for investigating!