Bug 1837692 - Do not snap -webkit-text-stroke-width to dev pixels. r=jfkthame

Introduce LineWidth (which doesn't snap) and let BorderSideWidth
wrap it and actually do the snapping.

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

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

@@ -46,7 +46,7 @@ ${helpers.predefined_type(
     "BorderSideWidth",
     "app_units::Au::from_px(3)",
     engines="gecko",
-    initial_specified_value="specified::BorderSideWidth::Medium",
+    initial_specified_value="specified::BorderSideWidth::medium()",
     spec="https://drafts.csswg.org/css-multicol/#propdef-column-rule-width",
     animation_value_type="NonNegativeLength",
 )}

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

@@ -282,10 +282,10 @@ ${helpers.predefined_type(
 
 ${helpers.predefined_type(
     "-webkit-text-stroke-width",
-    "BorderSideWidth",
+    "LineWidth",
     "app_units::Au(0)",
     engines="gecko",
-    initial_specified_value="specified::BorderSideWidth::zero()",
+    initial_specified_value="specified::LineWidth::zero()",
     spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-width",
     animation_value_type="discrete",
 )}

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

@@ -38,7 +38,7 @@ ${helpers.predefined_type(
     "app_units::Au::from_px(3)",
     engines="gecko servo-2013 servo-2020",
     servo_2020_pref="layout.2020.unimplemented",
-    initial_specified_value="specified::BorderSideWidth::Medium",
+    initial_specified_value="specified::BorderSideWidth::medium()",
     animation_value_type="NonNegativeLength",
     spec="https://drafts.csswg.org/css-ui/#propdef-outline-width",
 )}

servo/components/style/properties/shorthands/border.mako.rs

@@ -93,7 +93,7 @@ pub fn parse_border<'i, 't>(
     if !any {
         return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError))
     }
-    Ok((color.unwrap_or(Color::CurrentColor), style.unwrap_or(BorderStyle::None), width.unwrap_or(BorderSideWidth::Medium)))
+    Ok((color.unwrap_or(Color::CurrentColor), style.unwrap_or(BorderStyle::None), width.unwrap_or(BorderSideWidth::medium())))
 }
 
 % for side, logical in ALL_SIDES:

servo/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 -webkit-text-stroke-width.
+pub type LineWidth = Au;
+
 /// A computed value for border-width (and the like).
 pub type BorderSideWidth = Au;
 

servo/components/style/values/computed/mod.rs

@@ -48,7 +48,7 @@ pub use self::background::{BackgroundRepeat, BackgroundSize};
 pub use self::basic_shape::FillRule;
 pub use self::border::{
     BorderCornerRadius, BorderImageRepeat, BorderImageSideWidth, BorderImageSlice,
-    BorderImageWidth, BorderRadius, BorderSideWidth, BorderSpacing,
+    BorderImageWidth, BorderRadius, BorderSideWidth, BorderSpacing, LineWidth,
 };
 pub use self::box_::{
     Appearance, BaselineSource, BreakBetween, BreakWithin, Clear, Contain, ContainIntrinsicSize,

servo/components/style/values/specified/border.rs

@@ -4,7 +4,6 @@
 
 //! Specified types for CSS values related to borders.
 
-use app_units::Au;
 use crate::parser::{Parse, ParserContext};
 use crate::values::computed::{Context, ToComputedValue};
 use crate::values::generics::border::BorderCornerRadius as GenericBorderCornerRadius;
@@ -14,10 +13,11 @@ use crate::values::generics::border::BorderRadius as GenericBorderRadius;
 use crate::values::generics::border::BorderSpacing as GenericBorderSpacing;
 use crate::values::generics::rect::Rect;
 use crate::values::generics::size::Size2D;
-use crate::values::specified::length::{NonNegativeLength, NonNegativeLengthPercentage};
+use crate::values::specified::length::{Length, NonNegativeLength, NonNegativeLengthPercentage};
 use crate::values::specified::{AllowQuirks, NonNegativeNumber, NonNegativeNumberOrPercentage};
 use crate::values::specified::Color;
 use crate::Zero;
+use app_units::Au;
 use cssparser::Parser;
 use std::fmt::{self, Write};
 use style_traits::{CssWriter, ParseError, ToCss, values::SequenceWriter};
@@ -67,19 +67,6 @@ impl BorderStyle {
     }
 }
 
-/// A specified value for a single side of the `border-width` property.
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
-pub enum BorderSideWidth {
-    /// `thin`
-    Thin,
-    /// `medium`
-    Medium,
-    /// `thick`
-    Thick,
-    /// `<length>`
-    Length(NonNegativeLength),
-}
-
 /// A specified value for the `border-image-width` property.
 pub type BorderImageWidth = Rect<BorderImageSideWidth>;
 
@@ -110,11 +97,87 @@ impl BorderImageSlice {
     }
 }
 
-impl BorderSideWidth {
+/// https://drafts.csswg.org/css-backgrounds-3/#typedef-line-width
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
+pub enum LineWidth {
+    /// `thin`
+    Thin,
+    /// `medium`
+    Medium,
+    /// `thick`
+    Thick,
+    /// `<length>`
+    Length(NonNegativeLength),
+}
+
+impl LineWidth {
     /// Returns the `0px` value.
     #[inline]
     pub fn zero() -> Self {
-        BorderSideWidth::Length(NonNegativeLength::zero())
+        Self::Length(NonNegativeLength::zero())
+    }
+
+    fn parse_quirky<'i, 't>(
+        context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+        allow_quirks: AllowQuirks,
+    ) -> Result<Self, ParseError<'i>> {
+        if let Ok(length) =
+            input.try_parse(|i| NonNegativeLength::parse_quirky(context, i, allow_quirks))
+        {
+            return Ok(Self::Length(length));
+        }
+        Ok(try_match_ident_ignore_ascii_case! { input,
+            "thin" => Self::Thin,
+            "medium" => Self::Medium,
+            "thick" => Self::Thick,
+        })
+    }
+}
+
+impl Parse for LineWidth {
+    fn parse<'i>(
+        context: &ParserContext,
+        input: &mut Parser<'i, '_>,
+    ) -> Result<Self, ParseError<'i>> {
+        Self::parse_quirky(context, input, AllowQuirks::No)
+    }
+}
+
+impl ToComputedValue for LineWidth {
+    type ComputedValue = app_units::Au;
+
+    #[inline]
+    fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
+        match *self {
+            // https://drafts.csswg.org/css-backgrounds-3/#line-width
+            Self::Thin => Au::from_px(1),
+            Self::Medium => Au::from_px(3),
+            Self::Thick => Au::from_px(5),
+            Self::Length(ref length) => Au::from_f32_px(length.to_computed_value(context).px()),
+        }
+    }
+
+    #[inline]
+    fn from_computed_value(computed: &Self::ComputedValue) -> Self {
+        Self::Length(NonNegativeLength::from_px(computed.to_f32_px()))
+    }
+}
+
+/// A specified value for a single side of the `border-width` property. The difference between this
+/// and LineWidth is whether we snap to device pixels or not.
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss, ToShmem)]
+pub struct BorderSideWidth(LineWidth);
+
+impl BorderSideWidth {
+    /// Returns the `medium` value.
+    pub fn medium() -> Self {
+        Self(LineWidth::Medium)
+    }
+
+    /// Returns a bare px value from the argument.
+    pub fn from_px(px: f32) -> Self {
+        Self(LineWidth::Length(Length::from_px(px).into()))
     }
 
     /// Parses, with quirks.
@@ -123,23 +186,14 @@ impl BorderSideWidth {
         input: &mut Parser<'i, 't>,
         allow_quirks: AllowQuirks,
     ) -> Result<Self, ParseError<'i>> {
-        if let Ok(length) =
-            input.try_parse(|i| NonNegativeLength::parse_quirky(context, i, allow_quirks))
-        {
-            return Ok(BorderSideWidth::Length(length));
-        }
-        try_match_ident_ignore_ascii_case! { input,
-            "thin" => Ok(BorderSideWidth::Thin),
-            "medium" => Ok(BorderSideWidth::Medium),
-            "thick" => Ok(BorderSideWidth::Thick),
-        }
+        Ok(Self(LineWidth::parse_quirky(context, input, allow_quirks)?))
     }
 }
 
 impl Parse for BorderSideWidth {
-    fn parse<'i, 't>(
+    fn parse<'i>(
         context: &ParserContext,
-        input: &mut Parser<'i, 't>,
+        input: &mut Parser<'i, '_>,
     ) -> Result<Self, ParseError<'i>> {
         Self::parse_quirky(context, input, AllowQuirks::No)
     }
@@ -150,14 +204,7 @@ impl ToComputedValue for BorderSideWidth {
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Self::ComputedValue {
-        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()),
-        };
-
+        let width = self.0.to_computed_value(context);
         // 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) {
@@ -165,12 +212,15 @@ impl ToComputedValue for BorderSideWidth {
         }
 
         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))
+        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(NonNegativeLength::from_px(computed.to_f32_px()))
+        Self(LineWidth::from_computed_value(computed))
     }
 }
 

servo/components/style/values/specified/mod.rs

@@ -34,18 +34,17 @@ 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, BorderImageSlice, BorderImageWidth};
-pub use self::border::{BorderImageRepeat, BorderImageSideWidth};
-pub use self::border::{BorderRadius, BorderSideWidth, BorderSpacing, BorderStyle};
-pub use self::box_::{Appearance, BreakBetween, BreakWithin, ContainerName, ContainerType};
-pub use self::box_::{BaselineSource, TouchAction, VerticalAlign, WillChange};
-pub use self::box_::{
-    Clear, ContainIntrinsicSize, ContentVisibility, Float, LineClamp, Overflow, OverflowAnchor,
+pub use self::border::{
+    BorderCornerRadius, BorderImageRepeat, BorderImageSideWidth, BorderImageSlice,
+    BorderImageWidth, BorderRadius, BorderSideWidth, BorderSpacing, BorderStyle, LineWidth,
+};
+pub use self::box_::{
+    Appearance, BreakBetween, BaselineSource, BreakWithin, Contain, ContainerName, ContainerType,
+    Clear, ContainIntrinsicSize, ContentVisibility, Display, Float, LineClamp, Overflow,
+    OverflowAnchor, OverflowClipBox, OverscrollBehavior, Perspective, Resize, ScrollbarGutter,
+    ScrollSnapAlign, ScrollSnapAxis, ScrollSnapStop, ScrollSnapStrictness, ScrollSnapType,
+    TouchAction, VerticalAlign, WillChange,
 };
-pub use self::box_::{Contain, Display};
-pub use self::box_::{OverflowClipBox, OverscrollBehavior, Perspective, Resize, ScrollbarGutter};
-pub use self::box_::{ScrollSnapAlign, ScrollSnapAxis, ScrollSnapStop};
-pub use self::box_::{ScrollSnapStrictness, ScrollSnapType};
 pub use self::color::{
     Color, ColorOrAuto, ColorPropertyValue, ColorScheme, ForcedColorAdjust, PrintColorAdjust,
 };

servo/ports/geckolib/glue.rs

@@ -5347,10 +5347,10 @@ pub extern "C" fn Servo_DeclarationBlock_SetPixelValue(
     let prop = match_wrap_declared! { long,
         Height => Size::LengthPercentage(NonNegative(lp)),
         Width => Size::LengthPercentage(NonNegative(lp)),
-        BorderTopWidth => BorderSideWidth::Length(nocalc.into()),
-        BorderRightWidth => BorderSideWidth::Length(nocalc.into()),
-        BorderBottomWidth => BorderSideWidth::Length(nocalc.into()),
-        BorderLeftWidth => BorderSideWidth::Length(nocalc.into()),
+        BorderTopWidth => BorderSideWidth::from_px(value),
+        BorderRightWidth => BorderSideWidth::from_px(value),
+        BorderBottomWidth => BorderSideWidth::from_px(value),
+        BorderLeftWidth => BorderSideWidth::from_px(value),
         MarginTop => lp_or_auto,
         MarginRight => lp_or_auto,
         MarginBottom => lp_or_auto,

testing/web-platform/tests/css/css-text/text-stroke-width-subpixel-notref.html

@@ -0,0 +1,2 @@
+<!doctype html>
+<div style="font: 20px monospace; -webkit-text-stroke-width: 0.5px; -webkit-text-stroke-color: lime; color: white">A</div>

testing/web-platform/tests/css/css-text/text-stroke-width-subpixel.html

@@ -0,0 +1,5 @@
+<!doctype html>
+<title>-webkit-text-stroke-width is not snapped to device pixels</title>
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1837692">
+<link rel="mismatch" href="text-stroke-width-subpixel-notref.html">
+<div style="font: 20px monospace; -webkit-text-stroke-width: 0.1px; -webkit-text-stroke-color: lime; color: white">A</div>