From 8001151ea2aa2076c1f19336e51a03c29c59aa57 Mon Sep 17 00:00:00 2001
From: Arman Uguray <armansito@google.com>
Date: Tue, 21 Nov 2023 16:30:12 -0800
Subject: [PATCH] Address review comments

---
 crates/encoding/src/path.rs | 60 ++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/crates/encoding/src/path.rs b/crates/encoding/src/path.rs
index 91bca2c0b..69ca2cce9 100644
--- a/crates/encoding/src/path.rs
+++ b/crates/encoding/src/path.rs
@@ -523,9 +523,8 @@ impl<'a> PathEncoder<'a> {
             self.move_to(self.first_point[0], self.first_point[1]);
         }
         if self.state == PathState::MoveTo {
-            let p0 = (self.first_point[0], self.first_point[1]);
             // Ensure that we don't end up with a zero-length start tangent.
-            let Some((x, y)) = start_tangent_for_curve(p0, (x, y), None, None) else {
+            let Some((x, y)) = self.start_tangent_for_curve((x, y), None, None) else {
                 return;
             };
             self.first_start_tangent_end = [x, y];
@@ -552,9 +551,8 @@ impl<'a> PathEncoder<'a> {
             self.move_to(self.first_point[0], self.first_point[1]);
         }
         if self.state == PathState::MoveTo {
-            let p0 = (self.first_point[0], self.first_point[1]);
             // Ensure that we don't end up with a zero-length start tangent.
-            let Some((x, y)) = start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), None) else {
+            let Some((x, y)) = self.start_tangent_for_curve((x1, y1), Some((x2, y2)), None) else {
                 return;
             };
             self.first_start_tangent_end = [x, y];
@@ -581,10 +579,9 @@ impl<'a> PathEncoder<'a> {
             self.move_to(self.first_point[0], self.first_point[1]);
         }
         if self.state == PathState::MoveTo {
-            let p0 = (self.first_point[0], self.first_point[1]);
             // Ensure that we don't end up with a zero-length start tangent.
             let Some((x, y)) =
-                start_tangent_for_curve(p0, (x1, y1), Some((x2, y2)), Some((x3, y3)))
+                self.start_tangent_for_curve((x1, y1), Some((x2, y2)), Some((x3, y3)))
             else {
                 return;
             };
@@ -738,6 +735,33 @@ impl<'a> PathEncoder<'a> {
 
         !(x_max - x_min > EPSILON || y_max - y_min > EPSILON)
     }
+
+    // Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the
+    // curve is degenerate / has zero-length. The inputs are a sequence of control points that can
+    // represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be
+    // passed to this function by simply setting the invalid control point degrees equal to `None`.
+    //
+    // `self.first_point` is always treated as the first control point of the curve.
+    fn start_tangent_for_curve(
+        &self,
+        p1: (f32, f32),
+        p2: Option<(f32, f32)>,
+        p3: Option<(f32, f32)>,
+    ) -> Option<(f32, f32)> {
+        let p0 = (self.first_point[0], self.first_point[1]);
+        let p2 = p2.unwrap_or(p0);
+        let p3 = p3.unwrap_or(p0);
+        let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON {
+            p1
+        } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON {
+            p2
+        } else if (p3.0 - p0.0).abs() > EPSILON || (p3.1 - p0.1).abs() > EPSILON {
+            p3
+        } else {
+            return None;
+        };
+        Some(pt)
+    }
 }
 
 #[cfg(feature = "full")]
@@ -765,30 +789,6 @@ impl fello::scale::Pen for PathEncoder<'_> {
 
 const EPSILON: f32 = 1e-12;
 
-// Returns the end point of the start tangent of a curve starting at `(x0, y0)`, or `None` if the
-// curve is degenerate / has zero-length. The inputs are a sequence of control points that can
-// represent a line, a quadratic Bezier, or a cubic Bezier. Lines and quadratic Beziers can be
-// passed to this function by simply setting the invalid control point degrees equal to `(x0, y0)`.
-fn start_tangent_for_curve(
-    p0: (f32, f32),
-    p1: (f32, f32),
-    p2: Option<(f32, f32)>,
-    p3: Option<(f32, f32)>,
-) -> Option<(f32, f32)> {
-    let p2 = p2.unwrap_or(p0);
-    let p3 = p3.unwrap_or(p0);
-    let pt = if (p1.0 - p0.0).abs() > EPSILON || (p1.1 - p0.1).abs() > EPSILON {
-        p1
-    } else if (p2.0 - p0.0).abs() > EPSILON || (p2.1 - p0.1).abs() > EPSILON {
-        p2
-    } else if (p3.0 - p0.0).abs() > EPSILON || (p3.1 - p0.1).abs() > EPSILON {
-        p3
-    } else {
-        return None;
-    };
-    Some(pt)
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;