From 77c541b38a8eb4b1e42dddebbb5507bdd9703219 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 12 Jun 2023 14:57:53 +0000 Subject: [PATCH] style: Do not snap -webkit-text-stroke-width to dev pixels Introduce LineWidth (which doesn't snap) and let BorderSideWidth wrap it and actually do the snapping. Differential Revision: https://phabricator.services.mozilla.com/D180688 --- .../style/properties/longhands/column.mako.rs | 2 +- .../longhands/inherited_text.mako.rs | 4 +- .../properties/longhands/outline.mako.rs | 2 +- .../properties/shorthands/border.mako.rs | 2 +- components/style/values/computed/border.rs | 3 + components/style/values/computed/mod.rs | 2 +- components/style/values/specified/border.rs | 128 ++++++++++++------ components/style/values/specified/mod.rs | 21 ++- 8 files changed, 108 insertions(+), 56 deletions(-) diff --git a/components/style/properties/longhands/column.mako.rs b/components/style/properties/longhands/column.mako.rs index 75f17cf3ab3..6ad4c400c22 100644 --- a/components/style/properties/longhands/column.mako.rs +++ b/components/style/properties/longhands/column.mako.rs @@ -44,7 +44,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", )} diff --git a/components/style/properties/longhands/inherited_text.mako.rs b/components/style/properties/longhands/inherited_text.mako.rs index 22483cc0db2..8bb3c6355a9 100644 --- a/components/style/properties/longhands/inherited_text.mako.rs +++ b/components/style/properties/longhands/inherited_text.mako.rs @@ -283,10 +283,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", )} diff --git a/components/style/properties/longhands/outline.mako.rs b/components/style/properties/longhands/outline.mako.rs index 6d9af88dd7a..714cbc81ecc 100644 --- a/components/style/properties/longhands/outline.mako.rs +++ b/components/style/properties/longhands/outline.mako.rs @@ -36,7 +36,7 @@ ${helpers.predefined_type( "BorderSideWidth", "app_units::Au::from_px(3)", engines="gecko servo", - 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", )} diff --git a/components/style/properties/shorthands/border.mako.rs b/components/style/properties/shorthands/border.mako.rs index d9056a9252d..2eeb691e242 100644 --- a/components/style/properties/shorthands/border.mako.rs +++ b/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: diff --git a/components/style/values/computed/border.rs b/components/style/values/computed/border.rs index d5cb4dc919b..e073f671b39 100644 --- a/components/style/values/computed/border.rs +++ b/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; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index b9f4d38cdef..207e796a102 100644 --- a/components/style/values/computed/mod.rs +++ b/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, diff --git a/components/style/values/specified/border.rs b/components/style/values/specified/border.rs index 923da0a5738..f27d3aef61b 100644 --- a/components/style/values/specified/border.rs +++ b/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(NonNegativeLength), -} - /// A specified value for the `border-image-width` property. pub type BorderImageWidth = Rect; @@ -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(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> { + 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::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> { - 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::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)) } } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index ba9cb81b444..da7fcea9b46 100644 --- a/components/style/values/specified/mod.rs +++ b/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, };