style: Simplify border snapping

Make the computed value of border-like properties app units (which is
effectively what happens in Gecko already), and clamp at computed value
time.

Differential Revision: https://phabricator.services.mozilla.com/D179481

components/style/gecko/media_queries.rs

@@ -451,6 +451,14 @@ impl Device {
         unsafe { bindings::Gecko_VisitedStylesEnabled(self.document()) }
     }
 
+    /// Returns the number of app units per device pixel we're using currently.
+    pub fn app_units_per_device_pixel(&self) -> i32 {
+        match self.pres_context() {
+            Some(pc) => pc.mCurAppUnitsPerDevPixel,
+            None => AU_PER_PX,
+        }
+    }
+
     /// Returns the device pixel ratio.
     pub fn device_pixel_ratio(&self) -> Scale<f32, CSSPixel, DevicePixel> {
         let pc = match self.pres_context() {

components/style/gecko/values.rs

@@ -13,8 +13,6 @@ use crate::gecko_bindings::structs::CounterStylePtr;
 use crate::values::generics::CounterStyle;
 use crate::values::Either;
 use crate::Atom;
-use app_units::Au;
-use std::cmp::max;
 
 /// Convert a color value to `nscolor`.
 pub fn convert_absolute_color_to_nscolor(color: &AbsoluteColor) -> u32 {
@@ -38,21 +36,6 @@ pub fn convert_nscolor_to_absolute_color(color: u32) -> AbsoluteColor {
     )
 }
 
-/// Round `width` down to the nearest device pixel, but any non-zero value that
-/// would round down to zero is clamped to 1 device pixel.  Used for storing
-/// computed values of border-*-width and outline-width.
-#[inline]
-pub fn round_border_to_device_pixels(width: Au, au_per_device_px: Au) -> Au {
-    if width == Au(0) {
-        Au(0)
-    } else {
-        max(
-            au_per_device_px,
-            Au(width.0 / au_per_device_px.0 * au_per_device_px.0),
-        )
-    }
-}
-
 impl CounterStyle {
     /// Convert this counter style to a Gecko CounterStylePtr.
     #[inline]

components/style/properties/gecko.mako.rs

@@ -28,7 +28,6 @@ use crate::gecko_bindings::structs;
 use crate::gecko_bindings::structs::nsCSSPropertyID;
 use crate::gecko_bindings::structs::mozilla::PseudoStyleType;
 use crate::gecko::data::PerDocumentStyleData;
-use crate::gecko::values::round_border_to_device_pixels;
 use crate::logical_geometry::WritingMode;
 use crate::media_queries::Device;
 use crate::properties::longhands;
@@ -398,28 +397,16 @@ def set_gecko_property(ffi_name, expr):
 <%call expr="impl_simple_clone(ident, gecko_ffi_name)"></%call>
 </%def>
 
-<%def name="impl_non_negative_length(ident, gecko_ffi_name, inherit_from=None,
-                                     round_to_pixels=False)">
+<%def name="impl_border_width(ident, gecko_ffi_name, inherit_from)">
     #[allow(non_snake_case)]
-    pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
-        let value = {
-            % if round_to_pixels:
-            let au_per_device_px = Au(self.mTwipsPerPixel);
-            round_border_to_device_pixels(Au::from(v), au_per_device_px).0
-            % else:
-            v.0.to_i32_au()
-            % endif
-        };
-
-        % if inherit_from:
+    pub fn set_${ident}(&mut self, v: Au) {
+        let value = v.0;
         self.${inherit_from} = value;
-        % endif
         self.${gecko_ffi_name} = value;
     }
 
     #[allow(non_snake_case)]
     pub fn copy_${ident}_from(&mut self, other: &Self) {
-        % if inherit_from:
         self.${inherit_from} = other.${inherit_from};
         // NOTE: This is needed to easily handle the `unset` and `initial`
         // keywords, which are implemented calling this function.
@@ -430,9 +417,6 @@ def set_gecko_property(ffi_name, expr):
         // FIXME(emilio): We could clean this up a bit special-casing the reset_
         // function below.
         self.${gecko_ffi_name} = other.${inherit_from};
-        % else:
-        self.${gecko_ffi_name} = other.${gecko_ffi_name};
-        % endif
     }
 
     #[allow(non_snake_case)]
@@ -441,8 +425,8 @@ def set_gecko_property(ffi_name, expr):
     }
 
     #[allow(non_snake_case)]
-    pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
-        Au(self.${gecko_ffi_name}).into()
+    pub fn clone_${ident}(&self) -> Au {
+        Au(self.${gecko_ffi_name})
     }
 </%def>
 
@@ -678,7 +662,7 @@ fn static_assert() {
 
 <% skip_border_longhands = " ".join(["border-{0}-{1}".format(x.ident, y)
                                      for x in SIDES
-                                     for y in ["color", "style", "width"]] +
+                                     for y in ["style", "width"]] +
                                     ["border-{0}-radius".format(x.replace("_", "-"))
                                      for x in CORNERS]) %>
 
@@ -733,12 +717,7 @@ fn static_assert() {
         self.mBorderStyle[${side.index}]
     }
 
-    <% impl_simple("border_%s_color" % side.ident, "mBorder%sColor" % side.name) %>
-
-    <% impl_non_negative_length("border_%s_width" % side.ident,
-                                "mComputedBorder.%s" % side.ident,
-                                inherit_from="mBorder.%s" % side.ident,
-                                round_to_pixels=True) %>
+    ${impl_border_width("border_%s_width" % side.ident, "mComputedBorder.%s" % side.ident, "mBorder.%s" % side.ident)}
 
     pub fn border_${side.ident}_has_nonzero_width(&self) -> bool {
         self.mComputedBorder.${side.ident} != 0
@@ -862,9 +841,7 @@ fn static_assert() {
         self.mOutlineStyle.clone()
     }
 
-    <% impl_non_negative_length("outline_width", "mActualOutlineWidth",
-                                inherit_from="mOutlineWidth",
-                                round_to_pixels=True) %>
+    ${impl_border_width("outline_width", "mActualOutlineWidth", "mOutlineWidth")}
 
     pub fn outline_has_nonzero_width(&self) -> bool {
         self.mActualOutlineWidth != 0
@@ -1601,10 +1578,7 @@ fn static_assert() {
 </%self:impl_trait>
 
 
-<%self:impl_trait style_struct_name="InheritedText"
-                  skip_longhands="-webkit-text-stroke-width">
-    ${impl_non_negative_length('_webkit_text_stroke_width',
-                               'mWebkitTextStrokeWidth')}
+<%self:impl_trait style_struct_name="InheritedText">
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Text" skip_longhands="initial-letter">
@@ -1713,9 +1687,7 @@ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask-
         self.mColumnRuleStyle.clone()
     }
 
-    <% impl_non_negative_length("column_rule_width", "mActualColumnRuleWidth",
-                                inherit_from="mColumnRuleWidth",
-                                round_to_pixels=True) %>
+    ${impl_border_width("column_rule_width", "mActualColumnRuleWidth", "mColumnRuleWidth")}
 
     pub fn column_rule_has_nonzero_width(&self) -> bool {
         self.mActualColumnRuleWidth != 0

components/style/properties/helpers.mako.rs

@@ -9,8 +9,7 @@
 %>
 
 <%def name="predefined_type(name, type, initial_value, parse_method='parse',
-            vector=False,
-            computed_type=None, initial_specified_value=None,
+            vector=False, initial_specified_value=None,
             allow_quirks='No', allow_empty=False, **kwargs)">
     <%def name="predefined_type_inner(name, type, initial_value, parse_method)">
         #[allow(unused_imports)]
@@ -23,11 +22,7 @@
         use smallvec::SmallVec;
         pub use crate::values::specified::${type} as SpecifiedValue;
         pub mod computed_value {
-            % if computed_type:
-            pub use ${computed_type} as T;
-            % else:
             pub use crate::values::computed::${type} as T;
-            % endif
         }
         % if initial_value:
         #[inline] pub fn get_initial_value() -> computed_value::T { ${initial_value} }

components/style/properties/longhands/border.mako.rs

@@ -47,9 +47,8 @@
     ${helpers.predefined_type(
         "border-%s-width" % side_name,
         "BorderSideWidth",
-        "crate::values::computed::NonNegativeLength::new(3.)",
+        "app_units::Au::from_px(3)",
         engines="gecko servo",
-        computed_type="crate::values::computed::NonNegativeLength",
         aliases=maybe_moz_logical_alias(engine, side, "-moz-border-%s-width"),
         spec=maybe_logical_spec(side, "width"),
         animation_value_type="NonNegativeLength",

components/style/properties/longhands/column.mako.rs

@@ -42,10 +42,9 @@ ${helpers.single_keyword(
 ${helpers.predefined_type(
     "column-rule-width",
     "BorderSideWidth",
-    "crate::values::computed::NonNegativeLength::new(3.)",
+    "app_units::Au::from_px(3)",
     engines="gecko",
     initial_specified_value="specified::BorderSideWidth::Medium",
-    computed_type="crate::values::computed::NonNegativeLength",
     spec="https://drafts.csswg.org/css-multicol/#propdef-column-rule-width",
     animation_value_type="NonNegativeLength",
 )}

components/style/properties/longhands/inherited_text.mako.rs

@@ -284,10 +284,9 @@ ${helpers.predefined_type(
 ${helpers.predefined_type(
     "-webkit-text-stroke-width",
     "BorderSideWidth",
-    "crate::values::computed::NonNegativeLength::new(0.)",
+    "app_units::Au(0)",
     engines="gecko",
     initial_specified_value="specified::BorderSideWidth::zero()",
-    computed_type="crate::values::computed::NonNegativeLength",
     spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-width",
     animation_value_type="discrete",
 )}

components/style/properties/longhands/outline.mako.rs

@@ -34,10 +34,9 @@ ${helpers.predefined_type(
 ${helpers.predefined_type(
     "outline-width",
     "BorderSideWidth",
-    "crate::values::computed::NonNegativeLength::new(3.)",
+    "app_units::Au::from_px(3)",
     engines="gecko servo",
     initial_specified_value="specified::BorderSideWidth::Medium",
-    computed_type="crate::values::computed::NonNegativeLength",
     animation_value_type="NonNegativeLength",
     spec="https://drafts.csswg.org/css-ui/#propdef-outline-width",
 )}

components/style/properties/properties.mako.rs

@@ -10,7 +10,6 @@
 
 <%namespace name="helpers" file="/helpers.mako.rs" />
 
-#[cfg(feature = "servo")]
 use app_units::Au;
 use arrayvec::{ArrayVec, Drain as ArrayVecDrain};
 use servo_arc::{Arc, UniqueArc};
@@ -40,12 +39,9 @@ use to_shmem::impl_trivial_to_shmem;
 use crate::stylesheets::{CssRuleType, CssRuleTypes, Origin, UrlExtraData};
 use crate::use_counters::UseCounters;
 use crate::values::generics::text::LineHeight;
-use crate::values::{computed, resolved};
-use crate::values::computed::NonNegativeLength;
-use crate::values::serialize_atom_name;
+use crate::values::{computed, resolved, serialize_atom_name};
 use crate::values::specified::font::SystemFont;
 use crate::rule_tree::StrongRuleNode;
-use crate::Zero;
 use crate::str::{CssString, CssStringWriter};
 use std::cell::Cell;
 use super::declaration_block::AppendableValue;
@@ -4156,7 +4152,7 @@ pub fn adjust_border_width(style: &mut StyleBuilder) {
         // Like calling to_computed_value, which wouldn't type check.
         if style.get_border().clone_border_${side}_style().none_or_hidden() &&
            style.get_border().border_${side}_has_nonzero_width() {
-            style.set_border_${side}_width(NonNegativeLength::zero());
+            style.set_border_${side}_width(Au(0));
         }
     % endfor
 }

components/style/values/computed/border.rs

@@ -19,6 +19,9 @@ use app_units::Au;
 
 pub use crate::values::specified::border::BorderImageRepeat;
 
+/// A computed value for border-width (and the like).
+pub type BorderSideWidth = Au;
+
 /// A computed value for the `border-image-width` property.
 pub type BorderImageWidth = Rect<BorderImageSideWidth>;
 

components/style/values/computed/mod.rs

@@ -46,19 +46,16 @@ pub use self::animation::{AnimationIterationCount, AnimationName, AnimationTimel
 pub use self::animation::{ScrollAxis, ScrollTimelineName, TransitionProperty, ViewTimelineInset};
 pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::basic_shape::FillRule;
-pub use self::border::{BorderCornerRadius, BorderRadius, BorderSpacing};
-pub use self::border::{BorderImageRepeat, BorderImageSideWidth};
-pub use self::border::{BorderImageSlice, BorderImageWidth};
-pub use self::box_::{
-    Appearance, BreakBetween, BreakWithin, Clear, ContainIntrinsicSize, ContentVisibility, Float,
+pub use self::border::{
+    BorderCornerRadius, BorderImageRepeat, BorderImageSideWidth, BorderImageSlice,
+    BorderImageWidth, BorderRadius, BorderSideWidth, BorderSpacing,
 };
-pub use self::box_::{BaselineSource, TouchAction, VerticalAlign, WillChange};
 pub use self::box_::{
-    Contain, ContainerName, ContainerType, Display, LineClamp, Overflow, OverflowAnchor,
-};
-pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize, ScrollbarGutter};
-pub use self::box_::{
-    ScrollSnapAlign, ScrollSnapAxis, ScrollSnapStop, ScrollSnapStrictness, ScrollSnapType,
+    Appearance, BaselineSource, BreakBetween, BreakWithin, Clear, Contain, ContainIntrinsicSize,
+    ContainerName, ContainerType, ContentVisibility, Display, Float, LineClamp, Overflow,
+    OverflowAnchor, OverflowClipBox, OverscrollBehavior, Perspective, Resize, ScrollSnapAlign,
+    ScrollSnapAxis, ScrollSnapStop, ScrollSnapStrictness, ScrollSnapType, ScrollbarGutter,
+    TouchAction, VerticalAlign, WillChange,
 };
 pub use self::color::{
     Color, ColorOrAuto, ColorPropertyValue, ColorScheme, ForcedColorAdjust, PrintColorAdjust,

components/style/values/specified/border.rs

@@ -4,8 +4,9 @@
 
 //! Specified types for CSS values related to borders.
 
+use app_units::Au;
 use crate::parser::{Parse, ParserContext};
-use crate::values::computed::{self, Context, ToComputedValue};
+use crate::values::computed::{Context, ToComputedValue};
 use crate::values::generics::border::BorderCornerRadius as GenericBorderCornerRadius;
 use crate::values::generics::border::BorderImageSideWidth as GenericBorderImageSideWidth;
 use crate::values::generics::border::BorderImageSlice as GenericBorderImageSlice;
@@ -144,25 +145,31 @@ impl Parse for BorderSideWidth {
 }
 
 impl ToComputedValue for BorderSideWidth {
-    type ComputedValue = computed::NonNegativeLength;
+    type ComputedValue = app_units::Au;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        // We choose the pixel length of the keyword values the same as both spec and gecko.
-        // Spec: https://drafts.csswg.org/css-backgrounds-3/#line-width
-        // Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1312155#c0
-        match *self {
-            BorderSideWidth::Thin => NonNegativeLength::from_px(1.).to_computed_value(context),
-            BorderSideWidth::Medium => NonNegativeLength::from_px(3.).to_computed_value(context),
-            BorderSideWidth::Thick => NonNegativeLength::from_px(5.).to_computed_value(context),
-            BorderSideWidth::Length(ref length) => length.to_computed_value(context),
+        let width = match *self {
+            // https://drafts.csswg.org/css-backgrounds-3/#line-width
+            BorderSideWidth::Thin => Au::from_px(1),
+            BorderSideWidth::Medium => Au::from_px(3),
+            BorderSideWidth::Thick => Au::from_px(5),
+            BorderSideWidth::Length(ref length) => Au::from_f32_px(length.to_computed_value(context).px()),
+        };
+
+        // Round `width` down to the nearest device pixel, but any non-zero value that would round
+        // down to zero is clamped to 1 device pixel.
+        if width == Au(0) {
+            return width;
         }
-        .into()
+
+        let au_per_dev_px = context.device().app_units_per_device_pixel();
+        std::cmp::max(Au(au_per_dev_px), Au(width.0 / au_per_dev_px * au_per_dev_px))
     }
 
     #[inline]
     fn from_computed_value(computed: &Self::ComputedValue) -> Self {
-        BorderSideWidth::Length(ToComputedValue::from_computed_value(computed))
+        BorderSideWidth::Length(NonNegativeLength::from_px(computed.to_f32_px()))
     }
 }