From 4bde6430b37014dbe378bb803911c0179c2efb20 Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Thu, 24 Sep 2015 18:18:07 -0600 Subject: [PATCH] servo: Merge #7703 - gfx: Fix border-radius panic when a corner has 0px and >0px borders (from bjwbell:bugfix-0px-and-non-0px-border-widths); r=pcwalton 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 Source-Repo: https://github.com/servo/servo Source-Revision: 4d1be2f56cd7a37c4c803ba12733fca55b57d4de --- servo/components/gfx/paint_context.rs | 153 ++++++++++++++++++++------ 1 file changed, 122 insertions(+), 31 deletions(-) diff --git a/servo/components/gfx/paint_context.rs b/servo/components/gfx/paint_context.rs index 32a0aa42a0d6..4981f4f88566 100644 --- a/servo/components/gfx/paint_context.rs +++ b/servo/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, end: Point2D, } +impl Line { + /// Guarantees that `start.x <= end.x` for the returned `Line`. + fn new(start: Point2D, end: Point2D) -> 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, top_right: Point2D, @@ -392,12 +420,31 @@ impl<'a> PaintContext<'a> { (Some(x1), Some(x2)) } - fn intersect_ellipse_line(e: Ellipse, l: Line) -> (Option>, Option>) { - debug_assert!(l.end.x - l.start.x > f32::EPSILON, "Error line segment end.x > start.x!!"); - // shift the origin to center of the ellipse. - let line = Line { start: l.start - e.origin, - end: l.end - e.origin }; + fn intersect_ellipse_line(mut e: Ellipse, mut line: Line) -> (Option>, + Option>) { + let mut rotated_axes = false; + fn rotate_axes(point: Point2D, clockwise: bool) -> Point2D { + 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) => { - let p = Point2D::new(x0, a * x0 + b) + e.origin; - (Some(p), None) - }, - (None, Some(x1)) => { - let p = Point2D::new(x1, a * x1 + b) + e.origin; + (Some(x0), None) | (None, Some(x0)) => { + let mut p = Point2D::new(x0, a * x0 + b) + e.origin; + if rotated_axes { + p = rotate_axes(p, false); + } (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, radius: &Size2D, inner_border: &Point2D, @@ -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, - border1_width: f32, - border2_width: f32) -> bool { + // Returns true if the angular size for this border corner + // is PI/4. + fn simple_border_corner(border_corner_radius: &Size2D, + 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, - (outer_border.x - inner_border.x).abs(), - (outer_border.y - inner_border.y).abs()); + let simple_border = simple_border_corner(&radius, + (outer_border.x - inner_border.x).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 { - PaintContext::ellipse_leftmost_intersection(ellipse, - Line { start: *outer_border, - end: *inner_border }).unwrap() - } else { - PaintContext::ellipse_rightmost_intersection(ellipse, - Line { start: *inner_border, - end: *outer_border }).unwrap() + let corner_line = Line::new(*inner_border, *outer_border); + match corner { + BorderCorner::TopLeft | BorderCorner::BottomLeft => + PaintContext::ellipse_leftmost_intersection(ellipse, corner_line).unwrap(), + BorderCorner::TopRight | BorderCorner::BottomRight => + PaintContext::ellipse_rightmost_intersection(ellipse, corner_line).unwrap(), } }; - let (start_angle, end_angle) = match (inner_border.x <= outer_border.x, - inner_border.y >= outer_border.y) { + let (start_angle, end_angle) = match corner { // 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,