Add new Transform algorithm for transforming a geometry with PROJ.

[frewsxcv]

• I agree to follow the project's code of conduct.
• I added an entry to CHANGES.md if knowledge of this change could be valuable to users.
  • I will do this if we decide we want this
• Add entry to table of contents in lib.rs

[lnicola]

This is nice to have, but I'm wondering if it's better to take the transform instead of the two CRSes. I don't know how long these take to instantiate in practice, but it seems like you'd want to reuse them when possible.

[urschrei]

the transform instead of the two CRSes
it seems like you'd want to reuse them when possible.

I agree, even though the trade-off is a less-nice API. However, without something that takes a Proj instance, you can't use a ProjBuilder to enable grid downloads (etc.) requiring network access, which is common for people who need higher-accuracy transforms:

let pb = ProjBuilder::new();
pb.enable_network().unwrap();
let transform = pb.proj_known_crs("EPSG:4326", "EPSG:3857").unwrap(); // Proj instance!

[frewsxcv]

What if the argument was transform(transformer: TryInto<Proj>)? So you could pass it a Proj directly, and we could also consider adding a TryFrom<(&str, &str)> for Proj implementation in the proj crate, so you could do transform(("EPSG:4326", "EPSG:3857")) here.

[urschrei]

What if the argument was transform(transformer: TryInto)? So you could pass it a Proj directly, and we could also consider adding a TryFrom<(&str, &str)> for Proj implementation in the proj crate, so you could do transform(("EPSG:4326", "EPSG:3857")) here.

I like it.

[lnicola]

It's okay, but not really discoverable.

[frewsxcv]

I agree the TryFrom and TryInto trait implementations are hard to discover, so I would add documentation examples demonstrating you can use multiple options here. I added similar docs for ToSocketAddr a while back, which has multiple implementations.

[michaelkirk]

To recap, I agree that accepting a TryInto<Proj> param is the most flexible.

And ("EPSG:4326", "EPSG:3857").try_into() is concise, but I don't think that I'd guess for it to exist. The docs you're planning to add will help.

But I usually go to the docs only after I fail to intuit the API. If we think that often people will want to use the initially proposed interface (from: &str, to: &str), then I think it makes sense to expose a more discoverable helper method, like this:

pub trait Transform<T> {
    type Output;

    /// naming insperation from  https://proj.org/development/reference/functions.html#c.proj_create_crs_to_crs
    fn transform_crs_to_crs(&self, from: &str, to: &str)
        -> Result<Self::Output, TransformError> {
        self.transform((from, to).try_into()?)
    }

    fn transform(&self, proj: TryInto<Proj>)
         -> Result<Self::Output, TransformError>;
 }

WDYT?

If it's controversial, don't worry about it. We could always add it later.

[phayes]

I tried this out and it works great!

Question: Does it make sense to use Proj::convert_array instead of converting each point individually? Could be faster to make one call to Proj instead of many, especially on large polygons etc.

[michaelkirk]

Great point @phayes!

I think it's possible with the current API's we have (poly.interiors_mut(), line_string.0)... and if it's not possible we should consider making it so.

It's a perf improvement that doesn't seem like it would impact the API proposed here, so I'd also be fine with leaving that for a followup PR depending on how much time @frewsxcv wants to sink into this right now.

[frewsxcv]

I don't have much time to push this over the finish line over the next few days. If anyone wants to take over and push to my branch, I'd say go for it. I'm also not opposed to merging as-is (ish) and following up with some of these changes.

[phayes]

Thinking about the best performance here again, and I think the long-term solution is actually to add a convert_inter_mut method to Proj. If you look at the array processing code here (https://github.com/georust/proj/blob/83fca44d25b0def3fe7b313d85cf638aa7fdf493/src/proj.rs#L802-L865), we could avoid some allocations by passing an Into<Iterator> where Item: &'a mut Coord<F> or something similar. It also might make sense to grab num_coords first and do the simple thing if there's a small number of coords.

Anyways, long-term idea that doesn't affect the API surface here.

[frewsxcv]

This is semi-blocked on georust/proj#100 and a proj release

[phayes]

What do folks think of doing something like this?

georust/proj#105

[frewsxcv]

Once georust/proj#103 is reviewed, I will merge it and do a proj crate release. Then this PR will be ready for review.

[frewsxcv]

This is ready for review!

[frewsxcv]

bors r=lnicola

[bors]

Build succeeded:

• ci result