[librsvg: 1/5] Path.iter() - new function, replaces get_path_commands()

[Federico Mena Quintero <federico src gnome org>]

commit e9db621cdc79d31b8694d0f42cee4e02628ee145
Author: Federico Mena Quintero <federico gnome org>
Date:   Thu Mar 26 15:07:07 2020 -0600

    Path.iter() - new function, replaces get_path_commands()
    
    We'll move to a path iterator scheme instead of passing around a slice
    of PathCommand.  This will let us change the internals of Path without
    affecting the rest of the code.

 rsvg_internals/src/drawing_ctx.rs  |  8 +-------
 rsvg_internals/src/marker.rs       | 36 ++++++++++++++++++------------------
 rsvg_internals/src/path_builder.rs | 24 ++++++++++++++++++++----
 rsvg_internals/src/path_parser.rs  |  4 ++--
 4 files changed, 41 insertions(+), 31 deletions(-)
---
diff --git a/rsvg_internals/src/drawing_ctx.rs b/rsvg_internals/src/drawing_ctx.rs
index da4ff71d..ee143f25 100644
--- a/rsvg_internals/src/drawing_ctx.rs
+++ b/rsvg_internals/src/drawing_ctx.rs
@@ -997,13 +997,7 @@ impl DrawingCtx {
                 })?;
 
             if markers == Markers::Yes {
-                marker::render_markers_for_path(
-                    path.get_path_commands(),
-                    self,
-                    acquired_nodes,
-                    values,
-                    clipping,
-                )?;
+                marker::render_markers_for_path(path, self, acquired_nodes, values, clipping)?;
             }
 
             Ok(bbox)
diff --git a/rsvg_internals/src/marker.rs b/rsvg_internals/src/marker.rs
index c69b7e56..c98c9e05 100644
--- a/rsvg_internals/src/marker.rs
+++ b/rsvg_internals/src/marker.rs
@@ -19,7 +19,7 @@ use crate::iri::IRI;
 use crate::length::*;
 use crate::node::{CascadedValues, Node, NodeBorrow, NodeDraw};
 use crate::parsers::{Parse, ParseValue};
-use crate::path_builder::{arc_segment, ArcParameterization, CubicBezierCurve, PathCommand};
+use crate::path_builder::{arc_segment, ArcParameterization, CubicBezierCurve, Path, PathCommand};
 use crate::properties::{ComputedValues, ParsedProperty, SpecifiedValue, SpecifiedValues};
 use crate::property_bag::PropertyBag;
 use crate::rect::Rect;
@@ -337,8 +337,8 @@ impl Deref for Segments {
 // The tangent at the end point is given by the vector (P4 - P3).
 // The tangents also work if the segment refers to a lineto (they will
 // both just point in the same direction).
-impl From<&[PathCommand]> for Segments {
-    fn from(path_commands: &[PathCommand]) -> Segments {
+impl From<&Path> for Segments {
+    fn from(path: &Path) -> Segments {
         let mut last_x: f64;
         let mut last_y: f64;
         let mut cur_x: f64;
@@ -356,11 +356,11 @@ impl From<&[PathCommand]> for Segments {
         segments = Vec::new();
         state = SegmentState::Initial;
 
-        for path_command in path_commands {
+        for path_command in path.iter() {
             last_x = cur_x;
             last_y = cur_y;
 
-            match *path_command {
+            match path_command {
                 PathCommand::MoveTo(x, y) => {
                     cur_x = x;
                     cur_y = y;
@@ -617,7 +617,7 @@ where
 }
 
 pub fn render_markers_for_path(
-    path_commands: &[PathCommand],
+    path: &Path,
     draw_ctx: &mut DrawingCtx,
     acquired_nodes: &mut AcquiredNodes,
     values: &ComputedValues,
@@ -641,7 +641,7 @@ pub fn render_markers_for_path(
     }
 
     emit_markers_for_path(
-        path_commands,
+        path,
         draw_ctx.empty_bbox(),
         &mut |marker_type: MarkerType, x: f64, y: f64, computed_angle: Angle| {
             if let IRI::Resource(ref marker) = match marker_type {
@@ -667,7 +667,7 @@ pub fn render_markers_for_path(
 }
 
 fn emit_markers_for_path<E>(
-    path_commands: &[PathCommand],
+    path: &Path,
     empty_bbox: BoundingBox,
     emit_fn: &mut E,
 ) -> Result<BoundingBox, RenderingError>
@@ -682,7 +682,7 @@ where
     let mut bbox = empty_bbox;
 
     // Convert the path to a list of segments and bare points
-    let segments = Segments::from(path_commands);
+    let segments = Segments::from(path);
 
     let mut subpath_state = SubpathState::NoSubpath;
 
@@ -774,7 +774,7 @@ where
                 .unwrap_or_else(|| Angle::new(0.0));
 
             let angle = {
-                if let PathCommand::ClosePath = path_commands[segments.len()] {
+                if let PathCommand::ClosePath = path.iter().nth(segments.len()).unwrap() {
                     let outgoing = segments
                         .find_outgoing_angle_forwards(0)
                         .unwrap_or_else(|| Angle::new(0.0));
@@ -867,7 +867,7 @@ mod directionality_tests {
         builder.line_to(20.0, 10.0);
         builder.line_to(20.0, 20.0);
 
-        Segments::from(builder.into_path().get_path_commands())
+        Segments::from(&builder.into_path())
     }
 
     #[test]
@@ -892,7 +892,7 @@ mod directionality_tests {
         builder.curve_to(50.0, 35.0, 60.0, 60.0, 70.0, 70.0);
         builder.line_to(80.0, 90.0);
 
-        Segments::from(builder.into_path().get_path_commands())
+        Segments::from(&builder.into_path())
     }
 
     #[test]
@@ -917,7 +917,7 @@ mod directionality_tests {
         builder.line_to(20.0, 20.0);
         builder.close_path();
 
-        Segments::from(builder.into_path().get_path_commands())
+        Segments::from(&builder.into_path())
     }
 
     #[test]
@@ -947,7 +947,7 @@ mod directionality_tests {
         builder.line_to(80.0, 90.0);
         builder.close_path();
 
-        Segments::from(builder.into_path().get_path_commands())
+        Segments::from(&builder.into_path())
     }
 
     #[test]
@@ -977,7 +977,7 @@ mod directionality_tests {
 
         builder.line_to(40.0, 30.0);
 
-        Segments::from(builder.into_path().get_path_commands())
+        Segments::from(&builder.into_path())
     }
 
     #[test]
@@ -1011,7 +1011,7 @@ mod directionality_tests {
     // builder.move_to (30.0, 30.0);
     // builder.move_to (40.0, 40.0);
     //
-    // Segments::from(builder.into_path().get_path_commands())
+    // Segments::from(&builder.into_path())
     // }
     //
     // #[test]
@@ -1119,7 +1119,7 @@ mod marker_tests {
         let mut v = Vec::new();
 
         assert!(emit_markers_for_path(
-            builder.into_path().get_path_commands(),
+            &builder.into_path(),
             BoundingBox::new(),
             &mut |marker_type: MarkerType,
                   x: f64,
@@ -1155,7 +1155,7 @@ mod marker_tests {
         let mut v = Vec::new();
 
         assert!(emit_markers_for_path(
-            builder.into_path().get_path_commands(),
+            &builder.into_path(),
             BoundingBox::new(),
             &mut |marker_type: MarkerType,
                   x: f64,
diff --git a/rsvg_internals/src/path_builder.rs b/rsvg_internals/src/path_builder.rs
index b5738ffb..cba62e59 100644
--- a/rsvg_internals/src/path_builder.rs
+++ b/rsvg_internals/src/path_builder.rs
@@ -4,6 +4,7 @@ use smallvec::SmallVec;
 
 use std::f64;
 use std::f64::consts::*;
+use std::slice;
 
 use crate::float_eq_cairo::ApproxEqCairo;
 use crate::util::clamp;
@@ -308,6 +309,11 @@ pub struct Path {
     path_commands: Box<[PathCommand]>,
 }
 
+/// Iterator over a path's commands
+pub struct PathIter<'a> {
+    commands: slice::Iter<'a, PathCommand>,
+}
+
 impl PathBuilder {
     pub fn new() -> PathBuilder {
         PathBuilder {
@@ -367,18 +373,20 @@ impl PathBuilder {
 }
 
 impl Path {
-    pub fn get_path_commands(&self) -> &[PathCommand] {
-        &self.path_commands
+    pub fn iter(&self) -> PathIter {
+        PathIter {
+            commands: self.path_commands.iter(),
+        }
     }
 
     pub fn is_empty(&self) -> bool {
-        self.path_commands.is_empty()
+        self.iter().nth(0).is_none()
     }
 
     pub fn to_cairo(&self, cr: &cairo::Context) -> Result<(), cairo::Status> {
         assert!(!self.is_empty());
 
-        for s in self.path_commands.iter() {
+        for s in self.iter() {
             s.to_cairo(cr);
         }
 
@@ -400,6 +408,14 @@ impl Path {
     }
 }
 
+impl<'a> Iterator for PathIter<'a> {
+    type Item = PathCommand;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.commands.next().map(Clone::clone)
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
diff --git a/rsvg_internals/src/path_parser.rs b/rsvg_internals/src/path_parser.rs
index f33ab00f..2e05c3fa 100644
--- a/rsvg_internals/src/path_parser.rs
+++ b/rsvg_internals/src/path_parser.rs
@@ -977,9 +977,9 @@ mod tests {
         let result = parse_path_into_builder(path_str, &mut builder);
 
         let path = builder.into_path();
-        let commands = path.get_path_commands();
+        let commands = path.iter().collect::<Vec<_>>();
 
-        assert_eq!(expected_commands, commands);
+        assert_eq!(expected_commands, commands.as_slice());
         assert_eq!(expected_result, result);
     }