Auto merge of #7703 - bjwbell:bugfix-0px-and-non-0px-border-widths, r=pcwalton

gfx: Fix border-radius panic when a corner has 0px and >0px borders

When one border is 0px and the other is >0px then the border corner
drawing code panics when computing the values to use in drawing the
border corner arcs.

This fixes that bug and makes the `draw_corner` function more robust
by explicitly passing an enum, `BorderCorner`, naming which corner is
being drawn e.g. `BorderCorner::TL`.

Add a ref test,
`border_radius_zero_sizes_a.html/border_radius_zero_sizes_ref.html`.

Fixes https://github.com/servo/servo/issues/7700.

r? @pcwalton or @mbrubeck

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/7703)
<!-- Reviewable:end -->

components/gfx/paint_context.rs

@@ -63,6 +63,14 @@ enum Direction {
     Bottom
 }
 
+#[derive(Copy, Clone)]
+enum BorderCorner {
+    TopLeft,
+    TopRight,
+    BottomRight,
+    BottomLeft,
+}
+
 #[derive(Copy, Clone)]
 enum DashSize {
     DottedBorder = 1,
@@ -76,12 +84,32 @@ struct Ellipse {
     height: f32,
 }
 
+/// When `Line::new` creates a new `Line` it ensures `start.x <= end.x` for that line.
 #[derive(Copy, Clone, Debug)]
 struct Line {
     start: Point2D<f32>,
     end: Point2D<f32>,
 }
 
+impl Line {
+    /// Guarantees that `start.x <= end.x` for the returned `Line`.
+    fn new(start: Point2D<f32>, end: Point2D<f32>) -> Line {
+        let line = if start.x <= end.x {
+            Line { start: start, end: end }
+        } else {
+            Line { start: end, end: start }
+        };
+        debug_assert!(line.length_squared() > f32::EPSILON);
+        line
+    }
+
+    fn length_squared(&self) -> f32 {
+        let width = (self.end.x - self.start.x).abs();
+        let height = (self.end.y - self.start.y).abs();
+        width * width + height * height
+    }
+}
+
 struct CornerOrigin {
         top_left: Point2D<f32>,
         top_right: Point2D<f32>,
@@ -392,12 +420,31 @@ impl<'a> PaintContext<'a> {
         (Some(x1), Some(x2))
     }
 
-    fn intersect_ellipse_line(e: Ellipse, l: Line) -> (Option<Point2D<f32>>, Option<Point2D<f32>>) {
+    fn intersect_ellipse_line(mut e: Ellipse, mut line: Line) -> (Option<Point2D<f32>>,
-        debug_assert!(l.end.x - l.start.x > f32::EPSILON, "Error line segment end.x > start.x!!");
+                                                                  Option<Point2D<f32>>) {
-        // shift the origin to center of the ellipse.
+        let mut rotated_axes = false;
-        let line = Line { start: l.start - e.origin,
+        fn rotate_axes(point: Point2D<f32>, clockwise: bool) -> Point2D<f32> {
-                          end: l.end - e.origin };
+            if clockwise {
+                // rotate clockwise by 90 degrees
+                Point2D::new(point.y, -point.x)
+            } else {
+                // rotate counter clockwise by 90 degrees
+                Point2D::new(-point.y, point.x)
+            }
+        }
 
+        // if line height is greater than its width then rotate the axes by 90 degrees,
+        // i.e. (x, y) -> (y, -x).
+        if (line.end.x - line.start.x).abs() < (line.end.y - line.start.y).abs() {
+            rotated_axes = true;
+            line = Line::new(rotate_axes(line.start, true), rotate_axes(line.end, true));
+            e = Ellipse { origin: rotate_axes(e.origin, true),
+                          width: e.height, height: e.width };
+        }
+        debug_assert!(line.end.x - line.start.x > f32::EPSILON,
+                      "Error line segment end.x ({}) <= start.x ({})!", line.end.x, line.start.x);
+        // shift the origin to center of the ellipse.
+        line = Line::new(line.start - e.origin, line.end - e.origin);
         let a = (line.end.y - line.start.y)/(line.end.x - line.start.x);
         let b = line.start.y - (a * line.start.x);
         // given the equation of a line,
@@ -422,14 +469,17 @@ impl<'a> PaintContext<'a> {
                 if x0 > x1 {
                     mem::swap(&mut p0, &mut p1);
                 }
+                if rotated_axes {
+                    p0 = rotate_axes(p0, false);
+                    p1 = rotate_axes(p1, false);
+                }
                 (Some(p0), Some(p1))
             },
-            (Some(x0), None) => {
+            (Some(x0), None) | (None, Some(x0)) => {
-                let p = Point2D::new(x0, a * x0 + b) + e.origin;
+                let mut p = Point2D::new(x0, a * x0 + b) + e.origin;
-                (Some(p), None)
+                if rotated_axes {
-            },
+                    p = rotate_axes(p, false);
-            (None, Some(x1)) => {
+                }
-                let p = Point2D::new(x1, a * x1 + b) + e.origin;
                 (Some(p), None)
             },
             (None, None) => (None, None),
@@ -637,8 +687,30 @@ impl<'a> PaintContext<'a> {
          SideOffsets2D::new(elbow_TL, elbow_TR, elbow_BR, elbow_BL))
     }
 
+    /// `origin` is the origin point when drawing the corner e.g. it's the circle center
+    ///  when drawing radial borders.
+    ///
+    /// `corner` indicates which corner to draw e.g. top left or top right etc.
+    ///
+    /// `radius` is the border-radius width and height. If `radius.width == radius.height` then
+    ///  an arc from a circle is drawn instead of an arc from an ellipse.
+    ///
+    /// `inner_border` & `outer_border` are the inner and outer points on the border corner
+    ///  respectively. ASCII diagram:
+    ///      ---------------* =====> ("*" is the `outer_border` point)
+    ///                     |
+    ///                     |
+    ///                     |
+    ///      --------* ============> ("*" is the `inner_border` point)
+    ///              |      |
+    ///              |      |
+    ///
+    ///
+    ///  `dist_elbow` is the distance from `origin` to the inner part of the border corner.
+    ///  `clockwise` indicates direction to draw the border curve.
     #[allow(non_snake_case)]
     fn draw_corner(path_builder: &mut PathBuilder,
+                   corner: BorderCorner,
                    origin: &Point2D<AzFloat>,
                    radius: &Size2D<AzFloat>,
                    inner_border: &Point2D<AzFloat>,
@@ -653,9 +725,11 @@ impl<'a> PaintContext<'a> {
         let rad_TL = rad_L  + f32::consts::FRAC_PI_4;
         let rad_T  = rad_TL + f32::consts::FRAC_PI_4;
 
-        fn compatible_borders_corner(border_corner_radius: &Size2D<f32>,
+        // Returns true if the angular size for this border corner
-                                     border1_width: f32,
+        // is PI/4.
-                                     border2_width: f32) -> bool {
+        fn simple_border_corner(border_corner_radius: &Size2D<f32>,
+                                border1_width: f32,
+                                border2_width: f32) -> bool {
             (border_corner_radius.width - border_corner_radius.height).abs() <= f32::EPSILON &&
                 (border1_width - border2_width).abs() <= f32::EPSILON
         }
@@ -664,32 +738,33 @@ impl<'a> PaintContext<'a> {
             return;
         }
         let ellipse = Ellipse { origin: *origin, width: radius.width, height: radius.height };
-        let simple_border = compatible_borders_corner(&radius,
+        let simple_border = simple_border_corner(&radius,
-                                                      (outer_border.x - inner_border.x).abs(),
+                                                 (outer_border.x - inner_border.x).abs(),
-                                                      (outer_border.y - inner_border.y).abs());
+                                                 (outer_border.y - inner_border.y).abs());
         let corner_angle = if simple_border {
             f32::consts::FRAC_PI_4
         } else {
-            if inner_border.x >= outer_border.x {
+            let corner_line = Line::new(*inner_border, *outer_border);
-                PaintContext::ellipse_leftmost_intersection(ellipse,
+            match corner {
-                                                            Line { start: *outer_border,
+                BorderCorner::TopLeft | BorderCorner::BottomLeft =>
-                                                                   end: *inner_border }).unwrap()
+                    PaintContext::ellipse_leftmost_intersection(ellipse, corner_line).unwrap(),
-            } else {
+                BorderCorner::TopRight | BorderCorner::BottomRight =>
-                PaintContext::ellipse_rightmost_intersection(ellipse,
+                    PaintContext::ellipse_rightmost_intersection(ellipse, corner_line).unwrap(),
-                                                             Line { start: *inner_border,
-                                                                    end: *outer_border }).unwrap()
             }
         };
-        let (start_angle, end_angle) = match (inner_border.x <= outer_border.x,
+        let (start_angle, end_angle) = match corner {
-                                              inner_border.y >= outer_border.y) {
             // TR corner - top border & right border
-            (true, true) => if clockwise { (-rad_B, rad_R - corner_angle) } else { (rad_R - corner_angle, rad_R) },
+            BorderCorner::TopRight =>
+                if clockwise { (-rad_B, rad_R - corner_angle) } else { (rad_R - corner_angle, rad_R) },
             // BR corner - right border & bottom border
-            (true, false) => if clockwise { (rad_R, rad_R + corner_angle) } else { (rad_R + corner_angle, rad_B) },
+            BorderCorner::BottomRight =>
+                if clockwise { (rad_R, rad_R + corner_angle) } else { (rad_R + corner_angle, rad_B) },
             // TL corner - left border & top border
-            (false, true) => if clockwise { (rad_L, rad_L + corner_angle) } else { (rad_L + corner_angle, rad_T) },
+            BorderCorner::TopLeft =>
+                if clockwise { (rad_L, rad_L + corner_angle) } else { (rad_L + corner_angle, rad_T) },
             // BL corner - bottom border & left border
-            (false, false) => if clockwise { (rad_B, rad_L - corner_angle) } else { (rad_L - corner_angle, rad_L) },
+            BorderCorner::BottomLeft =>
+                if clockwise { (rad_B, rad_L - corner_angle) } else { (rad_L - corner_angle, rad_L) },
         };
         if clockwise {
             PaintContext::ellipse_to_bezier(path_builder, *origin, *radius, start_angle, end_angle);
@@ -760,6 +835,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_TR),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::TopRight,
                                           &corner_origin.top_right,
                                           &radii.top_right,
                                           &inner_TR,
@@ -774,6 +850,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_BL),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::TopLeft,
                                           &corner_origin.top_left,
                                           &radii.top_left,
                                           &inner_TL,
@@ -801,6 +878,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_TL),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::TopLeft,
                                           &corner_origin.top_left,
                                           &radii.top_left,
                                           &inner_TL,
@@ -815,6 +893,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_BR),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::BottomLeft,
                                           &corner_origin.bottom_left,
                                           &radii.bottom_left,
                                           &inner_BL,
@@ -842,6 +921,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_TL),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::TopRight,
                                           &corner_origin.top_right,
                                           &radii.top_right,
                                           &inner_TR,
@@ -856,6 +936,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_BR),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::BottomRight,
                                           &corner_origin.bottom_right,
                                           &radii.bottom_right,
                                           &inner_BR,
@@ -885,6 +966,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_TR),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::BottomRight,
                                           &corner_origin.bottom_right,
                                           &radii.bottom_right,
                                           &inner_BR,
@@ -899,6 +981,7 @@ impl<'a> PaintContext<'a> {
                     BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_BL),
                 }
                 PaintContext::draw_corner(path_builder,
+                                          BorderCorner::BottomLeft,
                                           &corner_origin.bottom_left,
                                           &radii.bottom_left,
                                           &inner_BL,
@@ -956,6 +1039,7 @@ impl<'a> PaintContext<'a> {
         path_builder.move_to(Point2D::new(bounds.origin.x + radii.top_left.width, bounds.origin.y));   // 1
         path_builder.line_to(Point2D::new(bounds.max_x() - radii.top_right.width, bounds.origin.y));   // 2
         PaintContext::draw_corner(path_builder,                                                        // 3
+                                  BorderCorner::TopRight,
                                   &origin_TR,
                                   &radii.top_right,
                                   &inner_TR,
@@ -963,6 +1047,7 @@ impl<'a> PaintContext<'a> {
                                   &zero_elbow,
                                   true);
         PaintContext::draw_corner(path_builder,                                                        // 3
+                                  BorderCorner::TopRight,
                                   &origin_TR,
                                   &radii.top_right,
                                   &inner_TR,
@@ -971,6 +1056,7 @@ impl<'a> PaintContext<'a> {
                                   false);
         path_builder.line_to(Point2D::new(bounds.max_x(), bounds.max_y() - radii.bottom_right.width)); // 4
         PaintContext::draw_corner(path_builder,                                                        // 5
+                                  BorderCorner::BottomRight,
                                   &origin_BR,
                                   &radii.bottom_right,
                                   &inner_BR,
@@ -978,6 +1064,7 @@ impl<'a> PaintContext<'a> {
                                   &zero_elbow,
                                   true);
         PaintContext::draw_corner(path_builder,                                                        // 5
+                                  BorderCorner::BottomRight,
                                   &origin_BR,
                                   &radii.bottom_right,
                                   &inner_BR,
@@ -987,6 +1074,7 @@ impl<'a> PaintContext<'a> {
         path_builder.line_to(Point2D::new(bounds.origin.x + radii.bottom_left.width,
                                           bounds.max_y()));                                            // 6
         PaintContext::draw_corner(path_builder,                                                        // 7
+                                  BorderCorner::BottomLeft,
                                   &origin_BL,
                                   &radii.bottom_left,
                                   &inner_BL,
@@ -994,6 +1082,7 @@ impl<'a> PaintContext<'a> {
                                   &zero_elbow,
                                   true);
         PaintContext::draw_corner(path_builder,                                                        // 7
+                                  BorderCorner::BottomLeft,
                                   &origin_BL,
                                   &radii.bottom_left,
                                   &inner_BL,
@@ -1003,6 +1092,7 @@ impl<'a> PaintContext<'a> {
         path_builder.line_to(Point2D::new(bounds.origin.x,
                                           bounds.origin.y + radii.top_left.height));                    // 8
         PaintContext::draw_corner(path_builder,                                                         // 9
+                                  BorderCorner::TopLeft,
                                   &origin_TL,
                                   &radii.top_left,
                                   &inner_TL,
@@ -1010,6 +1100,7 @@ impl<'a> PaintContext<'a> {
                                   &zero_elbow,
                                   true);
         PaintContext::draw_corner(path_builder,                                                         // 9
+                                  BorderCorner::TopLeft,
                                   &origin_TL,
                                   &radii.top_left,
                                   &inner_TL,

tests/ref/basic.list

@@ -69,6 +69,7 @@ flaky_cpu == append_style_a.html append_style_b.html
 == border_radius_elliptical_a.html border_radius_elliptical_ref.html
 == border_radius_overlapping_a.html border_radius_overlapping_ref.html
 == border_radius_shorthand_a.html border_radius_shorthand_ref.html
+== border_radius_zero_sizes_a.html border_radius_zero_sizes_ref.html
 == border_rounding_1px_invisible_issue_7184_a.html border_rounding_1px_invisible_issue_7184_ref.html
 == border_spacing_a.html border_spacing_ref.html
 == border_spacing_auto_layout_a.html border_spacing_ref.html

tests/ref/border_radius_zero_sizes_a.html

@@ -0,0 +1,10 @@
+<html>
+    <head>
+    </head>
+    <body>
+        <div style="border-top: 20px solid black; border-radius: 5px"></div>
+        <div style="border-right: 30px solid black; border-radius: 15px; height: 30px"></div>
+        <div style="border-bottom: 20px solid black; border-radius: 5px"></div>
+        <div style="border-left: 30px solid black; border-radius: 15px; height: 30px"></div>
+    </body>
+</html>

tests/ref/border_radius_zero_sizes_ref.html

@@ -0,0 +1,10 @@
+<html>
+    <head>
+    </head>
+    <body>
+        <div style="border: 10px solid black; border-top-left-radius: 5px; border-top-right-radius: 5px"></div>
+        <div style="border-right: 30px solid black; border-top-right-radius: 15px; border-bottom-right-radius: 15px; height: 30px"></div>
+        <div style="border: 10px solid black; border-bottom-left-radius: 5px; border-bottom-right-radius: 5px"></div>
+        <div style="border-left: 30px solid black; border-bottom-left-radius: 15px; border-top-left-radius: 15px; height: 30px"></div>
+    </body>
+</html>