andreesteve / voronoice

A nice and fast way to construct 2D Voronoi Diagrams
MIT License
34 stars 9 forks source link

Fix for uncentered BoundingBoxes #3

Closed Telzhaak closed 3 years ago

Telzhaak commented 3 years ago

Supposedly fixes #2

This commit changes the algorithm to not rely on mirrored coordinates, instead calculating left and bottom from the BoundingBox and checking for coordinates to be in range bottom <= y <= top and left <= x <= right. I chose not to rely on un-shift -> voronoi -> re-shift, since this could cause confusion for users.


The new output looks okay at first glance, but the BoundingBox Corners (e.g. top-left Voronoi-Diagram, upper right corner) are still a little broken. Haven't figured out why, yet. Voronoi_fix__corners_broken


Benchmark Comparison "large_input_benchmark":

100.000 1.000.000 5.000.000 10.000.000
Fix 28.814 ms 412.98 ms 2.4891 s 5.3512 s
Master 29.228 ms 418.78 ms 2.4628 s 5.3161 s
Telzhaak commented 3 years ago

After some testing, the broken corner cells were already present in the master-branch. So this PR "could" be seen as ready to merge.

I'll try to fix the corners over the weekend

Telzhaak commented 3 years ago

Opened new issue #4 for the corner-problem, as it is not related with any of the changes made here

andreesteve commented 3 years ago

@Telzhaak - thanks for the PR and reporting the issues! I started taking a look at the code and looks good. Thanks for adding the benchmark results, I was curious on that - but looks like no measurable to minimal impact.

Telzhaak commented 3 years ago

Short update: I added a UnitTest for the BoundingBox, but noticed that one of my changes results in a previously fine Unittest to fail:

---- cell_builder::test::extend_and_close_hull_test stdout ----
thread 'cell_builder::test::extend_and_close_hull_test' panicked at 'assertion failed: `(left == right)`
  left: `[32.75, -15.5]`,
 right: `[7.905417527999327, -3.0777087639996634]`:
Invalid vertex for position 1. Expected cell vertices [
    [0.5, 0.625],
    [32.75, -15.5],
    [0.5, 33],
], found [
    [0.5, 0.625],
    [7.905417527999327, -3.0777087639996634],
    [0.5, 9],
]. First cell', src\cell_builder.rs:513:13

I'm currently investigating the reason for that. It broke with the previous extend_vector normalized orthogonal commit. Most likely its the hardcoded, expected values, that are now outdated (depending on how you calculated them back when you implemented this test).

Telzhaak commented 3 years ago

Sry, I'm currently busy moving, and I forgot to get back at this.

It is indeed due to the expected values being outdated. Normalizing the orthogonal, and changing the scale factor to be the more easily computed as width+height of course changes where the extended vertices end up

Due to these changes, it is also very very difficult to find "pretty" numbers for tests. I changed the tested triangle to be

(-0.5,  1.0)
(-1.5, -1.0)
( 0.5, -1.0)

According to my hand-math:

circumcenter c = (-0.5, -0.25) new scale = width + height = 4 + 4 = 8 angles with +x-axis as reference

e1angle = 270° - atan(-1/-2) = 243.435°
e2angle = 0°
e3angle = 180° - atan(2/1) = 116.565°

The following vertices represent a rough estimate of where the extended vertice should end up

Instead of extending the half-edge-intersection point, the circumcenter is extended:

exv1.x = c.x + scale * cos(e1angle - 90°) = -0.5 + 8. * cos(153.435°) = -7.655
exv1.y = c.y + scale * sin(e1angle - 90°) = -0.25 + 8. * sin(153.453°) = 3.328

exv2.x = c.x + scale * cos(e2angle - 90°) = -0.5 + 8. * cos(-90°) = -0.5
exv2.y = c.y + scale * sin(e2angle - 90°) = -0.25 + 8. * sin(-90°) = -8.25

exv3.x = c.x + scale * cos(e3angle - 90°) = -0.5 + 8. * cos(26.565°) = 6.655
exv3.y = c.y + scale * sin(e3angle - 90°) = -0.25 + 8. * sin(26.565°) = 3.328

The following vertices are calculated by the code:

  1. (7.865633420552356, 2.8089875327071336)
  2. (8.155417527999326, 3.5777087639996634)
  3. (-0.5, -9) <- The proximity of each calculated point to its hand-estimated position is an indicator, that the expected test-values are correct. This is confirmed by the third point, whose mid-edge intersection point is trivial to find at (-0.5, -1.0). Extending this by scale in -90°-direction leads to exactly this value.

Thus, this PR finished its scope and can be merged