Aabb: fix min_max_dist_2 consistency with distance_2

[aschampion]

PR #35 changed the structure of floating point operations in this, which leads to precision issues and inconsistency with distance_2. This fixes the structure so that it is identical with results of the previous implementation of min_max_dist_2.

Performance difference is negligible.

These floating point differences compounded to significant changes in our data, so this is worth making consistent. This is also necessary for correctness of optimizations in a following PR.

[aschampion]

I finally got around to writing a regression test for a case that fails prior to this PR (and to the second commit) but passes with it. I left the new commits separate for clarity in what was changed to fix the addition order, but can squash. This should be ready to go.

[urschrei]

@aschampion Apologies for the long delay. We (georust) have taken over stewardship of this crate in order to facilitate maintenance and feature dev. If you're still interested, we'd like to get this merged – if you could rebase against master and update the changelog, we should be good to go.

[rmanoka]

Nice fix @aschampion . If I understand correct, the issue is that the present logic computes the distance to the further corner, and then subtracts the largest difference between min-max across dimensions. The subtraction after addition leads to the issue, as triggered in the simplest test case I could come up with:

#[test]
fn test_min_max_dist_2() {
    let q = [0., 0.];
    let p1 = [1., 1.];
    let p2 = [-1., -1.0e8];
    let aabb = AABB::from_corners(p1, p2);
    eprintln!("min max dist = {:.50}", aabb.min_max_dist_2(&q));
    // should print 2.0 but would print 0.0
}

Here, the square of second coord. of p2 has a ULP larger than 1.0 hence triggerring the issue.

[aschampion]

Exactly! That's a very clear minimal reproduction. Although even just adding the dimension-wise values in different order without subtraction (as this version did) can cause the same issue, since floating point addition is not commutative.

[aschampion]

Rebased, squashed the changes to min_max_dist_2, and pushed. I also confirmed that this fixes the test in #53 (although I did not commit the test).

[urschrei]

bors try

[bors[bot]]

try

Build succeeded:

• ci result

[urschrei]

bors r+

[bors[bot]]

Build succeeded:

• ci result

[urschrei]

Amazing work, and thanks for your patience @aschampion! We'll be pushing out a new release incorporating this before the weekend (hopefully!). As mentioned elsewhere, if you have any interest in helping out with crate maintenance in the form of tending the issue / PR garden, we'd be very happy to have you – we're on Discord (https://discord.gg/Fp2aape) or available via an issue…