geo icon indicating copy to clipboard operation
geo copied to clipboard

Expose fields in AffineTransform struct (or have a way to access the internal values)?

Open weiji14 opened this issue 11 months ago • 4 comments

Currently as of geo=0.28.0, the AffineTransform struct's fields (implemented in #866) are currently not publicly exposed:

https://github.com/georust/geo/blob/08d3f394a9552e90474b4dacd508927cba84a6d2/geo/src/algorithm/affine_ops.rs#L118-L119

The nine values in the tuple struct are:

[[a, b, xoff],
 [d, e, yoff],
 [0, 0, 1]]

where a/e are the x/y resolution, b/d are x/y rotation (typically 0), and xoff/yoff are the x/y origin coordinates. See https://www.perrygeo.com/python-affine-transforms.html which is a good reference on this.

Personally, I'd like to have a way to access these individual fields to build up an array of x and y coordinates for pixels in a raster image, e.g. when reading from GeoTIFFs (currently implementing this in https://github.com/weiji14/cog3pio/pull/8, but will do further work that will require access to a/xoff/e/yoff). I'm aware of https://docs.rs/gdal/0.16.0/gdal/type.GeoTransform.html which implements something similar, but would prefer to not depend on GDAL if possible :slightly_smiling_face:

Specifically, it would be nice to be able to do something like:

use geo::AffineTransform;

let transform = AffineTransform::new(200.0, 0.0, 499980.0, 0.0, -200.0, 5300040.0);

let x_scale: f64 = transform.a;
let x_rotation: f64 = transform.b;
let x_origin: f64 = transform.c;
let y_rotation: f64 = transform.d;
let y_scale: f64 = transform.e;
let y_origin: f64 = transform.f;

Yes, the affine transformation field names could possibly be renamed to something more easily interpretable.

Would be happy to open a PR for this if this sounds ok. Alternatively, if making the fields public would cause breaking changes, I'd also be ok with implementing a trait that can convert AffineTransform to a HashMap or some other type that allows for variable-based indexing.

weiji14 avatar Mar 11 '24 04:03 weiji14

I'm +1 on this. It doesn't look like there's any way to currently access those fields, even with a From impl. I don't think Rust has a concept of a getter so those attributes would have to become methods. I'm ok with using a-f letters for naming, since that seems pretty standard and we can additionally link to perrygeo's blog (I also go back to that blog post often!)

kylebarron avatar Mar 11 '24 16:03 kylebarron

Of course you can access the coefficients, you can apply the transform to (0, 0), (1, 0) and (0, 1) and compute them :smile:.

TBH they could even be public, but it's fine either way.

lnicola avatar Mar 11 '24 16:03 lnicola

I'm happy to have the fields be public, but I'm pretty sure the current field names are correct and meaningful in terms of affine transforms generally (see e.g. Shapely's affinity module); xoff and yoff are calculated offsets and I'd rather not rename them just to conform to Matt's terminology (it is a great blog post, btw).

urschrei avatar Mar 11 '24 23:03 urschrei

It doesn't look like there's any way to currently access those fields, even with a From impl. I don't think Rust has a concept of a getter so those attributes would have to become methods.

~Found out at https://users.rust-lang.org/t/access-tuple-struct-with-one-element-more-ergonomically/27236 that there's an Index trait in std (see https://doc.rust-lang.org/std/ops/trait.Index.html) which seems to allow indexing into the array/matrix. Have started a draft PR at #1159 as a proof of concept. This enables indexing using transform["a"] or transform.index("a")~. I could also rewrite things so that accessing the elements happens via transform.a() if that's more ergonomic. Edit: refactored implementation to use getter methods like transform.a().

Next point is to decide on the naming. To be clear, the current AffineTransform tuple struct doesn't set any names like c or xoff, it is only in the documentation. So we can go with either a,b,c,d,e,f naming or a,b,xoff,d,e,yoff.

weiji14 avatar Mar 12 '24 02:03 weiji14