From 642b34865ff47a016305f6035f391c2a376ab769 Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Wed, 9 Sep 2015 03:20:52 -0700 Subject: [PATCH 1/2] gfx: Fix bug with 1px width borders disappearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In to_nearest_azure_rect when rounding to pixel coordinates, maintain the invariant of rect non-overlap (if before rounding two rects don't overlap). The previous code rounded the rect top left corner to the nearest pixel with the size rounded to the nearest pixel multiple which can violate the non-overlap condition, e.g. 10px×9.60px at (0px,6.6px) & 10px×9.60px at (0px,16.2px) would round to 10px×10.0px at (0px,7.0px) & 10px×10.0px at (0px,16.0px), which overlap. Instead round each corner to the nearest pixel. For rects that dont need to satify the non-overlap condition and with width or height between 0.5px and 1px, rounding each rect corner to the nearest pixel can yield an empty rect e.g. 10px×0.6px at 0px,28.56px -> 10px×0px at 0px,29px. For this scenario a new function to_nearest_non_empty_azure_rect rounds the rect top left corner to the nearest pixel and the rect size to the nearest pixel multiple. It's possible for non-overlapping rects after this rounding to overlap. --- components/gfx/paint_context.rs | 31 ++++++++++++++++++- tests/ref/basic.list | 1 + ...r_rounding_1px_invisible_issue_7184_a.html | 28 +++++++++++++++++ ...rounding_1px_invisible_issue_7184_ref.html | 22 +++++++++++++ 4 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 tests/ref/border_rounding_1px_invisible_issue_7184_a.html create mode 100644 tests/ref/border_rounding_1px_invisible_issue_7184_ref.html diff --git a/components/gfx/paint_context.rs b/components/gfx/paint_context.rs index c4d883596b7..d8d8f74d8f5 100644 --- a/components/gfx/paint_context.rs +++ b/components/gfx/paint_context.rs @@ -1549,13 +1549,38 @@ impl ToAzurePoint for Point2D { pub trait ToAzureRect { fn to_nearest_azure_rect(&self) -> Rect; + fn to_nearest_non_empty_azure_rect(&self) -> Rect; fn to_azure_rect(&self) -> Rect; } impl ToAzureRect for Rect { + + /// Round rects to pixel coordinates, maintaining the invariant of non-overlap, + /// assuming that before rounding rects don't overlap. fn to_nearest_azure_rect(&self) -> Rect { + // Rounding the top left corner to the nearest pixel with the size rounded + // to the nearest pixel multiple would violate the non-overlap condition, + // e.g. + // 10px×9.60px at (0px,6.6px) & 10px×9.60px at (0px,16.2px) + // would round to + // 10px×10.0px at (0px,7.0px) & 10px×10.0px at (0px,16.0px), which overlap. + // + // Instead round each corner to the nearest pixel. + let top_left = self.origin.to_nearest_azure_point(); + let bottom_right = self.bottom_right().to_nearest_azure_point(); + Rect::new(top_left, Size2D::new((bottom_right.x - top_left.x) as AzFloat, + (bottom_right.y - top_left.y) as AzFloat)) + } + + /// For rects of width or height between 0.5px and 1px, rounding each rect corner to the + /// nearest pixel can yield an empty rect e.g. + /// 10px×0.6px at 0px,28.56px -> 10px×0px at 0px,29px + /// Instead round the top left to the nearest pixel and the size to the nearest pixel + /// multiple. It's possible for non-overlapping rects after this rounding to overlap. + fn to_nearest_non_empty_azure_rect(&self) -> Rect { Rect::new(self.origin.to_nearest_azure_point(), self.size.to_nearest_azure_size()) } + fn to_azure_rect(&self) -> Rect { Rect::new(self.origin.to_azure_point(), self.size.to_azure_size()) } @@ -1763,7 +1788,11 @@ impl DrawTargetExtensions for DrawTarget { } fn create_rectangular_path(&self, rect: &Rect) -> Path { - let rect = rect.to_nearest_azure_rect(); + // Explicitly round to the nearest non-empty rect because when drawing + // box-shadow the rect height can be between 0.5px & 1px and could + // otherwise round to an empty rect. + let rect = rect.to_nearest_non_empty_azure_rect(); + let path_builder = self.create_path_builder(); path_builder.move_to(rect.origin); path_builder.line_to(Point2D::new(rect.max_x(), rect.origin.y)); diff --git a/tests/ref/basic.list b/tests/ref/basic.list index 2c59c96f43f..fb1e989c636 100644 --- a/tests/ref/basic.list +++ b/tests/ref/basic.list @@ -67,6 +67,7 @@ flaky_cpu == append_style_a.html append_style_b.html != border_radius_dashed_a.html border_radius_dashed_ref.html == border_radius_elliptical_a.html border_radius_elliptical_ref.html == border_radius_overlapping_a.html border_radius_overlapping_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 == border_spacing_empty_table.html border_spacing_empty_table_ref.html diff --git a/tests/ref/border_rounding_1px_invisible_issue_7184_a.html b/tests/ref/border_rounding_1px_invisible_issue_7184_a.html new file mode 100644 index 00000000000..c97396bf19f --- /dev/null +++ b/tests/ref/border_rounding_1px_invisible_issue_7184_a.html @@ -0,0 +1,28 @@ + + + + + + +
+
+
1
+
2
+
3
+
4
+
+ + diff --git a/tests/ref/border_rounding_1px_invisible_issue_7184_ref.html b/tests/ref/border_rounding_1px_invisible_issue_7184_ref.html new file mode 100644 index 00000000000..1b7b0b5b148 --- /dev/null +++ b/tests/ref/border_rounding_1px_invisible_issue_7184_ref.html @@ -0,0 +1,22 @@ + + + + + + +
+
1
+
2
+
3
+
4
+
+ + From 1f4468641575a688cb36af5db1c6c6babdfc9a9b Mon Sep 17 00:00:00 2001 From: Bryan Bell Date: Wed, 9 Sep 2015 11:58:21 -0700 Subject: [PATCH 2/2] position-relative-035 reftest -> expected fail There is no easy way to pass position-relative-035 & also pass both tests/ref/border_rounding_1px_invisible_issue_7184 and tests/ref/text_decoration_underline_subpx. https://github.com/servo/servo/pull/7161, "Snap rectangles to nearest pixels...", did fix position-relative-035 (except for OS X) but broke 1px borders. --- .../metadata-css/css21_dev/html4/position-relative-035.htm.ini | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/wpt/metadata-css/css21_dev/html4/position-relative-035.htm.ini b/tests/wpt/metadata-css/css21_dev/html4/position-relative-035.htm.ini index 8f7aa9b2111..9cb4b4c28f7 100644 --- a/tests/wpt/metadata-css/css21_dev/html4/position-relative-035.htm.ini +++ b/tests/wpt/metadata-css/css21_dev/html4/position-relative-035.htm.ini @@ -1,4 +1,3 @@ [position-relative-035.htm] type: reftest - expected: - if os == "mac": FAIL + expected: FAIL