From 071acf476b92bedf6ef90e653ebfa483d4d6ee74 Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Mon, 21 Sep 2015 17:13:25 -0700 Subject: [PATCH] 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. --- components/gfx/paint_context.rs | 152 ++++++++++++++++---- tests/ref/basic.list | 1 + tests/ref/border_radius_zero_sizes_a.html | 10 ++ tests/ref/border_radius_zero_sizes_ref.html | 10 ++ 4 files changed, 146 insertions(+), 27 deletions(-) create mode 100644 tests/ref/border_radius_zero_sizes_a.html create mode 100644 tests/ref/border_radius_zero_sizes_ref.html diff --git a/components/gfx/paint_context.rs b/components/gfx/paint_context.rs index ff60e90a1af..d57205067d5 100644 --- a/components/gfx/paint_context.rs +++ b/components/gfx/paint_context.rs @@ -65,6 +65,14 @@ enum Direction { Bottom } +#[derive(Copy, Clone)] +enum BorderCorner { + TL, // top left + TR, // top right + BR, // bottom right + BL, // bottom left +} + #[derive(Copy, Clone)] enum DashSize { DottedBorder = 1, @@ -83,6 +91,23 @@ struct Line { start: Point2D, end: Point2D, } +impl 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, @@ -394,12 +419,34 @@ 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(_e: Ellipse, _l: Line) -> (Option>, + Option>) { + let mut line = _l; + let mut e = _e; + let mut rotated_axises = false; + fn rotate_axises(point: Point2D, clockwise: bool) -> Point2D { + if clockwise { + // rotate clockwise by 90 degress + Point2D::new(point.y, -point.x) + } else { + // rotate counter clockwise by 90 degress + Point2D::new(-point.y, point.x) + } + } + + // if line height is greater than it's width then rotate the axises 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_axises = true; + line = Line::new(rotate_axises(line.start, true), rotate_axises(line.end, true)); + e = Ellipse { origin: rotate_axises(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, @@ -424,14 +471,24 @@ impl<'a> PaintContext<'a> { if x0 > x1 { mem::swap(&mut p0, &mut p1); } + if rotated_axises { + p0 = rotate_axises(p0, false); + p1 = rotate_axises(p1, false); + } (Some(p0), Some(p1)) }, (Some(x0), None) => { - let p = Point2D::new(x0, a * x0 + b) + e.origin; + let mut p = Point2D::new(x0, a * x0 + b) + e.origin; + if rotated_axises { + p = rotate_axises(p, false); + } (Some(p), None) }, (None, Some(x1)) => { - let p = Point2D::new(x1, a * x1 + b) + e.origin; + let mut p = Point2D::new(x1, a * x1 + b) + e.origin; + if rotated_axises { + p = rotate_axises(p, false); + } (Some(p), None) }, (None, None) => (None, None), @@ -639,8 +696,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, @@ -655,9 +734,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 } @@ -666,32 +747,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::TL | BorderCorner::BL => + PaintContext::ellipse_leftmost_intersection(ellipse, corner_line).unwrap(), + BorderCorner::TR | BorderCorner::BR => + 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::TR => + 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::BR => + 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::TL => + 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::BL => + 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); @@ -762,6 +844,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_TR), } PaintContext::draw_corner(path_builder, + BorderCorner::TR, &corner_origin.top_right, &radii.top_right, &inner_TR, @@ -776,6 +859,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_BL), } PaintContext::draw_corner(path_builder, + BorderCorner::TL, &corner_origin.top_left, &radii.top_left, &inner_TL, @@ -803,6 +887,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_TL), } PaintContext::draw_corner(path_builder, + BorderCorner::TL, &corner_origin.top_left, &radii.top_left, &inner_TL, @@ -817,6 +902,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_BR), } PaintContext::draw_corner(path_builder, + BorderCorner::BL, &corner_origin.bottom_left, &radii.bottom_left, &inner_BL, @@ -844,6 +930,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_TL), } PaintContext::draw_corner(path_builder, + BorderCorner::TR, &corner_origin.top_right, &radii.top_right, &inner_TR, @@ -858,6 +945,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_BR), } PaintContext::draw_corner(path_builder, + BorderCorner::BR, &corner_origin.bottom_right, &radii.bottom_right, &inner_BR, @@ -887,6 +975,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(edge_TR), } PaintContext::draw_corner(path_builder, + BorderCorner::BR, &corner_origin.bottom_right, &radii.bottom_right, &inner_BR, @@ -901,6 +990,7 @@ impl<'a> PaintContext<'a> { BorderPathDrawingMode::CornersOnly => path_builder.move_to(corner_BL), } PaintContext::draw_corner(path_builder, + BorderCorner::BL, &corner_origin.bottom_left, &radii.bottom_left, &inner_BL, @@ -958,6 +1048,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::TR, &origin_TR, &radii.top_right, &inner_TR, @@ -965,6 +1056,7 @@ impl<'a> PaintContext<'a> { &zero_elbow, true); PaintContext::draw_corner(path_builder, // 3 + BorderCorner::TR, &origin_TR, &radii.top_right, &inner_TR, @@ -973,6 +1065,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::BR, &origin_BR, &radii.bottom_right, &inner_BR, @@ -980,6 +1073,7 @@ impl<'a> PaintContext<'a> { &zero_elbow, true); PaintContext::draw_corner(path_builder, // 5 + BorderCorner::BR, &origin_BR, &radii.bottom_right, &inner_BR, @@ -989,6 +1083,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::BL, &origin_BL, &radii.bottom_left, &inner_BL, @@ -996,6 +1091,7 @@ impl<'a> PaintContext<'a> { &zero_elbow, true); PaintContext::draw_corner(path_builder, // 7 + BorderCorner::BL, &origin_BL, &radii.bottom_left, &inner_BL, @@ -1005,6 +1101,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::TL, &origin_TL, &radii.top_left, &inner_TL, @@ -1012,6 +1109,7 @@ impl<'a> PaintContext<'a> { &zero_elbow, true); PaintContext::draw_corner(path_builder, // 9 + BorderCorner::TL, &origin_TL, &radii.top_left, &inner_TL, diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 404adece8de..1d78ac4485a 100644 --- a/tests/ref/basic.list +++ b/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 diff --git a/tests/ref/border_radius_zero_sizes_a.html b/tests/ref/border_radius_zero_sizes_a.html new file mode 100644 index 00000000000..d79ed6fd6ce --- /dev/null +++ b/tests/ref/border_radius_zero_sizes_a.html @@ -0,0 +1,10 @@ + + + + +
+
+
+
+ + diff --git a/tests/ref/border_radius_zero_sizes_ref.html b/tests/ref/border_radius_zero_sizes_ref.html new file mode 100644 index 00000000000..1f9d83440ae --- /dev/null +++ b/tests/ref/border_radius_zero_sizes_ref.html @@ -0,0 +1,10 @@ + + + + +
+
+
+
+ +