Bug 1485433 - Parse byte slice in PathParser. r=emilio
authorBoris Chiou <boris.chiou@gmail.com>
Mon, 27 Aug 2018 18:14:49 +0000
changeset 488657 70f6cffb8052574210987ad09ffb3da98fadf548
parent 488656 cb7af446b84177a4ee6cb18d31cbf52d026a6ac6
child 488658 c673b6f64457db9dba65a2cc235b6a38f29aef14
push id9734
push usershindli@mozilla.com
push dateThu, 30 Aug 2018 12:18:07 +0000
treeherdermozilla-beta@71c71ab3afae [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1485433
milestone63.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1485433 - Parse byte slice in PathParser. r=emilio We only care about ascii char for svg path, so we could parse the string as byte slice. Differential Revision: https://phabricator.services.mozilla.com/D4168
servo/components/style/values/specified/svg_path.rs
--- a/servo/components/style/values/specified/svg_path.rs
+++ b/servo/components/style/values/specified/svg_path.rs
@@ -2,18 +2,18 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Specified types for SVG Path.
 
 use cssparser::Parser;
 use parser::{Parse, ParserContext};
 use std::fmt::{self, Write};
-use std::iter::Peekable;
-use std::str::Chars;
+use std::iter::{Cloned, Peekable};
+use std::slice;
 use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss};
 use style_traits::values::SequenceWriter;
 use values::CSSFloat;
 
 
 /// The SVG path data.
 ///
 /// https://www.w3.org/TR/SVG11/paths.html#PathData
@@ -128,18 +128,18 @@ pub enum PathCommand {
 
 impl ToCss for PathCommand {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: fmt::Write
     {
         use self::PathCommand::*;
         match *self {
-            Unknown => dest.write_str("X"),
-            ClosePath => dest.write_str("Z"),
+            Unknown => dest.write_char('X'),
+            ClosePath => dest.write_char('Z'),
             MoveTo { point, absolute } => {
                 dest.write_char(if absolute { 'M' } else { 'm' })?;
                 dest.write_char(' ')?;
                 point.to_css(dest)
             }
             LineTo { point, absolute } => {
                 dest.write_char(if absolute { 'L' } else { 'l' })?;
                 dest.write_char(' ')?;
@@ -214,17 +214,17 @@ impl CoordPair {
     pub fn new(x: CSSFloat, y: CSSFloat) -> Self {
         CoordPair(x, y)
     }
 }
 
 
 /// SVG Path parser.
 struct PathParser<'a> {
-    chars: Peekable<Chars<'a>>,
+    chars: Peekable<Cloned<slice::Iter<'a, u8>>>,
     path: Vec<PathCommand>,
 }
 
 macro_rules! parse_arguments {
     (
         $parser:ident,
         $abs:ident,
         $enum:ident,
@@ -251,78 +251,78 @@ macro_rules! parse_arguments {
     }
 }
 
 impl<'a> PathParser<'a> {
     /// Return a PathParser.
     #[inline]
     fn new(string: &'a str) -> Self {
         PathParser {
-            chars: string.chars().peekable(),
+            chars: string.as_bytes().iter().cloned().peekable(),
             path: Vec::new(),
         }
     }
 
     /// Parse a sub-path.
     fn parse_subpath(&mut self) -> Result<(), ()> {
         // Handle "moveto" Command first. If there is no "moveto", this is not a valid sub-path
         // (i.e. not a valid moveto-drawto-command-group).
         self.parse_moveto()?;
 
         // Handle other commands.
         loop {
             skip_wsp(&mut self.chars);
-            if self.chars.peek().map_or(true, |m| *m == 'M' || *m == 'm') {
+            if self.chars.peek().map_or(true, |&m| m == b'M' || m == b'm') {
                 break;
             }
 
             match self.chars.next() {
                 Some(command) => {
-                    let abs = command.is_uppercase();
+                    let abs = command.is_ascii_uppercase();
                     macro_rules! parse_command {
                         ( $($($p:pat)|+ => $parse_func:ident,)* ) => {
                             match command {
                                 $(
                                     $($p)|+ => {
                                         skip_wsp(&mut self.chars);
                                         self.$parse_func(abs)?;
                                     },
                                 )*
                                 _ => return Err(()),
                             }
                         }
                     }
                     parse_command!(
-                        'Z' | 'z' => parse_closepath,
-                        'L' | 'l' => parse_lineto,
-                        'H' | 'h' => parse_h_lineto,
-                        'V' | 'v' => parse_v_lineto,
-                        'C' | 'c' => parse_curveto,
-                        'S' | 's' => parse_smooth_curveto,
-                        'Q' | 'q' => parse_quadratic_bezier_curveto,
-                        'T' | 't' => parse_smooth_quadratic_bezier_curveto,
-                        'A' | 'a' => parse_elliprical_arc,
+                        b'Z' | b'z' => parse_closepath,
+                        b'L' | b'l' => parse_lineto,
+                        b'H' | b'h' => parse_h_lineto,
+                        b'V' | b'v' => parse_v_lineto,
+                        b'C' | b'c' => parse_curveto,
+                        b'S' | b's' => parse_smooth_curveto,
+                        b'Q' | b'q' => parse_quadratic_bezier_curveto,
+                        b'T' | b't' => parse_smooth_quadratic_bezier_curveto,
+                        b'A' | b'a' => parse_elliprical_arc,
                     );
                 },
                 _ => break, // no more commands.
             }
         }
         Ok(())
     }
 
     /// Parse "moveto" command.
     fn parse_moveto(&mut self) -> Result<(), ()> {
         let command = match self.chars.next() {
-            Some(c) if c == 'M' || c == 'm' => c,
+            Some(c) if c == b'M' || c == b'm' => c,
             _ => return Err(()),
         };
 
         skip_wsp(&mut self.chars);
         let point = parse_coord(&mut self.chars)?;
-        let absolute = command == 'M';
+        let absolute = command == b'M';
         self.path.push(PathCommand::MoveTo { point, absolute } );
 
         // End of string or the next character is a possible new command.
         if !skip_wsp(&mut self.chars) ||
            self.chars.peek().map_or(true, |c| c.is_ascii_alphabetic()) {
             return Ok(());
         }
         skip_comma_wsp(&mut self.chars);
@@ -377,141 +377,139 @@ impl<'a> PathParser<'a> {
     /// Parse smooth quadratic B├ęzier curveto command.
     fn parse_smooth_quadratic_bezier_curveto(&mut self, absolute: bool) -> Result<(), ()> {
         parse_arguments!(self, absolute, SmoothQuadBezierCurveTo, [ point => parse_coord ])
     }
 
     /// Parse elliptical arc curve command.
     fn parse_elliprical_arc(&mut self, absolute: bool) -> Result<(), ()> {
         // Parse a flag whose value is '0' or '1'; otherwise, return Err(()).
-        let parse_flag = |iter: &mut Peekable<Chars>| -> Result<bool, ()> {
-            let value = match iter.peek() {
-                Some(c) if *c == '0' || *c == '1' => *c == '1',
-                _ => return Err(()),
-            };
-            iter.next();
-            Ok(value)
+        let parse_flag = |iter: &mut Peekable<Cloned<slice::Iter<u8>>>| -> Result<bool, ()> {
+            match iter.next() {
+                Some(c) if c == b'0' || c == b'1' => Ok(c == b'1'),
+                _ => Err(()),
+            }
         };
         parse_arguments!(self, absolute, EllipticalArc, [
             rx => parse_number,
             ry => parse_number,
             angle => parse_number,
             large_arc_flag => parse_flag,
             sweep_flag => parse_flag,
             point => parse_coord
         ])
     }
 }
 
 
 /// Parse a pair of numbers into CoordPair.
-fn parse_coord(iter: &mut Peekable<Chars>) -> Result<CoordPair, ()> {
+fn parse_coord(iter: &mut Peekable<Cloned<slice::Iter<u8>>>) -> Result<CoordPair, ()> {
     let x = parse_number(iter)?;
     skip_comma_wsp(iter);
     let y = parse_number(iter)?;
     Ok(CoordPair::new(x, y))
 }
 
 /// This is a special version which parses the number for SVG Path. e.g. "M 0.6.5" should be parsed
 /// as MoveTo with a coordinate of ("0.6", ".5"), instead of treating 0.6.5 as a non-valid floating
 /// point number. In other words, the logic here is similar with that of
 /// tokenizer::consume_numeric, which also consumes the number as many as possible, but here the
 /// input is a Peekable and we only accept an integer of a floating point number.
 ///
 /// The "number" syntax in https://www.w3.org/TR/SVG/paths.html#PathDataBNF
-fn parse_number(iter: &mut Peekable<Chars>) -> Result<CSSFloat, ()> {
+fn parse_number(iter: &mut Peekable<Cloned<slice::Iter<u8>>>) -> Result<CSSFloat, ()> {
     // 1. Check optional sign.
-    let sign = if iter.peek().map_or(false, |&sign: &char| sign == '+' || sign == '-') {
-        if iter.next().unwrap() == '-' { -1. } else { 1. }
+    let sign = if iter.peek().map_or(false, |&sign| sign == b'+' || sign == b'-') {
+        if iter.next().unwrap() == b'-' { -1. } else { 1. }
     } else {
         1.
     };
 
     // 2. Check integer part.
     let mut integral_part: f64 = 0.;
-    let got_dot = if !iter.peek().map_or(false, |&n: &char| n == '.') {
+    let got_dot = if !iter.peek().map_or(false, |&n| n == b'.') {
         // If the first digit in integer part is neither a dot nor a digit, this is not a number.
-        if iter.peek().map_or(true, |n: &char| !n.is_ascii_digit()) {
+        if iter.peek().map_or(true, |n| !n.is_ascii_digit()) {
             return Err(());
         }
 
-        while iter.peek().map_or(false, |n: &char| n.is_ascii_digit()) {
+        while iter.peek().map_or(false, |n| n.is_ascii_digit()) {
             integral_part =
-                integral_part * 10. + iter.next().unwrap().to_digit(10).unwrap() as f64;
+                integral_part * 10. + (iter.next().unwrap() - b'0') as f64;
         }
 
-        iter.peek().map_or(false, |&n: &char| n == '.')
+        iter.peek().map_or(false, |&n| n == b'.')
     } else {
         true
     };
 
     // 3. Check fractional part.
     let mut fractional_part: f64 = 0.;
     if got_dot {
         // Consume '.'.
         iter.next();
         // If the first digit in fractional part is not a digit, this is not a number.
-        if iter.peek().map_or(true, |n: &char| !n.is_ascii_digit()) {
+        if iter.peek().map_or(true, |n| !n.is_ascii_digit()) {
             return Err(());
         }
 
         let mut factor = 0.1;
-        while iter.peek().map_or(false, |n: &char| n.is_ascii_digit()) {
-            fractional_part += iter.next().unwrap().to_digit(10).unwrap() as f64 * factor;
+        while iter.peek().map_or(false, |n| n.is_ascii_digit()) {
+            fractional_part += (iter.next().unwrap() - b'0') as f64 * factor;
             factor *= 0.1;
         }
     }
 
     let mut value = sign * (integral_part + fractional_part);
 
     // 4. Check exp part. The segment name of SVG Path doesn't include 'E' or 'e', so it's ok to
     //    treat the numbers after 'E' or 'e' are in the exponential part.
-    if iter.peek().map_or(false, |&exp: &char| exp == 'E' || exp == 'e') {
+    if iter.peek().map_or(false, |&exp| exp == b'E' || exp == b'e') {
         // Consume 'E' or 'e'.
         iter.next();
-        let exp_sign = if iter.peek().map_or(false, |&sign: &char| sign == '+' || sign == '-') {
-            if iter.next().unwrap() == '-' { -1. } else { 1. }
+        let exp_sign = if iter.peek().map_or(false, |&sign| sign == b'+' || sign == b'-') {
+            if iter.next().unwrap() == b'-' { -1. } else { 1. }
         } else {
             1.
         };
 
         let mut exp: f64 = 0.;
-        while iter.peek().map_or(false, |n: &char| n.is_ascii_digit()) {
-            exp = exp * 10. + iter.next().unwrap().to_digit(10).unwrap() as f64;
+        while iter.peek().map_or(false, |n| n.is_ascii_digit()) {
+            exp = exp * 10. + (iter.next().unwrap() - b'0') as f64;
         }
 
         value *= f64::powf(10., exp * exp_sign);
     }
 
     if value.is_finite() {
         Ok(value.min(::std::f32::MAX as f64).max(::std::f32::MIN as f64) as CSSFloat)
     } else {
         Err(())
     }
 }
 
 /// Skip all svg whitespaces, and return true if |iter| hasn't finished.
 #[inline]
-fn skip_wsp(iter: &mut Peekable<Chars>) -> bool {
+fn skip_wsp(iter: &mut Peekable<Cloned<slice::Iter<u8>>>) -> bool {
     // Note: SVG 1.1 defines the whitespaces as \u{9}, \u{20}, \u{A}, \u{D}.
     //       However, SVG 2 has one extra whitespace: \u{C}.
     //       Therefore, we follow the newest spec for the definition of whitespace,
-    //       i.e. \u{9}, \u{20}, \u{A}, \u{C}, \u{D}, by is_ascii_whitespace().
-    while iter.peek().map_or(false, |c: &char| c.is_ascii_whitespace()) {
+    //       i.e. \u{9}, \u{20}, \u{A}, \u{C}, \u{D}.
+    while iter.peek().map_or(false, |c| c.is_ascii_whitespace()) {
         iter.next();
     }
     iter.peek().is_some()
 }
 
 /// Skip all svg whitespaces and one comma, and return true if |iter| hasn't finished.
 #[inline]
-fn skip_comma_wsp(iter: &mut Peekable<Chars>) -> bool {
+fn skip_comma_wsp(iter: &mut Peekable<Cloned<slice::Iter<u8>>>) -> bool {
     if !skip_wsp(iter) {
         return false;
     }
 
-    if *iter.peek().unwrap() != ',' {
+    if *iter.peek().unwrap() != b',' {
         return true;
     }
     iter.next();
 
     skip_wsp(iter)
 }