From 191c2a282b236b9acfaeb58185585d4a7d9a52b9 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Thu, 20 Jul 2017 13:11:37 +0800 Subject: [PATCH] Bug 1374233 - Part 2: Add NonNegativeAu. Add values::computed::NonNegativeAu, so BorderSideWith could be computed to this non-negative Au, for the following properties: 1. outline-width 2. border-{*}-width 3. column-rule-width 4. -webkit-text-stroke-width MozReview-Commit-ID: ASHaB2F7VtM --- components/layout/construct.rs | 8 ++--- components/layout/display_list_builder.rs | 2 +- components/layout/fragment.rs | 2 +- components/layout/query.rs | 16 ++++----- components/layout/table_row.rs | 8 ++--- components/style/properties/gecko.mako.rs | 33 ++++++++++--------- .../helpers/animated_properties.mako.rs | 2 +- .../style/properties/longhand/border.mako.rs | 6 ++-- .../style/properties/longhand/column.mako.rs | 6 ++-- .../longhand/inherited_text.mako.rs | 4 +-- .../style/properties/longhand/outline.mako.rs | 6 ++-- .../style/properties/properties.mako.rs | 27 +++++++-------- components/style/style_adjuster.rs | 2 +- components/style/values/animated/mod.rs | 16 +++++++++ components/style/values/computed/mod.rs | 30 +++++++++++++++++ components/style/values/specified/border.rs | 9 +++-- 16 files changed, 113 insertions(+), 64 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index c863219941e..c1643c9f92c 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -1860,10 +1860,10 @@ impl ComputedValueUtils for ComputedValues { !padding.padding_right.is_definitely_zero() || !padding.padding_bottom.is_definitely_zero() || !padding.padding_left.is_definitely_zero() || - border.border_top_width != Au(0) || - border.border_right_width != Au(0) || - border.border_bottom_width != Au(0) || - border.border_left_width != Au(0) + border.border_top_width.0 != Au(0) || + border.border_right_width.0 != Au(0) || + border.border_bottom_width.0 != Au(0) || + border.border_left_width.0 != Au(0) } } diff --git a/components/layout/display_list_builder.rs b/components/layout/display_list_builder.rs index f8b26899b57..4d10b9e8fa6 100644 --- a/components/layout/display_list_builder.rs +++ b/components/layout/display_list_builder.rs @@ -1572,7 +1572,7 @@ impl FragmentDisplayListBuilding for Fragment { clip: &Rect) { use style::values::Either; - let width = style.get_outline().outline_width; + let width = style.get_outline().outline_width.0; if width == Au(0) { return } diff --git a/components/layout/fragment.rs b/components/layout/fragment.rs index bd7d06be627..be596745318 100644 --- a/components/layout/fragment.rs +++ b/components/layout/fragment.rs @@ -2576,7 +2576,7 @@ impl Fragment { } // Outlines cause us to draw outside our border box. - let outline_width = self.style.get_outline().outline_width; + let outline_width = self.style.get_outline().outline_width.0; if outline_width != Au(0) { overflow.paint = overflow.paint.union(&border_box.inflate(outline_width, outline_width)) diff --git a/components/layout/query.rs b/components/layout/query.rs index 252071e9bcd..932abe29b5f 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -450,10 +450,10 @@ impl FragmentBorderBoxIterator for FragmentLocatingFragmentIterator { border_left_width: left_width, .. } = *fragment.style.get_border(); - self.client_rect.origin.y = top_width.to_px(); - self.client_rect.origin.x = left_width.to_px(); - self.client_rect.size.width = (border_box.size.width - left_width - right_width).to_px(); - self.client_rect.size.height = (border_box.size.height - top_width - bottom_width).to_px(); + self.client_rect.origin.y = top_width.0.to_px(); + self.client_rect.origin.x = left_width.0.to_px(); + self.client_rect.size.width = (border_box.size.width - left_width.0 - right_width.0).to_px(); + self.client_rect.size.height = (border_box.size.height - top_width.0 - bottom_width.0).to_px(); } fn should_process(&mut self, fragment: &Fragment) -> bool { @@ -476,10 +476,10 @@ impl FragmentBorderBoxIterator for UnioningFragmentScrollAreaIterator { border_left_width: left_border, .. } = *fragment.style.get_border(); - let right_padding = (border_box.size.width - right_border - left_border).to_px(); - let bottom_padding = (border_box.size.height - bottom_border - top_border).to_px(); - let top_padding = top_border.to_px(); - let left_padding = left_border.to_px(); + let right_padding = (border_box.size.width - right_border.0 - left_border.0).to_px(); + let bottom_padding = (border_box.size.height - bottom_border.0 - top_border.0).to_px(); + let top_padding = top_border.0.to_px(); + let left_padding = left_border.0.to_px(); match self.level { Some(start_level) if level <= start_level => { self.is_child = false; } diff --git a/components/layout/table_row.rs b/components/layout/table_row.rs index e960290bdd2..6deaa89e8b3 100644 --- a/components/layout/table_row.rs +++ b/components/layout/table_row.rs @@ -616,7 +616,7 @@ impl CollapsedBorder { -> CollapsedBorder { CollapsedBorder { style: css_style.get_border().border_top_style, - width: css_style.get_border().border_top_width, + width: css_style.get_border().border_top_width.0, color: css_style.get_border().border_top_color, provenance: provenance, } @@ -628,7 +628,7 @@ impl CollapsedBorder { -> CollapsedBorder { CollapsedBorder { style: css_style.get_border().border_right_style, - width: css_style.get_border().border_right_width, + width: css_style.get_border().border_right_width.0, color: css_style.get_border().border_right_color, provenance: provenance, } @@ -640,7 +640,7 @@ impl CollapsedBorder { -> CollapsedBorder { CollapsedBorder { style: css_style.get_border().border_bottom_style, - width: css_style.get_border().border_bottom_width, + width: css_style.get_border().border_bottom_width.0, color: css_style.get_border().border_bottom_color, provenance: provenance, } @@ -652,7 +652,7 @@ impl CollapsedBorder { -> CollapsedBorder { CollapsedBorder { style: css_style.get_border().border_left_style, - width: css_style.get_border().border_left_width, + width: css_style.get_border().border_left_width.0, color: css_style.get_border().border_left_color, provenance: provenance, } diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index e256394dea5..0085da6c518 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -803,15 +803,16 @@ def set_gecko_property(ffi_name, expr): } -<%def name="impl_app_units(ident, gecko_ffi_name, need_clone, inherit_from=None, round_to_pixels=False)"> +<%def name="impl_non_negative_app_units(ident, gecko_ffi_name, need_clone, inherit_from=None, + round_to_pixels=False)"> #[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.gecko.mTwipsPerPixel); - round_border_to_device_pixels(v, au_per_device_px).0 + round_border_to_device_pixels(v.0, au_per_device_px).0 % else: - v.0 + v.value() % endif }; @@ -847,7 +848,7 @@ def set_gecko_property(ffi_name, expr): %if need_clone: #[allow(non_snake_case)] pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { - Au(self.gecko.${gecko_ffi_name}) + Au(self.gecko.${gecko_ffi_name}).into() } % endif @@ -1312,11 +1313,11 @@ fn static_assert() { <% impl_color("border_%s_color" % side.ident, "(mBorderColor)[%s]" % side.index, need_clone=True) %> - <% impl_app_units("border_%s_width" % side.ident, - "mComputedBorder.%s" % side.ident, - inherit_from="mBorder.%s" % side.ident, - need_clone=True, - round_to_pixels=True) %> + <% impl_non_negative_app_units("border_%s_width" % side.ident, + "mComputedBorder.%s" % side.ident, + inherit_from="mBorder.%s" % side.ident, + need_clone=True, + round_to_pixels=True) %> pub fn border_${side.ident}_has_nonzero_width(&self) -> bool { self.gecko.mComputedBorder.${side.ident} != 0 @@ -2054,9 +2055,9 @@ fn static_assert() { } } - <% impl_app_units("outline_width", "mActualOutlineWidth", - inherit_from="mOutlineWidth", - need_clone=True, round_to_pixels=True) %> + <% impl_non_negative_app_units("outline_width", "mActualOutlineWidth", + inherit_from="mOutlineWidth", + need_clone=True, round_to_pixels=True) %> % for corner in CORNERS: <% impl_corner_style_coord("_moz_outline_radius_%s" % corner.ident.replace("_", ""), @@ -4787,7 +4788,9 @@ fn static_assert() { }) } - <%call expr="impl_app_units('_webkit_text_stroke_width', 'mWebkitTextStrokeWidth', need_clone=True)"> + <%call expr="impl_non_negative_app_units('_webkit_text_stroke_width', + 'mWebkitTextStrokeWidth', + need_clone=True)"> #[allow(non_snake_case)] pub fn set__moz_tab_size(&mut self, v: longhands::_moz_tab_size::computed_value::T) { @@ -5438,8 +5441,8 @@ clip-path } } - <% impl_app_units("column_rule_width", "mColumnRuleWidth", need_clone=True, - round_to_pixels=True) %> + <% impl_non_negative_app_units("column_rule_width", "mColumnRuleWidth", need_clone=True, + round_to_pixels=True) %> <%self:impl_trait style_struct_name="Counters" diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 1957c7d3268..64d94e871a7 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -43,7 +43,7 @@ use values::computed::{Angle, LengthOrPercentageOrAuto, LengthOrPercentageOrNone use values::computed::{BorderCornerRadius, ClipRect}; use values::computed::{CalcLengthOrPercentage, Color, Context, ComputedValueAsSpecified}; use values::computed::{LengthOrPercentage, MaxLength, MozLength, Percentage, ToComputedValue}; -use values::computed::NonNegativeNumber; +use values::computed::{NonNegativeAu, NonNegativeNumber}; use values::generics::{GreaterThanOrEqualToOne, NonNegative}; use values::generics::border::BorderCornerRadius as GenericBorderCornerRadius; use values::generics::effects::Filter; diff --git a/components/style/properties/longhand/border.mako.rs b/components/style/properties/longhand/border.mako.rs index c4f6aa888b1..d9a24263d55 100644 --- a/components/style/properties/longhand/border.mako.rs +++ b/components/style/properties/longhand/border.mako.rs @@ -40,11 +40,11 @@ ${helpers.predefined_type("border-%s-width" % side_name, "BorderSideWidth", - "Au::from_px(3)", - computed_type="::app_units::Au", + "::values::computed::NonNegativeAu::from_px(3)", + computed_type="::values::computed::NonNegativeAu", alias=maybe_moz_logical_alias(product, side, "-moz-border-%s-width"), spec=maybe_logical_spec(side, "width"), - animation_value_type="ComputedValue", + animation_value_type="NonNegativeAu", logical=is_logical, flags="APPLIES_TO_FIRST_LETTER", allow_quirks=not is_logical)} diff --git a/components/style/properties/longhand/column.mako.rs b/components/style/properties/longhand/column.mako.rs index 440e5d94d81..38668734fce 100644 --- a/components/style/properties/longhand/column.mako.rs +++ b/components/style/properties/longhand/column.mako.rs @@ -42,12 +42,12 @@ ${helpers.single_keyword("column-fill", "balance auto", extra_prefixes="moz", ${helpers.predefined_type("column-rule-width", "BorderSideWidth", - "Au::from_px(3)", + "::values::computed::NonNegativeAu::from_px(3)", initial_specified_value="specified::BorderSideWidth::Medium", - computed_type="::app_units::Au", + computed_type="::values::computed::NonNegativeAu", products="gecko", spec="https://drafts.csswg.org/css-multicol/#propdef-column-rule-width", - animation_value_type="ComputedValue", + animation_value_type="NonNegativeAu", extra_prefixes="moz")} // https://drafts.csswg.org/css-multicol-1/#crc diff --git a/components/style/properties/longhand/inherited_text.mako.rs b/components/style/properties/longhand/inherited_text.mako.rs index 50df62db72a..9fe33974020 100644 --- a/components/style/properties/longhand/inherited_text.mako.rs +++ b/components/style/properties/longhand/inherited_text.mako.rs @@ -742,9 +742,9 @@ ${helpers.predefined_type( ${helpers.predefined_type("-webkit-text-stroke-width", "BorderSideWidth", - "Au::from_px(0)", + "::values::computed::NonNegativeAu::from_px(0)", initial_specified_value="specified::BorderSideWidth::Length(specified::Length::zero())", - computed_type="::app_units::Au", + computed_type="::values::computed::NonNegativeAu", products="gecko", flags="APPLIES_TO_FIRST_LETTER APPLIES_TO_FIRST_LINE APPLIES_TO_PLACEHOLDER", spec="https://compat.spec.whatwg.org/#the-webkit-text-stroke-width", diff --git a/components/style/properties/longhand/outline.mako.rs b/components/style/properties/longhand/outline.mako.rs index d7b62dd3cfc..0b5957d8814 100644 --- a/components/style/properties/longhand/outline.mako.rs +++ b/components/style/properties/longhand/outline.mako.rs @@ -64,10 +64,10 @@ ${helpers.predefined_type("outline-color", "Color", "computed_value::T::currentc ${helpers.predefined_type("outline-width", "BorderSideWidth", - "Au::from_px(3)", + "::values::computed::NonNegativeAu::from_px(3)", initial_specified_value="specified::BorderSideWidth::Medium", - computed_type="::app_units::Au", - animation_value_type="ComputedValue", + computed_type="::values::computed::NonNegativeAu", + animation_value_type="NonNegativeAu", spec="https://drafts.csswg.org/css-ui/#propdef-outline-width")} // The -moz-outline-radius-* properties are non-standard and not on a standards track. diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 93e653b0813..e46a659ee97 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -10,13 +10,13 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> +#[cfg(feature = "servo")] use app_units::Au; use servo_arc::{Arc, UniqueArc}; use std::borrow::Cow; use std::collections::HashSet; use std::{fmt, mem, ops}; #[cfg(feature = "gecko")] use std::ptr; -use app_units::Au; #[cfg(feature = "servo")] use cssparser::RGBA; use cssparser::{Parser, TokenSerializationType, serialize_identifier}; use cssparser::ParserInput; @@ -43,6 +43,7 @@ use stylesheets::{CssRuleType, MallocSizeOf, MallocSizeOfFn, Origin, UrlExtraDat #[cfg(feature = "servo")] use values::Either; use values::generics::text::LineHeight; use values::computed; +use values::computed::NonNegativeAu; use cascade_info::CascadeInfo; use rule_tree::{CascadeLevel, StrongRuleNode}; use self::computed_value_flags::ComputedValueFlags; @@ -1752,7 +1753,7 @@ pub mod style_structs { /// Whether the border-${side} property has nonzero width. #[allow(non_snake_case)] pub fn border_${side}_has_nonzero_width(&self) -> bool { - self.border_${side}_width != ::app_units::Au(0) + self.border_${side}_width != NonNegativeAu::zero() } % endfor % elif style_struct.name == "Font": @@ -1790,7 +1791,7 @@ pub mod style_structs { /// Whether the outline-width property is non-zero. #[inline] pub fn outline_has_nonzero_width(&self) -> bool { - self.outline_width != ::app_units::Au(0) + self.outline_width != NonNegativeAu::zero() } % elif style_struct.name == "Text": /// Whether the text decoration has an underline. @@ -2202,10 +2203,10 @@ impl ComputedValuesInner { pub fn border_width_for_writing_mode(&self, writing_mode: WritingMode) -> LogicalMargin { let border_style = self.get_border(); LogicalMargin::from_physical(writing_mode, SideOffsets2D::new( - border_style.border_top_width, - border_style.border_right_width, - border_style.border_bottom_width, - border_style.border_left_width, + border_style.border_top_width.0, + border_style.border_right_width.0, + border_style.border_bottom_width.0, + border_style.border_left_width.0, )) } @@ -3360,7 +3361,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(Au(0)); + style.set_border_${side}_width(NonNegativeAu::zero()); } % endfor } @@ -3384,7 +3385,7 @@ pub fn modify_border_style_for_inline_sides(style: &mut Arc, PhysicalSide::Top => (border.border_top_width, border.border_top_style), PhysicalSide::Bottom => (border.border_bottom_width, border.border_bottom_style), }; - if current_style == (Au(0), BorderStyle::none) { + if current_style == (NonNegativeAu::zero(), BorderStyle::none) { return; } } @@ -3392,19 +3393,19 @@ pub fn modify_border_style_for_inline_sides(style: &mut Arc, let border = Arc::make_mut(&mut style.border); match side { PhysicalSide::Left => { - border.border_left_width = Au(0); + border.border_left_width = NonNegativeAu::zero(); border.border_left_style = BorderStyle::none; } PhysicalSide::Right => { - border.border_right_width = Au(0); + border.border_right_width = NonNegativeAu::zero(); border.border_right_style = BorderStyle::none; } PhysicalSide::Bottom => { - border.border_bottom_width = Au(0); + border.border_bottom_width = NonNegativeAu::zero(); border.border_bottom_style = BorderStyle::none; } PhysicalSide::Top => { - border.border_top_width = Au(0); + border.border_top_width = NonNegativeAu::zero(); border.border_top_style = BorderStyle::none; } } diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 1c93e6bae58..70d9880fb29 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -242,7 +242,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { fn adjust_for_outline(&mut self) { if self.style.get_outline().clone_outline_style().none_or_hidden() && self.style.get_outline().outline_has_nonzero_width() { - self.style.mutate_outline().set_outline_width(Au(0)); + self.style.mutate_outline().set_outline_width(Au(0).into()); } } diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index d6a4acea1b0..cbc2f63b311 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -9,8 +9,10 @@ //! module's raison d'être is to ultimately contain all these types. use app_units::Au; +use std::cmp::max; use values::computed::Angle as ComputedAngle; use values::computed::GreaterThanOrEqualToOneNumber as ComputedGreaterThanOrEqualToOneNumber; +use values::computed::NonNegativeAu; use values::computed::NonNegativeNumber as ComputedNonNegativeNumber; use values::specified::url::SpecifiedUrl; @@ -118,6 +120,20 @@ impl ToAnimatedValue for ComputedGreaterThanOrEqualToOneNumber { } } +impl ToAnimatedValue for NonNegativeAu { + type AnimatedValue = Self; + + #[inline] + fn to_animated_value(self) -> Self { + self + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + max(animated.0, Au(0)).into() + } +} + /// Returns a value similar to `self` that represents zero. pub trait ToAnimatedZero: Sized { /// Returns a value that, when added with an underlying value, will produce the underlying diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index f492279a559..271135b677c 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -591,3 +591,33 @@ impl ClipRectOrAuto { /// | auto pub type ColorOrAuto = Either; + +/// A wrapper of Au, but the value >= 0. +pub type NonNegativeAu = NonNegative; + +impl NonNegativeAu { + /// Return a zero value. + #[inline] + pub fn zero() -> Self { + NonNegative::(Au(0)) + } + + /// Return a NonNegativeAu from pixel. + #[inline] + pub fn from_px(px: i32) -> Self { + NonNegative::(Au::from_px(::std::cmp::max(px, 0))) + } + + /// Get the inner value of |NonNegativeAu.0|. + #[inline] + pub fn value(self) -> i32 { + (self.0).0 + } +} + +impl From for NonNegativeAu { + #[inline] + fn from(au: Au) -> NonNegativeAu { + NonNegative::(au) + } +} diff --git a/components/style/values/specified/border.rs b/components/style/values/specified/border.rs index 6a6da001db3..419074babe6 100644 --- a/components/style/values/specified/border.rs +++ b/components/style/values/specified/border.rs @@ -4,11 +4,10 @@ //! Specified types for CSS values related to borders. -use app_units::Au; use cssparser::Parser; use parser::{Parse, ParserContext}; use style_traits::ParseError; -use values::computed::{Context, ToComputedValue}; +use values::computed::{Context, NonNegativeAu, ToComputedValue}; use values::generics::border::BorderCornerRadius as GenericBorderCornerRadius; use values::generics::border::BorderImageSideWidth as GenericBorderImageSideWidth; use values::generics::border::BorderImageSlice as GenericBorderImageSlice; @@ -72,7 +71,7 @@ impl Parse for BorderSideWidth { } impl ToComputedValue for BorderSideWidth { - type ComputedValue = Au; + type ComputedValue = NonNegativeAu; #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { @@ -84,12 +83,12 @@ impl ToComputedValue for BorderSideWidth { BorderSideWidth::Medium => Length::from_px(3.).to_computed_value(context), BorderSideWidth::Thick => Length::from_px(5.).to_computed_value(context), BorderSideWidth::Length(ref length) => length.to_computed_value(context) - } + }.into() } #[inline] fn from_computed_value(computed: &Self::ComputedValue) -> Self { - BorderSideWidth::Length(ToComputedValue::from_computed_value(computed)) + BorderSideWidth::Length(ToComputedValue::from_computed_value(&computed.0)) } }