From 0dfbd38532bd1895d3818de5268d8d2c6d989a5b Mon Sep 17 00:00:00 2001 From: Sean Voisen Date: Tue, 17 Dec 2019 11:27:41 +0000 Subject: [PATCH 01/66] style: Add support for logical versions of overscroll-behavior. Differential Revision: https://phabricator.services.mozilla.com/D57363 --- components/style/properties/longhands/box.mako.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs index fba2fac66dc..862a373bdfe 100644 --- a/components/style/properties/longhands/box.mako.rs +++ b/components/style/properties/longhands/box.mako.rs @@ -472,13 +472,15 @@ ${helpers.predefined_type( animation_value_type="discrete", )} -% for axis in ["x", "y"]: +% for (axis, logical) in ALL_AXES: ${helpers.predefined_type( "overscroll-behavior-" + axis, "OverscrollBehavior", "computed::OverscrollBehavior::Auto", engines="gecko", needs_context=False, + logical_group="overscroll-behavior", + logical=logical, gecko_pref="layout.css.overscroll-behavior.enabled", spec="https://wicg.github.io/overscroll-behavior/#overscroll-behavior-properties", animation_value_type="discrete", From 2b499e48509ad102b0cbd6fc4997598e1b128445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 19 Dec 2019 01:19:04 +0000 Subject: [PATCH 02/66] style: Do not use synthetic display-inside values. This matches the new servo layout engine too, and thus removes some #[cfg] gunk. Just use `flow` since it doesn't simplify the layout code as much. Differential Revision: https://phabricator.services.mozilla.com/D45973 --- components/style/style_adjuster.rs | 4 +- components/style/values/specified/box.rs | 52 +++++++----------------- 2 files changed, 16 insertions(+), 40 deletions(-) diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 6a3dc75f4d8..36463aad6fb 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -12,8 +12,6 @@ use crate::properties::longhands::float::computed_value::T as Float; use crate::properties::longhands::overflow_x::computed_value::T as Overflow; use crate::properties::longhands::position::computed_value::T as Position; use crate::properties::{self, ComputedValues, StyleBuilder}; -#[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] -use crate::values::specified::box_::DisplayInside; use app_units::Au; /// A struct that implements all the adjustment methods. @@ -207,7 +205,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.style.pseudo.map_or(false, |p| p.is_marker()) && self.style.get_parent_list().clone_list_style_position() == ListStylePosition::Outside && - layout_parent_style.get_box().clone_display().inside() != DisplayInside::Inline + !layout_parent_style.get_box().clone_display().is_inline_flow() ); if !blockify { diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index ad8602ac431..1549f66e8e9 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -61,12 +61,9 @@ pub enum DisplayInside { None = 0, #[cfg(any(feature = "servo-layout-2020", feature = "gecko"))] Contents, - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - Block, + Flow, FlowRoot, #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - Inline, - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] Flex, #[cfg(feature = "gecko")] Grid, @@ -114,7 +111,6 @@ pub enum DisplayInside { MozGroupbox, #[cfg(feature = "gecko")] MozPopup, - Flow, // only used for parsing, not computed value } #[allow(missing_docs)] @@ -147,14 +143,8 @@ impl Display { pub const None: Self = Self::new(DisplayOutside::None, DisplayInside::None); #[cfg(any(feature = "servo-layout-2020", feature = "gecko"))] pub const Contents: Self = Self::new(DisplayOutside::None, DisplayInside::Contents); - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - pub const Inline: Self = Self::new(DisplayOutside::Inline, DisplayInside::Inline); - #[cfg(any(feature = "servo-layout-2020"))] pub const Inline: Self = Self::new(DisplayOutside::Inline, DisplayInside::Flow); pub const InlineBlock: Self = Self::new(DisplayOutside::Inline, DisplayInside::FlowRoot); - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - pub const Block: Self = Self::new(DisplayOutside::Block, DisplayInside::Block); - #[cfg(any(feature = "servo-layout-2020"))] pub const Block: Self = Self::new(DisplayOutside::Block, DisplayInside::Flow); #[cfg(feature = "gecko")] pub const FlowRoot: Self = Self::new(DisplayOutside::Block, DisplayInside::FlowRoot); @@ -171,7 +161,7 @@ impl Display { #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] pub const InlineTable: Self = Self::new(DisplayOutside::Inline, DisplayInside::Table); #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - pub const TableCaption: Self = Self::new(DisplayOutside::TableCaption, DisplayInside::Block); + pub const TableCaption: Self = Self::new(DisplayOutside::TableCaption, DisplayInside::Flow); #[cfg(feature = "gecko")] pub const Ruby: Self = Self::new(DisplayOutside::Inline, DisplayInside::Ruby); #[cfg(feature = "gecko")] @@ -256,18 +246,8 @@ impl Display { } /// Make a display enum value from and values. - /// We store `flow` as a synthetic `block` or `inline` inside-value to simplify - /// our layout code. #[inline] fn from3(outside: DisplayOutside, inside: DisplayInside, list_item: bool) -> Self { - let inside = match inside { - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - DisplayInside::Flow => match outside { - DisplayOutside::Inline => DisplayInside::Inline, - _ => DisplayInside::Block, - }, - _ => inside, - }; let v = Self::new(outside, inside); if !list_item { return v; @@ -290,6 +270,13 @@ impl Display { .unwrap() } + /// Whether this is `display: inline` (or `inline list-item`). + #[inline] + pub fn is_inline_flow(&self) -> bool { + self.outside() == DisplayOutside::Inline && + self.inside() == DisplayInside::Flow + } + /// Returns whether this `display` value is some kind of list-item. #[inline] pub const fn is_list_item(&self) -> bool { @@ -381,9 +368,8 @@ impl Display { match self.outside() { DisplayOutside::Inline => { let inside = match self.inside() { - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - DisplayInside::Inline | DisplayInside::FlowRoot => DisplayInside::Block, - #[cfg(feature = "servo-layout-2020")] + // `inline-block` blockifies to `block` rather than + // `flow-root`, for legacy reasons. DisplayInside::FlowRoot => DisplayInside::Flow, inside => inside, }; @@ -407,7 +393,9 @@ impl Display { match self.outside() { DisplayOutside::Block => { let inside = match self.inside() { - DisplayInside::Block => DisplayInside::FlowRoot, + // `display: block` inlinifies to `display: inline-block`, + // rather than `inline`, for legacy reasons. + DisplayInside::Flow => DisplayInside::FlowRoot, inside => inside, }; Display::from3(DisplayOutside::Inline, inside, self.is_list_item()) @@ -443,18 +431,8 @@ impl ToCss for Display { where W: fmt::Write, { - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - debug_assert_ne!( - self.inside(), - DisplayInside::Flow, - "`flow` fears in `display` computed value" - ); let outside = self.outside(); - let inside = match self.inside() { - #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] - DisplayInside::Block | DisplayInside::Inline => DisplayInside::Flow, - inside => inside, - }; + let inside = self.inside(); match *self { Display::Block | Display::Inline => outside.to_css(dest), Display::InlineBlock => dest.write_str("inline-block"), From d3e65000be4f6e19b5b85ab471b9e2a620dbb559 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 19 Dec 2019 20:43:06 +0000 Subject: [PATCH 03/66] style: Workaround LLVM ABI bug. All the gory details in https://bugzilla.mozilla.org/show_bug.cgi?id=1600735 and related LLVM / GCC bugs. Avoid the issue by forcing the relevant enum to be 32-bit wide, so as to not trigger the LLVM bug. Differential Revision: https://phabricator.services.mozilla.com/D57868 --- components/style/properties/properties.mako.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 30a03f79290..e4a07da1c77 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1803,8 +1803,11 @@ impl ToCss for PropertyId { } /// The counted unknown property list which is used for css use counters. +/// +/// FIXME: This should be just #[repr(u8)], but can't be because of ABI issues, +/// see https://bugs.llvm.org/show_bug.cgi?id=44228. #[derive(Clone, Copy, Debug, Eq, FromPrimitive, Hash, PartialEq)] -#[repr(u8)] +#[repr(u32)] pub enum CountedUnknownProperty { % for prop in data.counted_unknown_properties: /// ${prop.name} From 69bf0e40a6b1f4420e32464f04773f477c748b57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 26 Dec 2019 22:17:35 +0000 Subject: [PATCH 04/66] style: Remove unused FFI function to refcount nsIReferrerInfo. We don't use RefPtr in rust. Differential Revision: https://phabricator.services.mozilla.com/D58274 --- components/style/gecko_bindings/sugar/refptr.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/components/style/gecko_bindings/sugar/refptr.rs b/components/style/gecko_bindings/sugar/refptr.rs index e27b4a28c40..a141aef4e8b 100644 --- a/components/style/gecko_bindings/sugar/refptr.rs +++ b/components/style/gecko_bindings/sugar/refptr.rs @@ -303,11 +303,6 @@ impl_threadsafe_refcount!( bindings::Gecko_AddRefURLExtraDataArbitraryThread, bindings::Gecko_ReleaseURLExtraDataArbitraryThread ); -impl_threadsafe_refcount!( - structs::nsIReferrerInfo, - bindings::Gecko_AddRefnsIReferrerInfoArbitraryThread, - bindings::Gecko_ReleasensIReferrerInfoArbitraryThread -); impl_threadsafe_refcount!( structs::nsIURI, bindings::Gecko_AddRefnsIURIArbitraryThread, From 4f4d4803263f48030cfda400410e1b2b85cb8d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 31 Dec 2019 18:14:35 +0000 Subject: [PATCH 05/66] style: Do not incorrectly share style across elements with different part names. Do the same we do for classes for now. We could be more precise and achieve a bit more sharing with some more effort (left a comment there), but it seems unlikely to matter in practice (and if we did that, we'd probably want to do the same for classes). Differential Revision: https://phabricator.services.mozilla.com/D58453 --- components/style/sharing/checks.rs | 15 ++++++++- components/style/sharing/mod.rs | 53 ++++++++++++++++++++++++++---- 2 files changed, 60 insertions(+), 8 deletions(-) diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index 19c1c52acdc..9c860f0258f 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -81,7 +81,7 @@ where target.pres_hints() == candidate.pres_hints() } -/// Whether a given element has the same class attribute than a given candidate. +/// Whether a given element has the same class attribute as a given candidate. /// /// We don't try to share style across elements with different class attributes. pub fn have_same_class( @@ -94,6 +94,19 @@ where target.class_list() == candidate.class_list() } +/// Whether a given element has the same class attribute as a given candidate. +/// +/// We don't try to share style across elements with different part attributes. +pub fn have_same_parts( + target: &mut StyleSharingTarget, + candidate: &mut StyleSharingCandidate, +) -> bool +where + E: TElement, +{ + target.part_list() == candidate.part_list() +} + /// Whether a given element and a candidate match the same set of "revalidation" /// selectors. /// diff --git a/components/style/sharing/mod.rs b/components/style/sharing/mod.rs index 9b8c1275e4b..2d36aeea7ff 100644 --- a/components/style/sharing/mod.rs +++ b/components/style/sharing/mod.rs @@ -124,10 +124,16 @@ impl OpaqueComputedValues { pub struct ValidationData { /// The class list of this element. /// - /// TODO(emilio): See if it's worth to sort them, or doing something else in - /// a similar fashion as what Boris is doing for the ID attribute. + /// TODO(emilio): Maybe check whether rules for these classes apply to the + /// element? class_list: Option>, + /// The part list of this element. + /// + /// TODO(emilio): Maybe check whether rules with these part names apply to + /// the element? + part_list: Option>, + /// The list of presentational attributes of the element. pres_hints: Option>, @@ -161,22 +167,41 @@ impl ValidationData { }) } + /// Get or compute the part-list associated with this element. + pub fn part_list(&mut self, element: E) -> &[Atom] + where + E: TElement, + { + if !element.has_part_attr() { + return &[] + } + self.part_list.get_or_insert_with(|| { + let mut list = SmallVec::<[Atom; 5]>::new(); + element.each_part(|p| list.push(p.clone())); + // See below for the reasoning. + if !list.spilled() { + list.sort_unstable_by_key(|a| a.get_hash()); + } + list + }) + } + /// Get or compute the class-list associated with this element. pub fn class_list(&mut self, element: E) -> &[Atom] where E: TElement, { self.class_list.get_or_insert_with(|| { - let mut class_list = SmallVec::<[Atom; 5]>::new(); - element.each_class(|c| class_list.push(c.clone())); + let mut list = SmallVec::<[Atom; 5]>::new(); + element.each_class(|c| list.push(c.clone())); // Assuming there are a reasonable number of classes (we use the // inline capacity as "reasonable number"), sort them to so that // we don't mistakenly reject sharing candidates when one element // has "foo bar" and the other has "bar foo". - if !class_list.spilled() { - class_list.sort_by(|a, b| a.get_hash().cmp(&b.get_hash())); + if !list.spilled() { + list.sort_unstable_by_key(|a| a.get_hash()); } - class_list + list }) } @@ -273,6 +298,11 @@ impl StyleSharingCandidate { self.validation_data.class_list(self.element) } + /// Get the part list of this candidate. + fn part_list(&mut self) -> &[Atom] { + self.validation_data.part_list(self.element) + } + /// Get the pres hints of this candidate. fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] { self.validation_data.pres_hints(self.element) @@ -335,6 +365,10 @@ impl StyleSharingTarget { self.validation_data.class_list(self.element) } + fn part_list(&mut self) -> &[Atom] { + self.validation_data.part_list(self.element) + } + /// Get the pres hints of this candidate. fn pres_hints(&mut self) -> &[ApplicableDeclarationBlock] { self.validation_data.pres_hints(self.element) @@ -772,6 +806,11 @@ impl StyleSharingCache { return None; } + if !checks::have_same_parts(target, candidate) { + trace!("Miss: Shadow parts"); + return None; + } + if !checks::revalidate( target, candidate, From 07d0eea5fb2ef09b01981ff4f437eab073a713ae Mon Sep 17 00:00:00 2001 From: Ting-Yu Lin Date: Tue, 31 Dec 2019 16:15:12 +0000 Subject: [PATCH 06/66] style:- Remove column-span pref in column.mako.rs. Differential Revision: https://phabricator.services.mozilla.com/D58399 --- components/style/properties/longhands/column.mako.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/components/style/properties/longhands/column.mako.rs b/components/style/properties/longhands/column.mako.rs index 8b49679abf7..b987990844d 100644 --- a/components/style/properties/longhands/column.mako.rs +++ b/components/style/properties/longhands/column.mako.rs @@ -69,17 +69,14 @@ ${helpers.predefined_type( spec="https://drafts.csswg.org/css-multicol/#propdef-column-rule-color", )} -// FIXME: Remove enabled_in="ua" once column-span is enabled on nightly (bug 1423383). ${helpers.single_keyword( "column-span", "none all", engines="gecko", animation_value_type="discrete", gecko_enum_prefix="StyleColumnSpan", - gecko_pref="layout.css.column-span.enabled", - enabled_in="ua", spec="https://drafts.csswg.org/css-multicol/#propdef-column-span", - extra_prefixes="moz:layout.css.column-span.enabled", + extra_prefixes="moz", )} ${helpers.predefined_type( From 219c0f6328c1bf043a02cef3cce4d5035554c2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 5 Jan 2020 13:10:39 +0000 Subject: [PATCH 07/66] style: Use cbindgen for content property. This cleans up and also allows us to keep the distinction between content: none and content: normal, which allows us to fix the computed style we return from getComputedStyle(). Do this last bit from the resolved value instead of StyleAdjuster, because otherwise we need to tweak every initial struct for ::before / ::after. Differential Revision: https://phabricator.services.mozilla.com/D58276 --- components/style/counter_style/mod.rs | 2 + components/style/gecko/values.rs | 40 +-- .../style/gecko_string_cache/namespace.rs | 1 + components/style/lib.rs | 6 +- components/style/properties/gecko.mako.rs | 244 +----------------- components/style/values/computed/counters.rs | 4 +- components/style/values/computed/mod.rs | 1 + components/style/values/generics/counters.rs | 28 +- components/style/values/generics/mod.rs | 32 +-- components/style/values/resolved/counters.rs | 46 ++++ components/style/values/resolved/mod.rs | 2 + components/style/values/specified/counters.rs | 34 ++- components/style/values/specified/mod.rs | 25 +- 13 files changed, 122 insertions(+), 343 deletions(-) create mode 100644 components/style/values/resolved/counters.rs diff --git a/components/style/counter_style/mod.rs b/components/style/counter_style/mod.rs index 25cdc3f8737..d131f350393 100644 --- a/components/style/counter_style/mod.rs +++ b/components/style/counter_style/mod.rs @@ -409,6 +409,7 @@ impl ToCss for System { /// #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToShmem)] +#[repr(u8)] pub enum Symbol { /// String(crate::OwnedStr), @@ -554,6 +555,7 @@ impl Parse for Fallback { /// #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss, ToShmem)] +#[repr(C)] pub struct Symbols(#[css(iterable)] pub crate::OwnedSlice); impl Parse for Symbols { diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index b31485345b6..a755556e850 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -7,13 +7,13 @@ //! Different kind of helpers to interact with Gecko values. use crate::counter_style::{Symbol, Symbols}; +use crate::gecko_bindings::bindings; use crate::gecko_bindings::structs::CounterStylePtr; use crate::values::generics::CounterStyle; use crate::values::Either; use crate::Atom; use app_units::Au; use cssparser::RGBA; -use nsstring::{nsACString, nsCStr}; use std::cmp::max; /// Convert a given RGBA value to `nscolor`. @@ -51,43 +51,13 @@ pub fn round_border_to_device_pixels(width: Au, au_per_device_px: Au) -> Au { impl CounterStyle { /// Convert this counter style to a Gecko CounterStylePtr. - pub fn to_gecko_value(self, gecko_value: &mut CounterStylePtr) { - use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToName as set_name; - use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToSymbols as set_symbols; - match self { - CounterStyle::Name(name) => unsafe { - debug_assert_ne!(name.0, atom!("none")); - set_name(gecko_value, name.0.into_addrefed()); - }, - CounterStyle::Symbols(symbols_type, symbols) => { - let symbols: Vec<_> = symbols - .0 - .iter() - .map(|symbol| match *symbol { - Symbol::String(ref s) => nsCStr::from(&**s), - Symbol::Ident(_) => unreachable!("Should not have identifier in symbols()"), - }) - .collect(); - let symbols: Vec<_> = symbols - .iter() - .map(|symbol| symbol as &nsACString as *const _) - .collect(); - unsafe { - set_symbols( - gecko_value, - symbols_type.to_gecko_keyword(), - symbols.as_ptr(), - symbols.len() as u32, - ) - }; - }, - } + #[inline] + pub fn to_gecko_value(&self, gecko_value: &mut CounterStylePtr) { + unsafe { bindings::Gecko_CounterStyle_ToPtr(self, gecko_value) } } /// Convert Gecko CounterStylePtr to CounterStyle or String. pub fn from_gecko_value(gecko_value: &CounterStylePtr) -> Either { - use crate::gecko_bindings::bindings; - use crate::values::generics::SymbolsType; use crate::values::CustomIdent; let name = unsafe { bindings::Gecko_CounterStyle_GetName(gecko_value) }; @@ -103,7 +73,7 @@ impl CounterStyle { debug_assert_eq!(symbols.len(), 1); Either::Second(symbols[0].to_string()) } else { - let symbol_type = SymbolsType::from_gecko_keyword(anonymous.mSystem as u32); + let symbol_type = anonymous.mSymbolsType; let symbols = symbols .iter() .map(|gecko_symbol| Symbol::String(gecko_symbol.to_string().into())) diff --git a/components/style/gecko_string_cache/namespace.rs b/components/style/gecko_string_cache/namespace.rs index 33883e66941..2dba484e002 100644 --- a/components/style/gecko_string_cache/namespace.rs +++ b/components/style/gecko_string_cache/namespace.rs @@ -25,6 +25,7 @@ macro_rules! ns { /// A Gecko namespace is just a wrapped atom. #[derive(Clone, Debug, Default, Eq, Hash, MallocSizeOf, PartialEq, ToShmem)] +#[repr(transparent)] pub struct Namespace(pub Atom); impl PrecomputedHash for Namespace { diff --git a/components/style/lib.rs b/components/style/lib.rs index 354950f9c15..33862461aa9 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -173,10 +173,12 @@ pub mod values; pub use crate::gecko_string_cache as string_cache; #[cfg(feature = "gecko")] pub use crate::gecko_string_cache::Atom; +/// The namespace prefix type for Gecko, which is just an atom. #[cfg(feature = "gecko")] -pub use crate::gecko_string_cache::Atom as Prefix; +pub type Prefix = crate::gecko_string_cache::Atom; +/// The local name of an element for Gecko, which is just an atom. #[cfg(feature = "gecko")] -pub use crate::gecko_string_cache::Atom as LocalName; +pub type LocalName = crate::gecko_string_cache::Atom; #[cfg(feature = "gecko")] pub use crate::gecko_string_cache::Namespace; diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 14262e7a074..874bf1a83cb 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -36,7 +36,6 @@ use crate::gecko_bindings::bindings::Gecko_SetNullImageValue; use crate::gecko_bindings::structs; use crate::gecko_bindings::structs::nsCSSPropertyID; use crate::gecko_bindings::structs::mozilla::PseudoStyleType; -use crate::gecko_bindings::sugar::refptr::RefPtr; use crate::gecko::values::round_border_to_device_pixels; use crate::logical_geometry::WritingMode; use crate::media_queries::Device; @@ -45,7 +44,6 @@ use crate::properties::longhands; use crate::rule_tree::StrongRuleNode; use crate::selector_parser::PseudoElement; use servo_arc::{Arc, RawOffsetArc, UniqueArc}; -use std::marker::PhantomData; use std::mem::{forget, MaybeUninit}; use std::{cmp, ops, ptr}; use crate::values::{self, CustomIdent, Either, KeyframesName, None_}; @@ -2200,19 +2198,18 @@ fn static_assert() { } pub fn set_list_style_type(&mut self, v: longhands::list_style_type::computed_value::T) { - use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToName; - use crate::gecko_bindings::bindings::Gecko_SetCounterStyleToString; use nsstring::{nsACString, nsCStr}; use self::longhands::list_style_type::computed_value::T; match v { T::None => unsafe { - Gecko_SetCounterStyleToName(&mut self.gecko.mCounterStyle, - atom!("none").into_addrefed()); + bindings::Gecko_SetCounterStyleToNone(&mut self.gecko.mCounterStyle) } T::CounterStyle(s) => s.to_gecko_value(&mut self.gecko.mCounterStyle), T::String(s) => unsafe { - Gecko_SetCounterStyleToString(&mut self.gecko.mCounterStyle, - &nsCStr::from(&s) as &nsACString) + bindings::Gecko_SetCounterStyleToString( + &mut self.gecko.mCounterStyle, + &nsCStr::from(&s) as &nsACString, + ) } } } @@ -2580,236 +2577,9 @@ clip-path ${impl_simple('column_rule_style', 'mColumnRuleStyle')} -<%self:impl_trait style_struct_name="Counters" skip_longhands="content"> +<%self:impl_trait style_struct_name="Counters"> pub fn ineffective_content_property(&self) -> bool { - self.gecko.mContents.is_empty() - } - - pub fn set_content(&mut self, v: longhands::content::computed_value::T) { - use crate::values::CustomIdent; - use crate::values::generics::counters::{Content, ContentItem}; - use crate::values::generics::CounterStyle; - use crate::gecko_bindings::structs::nsStyleContentData; - use crate::gecko_bindings::structs::nsStyleContentAttr; - use crate::gecko_bindings::structs::StyleContentType; - use crate::gecko_bindings::bindings::Gecko_ClearAndResizeStyleContents; - - // Converts a string as utf16, and returns an owned, zero-terminated raw buffer. - fn as_utf16_and_forget(s: &str) -> *mut u16 { - use std::mem; - let mut vec = s.encode_utf16().collect::>(); - vec.push(0u16); - let ptr = vec.as_mut_ptr(); - mem::forget(vec); - ptr - } - - fn set_counter_function( - data: &mut nsStyleContentData, - content_type: StyleContentType, - name: CustomIdent, - sep: &str, - style: CounterStyle, - ) { - debug_assert!(content_type == StyleContentType::Counter || - content_type == StyleContentType::Counters); - let counter_func = unsafe { - bindings::Gecko_SetCounterFunction(data, content_type).as_mut().unwrap() - }; - counter_func.mIdent.set_move(unsafe { - RefPtr::from_addrefed(name.0.into_addrefed()) - }); - if content_type == StyleContentType::Counters { - counter_func.mSeparator.assign_str(sep); - } - style.to_gecko_value(&mut counter_func.mCounterStyle); - } - - match v { - Content::None | - Content::Normal => { - // Ensure destructors run, otherwise we could leak. - if !self.gecko.mContents.is_empty() { - unsafe { - Gecko_ClearAndResizeStyleContents(&mut *self.gecko, 0); - } - } - }, - Content::MozAltContent => { - unsafe { - Gecko_ClearAndResizeStyleContents(&mut *self.gecko, 1); - *self.gecko.mContents[0].mContent.mString.as_mut() = ptr::null_mut(); - } - self.gecko.mContents[0].mType = StyleContentType::AltContent; - }, - Content::Items(items) => { - unsafe { - Gecko_ClearAndResizeStyleContents(&mut *self.gecko, - items.len() as u32); - } - for (i, item) in items.into_vec().into_iter().enumerate() { - // NB: Gecko compares the mString value if type is not image - // or URI independently of whatever gets there. In the quote - // cases, they set it to null, so do the same here. - unsafe { - *self.gecko.mContents[i].mContent.mString.as_mut() = ptr::null_mut(); - } - match item { - ContentItem::String(ref value) => { - self.gecko.mContents[i].mType = StyleContentType::String; - unsafe { - // NB: we share allocators, so doing this is fine. - *self.gecko.mContents[i].mContent.mString.as_mut() = - as_utf16_and_forget(&value); - } - } - ContentItem::Attr(ref attr) => { - self.gecko.mContents[i].mType = StyleContentType::Attr; - unsafe { - // NB: we share allocators, so doing this is fine. - let maybe_ns = attr.namespace.clone(); - let attr_struct = Box::new(nsStyleContentAttr { - mName: structs::RefPtr { - mRawPtr: attr.attribute.clone().into_addrefed(), - _phantom_0: PhantomData, - }, - mNamespaceURL: structs::RefPtr { - mRawPtr: maybe_ns.map_or(ptr::null_mut(), |x| (x.1).0.into_addrefed()), - _phantom_0: PhantomData, - }, - }); - *self.gecko.mContents[i].mContent.mAttr.as_mut() = - Box::into_raw(attr_struct); - } - } - ContentItem::OpenQuote - => self.gecko.mContents[i].mType = StyleContentType::OpenQuote, - ContentItem::CloseQuote - => self.gecko.mContents[i].mType = StyleContentType::CloseQuote, - ContentItem::NoOpenQuote - => self.gecko.mContents[i].mType = StyleContentType::NoOpenQuote, - ContentItem::NoCloseQuote - => self.gecko.mContents[i].mType = StyleContentType::NoCloseQuote, - ContentItem::Counter(name, style) => { - set_counter_function( - &mut self.gecko.mContents[i], - StyleContentType::Counter, - name, - "", - style, - ); - } - ContentItem::Counters(name, sep, style) => { - set_counter_function( - &mut self.gecko.mContents[i], - StyleContentType::Counters, - name, - &sep, - style, - ); - } - ContentItem::Url(ref url) => { - unsafe { - bindings::Gecko_SetContentDataImageValue( - &mut self.gecko.mContents[i], - url, - ) - } - } - } - } - } - } - } - - pub fn copy_content_from(&mut self, other: &Self) { - use crate::gecko_bindings::bindings::Gecko_CopyStyleContentsFrom; - unsafe { - Gecko_CopyStyleContentsFrom(&mut *self.gecko, &*other.gecko) - } - } - - pub fn reset_content(&mut self, other: &Self) { - self.copy_content_from(other) - } - - pub fn clone_content(&self) -> longhands::content::computed_value::T { - use {Atom, Namespace}; - use crate::gecko::conversions::string_from_chars_pointer; - use crate::gecko_bindings::structs::StyleContentType; - use crate::values::generics::counters::{Content, ContentItem}; - use crate::values::{CustomIdent, Either}; - use crate::values::generics::CounterStyle; - use crate::values::specified::Attr; - - if self.gecko.mContents.is_empty() { - return Content::None; - } - - if self.gecko.mContents.len() == 1 && - self.gecko.mContents[0].mType == StyleContentType::AltContent { - return Content::MozAltContent; - } - - Content::Items( - self.gecko.mContents.iter().map(|gecko_content| { - match gecko_content.mType { - StyleContentType::OpenQuote => ContentItem::OpenQuote, - StyleContentType::CloseQuote => ContentItem::CloseQuote, - StyleContentType::NoOpenQuote => ContentItem::NoOpenQuote, - StyleContentType::NoCloseQuote => ContentItem::NoCloseQuote, - StyleContentType::String => { - let gecko_chars = unsafe { gecko_content.mContent.mString.as_ref() }; - let string = unsafe { string_from_chars_pointer(*gecko_chars) }; - ContentItem::String(string.into_boxed_str()) - }, - StyleContentType::Attr => { - let (namespace, attribute) = unsafe { - let s = &**gecko_content.mContent.mAttr.as_ref(); - let ns = if s.mNamespaceURL.mRawPtr.is_null() { - None - } else { - // FIXME(bholley): We don't have any way to get the prefix here. :-( - let prefix = atom!(""); - Some((prefix, Namespace(Atom::from_raw(s.mNamespaceURL.mRawPtr)))) - }; - (ns, Atom::from_raw(s.mName.mRawPtr)) - }; - ContentItem::Attr(Attr { namespace, attribute }) - }, - StyleContentType::Counter | StyleContentType::Counters => { - let gecko_function = - unsafe { &**gecko_content.mContent.mCounters.as_ref() }; - let ident = CustomIdent(unsafe { - Atom::from_raw(gecko_function.mIdent.mRawPtr) - }); - let style = - CounterStyle::from_gecko_value(&gecko_function.mCounterStyle); - let style = match style { - Either::First(counter_style) => counter_style, - Either::Second(_) => - unreachable!("counter function shouldn't have single string type"), - }; - if gecko_content.mType == StyleContentType::Counter { - ContentItem::Counter(ident, style) - } else { - let separator = gecko_function.mSeparator.to_string(); - ContentItem::Counters(ident, separator.into_boxed_str(), style) - } - }, - StyleContentType::Image => { - unsafe { - let gecko_image_request = - &**gecko_content.mContent.mImage.as_ref(); - ContentItem::Url( - ComputedImageUrl::from_image_request(gecko_image_request) - ) - } - }, - _ => panic!("Found unexpected value in style struct for content property"), - } - }).collect::>().into_boxed_slice() - ) + !self.gecko.mContent.is_items() } diff --git a/components/style/values/computed/counters.rs b/components/style/values/computed/counters.rs index 3a083632eb9..40cfe46efa6 100644 --- a/components/style/values/computed/counters.rs +++ b/components/style/values/computed/counters.rs @@ -16,7 +16,7 @@ pub type CounterIncrement = GenericCounterIncrement; pub type CounterSetOrReset = GenericCounterSetOrReset; /// A computed value for the `content` property. -pub type Content = generics::Content; +pub type Content = generics::GenericContent; /// A computed content item. -pub type ContentItem = generics::ContentItem; +pub type ContentItem = generics::GenericContentItem; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index f5b3440f426..958deaaf44e 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -471,6 +471,7 @@ trivial_to_computed_value!(Atom); trivial_to_computed_value!(Prefix); trivial_to_computed_value!(String); trivial_to_computed_value!(Box); +trivial_to_computed_value!(crate::OwnedStr); /// A `` value. pub type Number = CSSFloat; diff --git a/components/style/values/generics/counters.rs b/components/style/values/generics/counters.rs index 69893c75561..9ed8cdba68b 100644 --- a/components/style/values/generics/counters.rs +++ b/components/style/values/generics/counters.rs @@ -153,22 +153,26 @@ fn is_decimal(counter_type: &CounterStyleType) -> bool { SpecifiedValueInfo, ToComputedValue, ToCss, - ToResolvedValue, ToShmem, )] -pub enum Content { +#[repr(u8)] +pub enum GenericContent { /// `normal` reserved keyword. Normal, /// `none` reserved keyword. None, - /// `-moz-alt-content`. - #[cfg(feature = "gecko")] - MozAltContent, /// Content items. - Items(#[css(iterable)] Box<[ContentItem]>), + Items(#[css(iterable)] crate::OwnedSlice>), } +pub use self::GenericContent as Content; + impl Content { + #[inline] + pub(crate) fn is_items(&self) -> bool { + matches!(*self, Self::Items(..)) + } + /// Set `content` property to `normal`. #[inline] pub fn normal() -> Self { @@ -189,9 +193,10 @@ impl Content { ToResolvedValue, ToShmem, )] -pub enum ContentItem { +#[repr(u8)] +pub enum GenericContentItem { /// Literal string content. - String(Box), + String(crate::OwnedStr), /// `counter(name, style)`. #[css(comma, function)] Counter(CustomIdent, #[css(skip_if = "is_decimal")] CounterStyleType), @@ -199,7 +204,7 @@ pub enum ContentItem { #[css(comma, function)] Counters( CustomIdent, - Box, + crate::OwnedStr, #[css(skip_if = "is_decimal")] CounterStyleType, ), /// `open-quote`. @@ -210,9 +215,14 @@ pub enum ContentItem { NoOpenQuote, /// `no-close-quote`. NoCloseQuote, + /// `-moz-alt-content`. + #[cfg(feature = "gecko")] + MozAltContent, /// `attr([namespace? `|`]? ident)` #[cfg(feature = "gecko")] Attr(Attr), /// `url(url)` Url(ImageUrl), } + +pub use self::GenericContentItem as ContentItem; diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index 4db80abc8a8..a97d9e1018a 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -39,7 +39,7 @@ pub mod transform; pub mod ui; pub mod url; -// https://drafts.csswg.org/css-counter-styles/#typedef-symbols-type +/// https://drafts.csswg.org/css-counter-styles/#typedef-symbols-type #[allow(missing_docs)] #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))] #[derive( @@ -55,6 +55,7 @@ pub mod url; ToResolvedValue, ToShmem, )] +#[repr(u8)] pub enum SymbolsType { Cyclic, Numeric, @@ -63,39 +64,12 @@ pub enum SymbolsType { Fixed, } -#[cfg(feature = "gecko")] -impl SymbolsType { - /// Convert symbols type to their corresponding Gecko values. - pub fn to_gecko_keyword(self) -> u8 { - use crate::gecko_bindings::structs; - match self { - SymbolsType::Cyclic => structs::NS_STYLE_COUNTER_SYSTEM_CYCLIC as u8, - SymbolsType::Numeric => structs::NS_STYLE_COUNTER_SYSTEM_NUMERIC as u8, - SymbolsType::Alphabetic => structs::NS_STYLE_COUNTER_SYSTEM_ALPHABETIC as u8, - SymbolsType::Symbolic => structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC as u8, - SymbolsType::Fixed => structs::NS_STYLE_COUNTER_SYSTEM_FIXED as u8, - } - } - - /// Convert Gecko value to symbol type. - pub fn from_gecko_keyword(gecko_value: u32) -> SymbolsType { - use crate::gecko_bindings::structs; - match gecko_value { - structs::NS_STYLE_COUNTER_SYSTEM_CYCLIC => SymbolsType::Cyclic, - structs::NS_STYLE_COUNTER_SYSTEM_NUMERIC => SymbolsType::Numeric, - structs::NS_STYLE_COUNTER_SYSTEM_ALPHABETIC => SymbolsType::Alphabetic, - structs::NS_STYLE_COUNTER_SYSTEM_SYMBOLIC => SymbolsType::Symbolic, - structs::NS_STYLE_COUNTER_SYSTEM_FIXED => SymbolsType::Fixed, - x => panic!("Unexpected value for symbol type {}", x), - } - } -} - /// /// /// Note that 'none' is not a valid name. #[cfg_attr(feature = "gecko", derive(MallocSizeOf))] #[derive(Clone, Debug, Eq, PartialEq, ToComputedValue, ToCss, ToResolvedValue, ToShmem)] +#[repr(u8)] pub enum CounterStyle { /// `` Name(CustomIdent), diff --git a/components/style/values/resolved/counters.rs b/components/style/values/resolved/counters.rs new file mode 100644 index 00000000000..daf70359061 --- /dev/null +++ b/components/style/values/resolved/counters.rs @@ -0,0 +1,46 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +//! Resolved values for counter properties + +use super::{Context, ToResolvedValue}; +use crate::values::computed; + +/// https://drafts.csswg.org/css-content/#content-property +/// +/// We implement this at resolved value time because otherwise it causes us to +/// allocate a bunch of useless initial structs for ::before / ::after, which is +/// a bit unfortunate. +/// +/// Though these should be temporary, mostly, so if this causes complexity in +/// other places, it should be fine to move to `StyleAdjuster`. +/// +/// See https://github.com/w3c/csswg-drafts/issues/4632 for where some related +/// issues are being discussed. +impl ToResolvedValue for computed::Content { + type ResolvedValue = Self; + + #[inline] + fn to_resolved_value(self, context: &Context) -> Self { + let is_before_or_after = + context.style.pseudo().map_or(false, |p| p.is_before_or_after()); + + match self { + Self::Normal if is_before_or_after => Self::None, + // For now, make `content: none` compute to `normal` on other + // elements, as we don't respect it. + // + // FIXME(emilio, bug 1605473): for marker this should be preserved + // and respected, probably. + Self::None if !is_before_or_after => Self::Normal, + other => other, + } + } + + #[inline] + fn from_resolved_value(resolved: Self) -> Self { + resolved + } +} + diff --git a/components/style/values/resolved/mod.rs b/components/style/values/resolved/mod.rs index e64178a1c95..f50f58b5db4 100644 --- a/components/style/values/resolved/mod.rs +++ b/components/style/values/resolved/mod.rs @@ -10,6 +10,7 @@ use cssparser; use smallvec::SmallVec; mod color; +mod counters; use crate::values::computed; @@ -68,6 +69,7 @@ trivial_to_resolved_value!(u32); trivial_to_resolved_value!(usize); trivial_to_resolved_value!(String); trivial_to_resolved_value!(Box); +trivial_to_resolved_value!(crate::OwnedStr); trivial_to_resolved_value!(cssparser::RGBA); trivial_to_resolved_value!(crate::Atom); trivial_to_resolved_value!(app_units::Au); diff --git a/components/style/values/specified/counters.rs b/components/style/values/specified/counters.rs index 09020e73448..15898b58f47 100644 --- a/components/style/values/specified/counters.rs +++ b/components/style/values/specified/counters.rs @@ -9,8 +9,6 @@ use crate::computed_values::list_style_type::T as ListStyleType; use crate::parser::{Parse, ParserContext}; use crate::values::generics::counters as generics; use crate::values::generics::counters::CounterPair; -use crate::values::generics::counters::GenericCounterIncrement; -use crate::values::generics::counters::GenericCounterSetOrReset; #[cfg(feature = "gecko")] use crate::values::generics::CounterStyle; use crate::values::specified::url::SpecifiedImageUrl; @@ -23,7 +21,7 @@ use selectors::parser::SelectorParseErrorKind; use style_traits::{ParseError, StyleParseErrorKind}; /// A specified value for the `counter-increment` property. -pub type CounterIncrement = GenericCounterIncrement; +pub type CounterIncrement = generics::GenericCounterIncrement; impl Parse for CounterIncrement { fn parse<'i, 't>( @@ -35,7 +33,7 @@ impl Parse for CounterIncrement { } /// A specified value for the `counter-set` and `counter-reset` properties. -pub type CounterSetOrReset = GenericCounterSetOrReset; +pub type CounterSetOrReset = generics::GenericCounterSetOrReset; impl Parse for CounterSetOrReset { fn parse<'i, 't>( @@ -84,10 +82,10 @@ fn parse_counters<'i, 't>( } /// The specified value for the `content` property. -pub type Content = generics::Content; +pub type Content = generics::GenericContent; /// The specified value for a content item in the `content` property. -pub type ContentItem = generics::ContentItem; +pub type ContentItem = generics::GenericContentItem; impl Content { #[cfg(feature = "servo")] @@ -131,17 +129,9 @@ impl Parse for Content { { return Ok(generics::Content::None); } - #[cfg(feature = "gecko")] - { - if input - .try(|input| input.expect_ident_matching("-moz-alt-content")) - .is_ok() - { - return Ok(generics::Content::MozAltContent); - } - } let mut content = vec![]; + let mut has_alt_content = false; loop { #[cfg(feature = "gecko")] { @@ -153,7 +143,7 @@ impl Parse for Content { match input.next() { Ok(&Token::QuotedString(ref value)) => { content.push(generics::ContentItem::String( - value.as_ref().to_owned().into_boxed_str(), + value.as_ref().to_owned().into(), )); }, Ok(&Token::Function(ref name)) => { @@ -168,7 +158,7 @@ impl Parse for Content { let location = input.current_source_location(); let name = CustomIdent::from_ident(location, input.expect_ident()?, &[])?; input.expect_comma()?; - let separator = input.expect_string()?.as_ref().to_owned().into_boxed_str(); + let separator = input.expect_string()?.as_ref().to_owned().into(); let style = Content::parse_counter_style(context, input); Ok(generics::ContentItem::Counters(name, separator, style)) }), @@ -191,6 +181,11 @@ impl Parse for Content { "close-quote" => generics::ContentItem::CloseQuote, "no-open-quote" => generics::ContentItem::NoOpenQuote, "no-close-quote" => generics::ContentItem::NoCloseQuote, + #[cfg(feature = "gecko")] + "-moz-alt-content" => { + has_alt_content = true; + generics::ContentItem::MozAltContent + }, _ =>{ let ident = ident.clone(); return Err(input.new_custom_error( @@ -206,9 +201,10 @@ impl Parse for Content { }, } } - if content.is_empty() { + // We don't allow to parse `-moz-alt-content in multiple positions. + if content.is_empty() || (has_alt_content && content.len() != 1) { return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - Ok(generics::Content::Items(content.into_boxed_slice())) + Ok(generics::Content::Items(content.into())) } } diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 7f3ecfb96b3..7e16ba09601 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -767,9 +767,12 @@ impl AllowQuirks { ToShmem, )] #[css(function)] +#[repr(C)] pub struct Attr { - /// Optional namespace prefix and URL. - pub namespace: Option<(Prefix, Namespace)>, + /// Optional namespace prefix. + pub namespace_prefix: Prefix, + /// Optional namespace URL. + pub namespace_url: Namespace, /// Attribute name pub attribute: Atom, } @@ -814,7 +817,7 @@ impl Attr { ref t => return Err(location.new_unexpected_token_error(t.clone())), }; - let prefix_and_ns = if let Some(ns) = first { + let (namespace_prefix, namespace_url) = if let Some(ns) = first { let prefix = Prefix::from(ns.as_ref()); let ns = match get_namespace_for_prefix(&prefix, context) { Some(ns) => ns, @@ -823,17 +826,18 @@ impl Attr { .new_custom_error(StyleParseErrorKind::UnspecifiedError)); }, }; - Some((prefix, ns)) + (prefix, ns) } else { - None + (Prefix::default(), Namespace::default()) }; return Ok(Attr { - namespace: prefix_and_ns, + namespace_prefix, + namespace_url, attribute: Atom::from(second_token.as_ref()), }); }, // In the case of attr(foobar ) we don't want to error out - // because of the trailing whitespace + // because of the trailing whitespace. Token::WhiteSpace(..) => {}, ref t => return Err(input.new_unexpected_token_error(t.clone())), } @@ -841,7 +845,8 @@ impl Attr { if let Some(first) = first { Ok(Attr { - namespace: None, + namespace_prefix: Prefix::default(), + namespace_url: Namespace::default(), attribute: Atom::from(first.as_ref()), }) } else { @@ -856,8 +861,8 @@ impl ToCss for Attr { W: Write, { dest.write_str("attr(")?; - if let Some((ref prefix, ref _url)) = self.namespace { - serialize_atom_identifier(prefix, dest)?; + if !self.namespace_prefix.is_empty() { + serialize_atom_identifier(&self.namespace_prefix, dest)?; dest.write_str("|")?; } serialize_atom_identifier(&self.attribute, dest)?; From 718a9b1f05292be3afc552eab20fcf65803348ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 7 Jan 2020 21:00:42 +0000 Subject: [PATCH 08/66] style: Make display: -moz-box more similar to other display types for block layout. It is unexpected (see bug) that a -moz-box is affected by baseline alignment. Make -moz-box be block-outside, and -moz-inline-box be inline-outside, instead of the bespoke thing we have now. This is more similar to everything else, and fixes the bug. Differential Revision: https://phabricator.services.mozilla.com/D58726 --- components/style/values/specified/box.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 1549f66e8e9..2f114b5bb0a 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -98,8 +98,6 @@ pub enum DisplayInside { #[cfg(feature = "gecko")] MozBox, #[cfg(feature = "gecko")] - MozInlineBox, - #[cfg(feature = "gecko")] MozGrid, #[cfg(feature = "gecko")] MozGridGroup, @@ -221,9 +219,9 @@ impl Display { /// XUL boxes. #[cfg(feature = "gecko")] - pub const MozBox: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozBox); + pub const MozBox: Self = Self::new(DisplayOutside::Block, DisplayInside::MozBox); #[cfg(feature = "gecko")] - pub const MozInlineBox: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozInlineBox); + pub const MozInlineBox: Self = Self::new(DisplayOutside::Inline, DisplayInside::MozBox); #[cfg(feature = "gecko")] pub const MozGrid: Self = Self::new(DisplayOutside::XUL, DisplayInside::MozGrid); #[cfg(feature = "gecko")] @@ -375,11 +373,6 @@ impl Display { }; Display::from3(DisplayOutside::Block, inside, self.is_list_item()) }, - #[cfg(feature = "gecko")] - DisplayOutside::XUL => match self.inside() { - DisplayInside::MozInlineBox | DisplayInside::MozBox => Display::MozBox, - _ => Display::Block, - }, DisplayOutside::Block | DisplayOutside::None => *self, #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] _ => Display::Block, @@ -400,11 +393,6 @@ impl Display { }; Display::from3(DisplayOutside::Inline, inside, self.is_list_item()) }, - #[cfg(feature = "gecko")] - DisplayOutside::XUL => match self.inside() { - DisplayInside::MozBox => Display::MozInlineBox, - _ => *self, - }, _ => *self, } } From d459e3453ae309302537a4c9937336a14f632e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 8 Jan 2020 00:55:18 +0000 Subject: [PATCH 09/66] style: Make an assertion a release assert in nightly. If this can happen legitimately, this will help fuzzers to catch it. Differential Revision: https://phabricator.services.mozilla.com/D59066 --- components/style/rule_tree/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 19e3accb851..fa859d79f06 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -1719,7 +1719,10 @@ impl Drop for StrongRuleNode { return; } - debug_assert!(node.children.read().is_empty()); + if cfg!(debug_assertions) || crate::gecko_bindings::structs::GECKO_IS_NIGHTLY { + assert!(node.children.read().is_empty()); + } + if node.parent.is_none() { debug!("Dropping root node!"); // The free list should be null by this point From 25fda147f38ecb65ebb8e58b5cfce6b92a7bdf8a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 9 Jan 2020 11:36:02 +0000 Subject: [PATCH 10/66] style: Minor comment fix. Differential Revision: https://phabricator.services.mozilla.com/D59310 --- components/style/sharing/checks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/sharing/checks.rs b/components/style/sharing/checks.rs index 9c860f0258f..5e8350e78d3 100644 --- a/components/style/sharing/checks.rs +++ b/components/style/sharing/checks.rs @@ -94,7 +94,7 @@ where target.class_list() == candidate.class_list() } -/// Whether a given element has the same class attribute as a given candidate. +/// Whether a given element has the same part attribute as a given candidate. /// /// We don't try to share style across elements with different part attributes. pub fn have_same_parts( From ec37e7a5b815d4ac5e393fdf5d035495a8d9d49c Mon Sep 17 00:00:00 2001 From: Xidorn Quan Date: Fri, 10 Jan 2020 23:47:28 +0000 Subject: [PATCH 11/66] style: Ensure nested ruby level container don't escape from line break suppression. Differential Revision: https://phabricator.services.mozilla.com/D58351 --- components/style/style_adjuster.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/components/style/style_adjuster.rs b/components/style/style_adjuster.rs index 36463aad6fb..a66c9462c92 100644 --- a/components/style/style_adjuster.rs +++ b/components/style/style_adjuster.rs @@ -547,7 +547,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { } #[cfg(feature = "gecko")] - fn should_suppress_linebreak(&self, layout_parent_style: &ComputedValues) -> bool { + fn should_suppress_linebreak( + &self, + layout_parent_style: &ComputedValues, + element: Option, + ) -> bool + where + E: TElement, + { // Line break suppression should only be propagated to in-flow children. if self.style.is_floating() || self.style.is_absolutely_positioned() { return false; @@ -567,11 +574,18 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { // Ruby base and text are always non-breakable. Display::RubyBase | Display::RubyText => true, // Ruby base container and text container are breakable. + // Non-HTML elements may not form ruby base / text container because + // they may not respect ruby-internal display values, so we can't + // make them escaped from line break suppression. // Note that, when certain HTML tags, e.g. form controls, have ruby // level container display type, they could also escape from the // line break suppression flag while they shouldn't. However, it is - // generally fine since they themselves are non-breakable. - Display::RubyBaseContainer | Display::RubyTextContainer => false, + // generally fine as far as they can't break the line inside them. + Display::RubyBaseContainer | Display::RubyTextContainer + if element.map_or(true, |e| e.is_html_element()) => + { + false + }, // Anything else is non-breakable if and only if its layout parent // has a ruby display type, because any of the ruby boxes can be // anonymous. @@ -593,7 +607,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { let self_display = self.style.get_box().clone_display(); // Check whether line break should be suppressed for this element. - if self.should_suppress_linebreak(layout_parent_style) { + if self.should_suppress_linebreak(layout_parent_style, element) { self.style .add_flags(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK); // Inlinify the display type if allowed. From 61f3ff1de3b973efc2074e4b7251e11692bee12f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 13 Jan 2020 13:21:58 +0000 Subject: [PATCH 12/66] style: Split LengthPercentage again. This is needed to support min() / max() / clamp(), etc, as those need to be a tree of values and thus need heap storage. This unfortunately grows LengthPercentage to be two pointers, which is bad as it blows up the size of nsStylePosition enough to trigger the size assertions. This patch comments out the assertion for now, the follow-up patches will uncomment them. Differential Revision: https://phabricator.services.mozilla.com/D58700 --- components/style/gecko/media_queries.rs | 3 +- components/style/values/animated/length.rs | 11 +- components/style/values/animated/mod.rs | 10 + components/style/values/computed/length.rs | 453 +++++++++++------- components/style/values/specified/gecko.rs | 2 +- components/style/values/specified/position.rs | 2 +- 6 files changed, 304 insertions(+), 177 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 3f82b832bce..390c1d4fe58 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -138,8 +138,7 @@ impl Device { /// Set the font size of the root element (for rem) pub fn set_root_font_size(&self, size: Au) { - self.root_font_size - .store(size.0 as isize, Ordering::Relaxed) + self.root_font_size.store(size.0 as isize, Ordering::Relaxed) } /// Sets the body text color for the "inherit color from body" quirk. diff --git a/components/style/values/animated/length.rs b/components/style/values/animated/length.rs index 04a0844dbe0..ece26a9c491 100644 --- a/components/style/values/animated/length.rs +++ b/components/style/values/animated/length.rs @@ -5,7 +5,7 @@ //! Animation implementation for various length-related types. use super::{Animate, Procedure}; -use crate::values::computed::length::LengthPercentage; +use crate::values::computed::length::{LengthPercentage, CalcLengthPercentage}; use crate::values::computed::Percentage; /// @@ -26,10 +26,9 @@ impl Animate for LengthPercentage { .animate(&other.unclamped_length(), procedure)?; let percentage = animate_percentage_half(self.specified_percentage(), other.specified_percentage())?; - Ok(Self::with_clamping_mode( - length, - percentage, - self.clamping_mode, - )) + + // Gets clamped as needed after the animation, so no need to specify any + // particular AllowedNumericType. + Ok(CalcLengthPercentage::new(length, percentage).to_length_percentge()) } } diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index ea43d01112e..7cac407adcb 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -424,6 +424,16 @@ impl ToAnimatedZero for i32 { } } +impl ToAnimatedZero for Box +where + T: ToAnimatedZero, +{ + #[inline] + fn to_animated_zero(&self) -> Result { + Ok(Box::new((**self).to_animated_zero()?)) + } +} + impl ToAnimatedZero for Option where T: ToAnimatedZero, diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 485ef954d8e..285d9d2e6d4 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -18,7 +18,6 @@ use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativ use crate::values::{specified, CSSFloat}; use crate::Zero; use app_units::Au; -use ordered_float::NotNan; use std::fmt::{self, Write}; use std::ops::{Add, AddAssign, Div, Mul, Neg, Sub}; use style_traits::values::specified::AllowedNumericType; @@ -54,13 +53,13 @@ impl ToComputedValue for specified::NoCalcLength { } impl ToComputedValue for specified::Length { - type ComputedValue = CSSPixelLength; + type ComputedValue = Length; #[inline] fn to_computed_value(&self, context: &Context) -> Self::ComputedValue { match *self { specified::Length::NoCalc(l) => l.to_computed_value(context), - specified::Length::Calc(ref calc) => calc.to_computed_value(context).length(), + specified::Length::Calc(ref calc) => calc.to_computed_value(context).length_component(), } } @@ -73,15 +72,32 @@ impl ToComputedValue for specified::Length { /// A `` value. This can be either a ``, a /// ``, or a combination of both via `calc()`. /// +/// cbindgen:private-default-tagged-enum-constructor=false +/// cbindgen:derive-mut-casts=true +/// /// https://drafts.csswg.org/css-values-4/#typedef-length-percentage #[allow(missing_docs)] -#[derive(Clone, Debug, Deserialize, MallocSizeOf, Serialize, ToAnimatedZero, ToResolvedValue)] +#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, ToAnimatedZero, ToResolvedValue)] +#[repr(u8)] +pub enum LengthPercentage { + Length(Length), + Percentage(Percentage), + Calc(Box), +} + +/// The representation of a calc() function. +#[derive( + Clone, Debug, Deserialize, MallocSizeOf, Serialize, ToAnimatedZero, ToResolvedValue, +)] #[repr(C)] -pub struct LengthPercentage { +pub struct CalcLengthPercentage { length: Length, + percentage: Percentage, + #[animation(constant)] - pub clamping_mode: AllowedNumericType, + clamping_mode: AllowedNumericType, + /// Whether we specified a percentage or not. #[animation(constant)] pub has_percentage: bool, @@ -99,7 +115,7 @@ pub struct LengthPercentage { // We may want to just eagerly-detect whether we can clamp in // `LengthPercentage::new` and switch to `AllowedNumericType::NonNegative` then, // maybe. -impl PartialEq for LengthPercentage { +impl PartialEq for CalcLengthPercentage { fn eq(&self, other: &Self) -> bool { self.length == other.length && self.percentage == other.percentage && @@ -115,123 +131,8 @@ impl ComputeSquaredDistance for LengthPercentage { Ok(self .unclamped_length() .compute_squared_distance(&other.unclamped_length())? + - self.percentage - .compute_squared_distance(&other.percentage)?) - } -} - -impl LengthPercentage { - /// Returns a new `LengthPercentage`. - #[inline] - pub fn new(length: Length, percentage: Option) -> Self { - Self::with_clamping_mode(length, percentage, AllowedNumericType::All) - } - - /// Returns a new `LengthPercentage` with zero length and some percentage. - pub fn new_percent(percentage: Percentage) -> Self { - Self::new(Length::zero(), Some(percentage)) - } - - /// Returns a new `LengthPercentage` with a specific clamping mode. - #[inline] - pub fn with_clamping_mode( - length: Length, - percentage: Option, - clamping_mode: AllowedNumericType, - ) -> Self { - Self { - clamping_mode, - length, - percentage: percentage.unwrap_or_default(), - has_percentage: percentage.is_some(), - } - } - - /// Returns this `calc()` as a ``. - /// - /// Panics in debug mode if a percentage is present in the expression. - #[inline] - pub fn length(&self) -> CSSPixelLength { - debug_assert!(!self.has_percentage); - self.length_component() - } - - /// Returns the length component of this `calc()` - #[inline] - pub fn length_component(&self) -> CSSPixelLength { - CSSPixelLength::new(self.clamping_mode.clamp(self.length.px())) - } - - /// Returns the `` component of this `calc()`, unclamped. - #[inline] - pub fn unclamped_length(&self) -> CSSPixelLength { - self.length - } - - /// Returns the percentage component of this `calc()` - #[inline] - pub fn percentage_component(&self) -> Percentage { - Percentage(self.clamping_mode.clamp(self.percentage.0)) - } - - /// Return the percentage value as CSSFloat. - #[inline] - pub fn percentage(&self) -> CSSFloat { - self.percentage.0 - } - - /// Return the specified percentage if any. - #[inline] - pub fn specified_percentage(&self) -> Option { - if self.has_percentage { - Some(self.percentage) - } else { - None - } - } - - /// Returns the length component if this could be represented as a - /// non-calc length. - pub fn as_length(&self) -> Option { - if !self.has_percentage { - Some(self.length_component()) - } else { - None - } - } - - /// Returns the percentage component if this could be represented as a - /// non-calc percentage. - pub fn as_percentage(&self) -> Option { - if !self.has_percentage || self.length.px() != 0. { - return None; - } - - Some(Percentage(self.clamping_mode.clamp(self.percentage.0))) - } - - /// Resolves the percentage. - #[inline] - pub fn percentage_relative_to(&self, basis: Length) -> Length { - let length = self.unclamped_length().0 + basis.0 * self.percentage.0; - Length::new(self.clamping_mode.clamp(length)) - } - - /// Convert the computed value into used value. - #[inline] - pub fn maybe_to_used_value(&self, container_len: Option) -> Option { - self.maybe_percentage_relative_to(container_len) - .map(Au::from) - } - - /// If there are special rules for computing percentages in a value (e.g. - /// the height property), they apply whenever a calc() expression contains - /// percentages. - pub fn maybe_percentage_relative_to(&self, container_len: Option) -> Option { - if self.has_percentage { - return Some(self.percentage_relative_to(container_len?)); - } - Some(self.length()) + self.percentage() + .compute_squared_distance(&other.percentage())?) } } @@ -251,7 +152,7 @@ impl specified::CalcLengthPercentage { context: &Context, zoom_fn: F, base_size: FontBaseSize, - ) -> LengthPercentage + ) -> CalcLengthPercentage where F: Fn(Length) -> Length, { @@ -285,7 +186,7 @@ impl specified::CalcLengthPercentage { } } - LengthPercentage::with_clamping_mode( + CalcLengthPercentage::with_clamping_mode( Length::new(length.min(f32::MAX).max(f32::MIN)), self.percentage, self.clamping_mode, @@ -297,7 +198,7 @@ impl specified::CalcLengthPercentage { &self, context: &Context, base_size: FontBaseSize, - ) -> LengthPercentage { + ) -> CalcLengthPercentage { self.to_computed_value_with_zoom( context, |abs| context.maybe_zoom_text(abs.into()), @@ -329,18 +230,14 @@ impl specified::CalcLengthPercentage { }, } } -} -impl ToComputedValue for specified::CalcLengthPercentage { - type ComputedValue = LengthPercentage; - - fn to_computed_value(&self, context: &Context) -> LengthPercentage { + fn to_computed_value(&self, context: &Context) -> CalcLengthPercentage { // normal properties don't zoom, and compute em units against the current style's font-size self.to_computed_value_with_zoom(context, |abs| abs, FontBaseSize::CurrentStyle) } #[inline] - fn from_computed_value(computed: &LengthPercentage) -> Self { + fn from_computed_value(computed: &CalcLengthPercentage) -> Self { specified::CalcLengthPercentage { clamping_mode: computed.clamping_mode, absolute: Some(AbsoluteLength::from_computed_value(&computed.length)), @@ -353,50 +250,262 @@ impl ToComputedValue for specified::CalcLengthPercentage { impl LengthPercentage { /// 1px length value for SVG defaults #[inline] - pub fn one() -> LengthPercentage { - LengthPercentage::new(Length::new(1.), None) + pub fn one() -> Self { + Self::Length(Length::new(1.)) + } + + /// Constructs a length value. + #[inline] + pub fn new_length(l: Length) -> Self { + Self::Length(l) + } + + /// Constructs a percentage value. + #[inline] + pub fn new_percent(p: Percentage) -> Self { + Self::Percentage(p) + } + + /// Constructs a `calc()` value. + #[inline] + pub fn new_calc(l: Length, percentage: Percentage) -> Self { + CalcLengthPercentage::new(l, Some(percentage)).to_length_percentge() } /// Returns true if the computed value is absolute 0 or 0%. #[inline] pub fn is_definitely_zero(&self) -> bool { - self.unclamped_length().px() == 0.0 && self.percentage.0 == 0.0 + match *self { + Self::Length(l) => l.px() == 0.0, + Self::Percentage(p) => p.0 == 0.0, + Self::Calc(ref c) => c.is_definitely_zero(), + } } - // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we - // still use Au as the hash key. - #[allow(missing_docs)] - pub fn to_hash_key(&self) -> (Au, NotNan) { - ( - Au::from(self.unclamped_length()), - NotNan::new(self.percentage.0).unwrap(), - ) + /// Returns the `` component of this `calc()`, unclamped. + #[inline] + pub fn unclamped_length(&self) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(..) => Zero::zero(), + Self::Calc(ref c) => c.unclamped_length(), + } + } + + /// Returns this `calc()` as a ``. + /// + /// Panics in debug mode if a percentage is present in the expression. + #[inline] + fn length(&self) -> Length { + debug_assert!(!self.has_percentage()); + self.length_component() + } + + /// Returns the `` component of this `calc()`, clamped. + #[inline] + pub fn length_component(&self) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(..) => Zero::zero(), + Self::Calc(ref c) => c.length_component(), + } + } + + /// Returns the `` component of this `calc()`, unclamped, as a + /// float. + /// + /// FIXME: This are very different semantics from length(), we should + /// probably rename this. + #[inline] + pub fn percentage(&self) -> CSSFloat { + match *self { + Self::Length(..) => 0., + Self::Percentage(p) => p.0, + Self::Calc(ref c) => c.percentage.0, + } + } + + /// Returns the `` component of this `calc()`, clamped. + #[inline] + pub fn as_percentage(&self) -> Option { + match *self { + Self::Length(..) => None, + Self::Percentage(p) => Some(p), + Self::Calc(ref c) => c.as_percentage(), + } + } + + /// Resolves the percentage. + #[inline] + pub fn resolve(&self, basis: Length) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(p) => Length::new(basis.0 * p.0), + Self::Calc(ref c) => c.resolve(basis), + } + } + + /// Resolves the percentage. Just an alias of resolve(). + #[inline] + pub fn percentage_relative_to(&self, basis: Length) -> Length { + self.resolve(basis) + } + + /// Return whether there's any percentage in this value. + #[inline] + pub fn has_percentage(&self) -> bool { + match *self { + Self::Length(..) => false, + Self::Percentage(..) => true, + Self::Calc(ref c) => c.has_percentage, + } + } + + /// Return the specified percentage if any. + #[inline] + pub fn specified_percentage(&self) -> Option { + match *self { + Self::Length(..) => None, + Self::Percentage(p) => Some(p), + Self::Calc(ref c) => c.specified_percentage(), + } } /// Returns the used value. + #[inline] pub fn to_used_value(&self, containing_length: Au) -> Au { Au::from(self.to_pixel_length(containing_length)) } /// Returns the used value as CSSPixelLength. + #[inline] pub fn to_pixel_length(&self, containing_length: Au) -> Length { - self.percentage_relative_to(containing_length.into()) + self.resolve(containing_length.into()) + } + + /// Convert the computed value into used value. + #[inline] + fn maybe_to_used_value(&self, container_len: Option) -> Option { + self.maybe_percentage_relative_to(container_len).map(Au::from) + } + + /// If there are special rules for computing percentages in a value (e.g. + /// the height property), they apply whenever a calc() expression contains + /// percentages. + pub fn maybe_percentage_relative_to(&self, container_len: Option) -> Option { + if self.has_percentage() { + return Some(self.resolve(container_len?)); + } + Some(self.length()) } /// Returns the clamped non-negative values. #[inline] pub fn clamp_to_non_negative(self) -> Self { - if let Some(p) = self.specified_percentage() { + match self { + Self::Length(l) => Self::Length(l.clamp_to_non_negative()), + Self::Percentage(p) => Self::Percentage(p.clamp_to_non_negative()), + Self::Calc(c) => c.clamp_to_non_negative().to_length_percentge(), + } + } +} + +impl CalcLengthPercentage { + /// Returns a new `LengthPercentage`. + #[inline] + pub fn new(length: Length, percentage: Option) -> Self { + Self::with_clamping_mode(length, percentage, AllowedNumericType::All) + } + + /// Converts this to a `LengthPercentage`, simplifying if possible. + #[inline] + pub fn to_length_percentge(self) -> LengthPercentage { + if !self.has_percentage { + return LengthPercentage::Length(self.length_component()) + } + if self.length.is_zero() { + return LengthPercentage::Percentage(Percentage(self.clamping_mode.clamp(self.percentage.0))); + } + LengthPercentage::Calc(Box::new(self)) + } + + fn specified_percentage(&self) -> Option { + if self.has_percentage { + Some(self.percentage) + } else { + None + } + } + + /// Returns a new `LengthPercentage` with a specific clamping mode. + #[inline] + fn with_clamping_mode( + length: Length, + percentage: Option, + clamping_mode: AllowedNumericType, + ) -> Self { + Self { + clamping_mode, + length, + percentage: percentage.unwrap_or_default(), + has_percentage: percentage.is_some(), + } + } + + /// Returns the length component of this `calc()` + #[inline] + fn length_component(&self) -> CSSPixelLength { + Length::new(self.clamping_mode.clamp(self.length.px())) + } + + /// Returns the percentage component if this could be represented as a + /// non-calc percentage. + fn as_percentage(&self) -> Option { + if !self.has_percentage || self.length.px() != 0. { + return None; + } + + Some(Percentage(self.clamping_mode.clamp(self.percentage.0))) + } + + /// Resolves the percentage. + #[inline] + pub fn resolve(&self, basis: Length) -> Length { + let length = self.length.0 + basis.0 * self.percentage.0; + Length::new(self.clamping_mode.clamp(length)) + } + + /// Resolves the percentage. + #[inline] + pub fn percentage_relative_to(&self, basis: Length) -> Length { + self.resolve(basis) + } + + /// Returns the length, without clamping. + #[inline] + pub fn unclamped_length(&self) -> Length { + self.length + } + + /// Returns true if the computed value is absolute 0 or 0%. + #[inline] + fn is_definitely_zero(&self) -> bool { + self.length.px() == 0.0 && self.percentage.0 == 0.0 + } + + /// Returns the clamped non-negative values. + #[inline] + fn clamp_to_non_negative(self) -> Self { + if self.has_percentage { // If we can eagerly clamp the percentage then just do that. if self.length.is_zero() { return Self::with_clamping_mode( Length::zero(), - Some(p.clamp_to_non_negative()), + Some(self.percentage.clamp_to_non_negative()), AllowedNumericType::NonNegative, ); } - - return Self::with_clamping_mode(self.length, Some(p), AllowedNumericType::NonNegative); + return Self::with_clamping_mode(self.length, Some(self.percentage), AllowedNumericType::NonNegative); } Self::with_clamping_mode( @@ -413,31 +522,41 @@ impl ToComputedValue for specified::LengthPercentage { fn to_computed_value(&self, context: &Context) -> LengthPercentage { match *self { specified::LengthPercentage::Length(ref value) => { - LengthPercentage::new(value.to_computed_value(context), None) + LengthPercentage::Length(value.to_computed_value(context)) + }, + specified::LengthPercentage::Percentage(value) => { + LengthPercentage::Percentage(value) + }, + specified::LengthPercentage::Calc(ref calc) => { + (**calc).to_computed_value(context).to_length_percentge() }, - specified::LengthPercentage::Percentage(value) => LengthPercentage::new_percent(value), - specified::LengthPercentage::Calc(ref calc) => (**calc).to_computed_value(context), } } fn from_computed_value(computed: &LengthPercentage) -> Self { - if let Some(p) = computed.as_percentage() { - return specified::LengthPercentage::Percentage(p); + match *computed { + LengthPercentage::Length(ref l) => { + specified::LengthPercentage::Length(ToComputedValue::from_computed_value(l)) + } + LengthPercentage::Percentage(p) => { + specified::LengthPercentage::Percentage(p) + } + LengthPercentage::Calc(ref c) => { + if let Some(p) = c.as_percentage() { + return specified::LengthPercentage::Percentage(p) + } + if !c.has_percentage { + return specified::LengthPercentage::Length(ToComputedValue::from_computed_value(&c.length_component())) + } + specified::LengthPercentage::Calc(Box::new(specified::CalcLengthPercentage::from_computed_value(c))) + } } - - if !computed.has_percentage { - return specified::LengthPercentage::Length(ToComputedValue::from_computed_value( - &computed.length(), - )); - } - - specified::LengthPercentage::Calc(Box::new(ToComputedValue::from_computed_value(computed))) } } impl Zero for LengthPercentage { fn zero() -> Self { - LengthPercentage::new(Length::zero(), None) + LengthPercentage::Length(Length::zero()) } #[inline] @@ -541,7 +660,7 @@ impl ToAnimatedValue for NonNegativeLengthPercentage { impl From for NonNegativeLengthPercentage { #[inline] fn from(length: NonNegativeLength) -> Self { - NonNegative(LengthPercentage::new(length.0, None)) + NonNegative(LengthPercentage::new_length(length.0)) } } @@ -557,7 +676,7 @@ impl From for NonNegativeLengthPercentage { impl From for LengthPercentage { #[inline] fn from(length: Au) -> Self { - LengthPercentage::new(length.into(), None) + LengthPercentage::new_length(length.into()) } } @@ -572,7 +691,7 @@ impl NonNegativeLengthPercentage { #[inline] pub fn to_used_value(&self, containing_length: Au) -> Au { let resolved = self.0.to_used_value(containing_length); - ::std::cmp::max(resolved, Au(0)) + std::cmp::max(resolved, Au(0)) } /// Convert the computed value into used value. @@ -581,7 +700,7 @@ impl NonNegativeLengthPercentage { let resolved = self .0 .maybe_to_used_value(containing_length.map(|v| v.into()))?; - Some(::std::cmp::max(resolved, Au(0))) + Some(std::cmp::max(resolved, Au(0))) } } diff --git a/components/style/values/specified/gecko.rs b/components/style/values/specified/gecko.rs index 4c85d1df668..3e3085c8849 100644 --- a/components/style/values/specified/gecko.rs +++ b/components/style/values/specified/gecko.rs @@ -23,7 +23,7 @@ fn parse_pixel_or_percent<'i, 't>( value, ref unit, .. } => { match_ignore_ascii_case! { unit, - "px" => Ok(LengthPercentage::new(Length::new(value), None)), + "px" => Ok(LengthPercentage::new_length(Length::new(value))), _ => Err(()), } }, diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 8d35671991d..420133810ed 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -297,7 +297,7 @@ impl ToComputedValue for PositionComponent { let p = Percentage(1. - length.percentage()); let l = -length.unclamped_length(); // We represent ` ` as `calc(100% - )`. - ComputedLengthPercentage::with_clamping_mode(l, Some(p), length.clamping_mode) + ComputedLengthPercentage::new_calc(l, p) }, PositionComponent::Side(_, Some(ref length)) | PositionComponent::Length(ref length) => length.to_computed_value(context), From 4d5bd94a2b5deb2a1817c6351bc42fa5b20bd910 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 13 Jan 2020 13:23:44 +0000 Subject: [PATCH 13/66] style: Move LengthPercentage to its own file. I'm (sadly) about to make it a bit more complicated to pack it better. So we may as well do this so it is easier to reason about navigate. I also reordered things a bit, and removed some From<> implementations and such. Differential Revision: https://phabricator.services.mozilla.com/D58701 --- components/style/values/animated/length.rs | 4 +- components/style/values/computed/length.rs | 568 +----------------- .../values/computed/length_percentage.rs | 559 +++++++++++++++++ components/style/values/computed/mod.rs | 1 + components/style/values/specified/position.rs | 2 +- 5 files changed, 566 insertions(+), 568 deletions(-) create mode 100644 components/style/values/computed/length_percentage.rs diff --git a/components/style/values/animated/length.rs b/components/style/values/animated/length.rs index ece26a9c491..837b3ee86c3 100644 --- a/components/style/values/animated/length.rs +++ b/components/style/values/animated/length.rs @@ -5,7 +5,7 @@ //! Animation implementation for various length-related types. use super::{Animate, Procedure}; -use crate::values::computed::length::{LengthPercentage, CalcLengthPercentage}; +use crate::values::computed::length::LengthPercentage; use crate::values::computed::Percentage; /// @@ -29,6 +29,6 @@ impl Animate for LengthPercentage { // Gets clamped as needed after the animation, so no need to specify any // particular AllowedNumericType. - Ok(CalcLengthPercentage::new(length, percentage).to_length_percentge()) + Ok(LengthPercentage::new_calc(length, percentage)) } } diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index 285d9d2e6d4..d90e4e543b7 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -4,28 +4,26 @@ //! `` computed values, and related ones. -use super::{Context, Number, Percentage, ToComputedValue}; +use super::{Context, Number, ToComputedValue}; use crate::values::animated::ToAnimatedValue; use crate::values::computed::NonNegativeNumber; -use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::generics::length as generics; use crate::values::generics::length::{ GenericLengthOrNumber, GenericLengthPercentageOrNormal, GenericMaxSize, GenericSize, }; use crate::values::generics::NonNegative; -use crate::values::specified::length::ViewportPercentageLength; -use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength}; +use crate::values::specified::length::{AbsoluteLength, FontBaseSize}; use crate::values::{specified, CSSFloat}; use crate::Zero; use app_units::Au; use std::fmt::{self, Write}; use std::ops::{Add, AddAssign, Div, Mul, Neg, Sub}; -use style_traits::values::specified::AllowedNumericType; use style_traits::{CSSPixel, CssWriter, ToCss}; pub use super::image::Image; pub use crate::values::specified::url::UrlOrNone; pub use crate::values::specified::{Angle, BorderStyle, Time}; +pub use super::length_percentage::{LengthPercentage, NonNegativeLengthPercentage}; impl ToComputedValue for specified::NoCalcLength { type ComputedValue = Length; @@ -69,502 +67,6 @@ impl ToComputedValue for specified::Length { } } -/// A `` value. This can be either a ``, a -/// ``, or a combination of both via `calc()`. -/// -/// cbindgen:private-default-tagged-enum-constructor=false -/// cbindgen:derive-mut-casts=true -/// -/// https://drafts.csswg.org/css-values-4/#typedef-length-percentage -#[allow(missing_docs)] -#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, ToAnimatedZero, ToResolvedValue)] -#[repr(u8)] -pub enum LengthPercentage { - Length(Length), - Percentage(Percentage), - Calc(Box), -} - -/// The representation of a calc() function. -#[derive( - Clone, Debug, Deserialize, MallocSizeOf, Serialize, ToAnimatedZero, ToResolvedValue, -)] -#[repr(C)] -pub struct CalcLengthPercentage { - length: Length, - - percentage: Percentage, - - #[animation(constant)] - clamping_mode: AllowedNumericType, - - /// Whether we specified a percentage or not. - #[animation(constant)] - pub has_percentage: bool, -} - -// NOTE(emilio): We don't compare `clamping_mode` since we want to preserve the -// invariant that `from_computed_value(length).to_computed_value(..) == length`. -// -// Right now for e.g. a non-negative length, we set clamping_mode to `All` -// unconditionally for non-calc values, and to `NonNegative` for calc. -// -// If we determine that it's sound, from_computed_value() can generate an -// absolute length, which then would get `All` as the clamping mode. -// -// We may want to just eagerly-detect whether we can clamp in -// `LengthPercentage::new` and switch to `AllowedNumericType::NonNegative` then, -// maybe. -impl PartialEq for CalcLengthPercentage { - fn eq(&self, other: &Self) -> bool { - self.length == other.length && - self.percentage == other.percentage && - self.has_percentage == other.has_percentage - } -} - -impl ComputeSquaredDistance for LengthPercentage { - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - // FIXME(nox): This looks incorrect to me, to add a distance between lengths - // with a distance between percentages. - Ok(self - .unclamped_length() - .compute_squared_distance(&other.unclamped_length())? + - self.percentage() - .compute_squared_distance(&other.percentage())?) - } -} - -impl ToCss for LengthPercentage { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - specified::LengthPercentage::from_computed_value(self).to_css(dest) - } -} - -impl specified::CalcLengthPercentage { - /// Compute the value, zooming any absolute units by the zoom function. - fn to_computed_value_with_zoom( - &self, - context: &Context, - zoom_fn: F, - base_size: FontBaseSize, - ) -> CalcLengthPercentage - where - F: Fn(Length) -> Length, - { - use std::f32; - let mut length = 0.; - - if let Some(absolute) = self.absolute { - length += zoom_fn(absolute.to_computed_value(context)).px(); - } - - for val in &[ - self.vw.map(ViewportPercentageLength::Vw), - self.vh.map(ViewportPercentageLength::Vh), - self.vmin.map(ViewportPercentageLength::Vmin), - self.vmax.map(ViewportPercentageLength::Vmax), - ] { - if let Some(val) = *val { - let viewport_size = context.viewport_size_for_viewport_unit_resolution(); - length += val.to_computed_value(viewport_size).px(); - } - } - - for val in &[ - self.ch.map(FontRelativeLength::Ch), - self.em.map(FontRelativeLength::Em), - self.ex.map(FontRelativeLength::Ex), - self.rem.map(FontRelativeLength::Rem), - ] { - if let Some(val) = *val { - length += val.to_computed_value(context, base_size).px(); - } - } - - CalcLengthPercentage::with_clamping_mode( - Length::new(length.min(f32::MAX).max(f32::MIN)), - self.percentage, - self.clamping_mode, - ) - } - - /// Compute font-size or line-height taking into account text-zoom if necessary. - pub fn to_computed_value_zoomed( - &self, - context: &Context, - base_size: FontBaseSize, - ) -> CalcLengthPercentage { - self.to_computed_value_with_zoom( - context, - |abs| context.maybe_zoom_text(abs.into()), - base_size, - ) - } - - /// Compute the value into pixel length as CSSFloat without context, - /// so it returns Err(()) if there is any non-absolute unit. - pub fn to_computed_pixel_length_without_context(&self) -> Result { - if self.vw.is_some() || - self.vh.is_some() || - self.vmin.is_some() || - self.vmax.is_some() || - self.em.is_some() || - self.ex.is_some() || - self.ch.is_some() || - self.rem.is_some() || - self.percentage.is_some() - { - return Err(()); - } - - match self.absolute { - Some(abs) => Ok(abs.to_px()), - None => { - debug_assert!(false, "Someone forgot to handle an unit here: {:?}", self); - Err(()) - }, - } - } - - fn to_computed_value(&self, context: &Context) -> CalcLengthPercentage { - // normal properties don't zoom, and compute em units against the current style's font-size - self.to_computed_value_with_zoom(context, |abs| abs, FontBaseSize::CurrentStyle) - } - - #[inline] - fn from_computed_value(computed: &CalcLengthPercentage) -> Self { - specified::CalcLengthPercentage { - clamping_mode: computed.clamping_mode, - absolute: Some(AbsoluteLength::from_computed_value(&computed.length)), - percentage: computed.specified_percentage(), - ..Default::default() - } - } -} - -impl LengthPercentage { - /// 1px length value for SVG defaults - #[inline] - pub fn one() -> Self { - Self::Length(Length::new(1.)) - } - - /// Constructs a length value. - #[inline] - pub fn new_length(l: Length) -> Self { - Self::Length(l) - } - - /// Constructs a percentage value. - #[inline] - pub fn new_percent(p: Percentage) -> Self { - Self::Percentage(p) - } - - /// Constructs a `calc()` value. - #[inline] - pub fn new_calc(l: Length, percentage: Percentage) -> Self { - CalcLengthPercentage::new(l, Some(percentage)).to_length_percentge() - } - - /// Returns true if the computed value is absolute 0 or 0%. - #[inline] - pub fn is_definitely_zero(&self) -> bool { - match *self { - Self::Length(l) => l.px() == 0.0, - Self::Percentage(p) => p.0 == 0.0, - Self::Calc(ref c) => c.is_definitely_zero(), - } - } - - /// Returns the `` component of this `calc()`, unclamped. - #[inline] - pub fn unclamped_length(&self) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(..) => Zero::zero(), - Self::Calc(ref c) => c.unclamped_length(), - } - } - - /// Returns this `calc()` as a ``. - /// - /// Panics in debug mode if a percentage is present in the expression. - #[inline] - fn length(&self) -> Length { - debug_assert!(!self.has_percentage()); - self.length_component() - } - - /// Returns the `` component of this `calc()`, clamped. - #[inline] - pub fn length_component(&self) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(..) => Zero::zero(), - Self::Calc(ref c) => c.length_component(), - } - } - - /// Returns the `` component of this `calc()`, unclamped, as a - /// float. - /// - /// FIXME: This are very different semantics from length(), we should - /// probably rename this. - #[inline] - pub fn percentage(&self) -> CSSFloat { - match *self { - Self::Length(..) => 0., - Self::Percentage(p) => p.0, - Self::Calc(ref c) => c.percentage.0, - } - } - - /// Returns the `` component of this `calc()`, clamped. - #[inline] - pub fn as_percentage(&self) -> Option { - match *self { - Self::Length(..) => None, - Self::Percentage(p) => Some(p), - Self::Calc(ref c) => c.as_percentage(), - } - } - - /// Resolves the percentage. - #[inline] - pub fn resolve(&self, basis: Length) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(p) => Length::new(basis.0 * p.0), - Self::Calc(ref c) => c.resolve(basis), - } - } - - /// Resolves the percentage. Just an alias of resolve(). - #[inline] - pub fn percentage_relative_to(&self, basis: Length) -> Length { - self.resolve(basis) - } - - /// Return whether there's any percentage in this value. - #[inline] - pub fn has_percentage(&self) -> bool { - match *self { - Self::Length(..) => false, - Self::Percentage(..) => true, - Self::Calc(ref c) => c.has_percentage, - } - } - - /// Return the specified percentage if any. - #[inline] - pub fn specified_percentage(&self) -> Option { - match *self { - Self::Length(..) => None, - Self::Percentage(p) => Some(p), - Self::Calc(ref c) => c.specified_percentage(), - } - } - - /// Returns the used value. - #[inline] - pub fn to_used_value(&self, containing_length: Au) -> Au { - Au::from(self.to_pixel_length(containing_length)) - } - - /// Returns the used value as CSSPixelLength. - #[inline] - pub fn to_pixel_length(&self, containing_length: Au) -> Length { - self.resolve(containing_length.into()) - } - - /// Convert the computed value into used value. - #[inline] - fn maybe_to_used_value(&self, container_len: Option) -> Option { - self.maybe_percentage_relative_to(container_len).map(Au::from) - } - - /// If there are special rules for computing percentages in a value (e.g. - /// the height property), they apply whenever a calc() expression contains - /// percentages. - pub fn maybe_percentage_relative_to(&self, container_len: Option) -> Option { - if self.has_percentage() { - return Some(self.resolve(container_len?)); - } - Some(self.length()) - } - - /// Returns the clamped non-negative values. - #[inline] - pub fn clamp_to_non_negative(self) -> Self { - match self { - Self::Length(l) => Self::Length(l.clamp_to_non_negative()), - Self::Percentage(p) => Self::Percentage(p.clamp_to_non_negative()), - Self::Calc(c) => c.clamp_to_non_negative().to_length_percentge(), - } - } -} - -impl CalcLengthPercentage { - /// Returns a new `LengthPercentage`. - #[inline] - pub fn new(length: Length, percentage: Option) -> Self { - Self::with_clamping_mode(length, percentage, AllowedNumericType::All) - } - - /// Converts this to a `LengthPercentage`, simplifying if possible. - #[inline] - pub fn to_length_percentge(self) -> LengthPercentage { - if !self.has_percentage { - return LengthPercentage::Length(self.length_component()) - } - if self.length.is_zero() { - return LengthPercentage::Percentage(Percentage(self.clamping_mode.clamp(self.percentage.0))); - } - LengthPercentage::Calc(Box::new(self)) - } - - fn specified_percentage(&self) -> Option { - if self.has_percentage { - Some(self.percentage) - } else { - None - } - } - - /// Returns a new `LengthPercentage` with a specific clamping mode. - #[inline] - fn with_clamping_mode( - length: Length, - percentage: Option, - clamping_mode: AllowedNumericType, - ) -> Self { - Self { - clamping_mode, - length, - percentage: percentage.unwrap_or_default(), - has_percentage: percentage.is_some(), - } - } - - /// Returns the length component of this `calc()` - #[inline] - fn length_component(&self) -> CSSPixelLength { - Length::new(self.clamping_mode.clamp(self.length.px())) - } - - /// Returns the percentage component if this could be represented as a - /// non-calc percentage. - fn as_percentage(&self) -> Option { - if !self.has_percentage || self.length.px() != 0. { - return None; - } - - Some(Percentage(self.clamping_mode.clamp(self.percentage.0))) - } - - /// Resolves the percentage. - #[inline] - pub fn resolve(&self, basis: Length) -> Length { - let length = self.length.0 + basis.0 * self.percentage.0; - Length::new(self.clamping_mode.clamp(length)) - } - - /// Resolves the percentage. - #[inline] - pub fn percentage_relative_to(&self, basis: Length) -> Length { - self.resolve(basis) - } - - /// Returns the length, without clamping. - #[inline] - pub fn unclamped_length(&self) -> Length { - self.length - } - - /// Returns true if the computed value is absolute 0 or 0%. - #[inline] - fn is_definitely_zero(&self) -> bool { - self.length.px() == 0.0 && self.percentage.0 == 0.0 - } - - /// Returns the clamped non-negative values. - #[inline] - fn clamp_to_non_negative(self) -> Self { - if self.has_percentage { - // If we can eagerly clamp the percentage then just do that. - if self.length.is_zero() { - return Self::with_clamping_mode( - Length::zero(), - Some(self.percentage.clamp_to_non_negative()), - AllowedNumericType::NonNegative, - ); - } - return Self::with_clamping_mode(self.length, Some(self.percentage), AllowedNumericType::NonNegative); - } - - Self::with_clamping_mode( - self.length.clamp_to_non_negative(), - None, - AllowedNumericType::NonNegative, - ) - } -} - -impl ToComputedValue for specified::LengthPercentage { - type ComputedValue = LengthPercentage; - - fn to_computed_value(&self, context: &Context) -> LengthPercentage { - match *self { - specified::LengthPercentage::Length(ref value) => { - LengthPercentage::Length(value.to_computed_value(context)) - }, - specified::LengthPercentage::Percentage(value) => { - LengthPercentage::Percentage(value) - }, - specified::LengthPercentage::Calc(ref calc) => { - (**calc).to_computed_value(context).to_length_percentge() - }, - } - } - - fn from_computed_value(computed: &LengthPercentage) -> Self { - match *computed { - LengthPercentage::Length(ref l) => { - specified::LengthPercentage::Length(ToComputedValue::from_computed_value(l)) - } - LengthPercentage::Percentage(p) => { - specified::LengthPercentage::Percentage(p) - } - LengthPercentage::Calc(ref c) => { - if let Some(p) = c.as_percentage() { - return specified::LengthPercentage::Percentage(p) - } - if !c.has_percentage { - return specified::LengthPercentage::Length(ToComputedValue::from_computed_value(&c.length_component())) - } - specified::LengthPercentage::Calc(Box::new(specified::CalcLengthPercentage::from_computed_value(c))) - } - } - } -} - -impl Zero for LengthPercentage { - fn zero() -> Self { - LengthPercentage::Length(Length::zero()) - } - - #[inline] - fn is_zero(&self) -> bool { - self.is_definitely_zero() - } -} - /// Some boilerplate to share between negative and non-negative /// length-percentage or auto. macro_rules! computed_length_percentage_or_auto { @@ -640,70 +142,6 @@ impl NonNegativeLengthPercentageOrAuto { computed_length_percentage_or_auto!(NonNegativeLengthPercentage); } -/// A wrapper of LengthPercentage, whose value must be >= 0. -pub type NonNegativeLengthPercentage = NonNegative; - -impl ToAnimatedValue for NonNegativeLengthPercentage { - type AnimatedValue = LengthPercentage; - - #[inline] - fn to_animated_value(self) -> Self::AnimatedValue { - self.0 - } - - #[inline] - fn from_animated_value(animated: Self::AnimatedValue) -> Self { - NonNegative(animated.clamp_to_non_negative()) - } -} - -impl From for NonNegativeLengthPercentage { - #[inline] - fn from(length: NonNegativeLength) -> Self { - NonNegative(LengthPercentage::new_length(length.0)) - } -} - -impl From for NonNegativeLengthPercentage { - #[inline] - fn from(lp: LengthPercentage) -> Self { - NonNegative(lp) - } -} - -// TODO(emilio): This is a really generic impl which is only needed to implement -// Animated and co for Spacing<>. Get rid of this, probably? -impl From for LengthPercentage { - #[inline] - fn from(length: Au) -> Self { - LengthPercentage::new_length(length.into()) - } -} - -impl NonNegativeLengthPercentage { - /// Returns true if the computed value is absolute 0 or 0%. - #[inline] - pub fn is_definitely_zero(&self) -> bool { - self.0.is_definitely_zero() - } - - /// Returns the used value. - #[inline] - pub fn to_used_value(&self, containing_length: Au) -> Au { - let resolved = self.0.to_used_value(containing_length); - std::cmp::max(resolved, Au(0)) - } - - /// Convert the computed value into used value. - #[inline] - pub fn maybe_to_used_value(&self, containing_length: Option) -> Option { - let resolved = self - .0 - .maybe_to_used_value(containing_length.map(|v| v.into()))?; - Some(std::cmp::max(resolved, Au(0))) - } -} - #[cfg(feature = "servo")] impl MaxSize { /// Convert the computed value into used value. diff --git a/components/style/values/computed/length_percentage.rs b/components/style/values/computed/length_percentage.rs new file mode 100644 index 00000000000..ad108b9487a --- /dev/null +++ b/components/style/values/computed/length_percentage.rs @@ -0,0 +1,559 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ + +//! `` computed values, and related ones. + +use super::{Context, Length, Percentage, ToComputedValue}; +use crate::values::animated::ToAnimatedValue; +use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; +use crate::values::generics::NonNegative; +use crate::values::specified::length::FontBaseSize; +use crate::values::{specified, CSSFloat}; +use crate::Zero; +use app_units::Au; +use std::fmt::{self, Write}; +use style_traits::values::specified::AllowedNumericType; +use style_traits::{CssWriter, ToCss}; + +/// A `` value. This can be either a ``, a +/// ``, or a combination of both via `calc()`. +/// +/// cbindgen:private-default-tagged-enum-constructor=false +/// cbindgen:derive-mut-casts=true +/// +/// https://drafts.csswg.org/css-values-4/#typedef-length-percentage +#[allow(missing_docs)] +#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, ToAnimatedZero, ToResolvedValue)] +#[repr(u8)] +pub enum LengthPercentage { + Length(Length), + Percentage(Percentage), + Calc(Box), +} + +impl LengthPercentage { + /// 1px length value for SVG defaults + #[inline] + pub fn one() -> Self { + Self::new_length(Length::new(1.)) + } + + /// Constructs a length value. + #[inline] + pub fn new_length(l: Length) -> Self { + Self::Length(l) + } + + /// Constructs a percentage value. + #[inline] + pub fn new_percent(p: Percentage) -> Self { + Self::Percentage(p) + } + + /// Constructs a `calc()` value. + #[inline] + pub fn new_calc(l: Length, percentage: Option) -> Self { + CalcLengthPercentage::new(l, percentage).to_length_percentge() + } + + /// Returns true if the computed value is absolute 0 or 0%. + #[inline] + pub fn is_definitely_zero(&self) -> bool { + match *self { + Self::Length(l) => l.px() == 0.0, + Self::Percentage(p) => p.0 == 0.0, + Self::Calc(ref c) => c.is_definitely_zero(), + } + } + + /// Returns the `` component of this `calc()`, unclamped. + #[inline] + pub fn unclamped_length(&self) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(..) => Zero::zero(), + Self::Calc(ref c) => c.unclamped_length(), + } + } + + /// Returns this `calc()` as a ``. + /// + /// Panics in debug mode if a percentage is present in the expression. + #[inline] + fn length(&self) -> Length { + debug_assert!(!self.has_percentage()); + self.length_component() + } + + /// Returns the `` component of this `calc()`, clamped. + #[inline] + pub fn length_component(&self) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(..) => Zero::zero(), + Self::Calc(ref c) => c.length_component(), + } + } + + /// Returns the `` component of this `calc()`, unclamped, as a + /// float. + /// + /// FIXME: This are very different semantics from length(), we should + /// probably rename this. + #[inline] + pub fn percentage(&self) -> CSSFloat { + match *self { + Self::Length(..) => 0., + Self::Percentage(p) => p.0, + Self::Calc(ref c) => c.percentage.0, + } + } + + /// Returns the `` component of this `calc()`, clamped. + #[inline] + pub fn as_percentage(&self) -> Option { + match *self { + Self::Length(..) => None, + Self::Percentage(p) => Some(p), + Self::Calc(ref c) => c.as_percentage(), + } + } + + /// Resolves the percentage. + #[inline] + pub fn resolve(&self, basis: Length) -> Length { + match *self { + Self::Length(l) => l, + Self::Percentage(p) => Length::new(basis.px() * p.0), + Self::Calc(ref c) => c.resolve(basis), + } + } + + /// Resolves the percentage. Just an alias of resolve(). + #[inline] + pub fn percentage_relative_to(&self, basis: Length) -> Length { + self.resolve(basis) + } + + /// Return whether there's any percentage in this value. + #[inline] + pub fn has_percentage(&self) -> bool { + match *self { + Self::Length(..) => false, + Self::Percentage(..) => true, + Self::Calc(ref c) => c.has_percentage, + } + } + + /// Return the specified percentage if any. + #[inline] + pub fn specified_percentage(&self) -> Option { + match *self { + Self::Length(..) => None, + Self::Percentage(p) => Some(p), + Self::Calc(ref c) => c.specified_percentage(), + } + } + + /// Returns the used value. + #[inline] + pub fn to_used_value(&self, containing_length: Au) -> Au { + Au::from(self.to_pixel_length(containing_length)) + } + + /// Returns the used value as CSSPixelLength. + #[inline] + pub fn to_pixel_length(&self, containing_length: Au) -> Length { + self.resolve(containing_length.into()) + } + + /// Convert the computed value into used value. + #[inline] + fn maybe_to_used_value(&self, container_len: Option) -> Option { + self.maybe_percentage_relative_to(container_len).map(Au::from) + } + + /// If there are special rules for computing percentages in a value (e.g. + /// the height property), they apply whenever a calc() expression contains + /// percentages. + pub fn maybe_percentage_relative_to(&self, container_len: Option) -> Option { + if self.has_percentage() { + return Some(self.resolve(container_len?)); + } + Some(self.length()) + } + + /// Returns the clamped non-negative values. + #[inline] + pub fn clamp_to_non_negative(self) -> Self { + match self { + Self::Length(l) => Self::Length(l.clamp_to_non_negative()), + Self::Percentage(p) => Self::Percentage(p.clamp_to_non_negative()), + Self::Calc(c) => c.clamp_to_non_negative().to_length_percentge(), + } + } +} + +impl ToComputedValue for specified::LengthPercentage { + type ComputedValue = LengthPercentage; + + fn to_computed_value(&self, context: &Context) -> LengthPercentage { + match *self { + specified::LengthPercentage::Length(ref value) => { + LengthPercentage::Length(value.to_computed_value(context)) + }, + specified::LengthPercentage::Percentage(value) => { + LengthPercentage::Percentage(value) + }, + specified::LengthPercentage::Calc(ref calc) => { + (**calc).to_computed_value(context).to_length_percentge() + }, + } + } + + fn from_computed_value(computed: &LengthPercentage) -> Self { + match *computed { + LengthPercentage::Length(ref l) => { + specified::LengthPercentage::Length(ToComputedValue::from_computed_value(l)) + } + LengthPercentage::Percentage(p) => { + specified::LengthPercentage::Percentage(p) + } + LengthPercentage::Calc(ref c) => { + if let Some(p) = c.as_percentage() { + return specified::LengthPercentage::Percentage(p) + } + if !c.has_percentage { + return specified::LengthPercentage::Length(ToComputedValue::from_computed_value(&c.length_component())) + } + specified::LengthPercentage::Calc(Box::new(specified::CalcLengthPercentage::from_computed_value(c))) + } + } + } +} + +impl ComputeSquaredDistance for LengthPercentage { + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + // FIXME(nox): This looks incorrect to me, to add a distance between lengths + // with a distance between percentages. + Ok(self + .unclamped_length() + .compute_squared_distance(&other.unclamped_length())? + + self.percentage() + .compute_squared_distance(&other.percentage())?) + } +} + +impl ToCss for LengthPercentage { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + specified::LengthPercentage::from_computed_value(self).to_css(dest) + } +} + +impl Zero for LengthPercentage { + fn zero() -> Self { + LengthPercentage::Length(Length::zero()) + } + + #[inline] + fn is_zero(&self) -> bool { + self.is_definitely_zero() + } +} + +/// The representation of a calc() function. +#[derive( + Clone, Debug, Deserialize, MallocSizeOf, Serialize, ToAnimatedZero, ToResolvedValue, +)] +#[repr(C)] +pub struct CalcLengthPercentage { + length: Length, + + percentage: Percentage, + + #[animation(constant)] + clamping_mode: AllowedNumericType, + + /// Whether we specified a percentage or not. + #[animation(constant)] + pub has_percentage: bool, +} + +impl CalcLengthPercentage { + /// Returns a new `LengthPercentage`. + #[inline] + pub fn new(length: Length, percentage: Option) -> Self { + Self::with_clamping_mode(length, percentage, AllowedNumericType::All) + } + + /// Converts this to a `LengthPercentage`, simplifying if possible. + #[inline] + pub fn to_length_percentge(self) -> LengthPercentage { + if !self.has_percentage { + return LengthPercentage::Length(self.length_component()) + } + if self.length.is_zero() { + return LengthPercentage::Percentage(Percentage(self.clamping_mode.clamp(self.percentage.0))); + } + LengthPercentage::Calc(Box::new(self)) + } + + fn specified_percentage(&self) -> Option { + if self.has_percentage { + Some(self.percentage) + } else { + None + } + } + + /// Returns a new `LengthPercentage` with a specific clamping mode. + #[inline] + fn with_clamping_mode( + length: Length, + percentage: Option, + clamping_mode: AllowedNumericType, + ) -> Self { + Self { + clamping_mode, + length, + percentage: percentage.unwrap_or_default(), + has_percentage: percentage.is_some(), + } + } + + /// Returns the length component of this `calc()`, clamped. + #[inline] + pub fn length_component(&self) -> Length { + Length::new(self.clamping_mode.clamp(self.length.px())) + } + + /// Returns the percentage component if this could be represented as a + /// non-calc percentage. + fn as_percentage(&self) -> Option { + if !self.has_percentage || self.length.px() != 0. { + return None; + } + + Some(Percentage(self.clamping_mode.clamp(self.percentage.0))) + } + + /// Resolves the percentage. + #[inline] + pub fn resolve(&self, basis: Length) -> Length { + let length = self.length.px() + basis.px() * self.percentage.0; + Length::new(self.clamping_mode.clamp(length)) + } + + /// Resolves the percentage. + #[inline] + pub fn percentage_relative_to(&self, basis: Length) -> Length { + self.resolve(basis) + } + + /// Returns the length, without clamping. + #[inline] + pub fn unclamped_length(&self) -> Length { + self.length + } + + /// Returns true if the computed value is absolute 0 or 0%. + #[inline] + fn is_definitely_zero(&self) -> bool { + self.length.px() == 0.0 && self.percentage.0 == 0.0 + } + + /// Returns the clamped non-negative values. + #[inline] + fn clamp_to_non_negative(self) -> Self { + if self.has_percentage { + // If we can eagerly clamp the percentage then just do that. + if self.length.is_zero() { + return Self::with_clamping_mode( + Length::zero(), + Some(self.percentage.clamp_to_non_negative()), + AllowedNumericType::NonNegative, + ); + } + return Self::with_clamping_mode(self.length, Some(self.percentage), AllowedNumericType::NonNegative); + } + + Self::with_clamping_mode( + self.length.clamp_to_non_negative(), + None, + AllowedNumericType::NonNegative, + ) + } +} + +// NOTE(emilio): We don't compare `clamping_mode` since we want to preserve the +// invariant that `from_computed_value(length).to_computed_value(..) == length`. +// +// Right now for e.g. a non-negative length, we set clamping_mode to `All` +// unconditionally for non-calc values, and to `NonNegative` for calc. +// +// If we determine that it's sound, from_computed_value() can generate an +// absolute length, which then would get `All` as the clamping mode. +// +// We may want to just eagerly-detect whether we can clamp in +// `LengthPercentage::new` and switch to `AllowedNumericType::NonNegative` then, +// maybe. +impl PartialEq for CalcLengthPercentage { + fn eq(&self, other: &Self) -> bool { + self.length == other.length && + self.percentage == other.percentage && + self.has_percentage == other.has_percentage + } +} + +impl specified::CalcLengthPercentage { + /// Compute the value, zooming any absolute units by the zoom function. + fn to_computed_value_with_zoom( + &self, + context: &Context, + zoom_fn: F, + base_size: FontBaseSize, + ) -> CalcLengthPercentage + where + F: Fn(Length) -> Length, + { + use std::f32; + use crate::values::specified::length::{ViewportPercentageLength, FontRelativeLength}; + + let mut length = 0.; + + if let Some(absolute) = self.absolute { + length += zoom_fn(absolute.to_computed_value(context)).px(); + } + + for val in &[ + self.vw.map(ViewportPercentageLength::Vw), + self.vh.map(ViewportPercentageLength::Vh), + self.vmin.map(ViewportPercentageLength::Vmin), + self.vmax.map(ViewportPercentageLength::Vmax), + ] { + if let Some(val) = *val { + let viewport_size = context.viewport_size_for_viewport_unit_resolution(); + length += val.to_computed_value(viewport_size).px(); + } + } + + for val in &[ + self.ch.map(FontRelativeLength::Ch), + self.em.map(FontRelativeLength::Em), + self.ex.map(FontRelativeLength::Ex), + self.rem.map(FontRelativeLength::Rem), + ] { + if let Some(val) = *val { + length += val.to_computed_value(context, base_size).px(); + } + } + + CalcLengthPercentage::with_clamping_mode( + Length::new(length.min(f32::MAX).max(f32::MIN)), + self.percentage, + self.clamping_mode, + ) + } + + /// Compute font-size or line-height taking into account text-zoom if necessary. + pub fn to_computed_value_zoomed( + &self, + context: &Context, + base_size: FontBaseSize, + ) -> CalcLengthPercentage { + self.to_computed_value_with_zoom( + context, + |abs| context.maybe_zoom_text(abs.into()), + base_size, + ) + } + + /// Compute the value into pixel length as CSSFloat without context, + /// so it returns Err(()) if there is any non-absolute unit. + pub fn to_computed_pixel_length_without_context(&self) -> Result { + if self.vw.is_some() || + self.vh.is_some() || + self.vmin.is_some() || + self.vmax.is_some() || + self.em.is_some() || + self.ex.is_some() || + self.ch.is_some() || + self.rem.is_some() || + self.percentage.is_some() + { + return Err(()); + } + + match self.absolute { + Some(abs) => Ok(abs.to_px()), + None => { + debug_assert!(false, "Someone forgot to handle an unit here: {:?}", self); + Err(()) + }, + } + } + + /// Compute the calc using the current font-size (and without text-zoom). + pub fn to_computed_value(&self, context: &Context) -> CalcLengthPercentage { + self.to_computed_value_with_zoom(context, |abs| abs, FontBaseSize::CurrentStyle) + } + + #[inline] + fn from_computed_value(computed: &CalcLengthPercentage) -> Self { + use crate::values::specified::length::AbsoluteLength; + + specified::CalcLengthPercentage { + clamping_mode: computed.clamping_mode, + absolute: Some(AbsoluteLength::from_computed_value(&computed.length)), + percentage: computed.specified_percentage(), + ..Default::default() + } + } +} + +/// A wrapper of LengthPercentage, whose value must be >= 0. +pub type NonNegativeLengthPercentage = NonNegative; + +impl ToAnimatedValue for NonNegativeLengthPercentage { + type AnimatedValue = LengthPercentage; + + #[inline] + fn to_animated_value(self) -> Self::AnimatedValue { + self.0 + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + NonNegative(animated.clamp_to_non_negative()) + } +} + +impl NonNegativeLengthPercentage { + /// Returns true if the computed value is absolute 0 or 0%. + #[inline] + pub fn is_definitely_zero(&self) -> bool { + self.0.is_definitely_zero() + } + + /// Returns the used value. + #[inline] + pub fn to_used_value(&self, containing_length: Au) -> Au { + let resolved = self.0.to_used_value(containing_length); + std::cmp::max(resolved, Au(0)) + } + + /// Convert the computed value into used value. + #[inline] + pub fn maybe_to_used_value(&self, containing_length: Option) -> Option { + let resolved = self + .0 + .maybe_to_used_value(containing_length.map(|v| v.into()))?; + Some(std::cmp::max(resolved, Au(0))) + } +} + diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index 958deaaf44e..87c430fb265 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -108,6 +108,7 @@ pub mod flex; pub mod font; pub mod image; pub mod length; +pub mod length_percentage; pub mod list; pub mod motion; pub mod outline; diff --git a/components/style/values/specified/position.rs b/components/style/values/specified/position.rs index 420133810ed..57a6715b0a6 100644 --- a/components/style/values/specified/position.rs +++ b/components/style/values/specified/position.rs @@ -297,7 +297,7 @@ impl ToComputedValue for PositionComponent { let p = Percentage(1. - length.percentage()); let l = -length.unclamped_length(); // We represent ` ` as `calc(100% - )`. - ComputedLengthPercentage::new_calc(l, p) + ComputedLengthPercentage::new_calc(l, Some(p)) }, PositionComponent::Side(_, Some(ref length)) | PositionComponent::Length(ref length) => length.to_computed_value(context), From 88fe64d845feb6a6e3d2ae31b2d45edec2a31628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 13 Jan 2020 13:23:22 +0000 Subject: [PATCH 14/66] style: Pack LengthPercentage better. So that it takes one pointer instead of two, and doesn't make nsStylePosition's size blow up. This is not as ugly as I was fearing, thankfully, though it requires a bit of boilerplate. I think it's acceptable. Differential Revision: https://phabricator.services.mozilla.com/D58702 --- .../values/computed/length_percentage.rs | 372 +++++++++++++++--- components/style/values/resolved/mod.rs | 1 + 2 files changed, 315 insertions(+), 58 deletions(-) diff --git a/components/style/values/computed/length_percentage.rs b/components/style/values/computed/length_percentage.rs index ad108b9487a..bf489197b5a 100644 --- a/components/style/values/computed/length_percentage.rs +++ b/components/style/values/computed/length_percentage.rs @@ -3,19 +3,83 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ //! `` computed values, and related ones. +//! +//! The over-all design is a tagged pointer, with the lower bits of the pointer +//! being non-zero if it is a non-calc value. +//! +//! It is expected to take 64 bits both in x86 and x86-64. This is implemented +//! as a `union`, with 4 different variants: +//! +//! * The length and percentage variants have a { tag, f32 } (effectively) +//! layout. The tag has to overlap with the lower 2 bits of the calc variant. +//! +//! * The `calc()` variant is a { tag, pointer } in x86 (so same as the +//! others), or just a { pointer } in x86-64 (so that the two bits of the tag +//! can be obtained from the lower bits of the pointer). +//! +//! * There's a `tag` variant just to make clear when only the tag is intended +//! to be read. Note that the tag needs to be masked always by `TAG_MASK`, to +//! deal with the pointer variant in x86-64. +//! +//! The assertions in the constructor methods ensure that the tag getter matches +//! our expectations. use super::{Context, Length, Percentage, ToComputedValue}; -use crate::values::animated::ToAnimatedValue; +use crate::values::animated::{ToAnimatedValue, ToAnimatedZero}; use crate::values::distance::{ComputeSquaredDistance, SquaredDistance}; use crate::values::generics::NonNegative; use crate::values::specified::length::FontBaseSize; use crate::values::{specified, CSSFloat}; use crate::Zero; use app_units::Au; +use malloc_size_of::{MallocSizeOf, MallocSizeOfOps}; +use serde::{Serialize, Deserialize}; use std::fmt::{self, Write}; use style_traits::values::specified::AllowedNumericType; use style_traits::{CssWriter, ToCss}; +#[doc(hidden)] +#[derive(Copy, Clone)] +#[repr(C)] +pub struct LengthVariant { + tag: u32, + length: Length, +} + +#[doc(hidden)] +#[derive(Copy, Clone)] +#[repr(C)] +pub struct PercentageVariant { + tag: u32, + percentage: Percentage, +} + +// NOTE(emilio): cbindgen only understands the #[cfg] on the top level +// definition. +#[doc(hidden)] +#[derive(Copy, Clone)] +#[repr(C)] +#[cfg(target_pointer_width = "32")] +pub struct CalcVariant { + tag: u32, + ptr: *mut CalcLengthPercentage, +} + +#[doc(hidden)] +#[derive(Copy, Clone)] +#[repr(C)] +#[cfg(target_pointer_width = "64")] +pub struct CalcVariant { + ptr: *mut CalcLengthPercentage, +} + +#[doc(hidden)] +#[derive(Copy, Clone)] +#[repr(C)] +pub struct TagVariant { + tag: u32, +} + /// A `` value. This can be either a ``, a /// ``, or a combination of both via `calc()`. /// @@ -23,13 +87,87 @@ use style_traits::{CssWriter, ToCss}; /// cbindgen:derive-mut-casts=true /// /// https://drafts.csswg.org/css-values-4/#typedef-length-percentage -#[allow(missing_docs)] -#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize, ToAnimatedZero, ToResolvedValue)] -#[repr(u8)] -pub enum LengthPercentage { +/// +/// The tag is stored in the lower two bits. +/// +/// We need to use a struct instead of the union directly because unions with +/// Drop implementations are unstable, looks like. +/// +/// Also we need the union and the variants to be `pub` (even though the member +/// is private) so that cbindgen generates it. They're not part of the public +/// API otherwise. +#[repr(transparent)] +pub struct LengthPercentage(LengthPercentageUnion); + +#[doc(hidden)] +#[repr(C)] +pub union LengthPercentageUnion { + length: LengthVariant, + percentage: PercentageVariant, + calc: CalcVariant, + tag: TagVariant, +} + +impl LengthPercentageUnion { + #[doc(hidden)] // Need to be public so that cbindgen generates it. + pub const TAG_CALC: u32 = 0; + #[doc(hidden)] + pub const TAG_LENGTH: u32 = 1; + #[doc(hidden)] + pub const TAG_PERCENTAGE: u32 = 2; + #[doc(hidden)] + pub const TAG_MASK: u32 = 0b11; +} + +#[derive(Copy, Clone, Debug, PartialEq)] +#[repr(u32)] +enum Tag { + Calc = LengthPercentageUnion::TAG_CALC, + Length = LengthPercentageUnion::TAG_LENGTH, + Percentage = LengthPercentageUnion::TAG_PERCENTAGE, +} + +// All the members should be 64 bits, even in 32-bit builds. +#[allow(unused)] +unsafe fn static_assert() { + std::mem::transmute::(0u64); + std::mem::transmute::(0u64); + std::mem::transmute::(0u64); + std::mem::transmute::(0u64); +} + +impl Drop for LengthPercentage { + fn drop(&mut self) { + if self.tag() == Tag::Calc { + let _ = unsafe { Box::from_raw(self.0.calc.ptr) }; + } + } +} + +impl MallocSizeOf for LengthPercentage { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + match self.unpack() { + Unpacked::Length(..) | Unpacked::Percentage(..) => 0, + Unpacked::Calc(c) => unsafe { ops.malloc_size_of(c) }, + } + } +} + +/// An unpacked `` that borrows the `calc()` variant. +#[derive(Clone, Debug, PartialEq)] +enum Unpacked<'a> { + Calc(&'a CalcLengthPercentage), + Length(Length), + Percentage(Percentage), +} + +/// An unpacked `` that owns the `calc()` variant, for +/// serialization purposes. +#[derive(Deserialize, Serialize, PartialEq)] +enum Serializable { + Calc(CalcLengthPercentage), Length(Length), Percentage(Percentage), - Calc(Box), } impl LengthPercentage { @@ -41,14 +179,28 @@ impl LengthPercentage { /// Constructs a length value. #[inline] - pub fn new_length(l: Length) -> Self { - Self::Length(l) + pub fn new_length(length: Length) -> Self { + let length = Self(LengthPercentageUnion { + length: LengthVariant { + tag: LengthPercentageUnion::TAG_LENGTH, + length, + } + }); + debug_assert_eq!(length.tag(), Tag::Length); + length } /// Constructs a percentage value. #[inline] - pub fn new_percent(p: Percentage) -> Self { - Self::Percentage(p) + pub fn new_percent(percentage: Percentage) -> Self { + let percent = Self(LengthPercentageUnion { + percentage: PercentageVariant { + tag: LengthPercentageUnion::TAG_PERCENTAGE, + percentage, + } + }); + debug_assert_eq!(percent.tag(), Tag::Percentage); + percent } /// Constructs a `calc()` value. @@ -57,23 +209,77 @@ impl LengthPercentage { CalcLengthPercentage::new(l, percentage).to_length_percentge() } + /// Private version of new_calc() that constructs a calc() variant without + /// checking. + fn new_calc_unchecked(calc: Box) -> Self { + let ptr = Box::into_raw(calc); + let calc = Self(LengthPercentageUnion { + calc: CalcVariant { + #[cfg(target_pointer_width = "32")] + tag: LengthPercentageUnion::TAG_CALC, + ptr, + } + }); + debug_assert_eq!(calc.tag(), Tag::Calc); + calc + } + + #[inline] + fn tag(&self) -> Tag { + match unsafe { self.0.tag.tag & LengthPercentageUnion::TAG_MASK } { + LengthPercentageUnion::TAG_CALC => Tag::Calc, + LengthPercentageUnion::TAG_LENGTH => Tag::Length, + LengthPercentageUnion::TAG_PERCENTAGE => Tag::Percentage, + _ => unreachable!("Bogus tag?"), + } + } + + #[inline] + fn unpack<'a>(&'a self) -> Unpacked<'a> { + unsafe { + match self.tag() { + Tag::Calc => Unpacked::Calc(&*self.0.calc.ptr), + Tag::Length => Unpacked::Length(self.0.length.length), + Tag::Percentage => Unpacked::Percentage(self.0.percentage.percentage), + } + } + } + + #[inline] + fn to_serializable(&self) -> Serializable { + match self.unpack() { + Unpacked::Calc(c) => Serializable::Calc(c.clone()), + Unpacked::Length(l) => Serializable::Length(l), + Unpacked::Percentage(p) => Serializable::Percentage(p), + } + } + + #[inline] + fn from_serializable(s: Serializable) -> Self { + match s { + Serializable::Calc(c) => Self::new_calc_unchecked(Box::new(c)), + Serializable::Length(l) => Self::new_length(l), + Serializable::Percentage(p) => Self::new_percent(p), + } + } + /// Returns true if the computed value is absolute 0 or 0%. #[inline] pub fn is_definitely_zero(&self) -> bool { - match *self { - Self::Length(l) => l.px() == 0.0, - Self::Percentage(p) => p.0 == 0.0, - Self::Calc(ref c) => c.is_definitely_zero(), + match self.unpack() { + Unpacked::Length(l) => l.px() == 0.0, + Unpacked::Percentage(p) => p.0 == 0.0, + Unpacked::Calc(ref c) => c.is_definitely_zero(), } } /// Returns the `` component of this `calc()`, unclamped. #[inline] pub fn unclamped_length(&self) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(..) => Zero::zero(), - Self::Calc(ref c) => c.unclamped_length(), + match self.unpack() { + Unpacked::Length(l) => l, + Unpacked::Percentage(..) => Zero::zero(), + Unpacked::Calc(c) => c.unclamped_length(), } } @@ -89,10 +295,10 @@ impl LengthPercentage { /// Returns the `` component of this `calc()`, clamped. #[inline] pub fn length_component(&self) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(..) => Zero::zero(), - Self::Calc(ref c) => c.length_component(), + match self.unpack() { + Unpacked::Length(l) => l, + Unpacked::Percentage(..) => Zero::zero(), + Unpacked::Calc(c) => c.length_component(), } } @@ -103,30 +309,30 @@ impl LengthPercentage { /// probably rename this. #[inline] pub fn percentage(&self) -> CSSFloat { - match *self { - Self::Length(..) => 0., - Self::Percentage(p) => p.0, - Self::Calc(ref c) => c.percentage.0, + match self.unpack() { + Unpacked::Length(..) => 0., + Unpacked::Percentage(p) => p.0, + Unpacked::Calc(c) => c.percentage.0, } } /// Returns the `` component of this `calc()`, clamped. #[inline] pub fn as_percentage(&self) -> Option { - match *self { - Self::Length(..) => None, - Self::Percentage(p) => Some(p), - Self::Calc(ref c) => c.as_percentage(), + match self.unpack() { + Unpacked::Length(..) => None, + Unpacked::Percentage(p) => Some(p), + Unpacked::Calc(ref c) => c.as_percentage(), } } /// Resolves the percentage. #[inline] pub fn resolve(&self, basis: Length) -> Length { - match *self { - Self::Length(l) => l, - Self::Percentage(p) => Length::new(basis.px() * p.0), - Self::Calc(ref c) => c.resolve(basis), + match self.unpack() { + Unpacked::Length(l) => l, + Unpacked::Percentage(p) => Length::new(basis.px() * p.0), + Unpacked::Calc(ref c) => c.resolve(basis), } } @@ -139,20 +345,20 @@ impl LengthPercentage { /// Return whether there's any percentage in this value. #[inline] pub fn has_percentage(&self) -> bool { - match *self { - Self::Length(..) => false, - Self::Percentage(..) => true, - Self::Calc(ref c) => c.has_percentage, + match self.unpack() { + Unpacked::Length(..) => false, + Unpacked::Percentage(..) => true, + Unpacked::Calc(ref c) => c.has_percentage, } } /// Return the specified percentage if any. #[inline] pub fn specified_percentage(&self) -> Option { - match *self { - Self::Length(..) => None, - Self::Percentage(p) => Some(p), - Self::Calc(ref c) => c.specified_percentage(), + match self.unpack() { + Unpacked::Length(..) => None, + Unpacked::Percentage(p) => Some(p), + Unpacked::Calc(ref c) => c.specified_percentage(), } } @@ -186,11 +392,43 @@ impl LengthPercentage { /// Returns the clamped non-negative values. #[inline] - pub fn clamp_to_non_negative(self) -> Self { - match self { - Self::Length(l) => Self::Length(l.clamp_to_non_negative()), - Self::Percentage(p) => Self::Percentage(p.clamp_to_non_negative()), - Self::Calc(c) => c.clamp_to_non_negative().to_length_percentge(), + pub fn clamp_to_non_negative(&self) -> Self { + match self.unpack() { + Unpacked::Length(l) => Self::new_length(l.clamp_to_non_negative()), + Unpacked::Percentage(p) => Self::new_percent(p.clamp_to_non_negative()), + Unpacked::Calc(c) => c.clamp_to_non_negative().to_length_percentge(), + } + } +} + +impl PartialEq for LengthPercentage { + fn eq(&self, other: &Self) -> bool { + self.unpack() == other.unpack() + } +} + +impl fmt::Debug for LengthPercentage { + fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result { + self.unpack().fmt(formatter) + } +} + +impl ToAnimatedZero for LengthPercentage { + fn to_animated_zero(&self) -> Result { + Ok(match self.unpack() { + Unpacked::Length(l) => Self::new_length(l.to_animated_zero()?), + Unpacked::Percentage(p) => Self::new_percent(p.to_animated_zero()?), + Unpacked::Calc(c) => Self::new_calc_unchecked(Box::new(c.to_animated_zero()?)), + }) + } +} + +impl Clone for LengthPercentage { + fn clone(&self) -> Self { + match self.unpack() { + Unpacked::Length(l) => Self::new_length(l), + Unpacked::Percentage(p) => Self::new_percent(p), + Unpacked::Calc(c) => Self::new_calc_unchecked(Box::new(c.clone())) } } } @@ -201,10 +439,10 @@ impl ToComputedValue for specified::LengthPercentage { fn to_computed_value(&self, context: &Context) -> LengthPercentage { match *self { specified::LengthPercentage::Length(ref value) => { - LengthPercentage::Length(value.to_computed_value(context)) + LengthPercentage::new_length(value.to_computed_value(context)) }, specified::LengthPercentage::Percentage(value) => { - LengthPercentage::Percentage(value) + LengthPercentage::new_percent(value) }, specified::LengthPercentage::Calc(ref calc) => { (**calc).to_computed_value(context).to_length_percentge() @@ -213,14 +451,14 @@ impl ToComputedValue for specified::LengthPercentage { } fn from_computed_value(computed: &LengthPercentage) -> Self { - match *computed { - LengthPercentage::Length(ref l) => { + match computed.unpack() { + Unpacked::Length(ref l) => { specified::LengthPercentage::Length(ToComputedValue::from_computed_value(l)) } - LengthPercentage::Percentage(p) => { + Unpacked::Percentage(p) => { specified::LengthPercentage::Percentage(p) } - LengthPercentage::Calc(ref c) => { + Unpacked::Calc(c) => { if let Some(p) = c.as_percentage() { return specified::LengthPercentage::Percentage(p) } @@ -257,7 +495,7 @@ impl ToCss for LengthPercentage { impl Zero for LengthPercentage { fn zero() -> Self { - LengthPercentage::Length(Length::zero()) + LengthPercentage::new_length(Length::zero()) } #[inline] @@ -266,6 +504,24 @@ impl Zero for LengthPercentage { } } +impl Serialize for LengthPercentage { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.to_serializable().serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for LengthPercentage { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(Self::from_serializable(Serializable::deserialize(deserializer)?)) + } +} + /// The representation of a calc() function. #[derive( Clone, Debug, Deserialize, MallocSizeOf, Serialize, ToAnimatedZero, ToResolvedValue, @@ -295,12 +551,12 @@ impl CalcLengthPercentage { #[inline] pub fn to_length_percentge(self) -> LengthPercentage { if !self.has_percentage { - return LengthPercentage::Length(self.length_component()) + return LengthPercentage::new_length(self.length_component()) } if self.length.is_zero() { - return LengthPercentage::Percentage(Percentage(self.clamping_mode.clamp(self.percentage.0))); + return LengthPercentage::new_percent(Percentage(self.clamping_mode.clamp(self.percentage.0))); } - LengthPercentage::Calc(Box::new(self)) + LengthPercentage::new_calc_unchecked(Box::new(self)) } fn specified_percentage(&self) -> Option { @@ -369,7 +625,7 @@ impl CalcLengthPercentage { /// Returns the clamped non-negative values. #[inline] - fn clamp_to_non_negative(self) -> Self { + fn clamp_to_non_negative(&self) -> Self { if self.has_percentage { // If we can eagerly clamp the percentage then just do that. if self.length.is_zero() { diff --git a/components/style/values/resolved/mod.rs b/components/style/values/resolved/mod.rs index f50f58b5db4..022cb7893c6 100644 --- a/components/style/values/resolved/mod.rs +++ b/components/style/values/resolved/mod.rs @@ -78,6 +78,7 @@ trivial_to_resolved_value!(computed::url::ComputedUrl); trivial_to_resolved_value!(computed::url::ComputedImageUrl); #[cfg(feature = "servo")] trivial_to_resolved_value!(html5ever::Prefix); +trivial_to_resolved_value!(computed::LengthPercentage); impl ToResolvedValue for (A, B) where From c5bd98354d111de7a380835a1d97a43a55b248c1 Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 8 Jan 2020 11:55:28 +0000 Subject: [PATCH 15/66] style: Always use CssEnvironment from media query's device. Although CssEnvironment is in Device of media query implementation, some code creates CssEnvironment instance without Device. So I would like always to use it from Device of media query. Differential Revision: https://phabricator.services.mozilla.com/D52506 --- components/style/properties/declaration_block.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 90dec0a47d7..d51701b2ced 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -778,6 +778,7 @@ impl PropertyDeclarationBlock { dest: &mut CssStringWriter, computed_values: Option<&ComputedValues>, custom_properties_block: Option<&PropertyDeclarationBlock>, + environment: &CssEnvironment, ) -> fmt::Result { if let Ok(shorthand) = property.as_shorthand() { return self.shorthand_to_css(shorthand, dest); @@ -790,19 +791,13 @@ impl PropertyDeclarationBlock { None => return Err(fmt::Error), }; - // TODO(emilio): When we implement any environment variable without - // hard-coding the values we're going to need to get something - // meaningful out of here... All this code path is so terribly hacky - // ;_;. - let env = CssEnvironment; - let custom_properties = if let Some(cv) = computed_values { // If there are extra custom properties for this declaration block, // factor them in too. if let Some(block) = custom_properties_block { // FIXME(emilio): This is not super-efficient here, and all this // feels like a hack anyway... - block.cascade_custom_properties(cv.custom_properties(), &env) + block.cascade_custom_properties(cv.custom_properties(), environment) } else { cv.custom_properties().cloned() } @@ -825,7 +820,7 @@ impl PropertyDeclarationBlock { declaration.id, custom_properties.as_ref(), QuirksMode::NoQuirks, - &env, + environment, ) .to_css(dest) }, From 80a1b6438450b7df5b854112c43f7305b0fa34e2 Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 8 Jan 2020 11:56:25 +0000 Subject: [PATCH 16/66] style: Part 2. Use Device for parameter instead of CssEnvironment. CssEnvironment always is in Device, so use Device as parameter instead of CssEnvironment. Differential Revision: https://phabricator.services.mozilla.com/D52507 --- components/style/custom_properties.rs | 37 ++++++++++--------- components/style/properties/cascade.rs | 4 +- .../style/properties/declaration_block.rs | 14 +++---- .../helpers/animated_properties.mako.rs | 2 +- .../style/properties/properties.mako.rs | 4 +- 5 files changed, 31 insertions(+), 30 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 3717e1f0614..a8ccd96effb 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -7,6 +7,7 @@ //! [custom]: https://drafts.csswg.org/css-variables/ use crate::hash::map::Entry; +use crate::media_queries::Device; use crate::properties::{CSSWideKeyword, CustomDeclaration, CustomDeclarationValue}; use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher}; use crate::stylesheets::{Origin, PerOrigin}; @@ -497,14 +498,14 @@ pub struct CustomPropertiesBuilder<'a> { may_have_cycles: bool, custom_properties: Option, inherited: Option<&'a Arc>, - environment: &'a CssEnvironment, + device: &'a Device, } impl<'a> CustomPropertiesBuilder<'a> { /// Create a new builder, inheriting from a given custom properties map. pub fn new( inherited: Option<&'a Arc>, - environment: &'a CssEnvironment, + device: &'a Device, ) -> Self { Self { seen: PrecomputedHashSet::default(), @@ -512,7 +513,7 @@ impl<'a> CustomPropertiesBuilder<'a> { may_have_cycles: false, custom_properties: None, inherited, - environment, + device, } } @@ -554,7 +555,7 @@ impl<'a> CustomPropertiesBuilder<'a> { // of forcing a full traversal in `substitute_all` afterwards. let value = if !has_references && unparsed_value.references_environment { let result = - substitute_references_in_value(unparsed_value, &map, &self.environment); + substitute_references_in_value(unparsed_value, &map, &self.device); match result { Ok(new_value) => Arc::new(new_value), Err(..) => { @@ -632,7 +633,7 @@ impl<'a> CustomPropertiesBuilder<'a> { None => return self.inherited.cloned(), }; if self.may_have_cycles { - substitute_all(&mut map, self.environment); + substitute_all(&mut map, self.device); } Some(Arc::new(map)) } @@ -641,7 +642,7 @@ impl<'a> CustomPropertiesBuilder<'a> { /// Resolve all custom properties to either substituted or invalid. /// /// It does cycle dependencies removal at the same time as substitution. -fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: &CssEnvironment) { +fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, device: &Device) { // The cycle dependencies removal in this function is a variant // of Tarjan's algorithm. It is mostly based on the pseudo-code // listed in @@ -677,8 +678,8 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: /// all unfinished strong connected components. stack: SmallVec<[usize; 5]>, map: &'a mut CustomPropertiesMap, - /// The environment to substitute `env()` variables. - environment: &'a CssEnvironment, + /// to resolve the environment to substitute `env()` variables. + device: &'a Device, } /// This function combines the traversal for cycle removal and value @@ -813,7 +814,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: // Now we have shown that this variable is not in a loop, and // all of its dependencies should have been resolved. We can // start substitution now. - let result = substitute_references_in_value(&value, &context.map, &context.environment); + let result = substitute_references_in_value(&value, &context.map, &context.device); match result { Ok(computed_value) => { @@ -838,7 +839,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: stack: SmallVec::new(), var_info: SmallVec::new(), map: custom_properties_map, - environment, + device, }; traverse(name, &mut context); } @@ -848,7 +849,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: fn substitute_references_in_value<'i>( value: &'i VariableValue, custom_properties: &CustomPropertiesMap, - environment: &CssEnvironment, + device: &Device, ) -> Result> { debug_assert!(!value.references.is_empty() || value.references_environment); @@ -862,7 +863,7 @@ fn substitute_references_in_value<'i>( &mut position, &mut computed_value, custom_properties, - environment, + device, )?; computed_value.push_from(&input, position, last_token_type)?; @@ -884,7 +885,7 @@ fn substitute_block<'i>( position: &mut (SourcePosition, TokenSerializationType), partial_computed_value: &mut ComputedValue, custom_properties: &CustomPropertiesMap, - env: &CssEnvironment, + device: &Device, ) -> Result> { let mut last_token_type = TokenSerializationType::nothing(); let mut set_position_at_next_iteration = false; @@ -929,7 +930,7 @@ fn substitute_block<'i>( }; let value = if is_env { - env.get(&name) + device.environment().get(&name) } else { custom_properties.get(&name).map(|v| &**v) }; @@ -956,7 +957,7 @@ fn substitute_block<'i>( &mut position, partial_computed_value, custom_properties, - env, + device, )?; partial_computed_value.push_from(input, position, last_token_type)?; } @@ -974,7 +975,7 @@ fn substitute_block<'i>( position, partial_computed_value, custom_properties, - env, + device, ) })?; // It's the same type for CloseCurlyBracket and CloseSquareBracket. @@ -1000,7 +1001,7 @@ pub fn substitute<'i>( input: &'i str, first_token_type: TokenSerializationType, computed_values_map: Option<&Arc>, - env: &CssEnvironment, + device: &Device, ) -> Result> { let mut substituted = ComputedValue::empty(); let mut input = ParserInput::new(input); @@ -1016,7 +1017,7 @@ pub fn substitute<'i>( &mut position, &mut substituted, &custom_properties, - env, + device, )?; substituted.push_from(&input, position, last_token_type)?; Ok(substituted.css) diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index dd87916cf7d..234e06703bc 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -250,7 +250,7 @@ where let custom_properties = { let mut builder = CustomPropertiesBuilder::new( inherited_style.custom_properties(), - device.environment(), + device, ); for (declaration, origin) in iter_declarations() { @@ -424,7 +424,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> { declaration.id, self.context.builder.custom_properties.as_ref(), self.context.quirks_mode, - self.context.device().environment(), + self.context.device(), )) } diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index d51701b2ced..53b483887ce 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -8,7 +8,7 @@ use super::*; use crate::context::QuirksMode; -use crate::custom_properties::{CssEnvironment, CustomPropertiesBuilder}; +use crate::custom_properties::CustomPropertiesBuilder; use crate::error_reporting::{ContextualParseError, ParseErrorReporter}; use crate::parser::ParserContext; use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; @@ -778,7 +778,7 @@ impl PropertyDeclarationBlock { dest: &mut CssStringWriter, computed_values: Option<&ComputedValues>, custom_properties_block: Option<&PropertyDeclarationBlock>, - environment: &CssEnvironment, + device: &Device, ) -> fmt::Result { if let Ok(shorthand) = property.as_shorthand() { return self.shorthand_to_css(shorthand, dest); @@ -797,7 +797,7 @@ impl PropertyDeclarationBlock { if let Some(block) = custom_properties_block { // FIXME(emilio): This is not super-efficient here, and all this // feels like a hack anyway... - block.cascade_custom_properties(cv.custom_properties(), environment) + block.cascade_custom_properties(cv.custom_properties(), device) } else { cv.custom_properties().cloned() } @@ -820,7 +820,7 @@ impl PropertyDeclarationBlock { declaration.id, custom_properties.as_ref(), QuirksMode::NoQuirks, - environment, + device, ) .to_css(dest) }, @@ -868,7 +868,7 @@ impl PropertyDeclarationBlock { ) -> Option> { self.cascade_custom_properties( context.style().custom_properties(), - context.device().environment(), + context.device(), ) } @@ -878,9 +878,9 @@ impl PropertyDeclarationBlock { fn cascade_custom_properties( &self, inherited_custom_properties: Option<&Arc>, - environment: &CssEnvironment, + device: &Device, ) -> Option> { - let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties, environment); + let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties, device); for declaration in self.normal_declaration_iter() { if let PropertyDeclaration::Custom(ref declaration) = *declaration { diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index c81e186a129..84f843ec728 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -507,7 +507,7 @@ impl AnimationValue { declaration.id, custom_properties, context.quirks_mode, - context.device().environment(), + context.device(), ) }; return AnimationValue::from_declaration( diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index e4a07da1c77..d04d6d43d2e 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1584,7 +1584,7 @@ impl UnparsedValue { longhand_id: LonghandId, custom_properties: Option<<&Arc>, quirks_mode: QuirksMode, - environment: &::custom_properties::CssEnvironment, + device: &Device, ) -> PropertyDeclaration { let invalid_at_computed_value_time = || { let keyword = if longhand_id.inherited() { @@ -1602,7 +1602,7 @@ impl UnparsedValue { &self.css, self.first_token_type, custom_properties, - environment, + device, ) { Ok(css) => css, Err(..) => return invalid_at_computed_value_time(), From e9c14bb9fcd2a2f634eb234806865a214b0a75a3 Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Wed, 8 Jan 2020 11:56:25 +0000 Subject: [PATCH 17/66] style: Don't use hardcoded value for safearea. To implement safe area support on Gecko, we should get safe area from Device. Differential Revision: https://phabricator.services.mozilla.com/D52504 --- components/style/custom_properties.rs | 84 +++++++++++++++++-------- components/style/gecko/media_queries.rs | 7 ++- components/style/servo/media_queries.rs | 7 ++- 3 files changed, 69 insertions(+), 29 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index a8ccd96effb..06d0e71cb8b 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -12,7 +12,7 @@ use crate::properties::{CSSWideKeyword, CustomDeclaration, CustomDeclarationValu use crate::selector_map::{PrecomputedHashMap, PrecomputedHashSet, PrecomputedHasher}; use crate::stylesheets::{Origin, PerOrigin}; use crate::Atom; -use cssparser::{Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; +use cssparser::{CowRcStr, Delimiter, Parser, ParserInput, SourcePosition, Token, TokenSerializationType}; use indexmap::IndexMap; use selectors::parser::SelectorParseErrorKind; use servo_arc::Arc; @@ -30,50 +30,52 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; #[derive(Debug, MallocSizeOf)] pub struct CssEnvironment; +type EnvironmentEvaluator = fn(device: &Device) -> VariableValue; + struct EnvironmentVariable { name: Atom, - value: VariableValue, + evaluator: EnvironmentEvaluator, } macro_rules! make_variable { - ($name:expr, $value:expr) => {{ + ($name:expr, $evaluator:expr) => {{ EnvironmentVariable { name: $name, - value: { - // TODO(emilio): We could make this be more efficient (though a - // bit less convenient). - let mut input = ParserInput::new($value); - let mut input = Parser::new(&mut input); - - let (first_token_type, css, last_token_type) = - parse_self_contained_declaration_value(&mut input, None).unwrap(); - - VariableValue { - css: css.into_owned(), - first_token_type, - last_token_type, - references: Default::default(), - references_environment: false, - } - }, + evaluator: $evaluator, } }}; } +fn get_safearea_inset_top(device: &Device) -> VariableValue { + VariableValue::pixel(device.safe_area_insets().top) +} + +fn get_safearea_inset_bottom(device: &Device) -> VariableValue { + VariableValue::pixel(device.safe_area_insets().bottom) +} + +fn get_safearea_inset_left(device: &Device) -> VariableValue { + VariableValue::pixel(device.safe_area_insets().left) +} + +fn get_safearea_inset_right(device: &Device) -> VariableValue { + VariableValue::pixel(device.safe_area_insets().right) +} + lazy_static! { static ref ENVIRONMENT_VARIABLES: [EnvironmentVariable; 4] = [ - make_variable!(atom!("safe-area-inset-top"), "0px"), - make_variable!(atom!("safe-area-inset-bottom"), "0px"), - make_variable!(atom!("safe-area-inset-left"), "0px"), - make_variable!(atom!("safe-area-inset-right"), "0px"), + make_variable!(atom!("safe-area-inset-top"), get_safearea_inset_top), + make_variable!(atom!("safe-area-inset-bottom"), get_safearea_inset_bottom), + make_variable!(atom!("safe-area-inset-left"), get_safearea_inset_left), + make_variable!(atom!("safe-area-inset-right"), get_safearea_inset_right), ]; } impl CssEnvironment { #[inline] - fn get(&self, name: &Atom) -> Option<&VariableValue> { + fn get(&self, name: &Atom, device: &Device) -> Option { let var = ENVIRONMENT_VARIABLES.iter().find(|var| var.name == *name)?; - Some(&var.value) + Some((var.evaluator)(device)) } } @@ -253,6 +255,28 @@ impl VariableValue { references_environment: references.references_environment, })) } + + /// Create VariableValue from css pixel value + pub fn pixel(number: f32) -> Self { + // FIXME (https://github.com/servo/rust-cssparser/issues/266): + // No way to get TokenSerializationType::Dimension without creating + // Token object. + let token = Token::Dimension { + has_sign: false, + value: number, + int_value: None, + unit: CowRcStr::from("px"), + }; + let token_type = token.serialization_type(); + + VariableValue { + css: token.to_css_string(), + first_token_type: token_type, + last_token_type: token_type, + references: Default::default(), + references_environment: false, + } + } } /// Parse the value of a non-custom property that contains `var()` references. @@ -929,8 +953,14 @@ fn substitute_block<'i>( } }; + let env_value; let value = if is_env { - device.environment().get(&name) + if let Some(v) = device.environment().get(&name, device) { + env_value = v; + Some(&env_value) + } else { + None + } } else { custom_properties.get(&name).map(|v| &**v) }; diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 390c1d4fe58..39b99a4fefe 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -16,7 +16,7 @@ use crate::values::{CustomIdent, KeyframesName}; use app_units::{Au, AU_PER_PX}; use cssparser::RGBA; use euclid::default::Size2D; -use euclid::Scale; +use euclid::{Scale, SideOffsets2D}; use servo_arc::Arc; use std::fmt; use std::sync::atomic::{AtomicBool, AtomicIsize, AtomicUsize, Ordering}; @@ -301,4 +301,9 @@ impl Device { pub fn unzoom_text(&self, size: Au) -> Au { size.scale_by(1. / self.effective_text_zoom()) } + + /// Returns safe area insets + pub fn safe_area_insets(&self) -> SideOffsets2D { + SideOffsets2D::zero() + } } diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 4cdfc80eef7..b1e752234e5 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -16,7 +16,7 @@ use crate::values::KeyframesName; use app_units::Au; use cssparser::RGBA; use euclid::default::Size2D as UntypedSize2D; -use euclid::{Scale, Size2D}; +use euclid::{Scale, Size2D, SideOffsets2D}; use std::sync::atomic::{AtomicBool, AtomicIsize, Ordering}; use style_traits::viewport::ViewportConstraints; use style_traits::{CSSPixel, DevicePixel}; @@ -164,6 +164,11 @@ impl Device { pub fn default_background_color(&self) -> RGBA { RGBA::new(255, 255, 255, 255) } + + /// Returns safe area insets + pub fn safe_area_insets(&self) -> SideOffsets2D { + SideOffsets2D::zero() + } } /// https://drafts.csswg.org/mediaqueries-4/#width From c569d314a526074fefc660bb91ac7f53f2b30ff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 14 Jan 2020 20:25:03 +0000 Subject: [PATCH 18/66] style: Do not ignore color: transparent in high contrast mode. PDFJS uses it, for example to allow text selection. It's not great if it shows on top of the actual PDF :-) Differential Revision: https://phabricator.services.mozilla.com/D58703 --- components/style/gecko/media_queries.rs | 5 ++++ components/style/properties/cascade.rs | 36 ++++++++++++++----------- 2 files changed, 25 insertions(+), 16 deletions(-) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index 39b99a4fefe..ae4bad7a25a 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -280,6 +280,11 @@ impl Device { convert_nscolor_to_rgba(self.pref_sheet_prefs().mDefaultBackgroundColor) } + /// Returns the default foreground color. + pub fn default_color(&self) -> RGBA { + convert_nscolor_to_rgba(self.pref_sheet_prefs().mDefaultColor) + } + /// Returns the current effective text zoom. #[inline] fn effective_text_zoom(&self) -> f32 { diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 234e06703bc..f9f17a57121 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -25,7 +25,7 @@ use smallvec::SmallVec; use std::borrow::Cow; use std::cell::RefCell; use crate::style_adjuster::StyleAdjuster; -use crate::values::computed; +use crate::values::{computed, specified}; /// We split the cascade in two phases: 'early' properties, and 'late' /// properties. @@ -365,24 +365,28 @@ fn should_ignore_declaration_when_ignoring_document_colors( // a background image, if we're ignoring document colors). // Here we check backplate status to decide if ignoring background-image // is the right decision. - { - let background_color = match **declaration { - PropertyDeclaration::BackgroundColor(ref color) => color, - // In the future, if/when we remove the backplate pref, we can remove this - // special case along with the 'ignored_when_colors_disabled=True' mako line - // for the "background-image" property. - #[cfg(feature = "gecko")] - PropertyDeclaration::BackgroundImage(..) => return !static_prefs::pref!("browser.display.permit_backplate"), - _ => return true, - }; + let (is_background, is_transparent) = match **declaration { + PropertyDeclaration::BackgroundColor(ref color) => (true, color.is_transparent()), + PropertyDeclaration::Color(ref color) => (false, color.0.is_transparent()), + // In the future, if/when we remove the backplate pref, we can remove this + // special case along with the 'ignored_when_colors_disabled=True' mako line + // for the "background-image" property. + #[cfg(feature = "gecko")] + PropertyDeclaration::BackgroundImage(..) => return !static_prefs::pref!("browser.display.permit_backplate"), + _ => return true, + }; - if background_color.is_transparent() { - return false; - } + if is_transparent { + return false; } - let color = device.default_background_color(); - *declaration.to_mut() = PropertyDeclaration::BackgroundColor(color.into()); + if is_background { + let color = device.default_background_color(); + *declaration.to_mut() = PropertyDeclaration::BackgroundColor(color.into()); + } else { + let color = device.default_color(); + *declaration.to_mut() = PropertyDeclaration::Color(specified::ColorPropertyValue(color.into())); + } false } From 9026720f0477412b271d4e1979041278d6e3a087 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 14 Jan 2020 19:01:05 +0000 Subject: [PATCH 19/66] style: Rewrite to avoid an anonymous input. Instead, subclass nsTextControlFrame. This simplifies the code and avoids correctness issues. I kept the localization functionality though it is not spec compliant. But I filed a bug to remove it in a followup. Differential Revision: https://phabricator.services.mozilla.com/D57193 --- components/style/gecko/wrapper.rs | 17 +---------------- 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index e4b2d7f0be1..9a1196a11b5 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -1869,22 +1869,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> { #[inline] fn pseudo_element_originating_element(&self) -> Option { debug_assert!(self.is_pseudo_element()); - let parent = self.closest_anon_subtree_root_parent()?; - - // FIXME(emilio): Special-case for s - // pseudo-elements, which are nested NAC. Probably nsNumberControlFrame - // should instead inherit from nsTextControlFrame, and then this could - // go away. - if let Some(PseudoElement::MozNumberText) = parent.implemented_pseudo_element() { - debug_assert_eq!( - self.implemented_pseudo_element().unwrap(), - PseudoElement::Placeholder, - "You added a new pseudo, do you really want this?" - ); - return parent.closest_anon_subtree_root_parent(); - } - - Some(parent) + self.closest_anon_subtree_root_parent() } #[inline] From d74f90e3a7c6d6aff05ad50d378ac6b8d8304cbb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 15 Jan 2020 00:46:01 +0000 Subject: [PATCH 20/66] style: Centralize calc function parsing. So that extending it to support other math functions like min / max / etc is simpler. There should be no behavior change with this patch, though I added a comment to some places where we don't do calc() clamping correctly (though other browsers don't either so...). Differential Revision: https://phabricator.services.mozilla.com/D59939 --- components/style/values/specified/angle.rs | 15 ++-- components/style/values/specified/calc.rs | 73 +++++++++++++++---- components/style/values/specified/color.rs | 14 ++-- components/style/values/specified/length.rs | 17 ++--- components/style/values/specified/mod.rs | 28 +++---- .../style/values/specified/percentage.rs | 8 +- components/style/values/specified/time.rs | 19 +++-- 7 files changed, 116 insertions(+), 58 deletions(-) diff --git a/components/style/values/specified/angle.rs b/components/style/values/specified/angle.rs index 458f4178e59..6485bca6f8f 100644 --- a/components/style/values/specified/angle.rs +++ b/components/style/values/specified/angle.rs @@ -208,7 +208,9 @@ impl Angle { input: &mut Parser<'i, 't>, allow_unitless_zero: AllowUnitlessZeroAngle, ) -> Result> { + let location = input.current_source_location(); let t = input.next()?; + let allow_unitless_zero = matches!(allow_unitless_zero, AllowUnitlessZeroAngle::Yes); match *t { Token::Dimension { value, ref unit, .. @@ -221,15 +223,12 @@ impl Angle { }, } }, - Token::Number { value, .. } if value == 0. => match allow_unitless_zero { - AllowUnitlessZeroAngle::Yes => Ok(Angle::zero()), - AllowUnitlessZeroAngle::No => { - let t = t.clone(); - Err(input.new_unexpected_token_error(t)) - }, + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + CalcNode::parse_angle(context, input, function) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - return input.parse_nested_block(|i| CalcNode::parse_angle(context, i)); + Token::Number { value, .. } if value == 0. && allow_unitless_zero => { + Ok(Angle::zero()) }, ref t => { let t = t.clone(); diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index f9feb616fff..d951d0de08e 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -12,11 +12,19 @@ use crate::values::specified::length::ViewportPercentageLength; use crate::values::specified::length::{AbsoluteLength, FontRelativeLength, NoCalcLength}; use crate::values::specified::{Angle, Time}; use crate::values::{CSSFloat, CSSInteger}; -use cssparser::{AngleOrNumber, NumberOrPercentage, Parser, Token}; +use cssparser::{AngleOrNumber, CowRcStr, NumberOrPercentage, Parser, Token}; use std::fmt::{self, Write}; use style_traits::values::specified::AllowedNumericType; use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKind, ToCss}; +/// The name of the mathematical function that we're parsing. +#[derive(Debug, Copy, Clone)] +pub enum MathFunction { + /// `calc()` + Calc, + // FIXME: min() / max() / clamp +} + /// A node inside a `Calc` expression's AST. #[derive(Clone, Debug)] pub enum CalcNode { @@ -204,10 +212,13 @@ impl CalcNode { Ok(CalcNode::Percentage(unit_value)) }, (&Token::ParenthesisBlock, _) => { - input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit)) + input.parse_nested_block(|input| { + CalcNode::parse_argument(context, input, expected_unit) + }) }, - (&Token::Function(ref name), _) if name.eq_ignore_ascii_case("calc") => { - input.parse_nested_block(|i| CalcNode::parse(context, i, expected_unit)) + (&Token::Function(ref name), _) => { + let function = CalcNode::math_function(name, location)?; + CalcNode::parse(context, input, function, expected_unit) }, (t, _) => Err(location.new_unexpected_token_error(t.clone())), } @@ -217,6 +228,20 @@ impl CalcNode { /// /// This is in charge of parsing, for example, `2 + 3 * 100%`. fn parse<'i, 't>( + context: &ParserContext, + input: &mut Parser<'i, 't>, + _function: MathFunction, + expected_unit: CalcUnit, + ) -> Result> { + // TODO: Do something different based on the function name. In + // particular, for non-calc function we need to take a list of + // comma-separated arguments and such. + input.parse_nested_block(|input| { + Self::parse_argument(context, input, expected_unit) + }) + } + + fn parse_argument<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, expected_unit: CalcUnit, @@ -526,12 +551,26 @@ impl CalcNode { }) } + /// Given a function name, and the location from where the token came from, + /// return a mathematical function corresponding to that name or an error. + #[inline] + pub fn math_function<'i>( + name: &CowRcStr<'i>, + location: cssparser::SourceLocation, + ) -> Result> { + if !name.eq_ignore_ascii_case("calc") { + return Err(location.new_unexpected_token_error(Token::Function(name.clone()))); + } + Ok(MathFunction::Calc) + } + /// Convenience parsing function for integers. pub fn parse_integer<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - Self::parse_number(context, input).map(|n| n.round() as CSSInteger) + Self::parse_number(context, input, function).map(|n| n.round() as CSSInteger) } /// Convenience parsing function for ` | `. @@ -539,8 +578,9 @@ impl CalcNode { context: &ParserContext, input: &mut Parser<'i, 't>, clamping_mode: AllowedNumericType, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::LengthPercentage)? + Self::parse(context, input, function, CalcUnit::LengthPercentage)? .to_length_or_percentage(clamping_mode) .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -549,8 +589,9 @@ impl CalcNode { pub fn parse_percentage<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::Percentage)? + Self::parse(context, input, function, CalcUnit::Percentage)? .to_percentage() .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -560,8 +601,9 @@ impl CalcNode { context: &ParserContext, input: &mut Parser<'i, 't>, clamping_mode: AllowedNumericType, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::Length)? + Self::parse(context, input, function, CalcUnit::Length)? .to_length_or_percentage(clamping_mode) .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -570,8 +612,9 @@ impl CalcNode { pub fn parse_number<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::Number)? + Self::parse(context, input, function, CalcUnit::Number)? .to_number() .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -580,8 +623,9 @@ impl CalcNode { pub fn parse_angle<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::Angle)? + Self::parse(context, input, function, CalcUnit::Angle)? .to_angle() .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -590,8 +634,9 @@ impl CalcNode { pub fn parse_time<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - Self::parse(context, input, CalcUnit::Time)? + Self::parse(context, input, function, CalcUnit::Time)? .to_time() .map_err(|()| input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } @@ -600,8 +645,9 @@ impl CalcNode { pub fn parse_number_or_percentage<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - let node = Self::parse(context, input, CalcUnit::Percentage)?; + let node = Self::parse(context, input, function, CalcUnit::Percentage)?; if let Ok(value) = node.to_number() { return Ok(NumberOrPercentage::Number { value }); @@ -617,8 +663,9 @@ impl CalcNode { pub fn parse_angle_or_number<'i, 't>( context: &ParserContext, input: &mut Parser<'i, 't>, + function: MathFunction, ) -> Result> { - let node = Self::parse(context, input, CalcUnit::Angle)?; + let node = Self::parse(context, input, function, CalcUnit::Angle)?; if let Ok(angle) = node.to_angle() { let degrees = angle.degrees(); diff --git a/components/style/values/specified/color.rs b/components/style/values/specified/color.rs index 4e761f95338..be969f27cd4 100644 --- a/components/style/values/specified/color.rs +++ b/components/style/values/specified/color.rs @@ -298,8 +298,9 @@ impl<'a, 'b: 'a, 'i: 'a> ::cssparser::ColorComponentParser<'i> for ColorComponen Ok(AngleOrNumber::Angle { degrees }) }, Token::Number { value, .. } => Ok(AngleOrNumber::Number { value }), - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - input.parse_nested_block(|i| CalcNode::parse_angle_or_number(self.0, i)) + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + CalcNode::parse_angle_or_number(self.0, input, function) }, t => return Err(location.new_unexpected_token_error(t)), } @@ -323,15 +324,16 @@ impl<'a, 'b: 'a, 'i: 'a> ::cssparser::ColorComponentParser<'i> for ColorComponen ) -> Result> { let location = input.current_source_location(); - match input.next()?.clone() { + match *input.next()? { Token::Number { value, .. } => Ok(NumberOrPercentage::Number { value }), Token::Percentage { unit_value, .. } => { Ok(NumberOrPercentage::Percentage { unit_value }) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - input.parse_nested_block(|i| CalcNode::parse_number_or_percentage(self.0, i)) + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + CalcNode::parse_number_or_percentage(self.0, input, function) }, - t => return Err(location.new_unexpected_token_error(t)), + ref t => return Err(location.new_unexpected_token_error(t.clone())), } } } diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs index d9a51665ead..1ef76054382 100644 --- a/components/style/values/specified/length.rs +++ b/components/style/values/specified/length.rs @@ -599,11 +599,11 @@ impl Length { value, )))) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => input - .parse_nested_block(|input| { - CalcNode::parse_length(context, input, num_context) - .map(|calc| Length::Calc(Box::new(calc))) - }), + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let calc = CalcNode::parse_length(context, input, num_context, function)?; + Ok(Length::Calc(Box::new(calc))) + }, ref token => return Err(location.new_unexpected_token_error(token.clone())), } } @@ -822,10 +822,9 @@ impl LengthPercentage { return Ok(LengthPercentage::Length(NoCalcLength::from_px(value))); } }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - let calc = input.parse_nested_block(|i| { - CalcNode::parse_length_or_percentage(context, i, num_context) - })?; + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let calc = CalcNode::parse_length_or_percentage(context, input, num_context, function)?; Ok(LengthPercentage::Calc(Box::new(calc))) }, _ => return Err(location.new_unexpected_token_error(token.clone())), diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 7e16ba09601..8399456a491 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -144,8 +144,9 @@ fn parse_number_with_clamping_mode<'i, 't>( calc_clamping_mode: None, }) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - let result = input.parse_nested_block(|i| CalcNode::parse_number(context, i))?; + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let result = CalcNode::parse_number(context, input, function)?; Ok(Number { value: result.min(f32::MAX).max(f32::MIN), calc_clamping_mode: Some(clamping_mode), @@ -543,8 +544,9 @@ impl Parse for Integer { Token::Number { int_value: Some(v), .. } => Ok(Integer::new(v)), - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - let result = input.parse_nested_block(|i| CalcNode::parse_integer(context, i))?; + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let result = CalcNode::parse_integer(context, input, function)?; Ok(Integer::from_calc(result)) }, ref t => Err(location.new_unexpected_token_error(t.clone())), @@ -559,16 +561,16 @@ impl Integer { input: &mut Parser<'i, 't>, min: i32, ) -> Result> { - match Integer::parse(context, input) { - // FIXME(emilio): The spec asks us to avoid rejecting it at parse - // time except until computed value time. - // - // It's not totally clear it's worth it though, and no other browser - // does this. - Ok(value) if value.value() >= min => Ok(value), - Ok(_value) => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), - Err(e) => Err(e), + let value = Integer::parse(context, input)?; + // FIXME(emilio): The spec asks us to avoid rejecting it at parse + // time except until computed value time. + // + // It's not totally clear it's worth it though, and no other browser + // does this. + if value.value() < min { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } + Ok(value) } /// Parse a non-negative integer. diff --git a/components/style/values/specified/percentage.rs b/components/style/values/specified/percentage.rs index 50ebbb3bcf6..75549dca3be 100644 --- a/components/style/values/specified/percentage.rs +++ b/components/style/values/specified/percentage.rs @@ -117,14 +117,14 @@ impl Percentage { { Ok(Percentage::new(unit_value)) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - let result = - input.parse_nested_block(|i| CalcNode::parse_percentage(context, i))?; + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let value = CalcNode::parse_percentage(context, input, function)?; // TODO(emilio): -moz-image-rect is the only thing that uses // the clamping mode... I guess we could disallow it... Ok(Percentage { - value: result, + value, calc_clamping_mode: Some(num_context), }) }, diff --git a/components/style/values/specified/time.rs b/components/style/values/specified/time.rs index 0006ec45923..aba3f0a828f 100644 --- a/components/style/values/specified/time.rs +++ b/components/style/values/specified/time.rs @@ -69,7 +69,7 @@ impl Time { /// Returns a `Time` value from a CSS `calc()` expression. pub fn from_calc(seconds: CSSFloat) -> Self { Time { - seconds: seconds, + seconds, unit: TimeUnit::Second, was_calc: true, } @@ -95,11 +95,20 @@ impl Time { Time::parse_dimension(value, unit, /* from_calc = */ false) .map_err(|()| location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) }, - Token::Function(ref name) if name.eq_ignore_ascii_case("calc") => { - match input.parse_nested_block(|i| CalcNode::parse_time(context, i)) { - Ok(time) if clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) => Ok(time), - _ => Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)), + Token::Function(ref name) => { + let function = CalcNode::math_function(name, location)?; + let time = CalcNode::parse_time(context, input, function)?; + + // FIXME(emilio): Rejecting calc() at parse time is wrong, + // was_calc should probably be replaced by calc_clamping_mode or + // something like we do for numbers, or we should do the + // clamping here instead (simpler, but technically incorrect, + // though still more correct than this!). + if !clamping_mode.is_ok(ParsingMode::DEFAULT, time.seconds) { + return Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } + + Ok(time) }, ref t => return Err(location.new_unexpected_token_error(t.clone())), } From 6fa03e1d04ce0e4ad5ba0776fdf02b8af28eb74e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 15 Jan 2020 19:43:08 +0000 Subject: [PATCH 21/66] style: Implement min() / max() / clamp() for simple css types behind a pref. So for everything but and , which have more complex mechanics. The pref is off for now of course. Differential Revision: https://phabricator.services.mozilla.com/D60012 --- components/style/values/specified/calc.rs | 284 ++++++++++++---------- 1 file changed, 155 insertions(+), 129 deletions(-) diff --git a/components/style/values/specified/calc.rs b/components/style/values/specified/calc.rs index d951d0de08e..4fb3c009e04 100644 --- a/components/style/values/specified/calc.rs +++ b/components/style/values/specified/calc.rs @@ -20,9 +20,14 @@ use style_traits::{CssWriter, ParseError, SpecifiedValueInfo, StyleParseErrorKin /// The name of the mathematical function that we're parsing. #[derive(Debug, Copy, Clone)] pub enum MathFunction { - /// `calc()` + /// `calc()`: https://drafts.csswg.org/css-values-4/#funcdef-calc Calc, - // FIXME: min() / max() / clamp + /// `min()`: https://drafts.csswg.org/css-values-4/#funcdef-min + Min, + /// `max()`: https://drafts.csswg.org/css-values-4/#funcdef-max + Max, + /// `clamp()`: https://drafts.csswg.org/css-values-4/#funcdef-clamp + Clamp, } /// A node inside a `Calc` expression's AST. @@ -46,6 +51,19 @@ pub enum CalcNode { Mul(Box, Box), /// An expression of the form `x / y` Div(Box, Box), + /// A `min()` function. + Min(Box<[CalcNode]>), + /// A `max()` function. + Max(Box<[CalcNode]>), + /// A `clamp()` function. + Clamp { + /// The minimum value. + min: Box, + /// The central value. + center: Box, + /// The maximum value. + max: Box, + }, } /// An expected unit we intend to parse within a `calc()` expression. @@ -158,6 +176,80 @@ impl ToCss for CalcLengthPercentage { impl SpecifiedValueInfo for CalcLengthPercentage {} +macro_rules! impl_generic_to_type { + ($self:ident, $self_variant:ident, $to_self:ident, $to_float:ident, $from_float:path) => {{ + if let Self::$self_variant(ref v) = *$self { + return Ok(v.clone()); + } + + Ok(match *$self { + Self::Sub(ref a, ref b) => $from_float(a.$to_self()?.$to_float() - b.$to_self()?.$to_float()), + Self::Sum(ref a, ref b) => $from_float(a.$to_self()?.$to_float() + b.$to_self()?.$to_float()), + Self::Mul(ref a, ref b) => match a.$to_self() { + Ok(lhs) => { + let rhs = b.to_number()?; + $from_float(lhs.$to_float() * rhs) + }, + Err(..) => { + let lhs = a.to_number()?; + let rhs = b.$to_self()?; + $from_float(lhs * rhs.$to_float()) + }, + }, + Self::Div(ref a, ref b) => { + let lhs = a.$to_self()?; + let rhs = b.to_number()?; + if rhs == 0. { + return Err(()); + } + $from_float(lhs.$to_float() / rhs) + }, + Self::Clamp { ref min, ref center, ref max } => { + let min = min.$to_self()?; + let center = center.$to_self()?; + let max = max.$to_self()?; + + // Equivalent to cmp::max(min, cmp::min(center, max)) + // + // But preserving units when appropriate. + let mut result = center; + if result.$to_float() > max.$to_float() { + result = max; + } + if result.$to_float() < min.$to_float() { + result = min; + } + result + }, + Self::Min(ref nodes) => { + let mut min = nodes[0].$to_self()?; + for node in nodes.iter().skip(1) { + let candidate = node.$to_self()?; + if candidate.$to_float() < min.$to_float() { + min = candidate; + } + } + min + }, + Self::Max(ref nodes) => { + let mut max = nodes[0].$to_self()?; + for node in nodes.iter().skip(1) { + let candidate = node.$to_self()?; + if candidate.$to_float() > max.$to_float() { + max = candidate; + } + } + max + }, + Self::Length(..) | + Self::Angle(..) | + Self::Time(..) | + Self::Percentage(..) | + Self::Number(..) => return Err(()), + }) + }} +} + impl CalcNode { /// Tries to parse a single element in the expression, that is, a /// ``, ``, `