Interaction with Geo types

[urschrei]

From @bluenote10's comment:

I'm wondering if there is a good way to avoid the type conversion. Currently the robust crate interface relies on its own robust::Coord type. As far as I can see, computing orient2d on a geo::Coordinate requires to first duplicate the data into a robust::Coord, which seems unnecessary and a bit tedious. What is the best solution to that in Rust? Would it make sense to change the interface to orient2d(pa_x: f64, pa_y: f64, pb_x: f64, pb_y: f64, pc_x: f64, pc_y: f64)?

Several possibilities here:

• Switch to geo_types::Coordinate (I assume @frewsxcv didn't use it for a reason, but it may simply have been in order to get everything working)
• Write From impls for Geo types (and primitive types such as (T, T) and [T; 2]) to robust::Coord, and modify the functions to accept Into<Coord> instead, with an explicit into() call inside the functions.

Either way, we'll probably want to land #3 first.

[Stoeoef]

Switch to geo_types::Coordinate (I assume @frewsxcv didn't use it for a reason, but it may simply have been in order to get everything working)

This would include geo_types as a dependency, correct? Assuming that this crate should be attractive for other (non geo) projects, that seems troublesome. That makes the second option more preferable IMHO.

[bluenote10]

I did a few experiments and I would simply go for the pure float interface to avoid having to create extra temporary elements on the stack (which would be the case as well for using into() I guess). It looks like the compiler can't optimize away the temporary objects. It' a tiny overhead, but on the other hand on random data orient2d is basically just 5 subtractions and 2 multiplications, so it's in the measurable range. I've set up a few criterion.rs benchmarks and avoiding the extra elements on the stack seems to be ~2% faster. We may also consider making the function #[inline]. In combination I'm getting (benchmarking the signed_area function in rust-geo-booleanop, which basically just a call to orient2d):

[frewsxcv]

@bluenote10 Very interesting! Thanks for spending time on this. Did you push your branch anywhere? Would be curious to see what changes you made

[bluenote10]

@frewsxcv Nothing substantial yet and currently living only in a dump repo, but I'll try to contribute as soon as I'm done with my experiments.

[frewsxcv]

Another option is to add a Coord trait and make the robust functions generic:

pub trait Coord {
    fn x(&self) -> f64;
    fn y(&self) -> f64;
}

pub fn orient2d<T: Coord>(coord_a: T, coord_b: T, coord_c: T) -> f64 {
    ...
}

And then users of robust just need to implement the trait for their custom point types:

[frewsxcv]

https://github.com/georust/robust/pull/8

[frewsxcv]

my current feeling is that the georust ecosystem should make use of the mint crate. in particular, the mint::Point2 type. so in the case of robust, we'd add mint as a dependency, and in geo-types (and other georust crates), we'd alias Coordinate to mint::Point, or something along those lines. and then we can convert types between crates at no cost

[urschrei]

https://github.com/georust/robust/pull/9 has merged, so robust has a Coord type that's generic, but happy to keep this open for now as the mint question remains open.

[michaelkirk]

we'd alias Coordinate to mint::Point, or something along those lines. and then we can convert types between crates at no cost

This is true for any integration using geo-types (or the mint type if we go that way), but doesn't help people outside of geo-types.

Another option is to add a Coord trait and make the robust functions generic:

This is the route I went with in proj, and AFAIK it went ok: https://github.com/georust/proj/pull/41

I'd advocate for that route since it avoids a runtime conversion for geo-types and equally allows other folks to conform their type if they aren't using geo-types.