Skip to content

Commit

Permalink
Affine flip compose args to be conventional
Browse files Browse the repository at this point in the history 
FIX: #1195

e.g. for "conventional" https://gdal.org/api/raster_c_api.html#_CPPv424GDALComposeGeoTransformsPKdPKdPd

```
/// The resulting geotransform is the equivalent to padfGT1 and then padfGT2 being applied to a point.
/// Parameters:
///     padfGT1 -- the first geotransform, six values.
///     padfGT2 -- the second geotransform, six values.
///     padfGTOut -- the output geotransform, six values, may safely be the same array as padfGT1 or padfGT2.
void GDALComposeGeoTransforms(const double *padfGeoTransform1, const double *padfGeoTransform2, double *padfGeoTransformOut)
```

Note: `padfGT1` **and then** `padfGT2`, previously we were effectively doing `padfGT2` **and then** `padfGT1`.

I also simplified some of the examples to have more understandable
input/output. I'm not good enough at mental matrix math to know what to
expect from a rotated skew.
  • Loading branch information
@michaelkirk
michaelkirk committed Jun 25, 2024
1 parent af7b2bd commit 85eb674
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 58 deletions.
2 changes: 2 additions & 0 deletions geo/CHANGES.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* <https://github.com/georust/geo/pull/1159>
* Fix issue in Debug impl for AffineTransform where yoff is shown instead of xoff
* <https://github.com/georust/geo/pull/1191>
* Fix `AffineTransform::compose` ordering to be conventional - such that the argument is applied *after* self.
* <https://github.com/georust/geo/pull/1196>

## 0.28.0

Expand Down
125 changes: 67 additions & 58 deletions geo/src/algorithm/affine_ops.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,23 +22,19 @@ use std::{fmt, ops::Mul, ops::Neg};
/// ## Build up transforms by beginning with a constructor, then chaining mutation operations
/// ```
/// use geo::{AffineOps, AffineTransform};
/// use geo::{line_string, BoundingRect, Point, LineString};
/// use geo::{point, line_string, BoundingRect};
/// use approx::assert_relative_eq;
///
/// let ls: LineString = line_string![
/// (x: 0.0f64, y: 0.0f64),
/// (x: 0.0f64, y: 10.0f64),
/// ];
/// let center = ls.bounding_rect().unwrap().center();
/// let line_string = line_string![(x: 0.0, y: 0.0),(x: 1.0, y: 1.0)];
///
/// let transform = AffineTransform::skew(40.0, 40.0, center).rotated(45.0, center);
/// let transform = AffineTransform::translate(1.0, 1.0).scaled(2.0, 2.0, point!(x: 0.0, y: 0.0));
///
/// let skewed_rotated = ls.affine_transform(&transform);
/// let transformed_line_string = line_string.affine_transform(&transform);
///
/// assert_relative_eq!(skewed_rotated, line_string![
/// (x: 0.5688687f64, y: 4.4311312),
/// (x: -0.5688687, y: 5.5688687)
/// ], max_relative = 1.0);
/// assert_relative_eq!(
/// transformed_line_string,
/// line_string![(x: 2.0, y: 2.0),(x: 4.0, y: 4.0)]
/// );
/// ```
pub trait AffineOps<T: CoordNum> {
/// Apply `transform` immutably, outputting a new geometry.
Expand Down Expand Up @@ -97,23 +93,19 @@ impl<T: CoordNum, M: MapCoordsInPlace<T> + MapCoords<T, T, Output = Self>> Affin
/// ## Build up transforms by beginning with a constructor, then chaining mutation operations
/// ```
/// use geo::{AffineOps, AffineTransform};
/// use geo::{line_string, BoundingRect, Point, LineString};
/// use geo::{point, line_string, BoundingRect};
/// use approx::assert_relative_eq;
///
/// let ls: LineString = line_string![
/// (x: 0.0f64, y: 0.0f64),
/// (x: 0.0f64, y: 10.0f64),
/// ];
/// let center = ls.bounding_rect().unwrap().center();
/// let line_string = line_string![(x: 0.0, y: 0.0),(x: 1.0, y: 1.0)];
///
/// let transform = AffineTransform::skew(40.0, 40.0, center).rotated(45.0, center);
/// let transform = AffineTransform::translate(1.0, 1.0).scaled(2.0, 2.0, point!(x: 0.0, y: 0.0));
///
/// let skewed_rotated = ls.affine_transform(&transform);
/// let transformed_line_string = line_string.affine_transform(&transform);
///
/// assert_relative_eq!(skewed_rotated, line_string![
/// (x: 0.5688687f64, y: 4.4311312),
/// (x: -0.5688687, y: 5.5688687)
/// ], max_relative = 1.0);
/// assert_relative_eq!(
/// transformed_line_string,
/// line_string![(x: 2.0, y: 2.0),(x: 4.0, y: 4.0)]
/// );
/// ```
///
/// ## Create affine transform manually, and access elements using getter methods
Expand Down Expand Up @@ -150,38 +142,38 @@ impl<T: CoordNum> AffineTransform<T> {
// lol
Self([
[
(self.0[0][0] * other.0[0][0])
+ (self.0[0][1] * other.0[1][0])
+ (self.0[0][2] * other.0[2][0]),
(self.0[0][0] * other.0[0][1])
+ (self.0[0][1] * other.0[1][1])
+ (self.0[0][2] * other.0[2][1]),
(self.0[0][0] * other.0[0][2])
+ (self.0[0][1] * other.0[1][2])
+ (self.0[0][2] * other.0[2][2]),
(other.0[0][0] * self.0[0][0])
+ (other.0[0][1] * self.0[1][0])
+ (other.0[0][2] * self.0[2][0]),
(other.0[0][0] * self.0[0][1])
+ (other.0[0][1] * self.0[1][1])
+ (other.0[0][2] * self.0[2][1]),
(other.0[0][0] * self.0[0][2])
+ (other.0[0][1] * self.0[1][2])
+ (other.0[0][2] * self.0[2][2]),
],
[
(self.0[1][0] * other.0[0][0])
+ (self.0[1][1] * other.0[1][0])
+ (self.0[1][2] * other.0[2][0]),
(self.0[1][0] * other.0[0][1])
+ (self.0[1][1] * other.0[1][1])
+ (self.0[1][2] * other.0[2][1]),
(self.0[1][0] * other.0[0][2])
+ (self.0[1][1] * other.0[1][2])
+ (self.0[1][2] * other.0[2][2]),
(other.0[1][0] * self.0[0][0])
+ (other.0[1][1] * self.0[1][0])
+ (other.0[1][2] * self.0[2][0]),
(other.0[1][0] * self.0[0][1])
+ (other.0[1][1] * self.0[1][1])
+ (other.0[1][2] * self.0[2][1]),
(other.0[1][0] * self.0[0][2])
+ (other.0[1][1] * self.0[1][2])
+ (other.0[1][2] * self.0[2][2]),
],
[
// this section isn't technically necessary since the last row is invariant: [0, 0, 1]
(self.0[2][0] * other.0[0][0])
+ (self.0[2][1] * other.0[1][0])
+ (self.0[2][2] * other.0[2][0]),
(self.0[2][0] * other.0[0][1])
+ (self.0[2][1] * other.0[1][1])
+ (self.0[2][2] * other.0[2][1]),
(self.0[2][0] * other.0[0][2])
+ (self.0[2][1] * other.0[1][2])
+ (self.0[2][2] * other.0[2][2]),
(other.0[2][0] * self.0[0][0])
+ (other.0[2][1] * self.0[1][0])
+ (other.0[2][2] * self.0[2][0]),
(other.0[2][0] * self.0[0][1])
+ (other.0[2][1] * self.0[1][1])
+ (other.0[2][2] * self.0[2][1]),
(other.0[2][0] * self.0[0][2])
+ (other.0[2][1] * self.0[1][2])
+ (other.0[2][2] * self.0[2][2]),
],
])
}
Expand Down Expand Up @@ -578,12 +570,12 @@ mod tests {
let a = AffineTransform::new(1, 2, 5, 3, 4, 6);
let b = AffineTransform::new(7, 8, 11, 9, 10, 12);
let composed = a.compose(&b);
assert_eq!(composed.0[0][0], 25);
assert_eq!(composed.0[0][1], 28);
assert_eq!(composed.0[0][2], 40);
assert_eq!(composed.0[1][0], 57);
assert_eq!(composed.0[1][1], 64);
assert_eq!(composed.0[1][2], 87);
assert_eq!(composed.0[0][0], 31);
assert_eq!(composed.0[0][1], 46);
assert_eq!(composed.0[0][2], 94);
assert_eq!(composed.0[1][0], 39);
assert_eq!(composed.0[1][1], 58);
assert_eq!(composed.0[1][2], 117);
}
#[test]
fn test_transform_composition() {
Expand All @@ -610,7 +602,7 @@ mod tests {
let mut poly = wkt! { POLYGON((0.0 0.0,0.0 2.0,1.0 2.0)) };
poly.affine_transform_mut(&transform);

let expected = wkt! { POLYGON((1.0 1.0,1.0 5.0,3.0 5.0)) };
let expected = wkt! { POLYGON((2.0 2.0,2.0 6.0,4.0 6.0)) };
assert_eq!(expected, poly);
}
#[test]
Expand All @@ -636,4 +628,21 @@ mod tests {
assert_eq!(transform.e(), -10.0);
assert_eq!(transform.yoff(), 500_000.0);
}
#[test]
fn test_compose() {
let point = Point::new(1., 0.);

let translate = AffineTransform::translate(1., 0.);
let scale = AffineTransform::scale(4., 1., [0., 0.]);
let composed = translate.compose(&scale);

assert_eq!(point.affine_transform(&translate), Point::new(2., 0.));
assert_eq!(point.affine_transform(&scale), Point::new(4., 0.));
assert_eq!(
point.affine_transform(&translate).affine_transform(&scale),
Point::new(8., 0.)
);

assert_eq!(point.affine_transform(&composed), Point::new(8., 0.));
}
}

0 comments on commit 85eb674

Please sign in to comment.