From e12833a4768da4a37b9da5e6545f84a02154f3b2 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] 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 --- .../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 +- .../style/values/computed/border.rs | 3 + servo/components/style/values/computed/mod.rs | 2 +- .../style/values/specified/border.rs | 128 ++++++++++++------ .../components/style/values/specified/mod.rs | 21 ++- servo/ports/geckolib/glue.rs | 8 +- .../text-stroke-width-subpixel-notref.html | 2 + .../css-text/text-stroke-width-subpixel.html | 5 + 11 files changed, 119 insertions(+), 60 deletions(-) create mode 100644 testing/web-platform/tests/css/css-text/text-stroke-width-subpixel-notref.html create mode 100644 testing/web-platform/tests/css/css-text/text-stroke-width-subpixel.html diff --git a/servo/components/style/properties/longhands/column.mako.rs b/servo/components/style/properties/longhands/column.mako.rs index f46406c08999..1e0e92b7f3d6 100644 --- a/servo/components/style/properties/longhands/column.mako.rs +++ b/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", )} diff --git a/servo/components/style/properties/longhands/inherited_text.mako.rs b/servo/components/style/properties/longhands/inherited_text.mako.rs index cd79d37ef600..c1b110c08656 100644 --- a/servo/components/style/properties/longhands/inherited_text.mako.rs +++ b/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", )} diff --git a/servo/components/style/properties/longhands/outline.mako.rs b/servo/components/style/properties/longhands/outline.mako.rs index 6c856f8653e8..e5ae1810b438 100644 --- a/servo/components/style/properties/longhands/outline.mako.rs +++ b/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", )} diff --git a/servo/components/style/properties/shorthands/border.mako.rs b/servo/components/style/properties/shorthands/border.mako.rs index 9b9501096eb2..c6a87f3197b6 100644 --- a/servo/components/style/properties/shorthands/border.mako.rs +++ b/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: diff --git a/servo/components/style/values/computed/border.rs b/servo/components/style/values/computed/border.rs index d5cb4dc919ba..e073f671b395 100644 --- a/servo/components/style/values/computed/border.rs +++ b/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; diff --git a/servo/components/style/values/computed/mod.rs b/servo/components/style/values/computed/mod.rs index b9f4d38cdef9..207e796a1027 100644 --- a/servo/components/style/values/computed/mod.rs +++ b/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, diff --git a/servo/components/style/values/specified/border.rs b/servo/components/style/values/specified/border.rs index 923da0a5738f..f27d3aef61b0 100644 --- a/servo/components/style/values/specified/border.rs +++ b/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(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/servo/components/style/values/specified/mod.rs b/servo/components/style/values/specified/mod.rs index ba9cb81b4444..da7fcea9b462 100644 --- a/servo/components/style/values/specified/mod.rs +++ b/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, }; diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 4fa463c6739a..37889482fd9c 100644 --- a/servo/ports/geckolib/glue.rs +++ b/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, diff --git a/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel-notref.html b/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel-notref.html new file mode 100644 index 000000000000..ba5e8d90f3e9 --- /dev/null +++ b/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel-notref.html @@ -0,0 +1,2 @@ + +
A
diff --git a/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel.html b/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel.html new file mode 100644 index 000000000000..97cbc3f44de9 --- /dev/null +++ b/testing/web-platform/tests/css/css-text/text-stroke-width-subpixel.html @@ -0,0 +1,5 @@ + +-webkit-text-stroke-width is not snapped to device pixels + + +
A