From 51d0de09c6f7e2af4db3b65c3fe9595c501b82c9 Mon Sep 17 00:00:00 2001 From: Martin Haug Date: Tue, 3 May 2022 23:07:19 +0200 Subject: [PATCH] Code Review: Relax, it's much worse than you think --- src/geom/path.rs | 40 +++++-------------------- src/geom/rect.rs | 69 +++++++++++++++++++------------------------ src/geom/sides.rs | 8 +++++ src/geom/transform.rs | 17 ++++------- 4 files changed, 52 insertions(+), 82 deletions(-) diff --git a/src/geom/path.rs b/src/geom/path.rs index 721cc20bd..d0c3c75d0 100644 --- a/src/geom/path.rs +++ b/src/geom/path.rs @@ -72,33 +72,12 @@ impl Path { } } -/// Get the control points for a bezier curve that describes a circular arc -/// of this angle with the given radius. -pub fn bezier_arc( - angle: Angle, - radius: Length, - rotate: bool, - mirror_x: bool, - mirror_y: bool, -) -> [Point; 4] { - let end = Point::new(angle.cos() * radius - radius, angle.sin() * radius); - let center = Point::new(-radius, Length::zero()); - - let mut ts = if mirror_y { - Transform::mirror_y() - } else { - Transform::identity() - }; - - if mirror_x { - ts = ts.pre_concat(Transform::mirror_x()); - } - - if rotate { - ts = ts.pre_concat(Transform::rotate(Angle::deg(90.0))); - } - - let a = center * -1.0; +/// Get the control points for a bezier curve that describes a circular arc for +/// a start point, an end point and a center of the circle whose arc connects +/// the two. +pub fn bezier_arc(start: Point, center: Point, end: Point) -> [Point; 4] { + // https://stackoverflow.com/a/44829356/1567835 + let a = start - center; let b = end - center; let q1 = a.x.to_raw() * a.x.to_raw() + a.y.to_raw() * a.y.to_raw(); @@ -109,10 +88,5 @@ pub fn bezier_arc( let control_1 = Point::new(center.x + a.x - k2 * a.y, center.y + a.y + k2 * a.x); let control_2 = Point::new(center.x + b.x + k2 * b.y, center.y + b.y - k2 * b.x); - [ - Point::zero(), - control_1.transform(ts), - control_2.transform(ts), - end.transform(ts), - ] + [start, control_1, control_2, end] } diff --git a/src/geom/rect.rs b/src/geom/rect.rs index aa670f0a5..34160b041 100644 --- a/src/geom/rect.rs +++ b/src/geom/rect.rs @@ -112,52 +112,45 @@ fn draw_side( radius_right: Length, connection: Connection, ) { - let reversed = |angle: Angle, radius, rotate, mirror_x, mirror_y| { - let [a, b, c, d] = bezier_arc(angle, radius, rotate, mirror_x, mirror_y); - [d, c, b, a] - }; - let angle_left = Angle::deg(if connection.prev { 90.0 } else { 45.0 }); let angle_right = Angle::deg(if connection.next { 90.0 } else { 45.0 }); - let (arc1, arc2) = match side { - Side::Top => { - let arc1 = reversed(angle_left, radius_left, true, true, false) - .map(|x| x + Point::with_x(radius_left)); - let arc2 = bezier_arc(-angle_right, radius_right, true, true, false) - .map(|x| x + Point::with_x(size.x - radius_right)); + let length = size.get(side.axis()); - (arc1, arc2) - } - Side::Right => { - let arc1 = reversed(-angle_left, radius_left, false, false, false) - .map(|x| x + Point::new(size.x, radius_left)); + // The arcs for a border of the rectangle along the x-axis, starting at (0,0). + let p1 = Point::with_x(radius_left); + let mut arc1 = bezier_arc( + p1 + Point::new( + -angle_left.sin() * radius_left, + (1.0 - angle_left.cos()) * radius_left, + ), + Point::new(radius_left, radius_left), + p1, + ); - let arc2 = bezier_arc(angle_right, radius_right, false, false, false) - .map(|x| x + Point::new(size.x, size.y - radius_right)); + let p2 = Point::with_x(length - radius_right); + let mut arc2 = bezier_arc( + p2, + Point::new(length - radius_right, radius_right), + p2 + Point::new( + angle_right.sin() * radius_right, + (1.0 - angle_right.cos()) * radius_right, + ), + ); - (arc1, arc2) - } - Side::Bottom => { - let arc1 = reversed(-angle_left, radius_left, true, false, false) - .map(|x| x + Point::new(size.x - radius_left, size.y)); - - let arc2 = bezier_arc(angle_right, radius_right, true, false, false) - .map(|x| x + Point::new(radius_right, size.y)); - - (arc1, arc2) - } - Side::Left => { - let arc1 = reversed(angle_left, radius_left, false, false, true) - .map(|x| x + Point::with_y(size.y - radius_left)); - - let arc2 = bezier_arc(-angle_right, radius_right, false, false, true) - .map(|x| x + Point::with_y(radius_right)); - - (arc1, arc2) - } + let transform = match side { + Side::Left => Transform::rotate(Angle::deg(-90.0)) + .post_concat(Transform::translate(Length::zero(), size.y)), + Side::Bottom => Transform::rotate(Angle::deg(180.0)) + .post_concat(Transform::translate(size.x, size.y)), + Side::Right => Transform::rotate(Angle::deg(90.0)) + .post_concat(Transform::translate(size.x, Length::zero())), + _ => Transform::identity(), }; + arc1 = arc1.map(|x| x.transform(transform)); + arc2 = arc2.map(|x| x.transform(transform)); + if !connection.prev { path.move_to(if radius_left.is_zero() { arc1[3] } else { arc1[0] }); } diff --git a/src/geom/sides.rs b/src/geom/sides.rs index 555bbd62b..43e470d22 100644 --- a/src/geom/sides.rs +++ b/src/geom/sides.rs @@ -159,4 +159,12 @@ impl Side { Self::Bottom => Self::Right, } } + + /// Return the corresponding axis. + pub fn axis(self) -> SpecAxis { + match self { + Self::Left | Self::Right => SpecAxis::Vertical, + Self::Top | Self::Bottom => SpecAxis::Horizontal, + } + } } diff --git a/src/geom/transform.rs b/src/geom/transform.rs index 961ba4877..40c8e9e4f 100644 --- a/src/geom/transform.rs +++ b/src/geom/transform.rs @@ -24,16 +24,6 @@ impl Transform { } } - /// Transform by mirroring along the x-axis. - pub fn mirror_x() -> Self { - Self::scale(Ratio::one(), -Ratio::one()) - } - - /// Transform by mirroring along the y-axis. - pub fn mirror_y() -> Self { - Self::scale(-Ratio::one(), Ratio::one()) - } - /// A translate transform. pub const fn translate(tx: Length, ty: Length) -> Self { Self { tx, ty, ..Self::identity() } @@ -63,7 +53,7 @@ impl Transform { } /// Pre-concatenate another transformation. - pub fn pre_concat(&self, prev: Self) -> Self { + pub fn pre_concat(self, prev: Self) -> Self { Transform { sx: self.sx * prev.sx + self.kx * prev.ky, ky: self.ky * prev.sx + self.sy * prev.ky, @@ -73,6 +63,11 @@ impl Transform { ty: self.ky.of(prev.tx) + self.sy.of(prev.ty) + self.ty, } } + + /// Post-concatenate another transformation. + pub fn post_concat(self, next: Self) -> Self { + next.pre_concat(self) + } } impl Default for Transform {