From 2fac1335814280b6ed08c84032975272116a0b04 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 13:06:07 +0100 Subject: [PATCH 1/8] Improve TranslateScale Make interface for TranslateScale more consistent with other parts of kurbo --- src/translate_scale.rs | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index 4f195055..a67fcddc 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -9,7 +9,7 @@ use crate::{ Affine, Circle, CubicBez, Line, Point, QuadBez, Rect, RoundedRect, RoundedRectRadii, Vec2, }; -/// A transformation including scaling and translation. +/// A transformation including uniform scaling and translation. /// /// If the translation is `(x, y)` and the scale is `s`, then this /// transformation represents this augmented matrix: @@ -42,8 +42,10 @@ use crate::{ #[cfg_attr(feature = "schemars", derive(schemars::JsonSchema))] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct TranslateScale { - translation: Vec2, - scale: f64, + /// The translation component of this transformation + pub translation: Vec2, + /// The scale component of this transformation + pub scale: f64, } impl TranslateScale { @@ -55,21 +57,16 @@ impl TranslateScale { /// Create a new transformation with scale only. #[inline] - pub const fn scale(s: f64) -> TranslateScale { + pub const fn from_scale(s: f64) -> TranslateScale { TranslateScale::new(Vec2::ZERO, s) } /// Create a new transformation with translation only. #[inline] - pub const fn translate(t: Vec2) -> TranslateScale { + pub const fn from_translate(t: Vec2) -> TranslateScale { TranslateScale::new(t, 1.0) } - /// Decompose transformation into translation and scale. - pub fn as_tuple(self) -> (Vec2, f64) { - (self.translation, self.scale) - } - /// Compute the inverse transform. /// /// Multiplying a transform with its inverse (either on the @@ -101,7 +98,7 @@ impl TranslateScale { impl Default for TranslateScale { #[inline] fn default() -> TranslateScale { - TranslateScale::scale(1.0) + TranslateScale::new(Vec2::ZERO, 1.0) } } @@ -301,8 +298,11 @@ mod tests { let a: Affine = ts.into(); assert_near(ts * p, a * p); - assert_near((s * p.to_vec2()).to_point(), TranslateScale::scale(s) * p); - assert_near(p + t, TranslateScale::translate(t) * p); + assert_near( + (s * p.to_vec2()).to_point(), + TranslateScale::from_scale(s) * p, + ); + assert_near(p + t, TranslateScale::from_translate(t) * p); } #[test] From 9f4acbb6963715a1462978ad89cda871693e9ae4 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 13:26:31 +0100 Subject: [PATCH 2/8] Add `from_scale_about` method. --- src/translate_scale.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index a67fcddc..c9a20675 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -9,7 +9,7 @@ use crate::{ Affine, Circle, CubicBez, Line, Point, QuadBez, Rect, RoundedRect, RoundedRectRadii, Vec2, }; -/// A transformation including uniform scaling and translation. +/// A transformation consisting of a uniform scaling followed by a translation. /// /// If the translation is `(x, y)` and the scale is `s`, then this /// transformation represents this augmented matrix: @@ -63,8 +63,18 @@ impl TranslateScale { /// Create a new transformation with translation only. #[inline] - pub const fn from_translate(t: Vec2) -> TranslateScale { - TranslateScale::new(t, 1.0) + pub const fn from_translate(translation: Vec2) -> TranslateScale { + TranslateScale::new(translation, 1.0) + } + + /// Create a transform that scales about a point other than the origin. + pub fn from_scale_about(scale: f64, focus: Point) -> Self { + // We need to create a transform that is equivalent to translating `focus` + // to the origin, followed by a normal scale, followed by reversing the translation. + // We need to find the (scale ∘ translation) that matches this. + let focus = focus.to_vec2(); + let translation = focus - focus * scale; + Self::new(translation, scale) } /// Compute the inverse transform. @@ -287,6 +297,14 @@ mod tests { assert_near(ts * p, Point::new(11.0, 14.0)); } + #[test] + fn scale_about() { + let center = Point::new(1., 1.); + let ts = TranslateScale::from_scale_about(2., center); + // Should keep the point (1., 1.) stationary + assert_near(center, ts * center); + } + #[test] fn conversions() { let p = Point::new(3.0, 4.0); From 616f1e72a084aa8fe2fb486065cdb0bfead1de16 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 13:32:04 +0100 Subject: [PATCH 3/8] Fix docs --- src/translate_scale.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index c9a20675..e71c130f 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -71,7 +71,7 @@ impl TranslateScale { pub fn from_scale_about(scale: f64, focus: Point) -> Self { // We need to create a transform that is equivalent to translating `focus` // to the origin, followed by a normal scale, followed by reversing the translation. - // We need to find the (scale ∘ translation) that matches this. + // We need to find the (translation ∘ scale) that matches this. let focus = focus.to_vec2(); let translation = focus - focus * scale; Self::new(translation, scale) From 47046f1e2617f026a1dea1d4d8eead9d2f117443 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 13:33:19 +0100 Subject: [PATCH 4/8] Add extra test --- src/translate_scale.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index e71c130f..6d0beb90 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -302,7 +302,9 @@ mod tests { let center = Point::new(1., 1.); let ts = TranslateScale::from_scale_about(2., center); // Should keep the point (1., 1.) stationary - assert_near(center, ts * center); + assert_near(ts * center, center); + // (2., 2.) -> (3., 3.) + assert_near(ts * Point::new(2., 2.), Point::new(3., 3.)); } #[test] From 0e628dd592bbde72486db7bc6141ac672d92df52 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 14:01:34 +0100 Subject: [PATCH 5/8] Make test into doctest example --- src/translate_scale.rs | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index 6d0beb90..346274f7 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -68,6 +68,21 @@ impl TranslateScale { } /// Create a transform that scales about a point other than the origin. + /// + /// # Examples + /// + /// ``` + /// # use kurbo::{Point, TranslateScale}; + /// # fn assert_near(p0: Point, p1: Point) { + /// # assert!((p1 - p0).hypot() < 1e-9, "{p0:?} != {p1:?}"); + /// # } + /// let center = Point::new(1., 1.); + /// let ts = TranslateScale::from_scale_about(2., center); + /// // Should keep the point (1., 1.) stationary + /// assert_near(ts * center, center); + /// // (2., 2.) -> (3., 3.) + /// assert_near(ts * Point::new(2., 2.), Point::new(3., 3.)); + /// ``` pub fn from_scale_about(scale: f64, focus: Point) -> Self { // We need to create a transform that is equivalent to translating `focus` // to the origin, followed by a normal scale, followed by reversing the translation. @@ -297,16 +312,6 @@ mod tests { assert_near(ts * p, Point::new(11.0, 14.0)); } - #[test] - fn scale_about() { - let center = Point::new(1., 1.); - let ts = TranslateScale::from_scale_about(2., center); - // Should keep the point (1., 1.) stationary - assert_near(ts * center, center); - // (2., 2.) -> (3., 3.) - assert_near(ts * Point::new(2., 2.), Point::new(3., 3.)); - } - #[test] fn conversions() { let p = Point::new(3.0, 4.0); From 4cb545c7f098406c5822b2f226de76f78177275a Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Thu, 18 May 2023 20:21:40 +0100 Subject: [PATCH 6/8] Bump major version, since this is a breaking change. --- Cargo.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/Cargo.toml b/Cargo.toml index 0bb01e80..69938fac 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,5 +1,6 @@ [package] name = "kurbo" +# This should be bumped to 0.10 at the next release, because master has breaking changes version = "0.9.5" authors = ["Raph Levien "] license = "MIT/Apache-2.0" From f1d0b4d5e3f65335db0dfaa296902aec0779ac8a Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Fri, 19 May 2023 08:25:05 +0100 Subject: [PATCH 7/8] Remove breaking change --- Cargo.toml | 1 - src/translate_scale.rs | 13 ++++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 69938fac..0bb01e80 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,5 @@ [package] name = "kurbo" -# This should be bumped to 0.10 at the next release, because master has breaking changes version = "0.9.5" authors = ["Raph Levien "] license = "MIT/Apache-2.0" diff --git a/src/translate_scale.rs b/src/translate_scale.rs index 346274f7..3c22539a 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -57,13 +57,13 @@ impl TranslateScale { /// Create a new transformation with scale only. #[inline] - pub const fn from_scale(s: f64) -> TranslateScale { + pub const fn scale(s: f64) -> TranslateScale { TranslateScale::new(Vec2::ZERO, s) } /// Create a new transformation with translation only. #[inline] - pub const fn from_translate(translation: Vec2) -> TranslateScale { + pub const fn translate(translation: Vec2) -> TranslateScale { TranslateScale::new(translation, 1.0) } @@ -83,6 +83,7 @@ impl TranslateScale { /// // (2., 2.) -> (3., 3.) /// assert_near(ts * Point::new(2., 2.), Point::new(3., 3.)); /// ``` + #[inline] pub fn from_scale_about(scale: f64, focus: Point) -> Self { // We need to create a transform that is equivalent to translating `focus` // to the origin, followed by a normal scale, followed by reversing the translation. @@ -99,6 +100,7 @@ impl TranslateScale { /// (modulo floating point rounding errors). /// /// Produces NaN values when scale is zero. + #[inline] pub fn inverse(self) -> TranslateScale { let scale_recip = self.scale.recip(); TranslateScale { @@ -323,11 +325,8 @@ mod tests { let a: Affine = ts.into(); assert_near(ts * p, a * p); - assert_near( - (s * p.to_vec2()).to_point(), - TranslateScale::from_scale(s) * p, - ); - assert_near(p + t, TranslateScale::from_translate(t) * p); + assert_near((s * p.to_vec2()).to_point(), TranslateScale::scale(s) * p); + assert_near(p + t, TranslateScale::translate(t) * p); } #[test] From 20ff82782000092bf9b97308a61dbb6f1146bf58 Mon Sep 17 00:00:00 2001 From: Richard Dodd Date: Sat, 27 May 2023 12:59:16 +0100 Subject: [PATCH 8/8] Make recommended changes - Make `translate` constructor take `impl Into` - Add in removed function and mark deprecated (PR is now backwards-compatible) --- src/translate_scale.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/translate_scale.rs b/src/translate_scale.rs index 3c22539a..1801f3de 100644 --- a/src/translate_scale.rs +++ b/src/translate_scale.rs @@ -63,8 +63,15 @@ impl TranslateScale { /// Create a new transformation with translation only. #[inline] - pub const fn translate(translation: Vec2) -> TranslateScale { - TranslateScale::new(translation, 1.0) + pub fn translate(translation: impl Into) -> TranslateScale { + TranslateScale::new(translation.into(), 1.0) + } + + /// Decompose transformation into translation and scale. + #[deprecated(note = "use the struct fields directly")] + #[inline] + pub const fn as_tuple(self) -> (Vec2, f64) { + (self.translation, self.scale) } /// Create a transform that scales about a point other than the origin.