From 26040d1b0015599881f3282a15dbc67adc9486f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Wed, 24 Oct 2018 20:32:16 +0000 Subject: [PATCH 01/32] style: Remove unused style constant. Also remove specified-value-only keywords, since those are handled only in Rust code and C++ doesn't need to know about them. Differential Revision: https://phabricator.services.mozilla.com/D9634 --- components/style/values/specified/text.rs | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/components/style/values/specified/text.rs b/components/style/values/specified/text.rs index adeab96c984..6bd0af23eb2 100644 --- a/components/style/values/specified/text.rs +++ b/components/style/values/specified/text.rs @@ -488,12 +488,7 @@ impl TextAlign { /// Convert an enumerated value coming from Gecko to a `TextAlign`. #[cfg(feature = "gecko")] pub fn from_gecko_keyword(kw: u32) -> Self { - use gecko_bindings::structs::NS_STYLE_TEXT_ALIGN_MATCH_PARENT; - if kw == NS_STYLE_TEXT_ALIGN_MATCH_PARENT { - TextAlign::MatchParent - } else { - TextAlign::Keyword(TextAlignKeyword::from_gecko_keyword(kw)) - } + TextAlign::Keyword(TextAlignKeyword::from_gecko_keyword(kw)) } } From a9af61e3be3edcb247bce55173d2504fb206419a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 28 Oct 2018 23:26:53 +0000 Subject: [PATCH 02/32] style: Don't allow auto in grid line names. See https://github.com/w3c/csswg-drafts/issues/2856. Differential Revision: https://phabricator.services.mozilla.com/D9882 --- components/style/values/generics/grid.rs | 4 +++- components/style/values/specified/grid.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/style/values/generics/grid.rs b/components/style/values/generics/grid.rs index 56b6f463691..13c7fa54200 100644 --- a/components/style/values/generics/grid.rs +++ b/components/style/values/generics/grid.rs @@ -120,7 +120,9 @@ impl Parse for GridLine { if val_before_span || grid_line.ident.is_some() { return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)); } - grid_line.ident = Some(CustomIdent::from_ident(location, &name, &[])?); + // NOTE(emilio): `span` is consumed above, so we only need to + // reject `auto`. + grid_line.ident = Some(CustomIdent::from_ident(location, &name, &["auto"])?); } else { break; } diff --git a/components/style/values/specified/grid.rs b/components/style/values/specified/grid.rs index ad95264c595..ec00393febd 100644 --- a/components/style/values/specified/grid.rs +++ b/components/style/values/specified/grid.rs @@ -95,7 +95,7 @@ pub fn parse_line_names<'i, 't>( while let Ok((loc, ident)) = input.try(|i| -> Result<_, CssParseError<()>> { Ok((i.current_source_location(), i.expect_ident_cloned()?)) }) { - let ident = CustomIdent::from_ident(loc, &ident, &["span"])?; + let ident = CustomIdent::from_ident(loc, &ident, &["span", "auto"])?; values.push(ident); } From a3fcfb6435fa3cde019ed4c7c69e604225b45a89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 29 Oct 2018 12:04:25 +0000 Subject: [PATCH 03/32] style: Add a fast path for querySelector{,All} when we have classes or tags in the rightmost compound. Before this patch we were only optimizing the case of a single selector, which is fine, but not enough to catch ones like .foo .bar or so. This patch allows us to optimize classes and tags in the rightmost compound, while keeping the current optimization for #id selectors. Need to profile this, but code-wise should be ready for review. Differential Revision: https://phabricator.services.mozilla.com/D9351 --- components/style/dom_apis.rs | 97 +++++++++++++++++++++++++++++++----- 1 file changed, 84 insertions(+), 13 deletions(-) diff --git a/components/style/dom_apis.rs b/components/style/dom_apis.rs index 9d7ab917a91..b7a5fbd24c0 100644 --- a/components/style/dom_apis.rs +++ b/components/style/dom_apis.rs @@ -13,7 +13,7 @@ use invalidation::element::invalidator::{InvalidationProcessor, InvalidationVect use selectors::{Element, NthIndexCache, SelectorList}; use selectors::attr::CaseSensitivity; use selectors::matching::{self, MatchingContext, MatchingMode}; -use selectors::parser::{Combinator, Component, LocalName}; +use selectors::parser::{Combinator, Component, LocalName, SelectorImpl}; use smallvec::SmallVec; use std::borrow::Borrow; @@ -333,6 +333,19 @@ fn collect_elements_with_id( } } +#[inline(always)] +fn local_name_matches(element: E, local_name: &LocalName) -> bool +where + E: TElement, +{ + let LocalName { ref name, ref lower_name } = *local_name; + if element.is_html_element_in_html_document() { + element.local_name() == lower_name.borrow() + } else { + element.local_name() == name.borrow() + } +} + /// Fast paths for querySelector with a single simple selector. fn query_selector_single_query( root: E::ConcreteNode, @@ -357,16 +370,11 @@ where element.has_class(class, case_sensitivity) }) }, - Component::LocalName(LocalName { - ref name, - ref lower_name, - }) => collect_all_elements::(root, results, |element| { - if element.is_html_element_in_html_document() { - element.local_name() == lower_name.borrow() - } else { - element.local_name() == name.borrow() - } - }), + Component::LocalName(ref local_name) => { + collect_all_elements::(root, results, |element| { + local_name_matches(element, local_name) + }) + }, // TODO(emilio): More fast paths? _ => return Err(()), } @@ -374,8 +382,18 @@ where Ok(()) } +enum SimpleFilter<'a, Impl: SelectorImpl> { + Class(&'a Atom), + LocalName(&'a LocalName), +} + /// Fast paths for a given selector query. /// +/// When there's only one component, we go directly to +/// `query_selector_single_query`, otherwise, we try to optimize by looking just +/// at the subtrees rooted at ids in the selector, and otherwise we try to look +/// up by class name or local name in the rightmost compound. +/// /// FIXME(emilio, nbp): This may very well be a good candidate for code to be /// replaced by HolyJit :) fn query_selector_fast( @@ -410,7 +428,12 @@ where let mut iter = selector.iter(); let mut combinator: Option = None; - loop { + // We want to optimize some cases where there's no id involved whatsoever, + // like `.foo .bar`, but we don't want to make `#foo .bar` slower because of + // that. + let mut simple_filter = None; + + 'selector_loop: loop { debug_assert!(combinator.map_or(true, |c| !c.is_sibling())); 'component_loop: for component in &mut iter { @@ -469,13 +492,28 @@ where return Ok(()); }, + Component::Class(ref class) => { + if combinator.is_none() { + simple_filter = Some(SimpleFilter::Class(class)); + } + }, + Component::LocalName(ref local_name) => { + if combinator.is_none() { + // Prefer to look at class rather than local-name if + // both are present. + if let Some(SimpleFilter::Class(..)) = simple_filter { + continue; + } + simple_filter = Some(SimpleFilter::LocalName(local_name)); + } + }, _ => {}, } } loop { let next_combinator = match iter.next_sequence() { - None => return Err(()), + None => break 'selector_loop, Some(c) => c, }; @@ -492,6 +530,39 @@ where break; } } + + // We got here without finding any ID or such that we could handle. Try to + // use one of the simple filters. + let simple_filter = match simple_filter { + Some(f) => f, + None => return Err(()), + }; + + match simple_filter { + SimpleFilter::Class(ref class) => { + let case_sensitivity = quirks_mode.classes_and_ids_case_sensitivity(); + collect_all_elements::(root, results, |element| { + element.has_class(class, case_sensitivity) && + matching::matches_selector_list( + selector_list, + &element, + matching_context, + ) + }); + } + SimpleFilter::LocalName(ref local_name) => { + collect_all_elements::(root, results, |element| { + local_name_matches(element, local_name) && + matching::matches_selector_list( + selector_list, + &element, + matching_context, + ) + }); + } + } + + Ok(()) } // Slow path for a given selector query. From 62aaf865aa6ea1346066c04e9a79bdc08859cd5f Mon Sep 17 00:00:00 2001 From: Sean Voisen Date: Mon, 29 Oct 2018 10:38:50 +0000 Subject: [PATCH 04/32] style: Ignore border-image-source when overriding document colors. Differential Revision: https://phabricator.services.mozilla.com/D10017 --- components/style/properties/longhands/border.mako.rs | 1 + components/style/rule_tree/mod.rs | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/components/style/properties/longhands/border.mako.rs b/components/style/properties/longhands/border.mako.rs index f4b9187b88a..2f455a1716b 100644 --- a/components/style/properties/longhands/border.mako.rs +++ b/components/style/properties/longhands/border.mako.rs @@ -112,6 +112,7 @@ ${helpers.predefined_type( animation_value_type="discrete", flags="APPLIES_TO_FIRST_LETTER", boxed=product == "servo", + ignored_when_colors_disabled=True )} ${helpers.predefined_type( diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index ff8c1cc86de..7277afd4d9f 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -1289,10 +1289,11 @@ impl StrongRuleNode { } // If author colors are not allowed, only claim to have author-specified - // rules if we're looking at a non-color property or if we're looking at - // the background color and it's set to transparent. + // rules if we're looking at a non-color property, a border image, or if + // we're looking at the background color and it's set to transparent. const IGNORED_WHEN_COLORS_DISABLED: &'static [LonghandId] = &[ LonghandId::BackgroundImage, + LonghandId::BorderImageSource, LonghandId::BorderTopColor, LonghandId::BorderRightColor, LonghandId::BorderBottomColor, From badb8f398aefd244febfaf09a63eb68ec1a09470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 29 Oct 2018 23:34:12 +0100 Subject: [PATCH 05/32] style: Support ::before / ::after on ::slotted pseudos. See https://github.com/w3c/csswg-drafts/issues/3150 for the issue that would expand this to all pseudos. Differential Revision: https://phabricator.services.mozilla.com/D9994 --- components/selectors/parser.rs | 168 +++++++++++++++-------- components/style/gecko/pseudo_element.rs | 8 ++ 2 files changed, 115 insertions(+), 61 deletions(-) diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index fc4b26bb3a3..3e82a49b348 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -36,6 +36,11 @@ pub trait PseudoElement: Sized + ToCss { ) -> bool { false } + + /// Whether this pseudo-element is valid after a ::slotted(..) pseudo. + fn valid_after_slotted(&self) -> bool { + false + } } /// A trait that represents a pseudo-class. @@ -69,6 +74,8 @@ pub enum SelectorParseErrorKind<'i> { DanglingCombinator, NonSimpleSelectorInNegation, NonCompoundSelector, + NonPseudoElementAfterSlotted, + InvalidPseudoElementAfterSlotted, UnexpectedTokenInAttributeSelector(Token<'i>), PseudoElementExpectedColon(Token<'i>), PseudoElementExpectedIdent(Token<'i>), @@ -1832,7 +1839,6 @@ where input.skip_whitespace(); let mut empty = true; - let mut slot = false; if !parse_type_selector(parser, input, builder)? { if let Some(url) = parser.default_namespace() { // If there was no explicit type selector, but there is a @@ -1845,6 +1851,7 @@ where } let mut pseudo = false; + let mut slot = false; loop { let parse_result = match parse_one_simple_selector(parser, input, /* inside_negation = */ false)? { @@ -1852,75 +1859,111 @@ where Some(result) => result, }; + empty = false; + + let slotted_selector; + let pseudo_element; + match parse_result { SimpleSelectorParseResult::SimpleSelector(s) => { builder.push_simple_selector(s); - empty = false + continue; }, SimpleSelectorParseResult::PseudoElement(p) => { - // Try to parse state to its right. There are only 3 allowable - // state selectors that can go on pseudo-elements. - let mut state_selectors = SmallVec::<[Component; 3]>::new(); + slotted_selector = None; + pseudo_element = Some(p); + } + SimpleSelectorParseResult::SlottedPseudo(selector) => { + slotted_selector = Some(selector); + let maybe_pseudo = parse_one_simple_selector( + parser, + input, + /* inside_negation = */ false, + )?; - loop { - let location = input.current_source_location(); - match input.next_including_whitespace() { - Ok(&Token::Colon) => {}, - Ok(&Token::WhiteSpace(_)) | Err(_) => break, - Ok(t) => { - let e = SelectorParseErrorKind::PseudoElementExpectedColon(t.clone()); - return Err(location.new_custom_error(e)); - }, + pseudo_element = match maybe_pseudo { + None => None, + Some(SimpleSelectorParseResult::PseudoElement(pseudo)) => { + if !pseudo.valid_after_slotted() { + return Err(input.new_custom_error( + SelectorParseErrorKind::InvalidPseudoElementAfterSlotted + )); + } + Some(pseudo) } - - let location = input.current_source_location(); - // TODO(emilio): Functional pseudo-classes too? - // We don't need it for now. - let name = match input.next_including_whitespace()? { - &Token::Ident(ref name) => name.clone(), - t => { - return Err(location.new_custom_error( - SelectorParseErrorKind::NoIdentForPseudo(t.clone()), - )) - }, - }; - - let pseudo_class = - P::parse_non_ts_pseudo_class(parser, location, name.clone())?; - if !p.supports_pseudo_class(&pseudo_class) { + Some(SimpleSelectorParseResult::SimpleSelector(..)) | + Some(SimpleSelectorParseResult::SlottedPseudo(..)) => { return Err(input.new_custom_error( - SelectorParseErrorKind::UnsupportedPseudoClassOrElement(name), + SelectorParseErrorKind::NonPseudoElementAfterSlotted )); } - state_selectors.push(Component::NonTSPseudoClass(pseudo_class)); - } - - if !builder.is_empty() { - builder.push_combinator(Combinator::PseudoElement); - } - - builder.push_simple_selector(Component::PseudoElement(p)); - for state_selector in state_selectors.drain() { - builder.push_simple_selector(state_selector); - } - - pseudo = true; - empty = false; - break; - }, - SimpleSelectorParseResult::SlottedPseudo(selector) => { - empty = false; - slot = true; - if !builder.is_empty() { - builder.push_combinator(Combinator::SlotAssignment); - } - builder.push_simple_selector(Component::Slotted(selector)); - // FIXME(emilio): ::slotted() should support ::before and - // ::after after it, so we shouldn't break, but we shouldn't - // push more type selectors either. - break; - }, + }; + } } + + debug_assert!(slotted_selector.is_some() || pseudo_element.is_some()); + // Try to parse state to the right of the pseudo-element. + // + // There are only 3 allowable state selectors that can go on + // pseudo-elements as of right now. + let mut state_selectors = SmallVec::<[Component; 3]>::new(); + if let Some(ref p) = pseudo_element { + loop { + let location = input.current_source_location(); + match input.next_including_whitespace() { + Ok(&Token::Colon) => {}, + Ok(&Token::WhiteSpace(_)) | Err(_) => break, + Ok(t) => { + let e = SelectorParseErrorKind::PseudoElementExpectedColon(t.clone()); + return Err(location.new_custom_error(e)); + }, + } + + let location = input.current_source_location(); + // TODO(emilio): Functional pseudo-classes too? + // We don't need it for now. + let name = match input.next_including_whitespace()? { + &Token::Ident(ref name) => name.clone(), + t => { + return Err(location.new_custom_error( + SelectorParseErrorKind::NoIdentForPseudo(t.clone()), + )) + }, + }; + + let pseudo_class = + P::parse_non_ts_pseudo_class(parser, location, name.clone())?; + if !p.supports_pseudo_class(&pseudo_class) { + return Err(input.new_custom_error( + SelectorParseErrorKind::UnsupportedPseudoClassOrElement(name), + )); + } + state_selectors.push(Component::NonTSPseudoClass(pseudo_class)); + } + } + + if let Some(slotted) = slotted_selector { + slot = true; + if !builder.is_empty() { + builder.push_combinator(Combinator::SlotAssignment); + } + builder.push_simple_selector(Component::Slotted(slotted)); + } + + if let Some(p) = pseudo_element { + pseudo = true; + if !builder.is_empty() { + builder.push_combinator(Combinator::PseudoElement); + } + + builder.push_simple_selector(Component::PseudoElement(p)); + + for state_selector in state_selectors.drain() { + builder.push_simple_selector(state_selector); + } + } + + break; } if empty { // An empty selector is invalid. @@ -2133,6 +2176,10 @@ pub mod tests { PseudoClass::Active | PseudoClass::Lang(..) => false, } } + + fn valid_after_slotted(&self) -> bool { + true + } } impl parser::NonTSPseudoClass for PseudoClass { @@ -2818,8 +2865,7 @@ pub mod tests { assert!(parse("div + slot::slotted(div)").is_ok()); assert!(parse("div + slot::slotted(div.foo)").is_ok()); assert!(parse("slot::slotted(div,foo)::first-line").is_err()); - // TODO - assert!(parse("::slotted(div)::before").is_err()); + assert!(parse("::slotted(div)::before").is_ok()); assert!(parse("slot::slotted(div,foo)").is_err()); } diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index 23fea10189f..1164a39bdc4 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -27,6 +27,14 @@ include!(concat!( impl ::selectors::parser::PseudoElement for PseudoElement { type Impl = SelectorImpl; + fn valid_after_slotted(&self) -> bool { + // TODO(emilio): Remove this function or this comment after [1] is + // resolved. + // + // [1]: https://github.com/w3c/csswg-drafts/issues/3150 + self.is_before_or_after() + } + fn supports_pseudo_class(&self, pseudo_class: &NonTSPseudoClass) -> bool { if !self.supports_user_action_state() { return false; From edf6e6a54f0f0ce4c749c0a1f7dd974e16220dbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 30 Oct 2018 21:25:17 +0000 Subject: [PATCH 06/32] style: Update cbindgen due to breaking change. https://github.com/eqrion/cbindgen/pull/233 changed the way one of the options we use work. I think the new behavior is better, but we should do this sooner rather than later, and fix broken builds. Differential Revision: https://phabricator.services.mozilla.com/D10301 --- components/style/cbindgen.toml | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index 1c8d14e16e8..50252690054 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -38,19 +38,19 @@ derive_helper_methods = true [export] prefix = "Style" include = [ - "StyleAppearance", - "StyleComputedFontStretchRange", - "StyleComputedFontStyleDescriptor", - "StyleComputedFontWeightRange", - "StyleComputedTimingFunction", - "StyleDisplay", - "StyleDisplayMode", - "StyleFillRule", - "StyleFontDisplay", - "StyleFontFaceSourceListComponent", - "StyleFontLanguageOverride", - "StyleTimingFunction", - "StylePathCommand", - "StyleUnicodeRange", + "Appearance", + "ComputedFontStretchRange", + "ComputedFontStyleDescriptor", + "ComputedFontWeightRange", + "ComputedTimingFunction", + "Display", + "DisplayMode", + "FillRule", + "FontDisplay", + "FontFaceSourceListComponent", + "FontLanguageOverride", + "TimingFunction", + "PathCommand", + "UnicodeRange", ] item_types = ["enums", "structs", "typedefs"] From 8bc8a0bfa01679ea8505f7734bc05360fb491504 Mon Sep 17 00:00:00 2001 From: Brian Birtles Date: Tue, 30 Oct 2018 19:57:46 +0900 Subject: [PATCH 07/32] style: Interpolate the angle between mis-matched rotate() functions when the angle of one is zero. Bug: 1501176 Reviewed-by: hiro --- .../helpers/animated_properties.mako.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index b079f30b1a8..c8e72ca1df4 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -2351,10 +2351,21 @@ impl Animate for ComputedRotate { let from = ComputedRotate::resolve(self); let to = ComputedRotate::resolve(other); - let (fx, fy, fz, fa) = + let (mut fx, mut fy, mut fz, fa) = transform::get_normalized_vector_and_angle(from.0, from.1, from.2, from.3); - let (tx, ty, tz, ta) = + let (mut tx, mut ty, mut tz, ta) = transform::get_normalized_vector_and_angle(to.0, to.1, to.2, to.3); + + if fa == Angle::from_degrees(0.) { + fx = tx; + fy = ty; + fz = tz; + } else if ta == Angle::from_degrees(0.) { + tx = fx; + ty = fy; + tz = fz; + } + if (fx, fy, fz) == (tx, ty, tz) { return Ok(Rotate::Rotate3D(fx, fy, fz, fa.animate(&ta, procedure)?)); } From b00bbb3be7f18be985d6c88e0fdc0f046993586c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 06:19:52 +0000 Subject: [PATCH 08/32] style: Add a special list for cbindgen types to avoid generating redundant rust types. We will blacklist this type and add a module raw line to map the gecko type to its rust type (as an alias). Differential Revision: https://phabricator.services.mozilla.com/D10303 --- components/style/build_gecko.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index bee061fcc21..3e4200d28d0 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -392,6 +392,12 @@ mod bindings { .handle_str_items("whitelist-vars", |b, item| b.whitelist_var(item)) .handle_str_items("whitelist-types", |b, item| b.whitelist_type(item)) .handle_str_items("opaque-types", |b, item| b.opaque_type(item)) + .handle_table_items("cbindgen-types", |b, item| { + let gecko = item["gecko"].as_str().unwrap(); + let servo = item["servo"].as_str().unwrap(); + let line = format!("pub use {} as {};", servo, gecko.rsplit("::").next().unwrap()); + b.blacklist_type(gecko).module_raw_line("root::mozilla", line) + }) .handle_table_items("mapped-generic-types", |builder, item| { let generic = item["generic"].as_bool().unwrap(); let gecko = item["gecko"].as_str().unwrap(); From 591a47858b3a58392d1c9989e094ff1c3ec9e1db Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 06:20:00 +0000 Subject: [PATCH 09/32] style: Use alias for StyleAppearance. So we could avoid generating it in rust-bindgen and drop transmute. Differential Revision: https://phabricator.services.mozilla.com/D10304 --- components/style/properties/gecko.mako.rs | 30 +---------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 19d5ec8baba..47143343824 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -368,34 +368,6 @@ def set_gecko_property(ffi_name, expr): return "self.gecko.%s = %s;" % (ffi_name, expr) %> -<%def name="impl_cbindgen_keyword(ident, gecko_ffi_name)"> - #[allow(non_snake_case)] - #[inline] - pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { - // unsafe: cbindgen ensures the representations match. - ${set_gecko_property(gecko_ffi_name, "unsafe { transmute(v) }")} - } - - #[allow(non_snake_case)] - #[inline] - pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T { - // unsafe: cbindgen ensures the representations match. - unsafe { transmute(${get_gecko_property(gecko_ffi_name)}) } - } - - #[allow(non_snake_case)] - #[inline] - pub fn copy_${ident}_from(&mut self, other: &Self) { - self.gecko.${gecko_ffi_name} = other.gecko.${gecko_ffi_name}; - } - - #[allow(non_snake_case)] - #[inline] - pub fn reset_${ident}(&mut self, other: &Self) { - self.copy_${ident}_from(other) - } - - <%def name="impl_keyword_setter(ident, gecko_ffi_name, keyword, cast_type='u8', on_set=None)"> #[allow(non_snake_case)] pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) { @@ -3091,7 +3063,7 @@ fn static_assert() { unsafe { transmute(self.gecko.mDisplay) } } - ${impl_cbindgen_keyword('_moz_appearance', 'mAppearance')} + ${impl_simple('_moz_appearance', 'mAppearance')} <% float_keyword = Keyword("float", "Left Right None", gecko_enum_prefix="StyleFloat") %> ${impl_keyword('float', 'mFloat', float_keyword)} From cb2533d61ae62b1032f41cb42894e8a23649d82c Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 06:20:07 +0000 Subject: [PATCH 10/32] style: Use alias for StyleDisplay and StyleDisplayMode. Map these two types to their original rust type in rust-bindgen. Differential Revision: https://phabricator.services.mozilla.com/D10141 --- components/style/gecko/media_features.rs | 16 +++++----------- components/style/properties/gecko.mako.rs | 11 ++++------- 2 files changed, 9 insertions(+), 18 deletions(-) diff --git a/components/style/gecko/media_features.rs b/components/style/gecko/media_features.rs index 9e84a7722b1..e6f08c60f0c 100644 --- a/components/style/gecko/media_features.rs +++ b/components/style/gecko/media_features.rs @@ -179,7 +179,7 @@ fn eval_device_orientation(device: &Device, value: Option) -> bool } /// Values for the display-mode media feature. -#[derive(Clone, Copy, Debug, FromPrimitive, Parse, ToCss)] +#[derive(Clone, Copy, Debug, FromPrimitive, Parse, PartialEq, ToCss)] #[repr(u8)] #[allow(missing_docs)] pub enum DisplayMode { @@ -191,16 +191,10 @@ pub enum DisplayMode { /// https://w3c.github.io/manifest/#the-display-mode-media-feature fn eval_display_mode(device: &Device, query_value: Option) -> bool { - let query_value = match query_value { - Some(v) => v, - None => return true, - }; - - let gecko_display_mode = - unsafe { bindings::Gecko_MediaFeatures_GetDisplayMode(device.document()) }; - - // NOTE: cbindgen guarantees the same representation. - gecko_display_mode as u8 == query_value as u8 + match query_value { + Some(v) => v == unsafe { bindings::Gecko_MediaFeatures_GetDisplayMode(device.document()) }, + None => true, + } } /// https://drafts.csswg.org/mediaqueries-4/#grid diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 47143343824..b9eb6830777 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -3031,9 +3031,8 @@ fn static_assert() { <%self:impl_trait style_struct_name="Box" skip_longhands="${skip_box_longhands}"> #[inline] pub fn set_display(&mut self, v: longhands::display::computed_value::T) { - // unsafe: cbindgen ensures the representation is the same. - self.gecko.mDisplay = unsafe { transmute(v) }; - self.gecko.mOriginalDisplay = unsafe { transmute(v) }; + self.gecko.mDisplay = v; + self.gecko.mOriginalDisplay = v; } #[inline] @@ -3053,14 +3052,12 @@ fn static_assert() { v: longhands::display::computed_value::T, _is_item_or_root: bool ) { - // unsafe: cbindgen ensures the representation is the same. - self.gecko.mDisplay = unsafe { transmute(v) }; + self.gecko.mDisplay = v; } #[inline] pub fn clone_display(&self) -> longhands::display::computed_value::T { - // unsafe: cbindgen ensures the representation is the same. - unsafe { transmute(self.gecko.mDisplay) } + self.gecko.mDisplay } ${impl_simple('_moz_appearance', 'mAppearance')} From c7027e2676e670d5c4b9b53a4cf6a5a817e8c4d4 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 10:57:48 +0000 Subject: [PATCH 11/32] style: Use alias for StyleFillRule. This needs to update the "fill-rule" and "clip-rule" to use predefined_type to avoid some compilation errors. Differential Revision: https://phabricator.services.mozilla.com/D10142 --- components/style/gecko/conversions.rs | 18 ++++-------------- components/style/properties/data.py | 1 + components/style/properties/gecko.mako.rs | 7 +++---- .../properties/longhands/inherited_svg.mako.rs | 14 ++++++++------ .../style/values/computed/basic_shape.rs | 3 +++ components/style/values/computed/mod.rs | 1 + .../style/values/specified/basic_shape.rs | 5 ++++- components/style/values/specified/mod.rs | 1 + 8 files changed, 25 insertions(+), 25 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index dff1aef30cc..8cfe0c98169 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -649,7 +649,7 @@ pub mod basic_shape { use gecko::values::GeckoStyleCoordConvertible; use gecko_bindings::structs; - use gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType, StyleFillRule}; + use gecko_bindings::structs::{StyleBasicShape, StyleBasicShapeType}; use gecko_bindings::structs::{StyleGeometryBox, StyleShapeSource, StyleShapeSourceType}; use gecko_bindings::structs::{nsStyleCoord, nsStyleCorners}; use gecko_bindings::sugar::ns_style_coord::{CoordDataMut, CoordDataValue}; @@ -662,7 +662,7 @@ pub mod basic_shape { use values::computed::position; use values::computed::url::ComputedUrl; use values::generics::basic_shape::{BasicShape as GenericBasicShape, InsetRect, Polygon}; - use values::generics::basic_shape::{Circle, Ellipse, FillRule, Path, PolygonCoord}; + use values::generics::basic_shape::{Circle, Ellipse, Path, PolygonCoord}; use values::generics::basic_shape::{GeometryBox, ShapeBox, ShapeSource}; use values::generics::border::BorderRadius as GenericBorderRadius; use values::generics::rect::Rect; @@ -693,12 +693,7 @@ pub mod basic_shape { StyleShapeSourceType::URL | StyleShapeSourceType::Image => None, StyleShapeSourceType::Path => { let path = self.to_svg_path().expect("expect an SVGPathData"); - let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; - let fill = if gecko_path.mFillRule == StyleFillRule::Evenodd { - FillRule::Evenodd - } else { - FillRule::Nonzero - }; + let fill = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }.mFillRule; Some(ShapeSource::Path(Path { fill, path })) }, } @@ -805,11 +800,6 @@ pub mod basic_shape { position: (&other.mPosition).into(), }), StyleBasicShapeType::Polygon => { - let fill_rule = if other.mFillRule == StyleFillRule::Evenodd { - FillRule::Evenodd - } else { - FillRule::Nonzero - }; let mut coords = Vec::with_capacity(other.mCoordinates.len() / 2); for i in 0..(other.mCoordinates.len() / 2) { let x = 2 * i; @@ -828,7 +818,7 @@ pub mod basic_shape { )) } GenericBasicShape::Polygon(Polygon { - fill: fill_rule, + fill: other.mFillRule, coordinates: coords, }) }, diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 4d3e4ad2177..38b5273db26 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -298,6 +298,7 @@ class Longhand(object): "ColumnCount", "Contain", "Display", + "FillRule", "Float", "FontSizeAdjust", "FontStretch", diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index b9eb6830777..6f112343b0e 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1397,6 +1397,7 @@ impl Clone for ${style_struct.gecko_struct_name} { "length::LengthOrNormal": impl_style_coord, "length::NonNegativeLengthOrAuto": impl_style_coord, "length::NonNegativeLengthOrPercentageOrNormal": impl_style_coord, + "FillRule": impl_simple, "FlexBasis": impl_style_coord, "Length": impl_absolute_length, "LengthOrNormal": impl_style_coord, @@ -4953,8 +4954,7 @@ fn set_style_svg_path( gecko_path.mPath.assign_from_iter_pod(iter); // Setup fill-rule. - // unsafe: cbindgen ensures the representation is the same. - gecko_path.mFillRule = unsafe { transmute(fill) }; + gecko_path.mFillRule = fill; } <%def name="impl_shape_source(ident, gecko_ffi_name)"> @@ -5059,8 +5059,7 @@ fn set_style_svg_path( coord.0.to_gecko_style_coord(&mut shape.mCoordinates[2 * i]); coord.1.to_gecko_style_coord(&mut shape.mCoordinates[2 * i + 1]); } - // unsafe: cbindgen ensures the representation is the same. - shape.mFillRule = unsafe { transmute(poly.fill) }; + shape.mFillRule = poly.fill; } } diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 40c61b10129..3cdc5e76569 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -55,10 +55,11 @@ ${helpers.predefined_type( spec="https://www.w3.org/TR/SVG11/painting.html#FillOpacityProperty", )} -${helpers.single_keyword( +${helpers.predefined_type( "fill-rule", - "nonzero evenodd", - gecko_enum_prefix="StyleFillRule", + "FillRule", + "Default::default()", + needs_context=False, products="gecko", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG11/painting.html#FillRuleProperty", @@ -142,11 +143,12 @@ ${helpers.predefined_type( )} // Section 14 - Clipping, Masking and Compositing -${helpers.single_keyword( +${helpers.predefined_type( "clip-rule", - "nonzero evenodd", + "FillRule", + "Default::default()", + needs_context=False, products="gecko", - gecko_enum_prefix="StyleFillRule", animation_value_type="discrete", spec="https://www.w3.org/TR/SVG11/masking.html#ClipRuleProperty", )} diff --git a/components/style/values/computed/basic_shape.rs b/components/style/values/computed/basic_shape.rs index c140439af4e..e0409a9d904 100644 --- a/components/style/values/computed/basic_shape.rs +++ b/components/style/values/computed/basic_shape.rs @@ -13,6 +13,9 @@ use values::computed::{Image, LengthOrPercentage}; use values::computed::url::ComputedUrl; use values::generics::basic_shape as generic; +/// A computed alias for FillRule. +pub use values::generics::basic_shape::FillRule; + /// A computed clipping shape. pub type ClippingShape = generic::ClippingShape; diff --git a/components/style/values/computed/mod.rs b/components/style/values/computed/mod.rs index d1787decf46..8b9a33f0df2 100644 --- a/components/style/values/computed/mod.rs +++ b/components/style/values/computed/mod.rs @@ -36,6 +36,7 @@ pub use self::align::{AlignContent, AlignItems, JustifyContent, JustifyItems, Se pub use self::align::{AlignSelf, JustifySelf}; pub use self::angle::Angle; pub use self::background::{BackgroundRepeat, BackgroundSize}; +pub use self::basic_shape::FillRule; pub use self::border::{BorderImageRepeat, BorderImageSideWidth, BorderImageSlice, BorderImageWidth}; pub use self::border::{BorderCornerRadius, BorderRadius, BorderSpacing}; pub use self::font::{FontSize, FontSizeAdjust, FontStretch, FontSynthesis, FontVariantAlternates, FontWeight}; diff --git a/components/style/values/specified/basic_shape.rs b/components/style/values/specified/basic_shape.rs index 52343621b5e..dcf04dcc0df 100644 --- a/components/style/values/specified/basic_shape.rs +++ b/components/style/values/specified/basic_shape.rs @@ -14,7 +14,7 @@ use std::fmt::{self, Write}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; use values::computed::Percentage; use values::generics::basic_shape as generic; -use values::generics::basic_shape::{FillRule, GeometryBox, Path, PolygonCoord}; +use values::generics::basic_shape::{GeometryBox, Path, PolygonCoord}; use values::generics::basic_shape::{ShapeBox, ShapeSource}; use values::generics::rect::Rect; use values::specified::LengthOrPercentage; @@ -25,6 +25,9 @@ use values::specified::position::{HorizontalPosition, Position, PositionComponen use values::specified::position::{Side, VerticalPosition}; use values::specified::url::SpecifiedUrl; +/// A specified alias for FillRule. +pub use values::generics::basic_shape::FillRule; + /// A specified clipping shape. pub type ClippingShape = generic::ClippingShape; diff --git a/components/style/values/specified/mod.rs b/components/style/values/specified/mod.rs index 9955f7232be..0a910f9ce6a 100644 --- a/components/style/values/specified/mod.rs +++ b/components/style/values/specified/mod.rs @@ -29,6 +29,7 @@ pub use self::align::{AlignContent, AlignItems, AlignSelf, ContentDistribution}; #[cfg(feature = "gecko")] pub use self::align::{JustifyContent, JustifyItems, JustifySelf, SelfAlignment}; pub use self::background::{BackgroundRepeat, BackgroundSize}; +pub use self::basic_shape::FillRule; pub use self::border::{BorderCornerRadius, BorderImageSlice, BorderImageWidth}; pub use self::border::{BorderImageRepeat, BorderImageSideWidth}; pub use self::border::{BorderRadius, BorderSideWidth, BorderSpacing}; From b81bbb85b4c93ecece57a9728151e982bd148495 Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 06:20:32 +0000 Subject: [PATCH 12/32] style: Use alias for StylePathCommand. So we could drop transmute in svg_path related functions. Differential Revision: https://phabricator.services.mozilla.com/D10140 --- components/style/gecko/conversions.rs | 9 +-------- components/style/properties/gecko.mako.rs | 6 +----- 2 files changed, 2 insertions(+), 13 deletions(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 8cfe0c98169..6a2287dc033 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -701,18 +701,11 @@ pub mod basic_shape { /// Generate a SVGPathData from StyleShapeSource if possible. fn to_svg_path(&self) -> Option { - use gecko_bindings::structs::StylePathCommand; use values::specified::svg_path::PathCommand; match self.mType { StyleShapeSourceType::Path => { let gecko_path = unsafe { &*self.__bindgen_anon_1.mSVGPath.as_ref().mPtr }; - let result: Vec = gecko_path - .mPath - .iter() - .map(|gecko: &StylePathCommand| { - // unsafe: cbindgen ensures the representation is the same. - unsafe { ::std::mem::transmute(*gecko) } - }).collect(); + let result: Vec = gecko_path.mPath.iter().cloned().collect(); Some(SVGPathData::new(result.into_boxed_slice())) }, _ => None, diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 6f112343b0e..59beaea0a80 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -4947,11 +4947,7 @@ fn set_style_svg_path( &mut shape_source.__bindgen_anon_1.mSVGPath.as_mut().mPtr.as_mut().unwrap() }; - let iter = servo_path.commands().iter().map(|command| { - // unsafe: cbindgen ensures the representation is the same. - unsafe { transmute(*command) } - }); - gecko_path.mPath.assign_from_iter_pod(iter); + gecko_path.mPath.assign_from_iter_pod(servo_path.commands().iter().cloned()); // Setup fill-rule. gecko_path.mFillRule = fill; From 33b2514198fdf69ccc5c1e2e928911160c0b816a Mon Sep 17 00:00:00 2001 From: Boris Chiou Date: Wed, 31 Oct 2018 10:58:24 +0000 Subject: [PATCH 13/32] style: Drop "mozilla" prefix in cbindgen_types in ServoBindings.toml. A minor update to drop the redundant "mozilla" namespace prefix in `cbindgen_types` array. Depends on D10305 Differential Revision: https://phabricator.services.mozilla.com/D10325 --- components/style/build_gecko.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/style/build_gecko.rs b/components/style/build_gecko.rs index 3e4200d28d0..eff6d94027a 100644 --- a/components/style/build_gecko.rs +++ b/components/style/build_gecko.rs @@ -395,8 +395,8 @@ mod bindings { .handle_table_items("cbindgen-types", |b, item| { let gecko = item["gecko"].as_str().unwrap(); let servo = item["servo"].as_str().unwrap(); - let line = format!("pub use {} as {};", servo, gecko.rsplit("::").next().unwrap()); - b.blacklist_type(gecko).module_raw_line("root::mozilla", line) + b.blacklist_type(format!("mozilla::{}", gecko)) + .module_raw_line("root::mozilla", format!("pub use {} as {};", servo, gecko)) }) .handle_table_items("mapped-generic-types", |builder, item| { let generic = item["generic"].as_bool().unwrap(); From 5976956e1c9a00f8cb22bd4f0718085168ec5608 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Wed, 31 Oct 2018 10:40:56 +0000 Subject: [PATCH 14/32] style: Handle reversed ranges in @font-face descriptors. Differential Revision: https://phabricator.services.mozilla.com/D10327 --- components/style/font_face.rs | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/components/style/font_face.rs b/components/style/font_face.rs index b78947dd400..b207b0291bc 100644 --- a/components/style/font_face.rs +++ b/components/style/font_face.rs @@ -151,10 +151,20 @@ impl_range!(FontWeightRange, AbsoluteFontWeight); #[allow(missing_docs)] pub struct ComputedFontWeightRange(f32, f32); +#[inline] +fn sort_range(a: T, b: T) -> (T, T) { + if a > b { + (b, a) + } else { + (a, b) + } +} + impl FontWeightRange { /// Returns a computed font-stretch range. pub fn compute(&self) -> ComputedFontWeightRange { - ComputedFontWeightRange(self.0.compute().0, self.1.compute().0) + let (min, max) = sort_range(self.0.compute().0, self.1.compute().0); + ComputedFontWeightRange(min, max) } } @@ -182,7 +192,11 @@ impl FontStretchRange { } } - ComputedFontStretchRange(compute_stretch(&self.0), compute_stretch(&self.1)) + let (min, max) = sort_range( + compute_stretch(&self.0), + compute_stretch(&self.1), + ); + ComputedFontStretchRange(min, max) } } @@ -258,10 +272,11 @@ impl FontStyle { FontStyle::Normal => ComputedFontStyleDescriptor::Normal, FontStyle::Italic => ComputedFontStyleDescriptor::Italic, FontStyle::Oblique(ref first, ref second) => { - ComputedFontStyleDescriptor::Oblique( + let (min, max) = sort_range( SpecifiedFontStyle::compute_angle_degrees(first), SpecifiedFontStyle::compute_angle_degrees(second), - ) + ); + ComputedFontStyleDescriptor::Oblique(min, max) } } } From d43c4ce81e25f0f55a940a74822f3dc78745b6a3 Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Thu, 1 Nov 2018 01:35:26 +0000 Subject: [PATCH 15/32] style: Support unprefixed image-rendering: crisp-edges. For now, we keep supporting the prefixed version, since there are examples/instructions on the Web that don't include an unprefixed value. Differential Revision: https://phabricator.services.mozilla.com/D10451 --- .../style/properties/longhands/inherited_box.mako.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/components/style/properties/longhands/inherited_box.mako.rs b/components/style/properties/longhands/inherited_box.mako.rs index d810ec3d29a..2b49e0ca55c 100644 --- a/components/style/properties/longhands/inherited_box.mako.rs +++ b/components/style/properties/longhands/inherited_box.mako.rs @@ -65,9 +65,10 @@ ${helpers.single_keyword( // And, firefox doesn't support `pixelated` yet (https://bugzilla.mozilla.org/show_bug.cgi?id=856337) ${helpers.single_keyword( "image-rendering", - "auto", - extra_gecko_values="optimizespeed optimizequality -moz-crisp-edges", - extra_servo_values="pixelated crisp-edges", + "auto crisp-edges", + extra_gecko_values="optimizespeed optimizequality", + extra_servo_values="pixelated", + extra_gecko_aliases="-moz-crisp-edges=crisp-edges", custom_consts=image_rendering_custom_consts, animation_value_type="discrete", spec="https://drafts.csswg.org/css-images/#propdef-image-rendering", From 52c3ba00b3e0748239951088406176576e4e936e Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Thu, 1 Nov 2018 01:37:26 +0000 Subject: [PATCH 16/32] style: Fix inconsistent CRISPEDGES constant name. Differential Revision: https://phabricator.services.mozilla.com/D10452 --- components/style/properties/longhands/inherited_box.mako.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/components/style/properties/longhands/inherited_box.mako.rs b/components/style/properties/longhands/inherited_box.mako.rs index 2b49e0ca55c..feee09556d7 100644 --- a/components/style/properties/longhands/inherited_box.mako.rs +++ b/components/style/properties/longhands/inherited_box.mako.rs @@ -59,8 +59,6 @@ ${helpers.single_keyword( spec="https://drafts.csswg.org/css-color/#propdef-color-adjust", )} -<% image_rendering_custom_consts = { "crisp-edges": "CRISPEDGES", - "-moz-crisp-edges": "CRISPEDGES" } %> // According to to CSS-IMAGES-3, `optimizespeed` and `optimizequality` are synonyms for `auto` // And, firefox doesn't support `pixelated` yet (https://bugzilla.mozilla.org/show_bug.cgi?id=856337) ${helpers.single_keyword( @@ -69,7 +67,6 @@ ${helpers.single_keyword( extra_gecko_values="optimizespeed optimizequality", extra_servo_values="pixelated", extra_gecko_aliases="-moz-crisp-edges=crisp-edges", - custom_consts=image_rendering_custom_consts, animation_value_type="discrete", spec="https://drafts.csswg.org/css-images/#propdef-image-rendering", )} From a7b5ba14c0f1e8a3167fd21ca5b1916dd2220a9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 2 Nov 2018 18:19:06 +0000 Subject: [PATCH 17/32] style: Use references in the shapes code. It doesn't make much sense to return const UniquePtr& for something that can't be null, it's just confusing. Also make more stuff actually const. Differential Revision: https://phabricator.services.mozilla.com/D10647 --- components/style/gecko/conversions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 6a2287dc033..eeb13fce732 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -718,7 +718,7 @@ pub mod basic_shape { match other.mType { StyleShapeSourceType::URL => unsafe { let shape_image = &*other.__bindgen_anon_1.mShapeImage.as_ref().mPtr; - let other_url = RefPtr::new(*shape_image.__bindgen_anon_1.mURLValue.as_ref()); + let other_url = RefPtr::new(*shape_image.__bindgen_anon_1.mURLValue.as_ref() as *mut _); let url = ComputedUrl::from_url_value(other_url); ShapeSource::ImageOrUrl(url) }, From 33fed655978824c29dd4dcc4424b3148c3b2d26d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 00:05:12 +0000 Subject: [PATCH 18/32] style: Don't match document author rules if not needed for revalidation. When you're in a ShadowRoot and can share style with a sibling, the sharing code is smart enough to skip document author rules. But then it could get confused if you also include document rules, since revalidation selectors are matched against these. This is not a correctness issue, because we're matching more than what we need, and avoid sharing if we failed. Also fix the detection for user rules in any_applicable_rule_data. Differential Revision: https://phabricator.services.mozilla.com/D10117 --- components/style/stylist.rs | 53 +++++++++++++++++++++---------------- 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/components/style/stylist.rs b/components/style/stylist.rs index fe96fd78ad8..2772891a3db 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -605,11 +605,11 @@ impl Stylist { maybe = maybe || f(&*data); }); - if maybe || !doc_author_rules_apply { - return maybe; + if maybe || f(&self.cascade_data.user) { + return true; } - f(&self.cascade_data.author) || f(&self.cascade_data.user) + doc_author_rules_apply && f(&self.cascade_data.author) } /// Computes the style for a given "precomputed" pseudo-element, taking the @@ -1491,7 +1491,33 @@ impl Stylist { // the lookups, which means that the bitvecs are comparable. We verify // this in the caller by asserting that the bitvecs are same-length. let mut results = SmallBitVec::new(); - for (data, _) in self.cascade_data.iter_origins() { + + let matches_document_rules = + element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| { + matching_context.with_shadow_host(host, |matching_context| { + data.selectors_for_cache_revalidation.lookup( + element, + quirks_mode, + |selector_and_hashes| { + results.push(matches_selector( + &selector_and_hashes.selector, + selector_and_hashes.selector_offset, + Some(&selector_and_hashes.hashes), + &element, + matching_context, + flags_setter, + )); + true + }, + ); + }) + }); + + for (data, origin) in self.cascade_data.iter_origins() { + if origin == Origin::Author && !matches_document_rules { + continue; + } + data.selectors_for_cache_revalidation.lookup( element, self.quirks_mode, @@ -1509,25 +1535,6 @@ impl Stylist { ); } - element.each_applicable_non_document_style_rule_data(|data, quirks_mode, host| { - matching_context.with_shadow_host(host, |matching_context| { - data.selectors_for_cache_revalidation.lookup( - element, - quirks_mode, - |selector_and_hashes| { - results.push(matches_selector( - &selector_and_hashes.selector, - selector_and_hashes.selector_offset, - Some(&selector_and_hashes.hashes), - &element, - matching_context, - flags_setter, - )); - true - }, - ); - }) - }); results } From d035d02517f37573e52260bf7870b1932ddecc5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 00:16:58 +0000 Subject: [PATCH 19/32] style: Don't keep a separate list of ignored-when-colors-disabled longhands. Most of the change is moving sets around to be static functions on LonghandIdSet. I think I like that pattern, but I can also make the new set a global static and add mako code to be `pub` or something. Though I think the LonghandIdSet::foo().contains(..) pattern is nice to read :) Differential Revision: https://phabricator.services.mozilla.com/D10653 --- .../style/properties/properties.mako.rs | 69 ++++++++++++++----- components/style/rule_tree/mod.rs | 23 ++----- 2 files changed, 56 insertions(+), 36 deletions(-) diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 330ceea7f84..027a3eba28b 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -758,6 +758,41 @@ impl<'a> Iterator for LonghandIdSetIterator<'a> { } impl LonghandIdSet { + #[inline] + fn reset() -> &'static Self { + ${static_longhand_id_set("RESET", lambda p: not p.style_struct.inherited)} + &RESET + } + + #[inline] + fn animatable() -> &'static Self { + ${static_longhand_id_set("ANIMATABLE", lambda p: p.animatable)} + &ANIMATABLE + } + + #[inline] + fn discrete_animatable() -> &'static Self { + ${static_longhand_id_set("DISCRETE_ANIMATABLE", lambda p: p.animation_value_type == "discrete")} + &DISCRETE_ANIMATABLE + } + + #[inline] + fn logical() -> &'static Self { + ${static_longhand_id_set("LOGICAL", lambda p: p.logical)} + &LOGICAL + } + + /// Returns the set of longhands that are ignored when document colors are + /// disabled. + #[inline] + pub fn ignored_when_colors_disabled() -> &'static Self { + ${static_longhand_id_set( + "IGNORED_WHEN_COLORS_DISABLED", + lambda p: p.ignored_when_colors_disabled + )} + &IGNORED_WHEN_COLORS_DISABLED + } + /// Iterate over the current longhand id set. pub fn iter(&self) -> LonghandIdSetIterator { LonghandIdSetIterator { longhands: self, cur: 0, } @@ -784,6 +819,14 @@ impl LonghandIdSet { false } + /// Remove all the given properties from the set. + #[inline] + pub fn remove_all(&mut self, other: &Self) { + for (self_cell, other_cell) in self.storage.iter_mut().zip(other.storage.iter()) { + *self_cell &= !*other_cell; + } + } + /// Create an empty set #[inline] pub fn new() -> LonghandIdSet { @@ -800,8 +843,7 @@ impl LonghandIdSet { /// Return whether this set contains any reset longhand. #[inline] pub fn contains_any_reset(&self) -> bool { - ${static_longhand_id_set("RESET", lambda p: not p.style_struct.inherited)} - self.contains_any(&RESET) + self.contains_any(Self::reset()) } /// Add the given property to the set @@ -961,9 +1003,8 @@ impl LonghandId { /// Returns whether the longhand property is inherited by default. #[inline] - pub fn inherited(&self) -> bool { - ${static_longhand_id_set("INHERITED", lambda p: p.style_struct.inherited)} - INHERITED.contains(*self) + pub fn inherited(self) -> bool { + !LonghandIdSet::reset().contains(self) } fn shorthands(&self) -> NonCustomPropertyIterator { @@ -1034,15 +1075,13 @@ impl LonghandId { /// Returns whether this property is animatable. #[inline] pub fn is_animatable(self) -> bool { - ${static_longhand_id_set("ANIMATABLE", lambda p: p.animatable)} - ANIMATABLE.contains(self) + LonghandIdSet::animatable().contains(self) } /// Returns whether this property is animatable in a discrete way. #[inline] pub fn is_discrete_animatable(self) -> bool { - ${static_longhand_id_set("DISCRETE_ANIMATABLE", lambda p: p.animation_value_type == "discrete")} - DISCRETE_ANIMATABLE.contains(self) + LonghandIdSet::discrete_animatable().contains(self) } /// Converts from a LonghandId to an adequate nsCSSPropertyID. @@ -1065,9 +1104,8 @@ impl LonghandId { /// Return whether this property is logical. #[inline] - pub fn is_logical(&self) -> bool { - ${static_longhand_id_set("LOGICAL", lambda p: p.logical)} - LOGICAL.contains(*self) + pub fn is_logical(self) -> bool { + LonghandIdSet::logical().contains(self) } /// If this is a logical property, return the corresponding physical one in @@ -1157,12 +1195,7 @@ impl LonghandId { /// colors are disabled. #[inline] fn ignored_when_document_colors_disabled(self) -> bool { - ${static_longhand_id_set( - "IGNORED_WHEN_COLORS_DISABLED", - lambda p: p.ignored_when_colors_disabled - )} - - IGNORED_WHEN_COLORS_DISABLED.contains(self) + LonghandIdSet::ignored_when_colors_disabled().contains(self) } /// The computed value of some properties depends on the (sometimes diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index 7277afd4d9f..59c06e58923 100644 --- a/components/style/rule_tree/mod.rs +++ b/components/style/rule_tree/mod.rs @@ -1288,25 +1288,12 @@ impl StrongRuleNode { } } - // If author colors are not allowed, only claim to have author-specified - // rules if we're looking at a non-color property, a border image, or if - // we're looking at the background color and it's set to transparent. - const IGNORED_WHEN_COLORS_DISABLED: &'static [LonghandId] = &[ - LonghandId::BackgroundImage, - LonghandId::BorderImageSource, - LonghandId::BorderTopColor, - LonghandId::BorderRightColor, - LonghandId::BorderBottomColor, - LonghandId::BorderLeftColor, - LonghandId::BorderInlineStartColor, - LonghandId::BorderInlineEndColor, - LonghandId::BorderBlockStartColor, - LonghandId::BorderBlockEndColor, - ]; - + // If author colors are not allowed, don't look at those properties + // (except for background-color which is special and we handle below). if !author_colors_allowed { - for id in IGNORED_WHEN_COLORS_DISABLED { - properties.remove(*id); + properties.remove_all(LonghandIdSet::ignored_when_colors_disabled()); + if rule_type_mask & NS_AUTHOR_SPECIFIED_BACKGROUND != 0 { + properties.insert(LonghandId::BackgroundColor); } } From 282edf1a13c57831e7cfdcde85926472388b2bab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 23:10:17 +0000 Subject: [PATCH 20/32] style: Move shorthand IDL order stuff out of animated_properties. Doesn't really need to be in a mako file. Differential Revision: https://phabricator.services.mozilla.com/D10839 --- .../helpers/animated_properties.mako.rs | 37 ++++++------------- .../style/properties/properties.mako.rs | 19 ++++++++++ 2 files changed, 30 insertions(+), 26 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index c8e72ca1df4..cb8f2970ca0 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -3057,32 +3057,17 @@ pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Orderin return a_category.cmp(&b_category); } - if a_category == PropertyCategory::Shorthand { - let a = a.as_shorthand().unwrap(); - let b = b.as_shorthand().unwrap(); - // Within shorthands, sort by the number of subproperties, then by IDL - // name. - let subprop_count_a = a.longhands().count(); - let subprop_count_b = b.longhands().count(); - return subprop_count_a.cmp(&subprop_count_b).then_with(|| { - get_idl_name_sort_order(a).cmp(&get_idl_name_sort_order(b)) - }); + if a_category != PropertyCategory::Shorthand { + return cmp::Ordering::Equal; } - cmp::Ordering::Equal -} - -fn get_idl_name_sort_order(shorthand: ShorthandId) -> u32 { -<% -# Sort by IDL name. -sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident)) - -# Annotate with sorted position -sorted_shorthands = [(p, position) for position, p in enumerate(sorted_shorthands)] -%> - match shorthand { - % for property, position in sorted_shorthands: - ShorthandId::${property.camel_case} => ${position}, - % endfor - } + let a = a.as_shorthand().unwrap(); + let b = b.as_shorthand().unwrap(); + // Within shorthands, sort by the number of subproperties, then by IDL + // name. + let subprop_count_a = a.longhands().count(); + let subprop_count_b = b.longhands().count(); + subprop_count_a.cmp(&subprop_count_b).then_with(|| { + a.idl_name_sort_order().cmp(&b.idl_name_sort_order()) + }) } diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 027a3eba28b..6676e956e38 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1453,6 +1453,25 @@ impl ShorthandId { } } + /// Returns the order in which this property appears relative to other + /// shorthands in idl-name-sorting order. + #[inline] + pub fn idl_name_sort_order(self) -> u32 { + <% + from data import to_idl_name + ordered = {} + sorted_shorthands = sorted(data.shorthands, key=lambda p: to_idl_name(p.ident)) + for order, shorthand in enumerate(sorted_shorthands): + ordered[shorthand.ident] = order + %> + static IDL_NAME_SORT_ORDER: [u32; ${len(data.shorthands)}] = [ + % for property in data.shorthands: + ${ordered[property.ident]}, + % endfor + ]; + IDL_NAME_SORT_ORDER[self as usize] + } + fn parse_into<'i, 't>( &self, declarations: &mut SourcePropertyDeclaration, From f159c201982648a5c7e517838b6f52d83d6450f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 23:12:24 +0000 Subject: [PATCH 21/32] style: Move the keyframes property priority stuff outside of animated_properties. Similarly, no need to be here. Differential Revision: https://phabricator.services.mozilla.com/D10840 --- .../helpers/animated_properties.mako.rs | 64 ------------------- components/style/values/animated/mod.rs | 64 +++++++++++++++++++ 2 files changed, 64 insertions(+), 64 deletions(-) diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index cb8f2970ca0..0b7cc3db957 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -19,7 +19,6 @@ use properties::{CSSWideKeyword, PropertyDeclaration}; use properties::longhands; use properties::longhands::font_weight::computed_value::T as FontWeight; use properties::longhands::visibility::computed_value::T as Visibility; -use properties::PropertyId; use properties::{LonghandId, ShorthandId}; use servo_arc::Arc; use smallvec::SmallVec; @@ -3008,66 +3007,3 @@ impl ToAnimatedZero for AnimatedFilter { } } } - -/// The category a property falls into for ordering purposes. -/// -/// https://drafts.csswg.org/web-animations/#calculating-computed-keyframes -/// -#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] -enum PropertyCategory { - Custom, - PhysicalLonghand, - LogicalLonghand, - Shorthand, -} - -impl PropertyCategory { - fn of(id: &PropertyId) -> Self { - match *id { - PropertyId::Shorthand(..) | - PropertyId::ShorthandAlias(..) => PropertyCategory::Shorthand, - PropertyId::Longhand(id) | - PropertyId::LonghandAlias(id, ..) => { - if id.is_logical() { - PropertyCategory::LogicalLonghand - } else { - PropertyCategory::PhysicalLonghand - } - } - PropertyId::Custom(..) => PropertyCategory::Custom, - } - } -} - -/// A comparator to sort PropertyIds such that physical longhands are sorted -/// before logical longhands and shorthands, shorthands with fewer components -/// are sorted before shorthands with more components, and otherwise shorthands -/// are sorted by IDL name as defined by [Web Animations][property-order]. -/// -/// Using this allows us to prioritize values specified by longhands (or smaller -/// shorthand subsets) when longhands and shorthands are both specified on the -/// one keyframe. -/// -/// [property-order] https://drafts.csswg.org/web-animations/#calculating-computed-keyframes -pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Ordering { - let a_category = PropertyCategory::of(a); - let b_category = PropertyCategory::of(b); - - if a_category != b_category { - return a_category.cmp(&b_category); - } - - if a_category != PropertyCategory::Shorthand { - return cmp::Ordering::Equal; - } - - let a = a.as_shorthand().unwrap(); - let b = b.as_shorthand().unwrap(); - // Within shorthands, sort by the number of subproperties, then by IDL - // name. - let subprop_count_a = a.longhands().count(); - let subprop_count_b = b.longhands().count(); - subprop_count_a.cmp(&subprop_count_b).then_with(|| { - a.idl_name_sort_order().cmp(&b.idl_name_sort_order()) - }) -} diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 8944f3fc5f4..41482418b1d 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -10,7 +10,9 @@ use app_units::Au; use euclid::{Point2D, Size2D}; +use properties::PropertyId; use smallvec::SmallVec; +use std::cmp; use values::computed::Angle as ComputedAngle; use values::computed::BorderCornerRadius as ComputedBorderCornerRadius; use values::computed::MaxLength as ComputedMaxLength; @@ -21,6 +23,68 @@ use values::computed::url::ComputedUrl; pub mod color; pub mod effects; +/// The category a property falls into for ordering purposes. +/// +/// https://drafts.csswg.org/web-animations/#calculating-computed-keyframes +#[derive(Clone, Copy, Eq, Ord, PartialEq, PartialOrd)] +enum PropertyCategory { + Custom, + PhysicalLonghand, + LogicalLonghand, + Shorthand, +} + +impl PropertyCategory { + fn of(id: &PropertyId) -> Self { + match *id { + PropertyId::Shorthand(..) | + PropertyId::ShorthandAlias(..) => PropertyCategory::Shorthand, + PropertyId::Longhand(id) | + PropertyId::LonghandAlias(id, ..) => { + if id.is_logical() { + PropertyCategory::LogicalLonghand + } else { + PropertyCategory::PhysicalLonghand + } + } + PropertyId::Custom(..) => PropertyCategory::Custom, + } + } +} + +/// A comparator to sort PropertyIds such that physical longhands are sorted +/// before logical longhands and shorthands, shorthands with fewer components +/// are sorted before shorthands with more components, and otherwise shorthands +/// are sorted by IDL name as defined by [Web Animations][property-order]. +/// +/// Using this allows us to prioritize values specified by longhands (or smaller +/// shorthand subsets) when longhands and shorthands are both specified on the +/// one keyframe. +/// +/// [property-order] https://drafts.csswg.org/web-animations/#calculating-computed-keyframes +pub fn compare_property_priority(a: &PropertyId, b: &PropertyId) -> cmp::Ordering { + let a_category = PropertyCategory::of(a); + let b_category = PropertyCategory::of(b); + + if a_category != b_category { + return a_category.cmp(&b_category); + } + + if a_category != PropertyCategory::Shorthand { + return cmp::Ordering::Equal; + } + + let a = a.as_shorthand().unwrap(); + let b = b.as_shorthand().unwrap(); + // Within shorthands, sort by the number of subproperties, then by IDL + // name. + let subprop_count_a = a.longhands().count(); + let subprop_count_b = b.longhands().count(); + subprop_count_a.cmp(&subprop_count_b).then_with(|| { + a.idl_name_sort_order().cmp(&b.idl_name_sort_order()) + }) +} + /// Animate from one value to another. /// /// This trait is derivable with `#[derive(Animate)]`. The derived From 778ae7d745eaf83fc0fb37afd6f31468eae61fed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 23:16:07 +0000 Subject: [PATCH 22/32] style: Move various length animation implementations to its own file. It's nicer, I think. Differential Revision: https://phabricator.services.mozilla.com/D10841 --- .../helpers/animated_properties.mako.rs | 52 +------- components/style/values/animated/length.rs | 118 ++++++++++++++++++ components/style/values/animated/mod.rs | 63 +--------- 3 files changed, 121 insertions(+), 112 deletions(-) create mode 100644 components/style/values/animated/length.rs diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 0b7cc3db957..85c588502c5 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -31,10 +31,9 @@ use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; use values::animated::color::Color as AnimatedColor; use values::animated::effects::Filter as AnimatedFilter; #[cfg(feature = "gecko")] use values::computed::TransitionProperty; -use values::computed::{Angle, CalcLengthOrPercentage}; +use values::computed::Angle; use values::computed::{ClipRect, Context}; -use values::computed::{Length, LengthOrPercentage, LengthOrPercentageOrAuto}; -use values::computed::LengthOrPercentageOrNone; +use values::computed::{Length, LengthOrPercentage}; use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, Percentage}; use values::computed::length::NonNegativeLengthOrPercentage; use values::computed::ToComputedValue; @@ -845,53 +844,6 @@ impl ToAnimatedZero for Visibility { } } -/// -impl Animate for CalcLengthOrPercentage { - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - let animate_percentage_half = |this: Option, other: Option| { - if this.is_none() && other.is_none() { - return Ok(None); - } - let this = this.unwrap_or_default(); - let other = other.unwrap_or_default(); - Ok(Some(this.animate(&other, procedure)?)) - }; - - let length = self.unclamped_length().animate(&other.unclamped_length(), procedure)?; - let percentage = animate_percentage_half(self.percentage, other.percentage)?; - Ok(CalcLengthOrPercentage::with_clamping_mode(length, percentage, self.clamping_mode)) - } -} - -impl ToAnimatedZero for LengthOrPercentageOrAuto { - #[inline] - fn to_animated_zero(&self) -> Result { - match *self { - LengthOrPercentageOrAuto::Length(_) | - LengthOrPercentageOrAuto::Percentage(_) | - LengthOrPercentageOrAuto::Calc(_) => { - Ok(LengthOrPercentageOrAuto::Length(Length::new(0.))) - }, - LengthOrPercentageOrAuto::Auto => Err(()), - } - } -} - -impl ToAnimatedZero for LengthOrPercentageOrNone { - #[inline] - fn to_animated_zero(&self) -> Result { - match *self { - LengthOrPercentageOrNone::Length(_) | - LengthOrPercentageOrNone::Percentage(_) | - LengthOrPercentageOrNone::Calc(_) => { - Ok(LengthOrPercentageOrNone::Length(Length::new(0.))) - }, - LengthOrPercentageOrNone::None => Err(()), - } - } -} - impl ToAnimatedZero for FontWeight { #[inline] fn to_animated_zero(&self) -> Result { diff --git a/components/style/values/animated/length.rs b/components/style/values/animated/length.rs new file mode 100644 index 00000000000..f6bbaa6df38 --- /dev/null +++ b/components/style/values/animated/length.rs @@ -0,0 +1,118 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Animation implementation for various length-related types. + +use values::computed::MaxLength as ComputedMaxLength; +use values::computed::MozLength as ComputedMozLength; +use values::computed::Percentage; +use values::computed::length::{Length, CalcLengthOrPercentage, LengthOrPercentageOrNone, LengthOrPercentageOrAuto}; +use super::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; + +/// +impl Animate for CalcLengthOrPercentage { + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + let animate_percentage_half = |this: Option, other: Option| { + if this.is_none() && other.is_none() { + return Ok(None); + } + let this = this.unwrap_or_default(); + let other = other.unwrap_or_default(); + Ok(Some(this.animate(&other, procedure)?)) + }; + + let length = self.unclamped_length().animate(&other.unclamped_length(), procedure)?; + let percentage = animate_percentage_half(self.percentage, other.percentage)?; + Ok(CalcLengthOrPercentage::with_clamping_mode(length, percentage, self.clamping_mode)) + } +} + +impl ToAnimatedZero for LengthOrPercentageOrAuto { + #[inline] + fn to_animated_zero(&self) -> Result { + match *self { + LengthOrPercentageOrAuto::Length(_) | + LengthOrPercentageOrAuto::Percentage(_) | + LengthOrPercentageOrAuto::Calc(_) => { + Ok(LengthOrPercentageOrAuto::Length(Length::new(0.))) + }, + LengthOrPercentageOrAuto::Auto => Err(()), + } + } +} + +impl ToAnimatedZero for LengthOrPercentageOrNone { + #[inline] + fn to_animated_zero(&self) -> Result { + match *self { + LengthOrPercentageOrNone::Length(_) | + LengthOrPercentageOrNone::Percentage(_) | + LengthOrPercentageOrNone::Calc(_) => { + Ok(LengthOrPercentageOrNone::Length(Length::new(0.))) + }, + LengthOrPercentageOrNone::None => Err(()), + } + } +} + +impl ToAnimatedValue for ComputedMaxLength { + type AnimatedValue = Self; + + #[inline] + fn to_animated_value(self) -> Self { + self + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + use values::computed::{Length, LengthOrPercentageOrNone, Percentage}; + use values::generics::length::MaxLength as GenericMaxLength; + match animated { + GenericMaxLength::LengthOrPercentageOrNone(lopn) => { + let result = match lopn { + LengthOrPercentageOrNone::Length(px) => { + LengthOrPercentageOrNone::Length(Length::new(px.px().max(0.))) + }, + LengthOrPercentageOrNone::Percentage(percentage) => { + LengthOrPercentageOrNone::Percentage(Percentage(percentage.0.max(0.))) + }, + _ => lopn, + }; + GenericMaxLength::LengthOrPercentageOrNone(result) + }, + _ => animated, + } + } +} + +impl ToAnimatedValue for ComputedMozLength { + type AnimatedValue = Self; + + #[inline] + fn to_animated_value(self) -> Self { + self + } + + #[inline] + fn from_animated_value(animated: Self::AnimatedValue) -> Self { + use values::computed::{Length, LengthOrPercentageOrAuto, Percentage}; + use values::generics::length::MozLength as GenericMozLength; + match animated { + GenericMozLength::LengthOrPercentageOrAuto(lopa) => { + let result = match lopa { + LengthOrPercentageOrAuto::Length(px) => { + LengthOrPercentageOrAuto::Length(Length::new(px.px().max(0.))) + }, + LengthOrPercentageOrAuto::Percentage(percentage) => { + LengthOrPercentageOrAuto::Percentage(Percentage(percentage.0.max(0.))) + }, + _ => lopa, + }; + GenericMozLength::LengthOrPercentageOrAuto(result) + }, + _ => animated, + } + } +} diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 41482418b1d..7d781f19a5d 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -15,13 +15,12 @@ use smallvec::SmallVec; use std::cmp; use values::computed::Angle as ComputedAngle; use values::computed::BorderCornerRadius as ComputedBorderCornerRadius; -use values::computed::MaxLength as ComputedMaxLength; -use values::computed::MozLength as ComputedMozLength; use values::computed::length::CalcLengthOrPercentage; use values::computed::url::ComputedUrl; pub mod color; pub mod effects; +mod length; /// The category a property falls into for ordering purposes. /// @@ -345,66 +344,6 @@ impl ToAnimatedValue for ComputedBorderCornerRadius { } } -impl ToAnimatedValue for ComputedMaxLength { - type AnimatedValue = Self; - - #[inline] - fn to_animated_value(self) -> Self { - self - } - - #[inline] - fn from_animated_value(animated: Self::AnimatedValue) -> Self { - use values::computed::{Length, LengthOrPercentageOrNone, Percentage}; - use values::generics::length::MaxLength as GenericMaxLength; - match animated { - GenericMaxLength::LengthOrPercentageOrNone(lopn) => { - let result = match lopn { - LengthOrPercentageOrNone::Length(px) => { - LengthOrPercentageOrNone::Length(Length::new(px.px().max(0.))) - }, - LengthOrPercentageOrNone::Percentage(percentage) => { - LengthOrPercentageOrNone::Percentage(Percentage(percentage.0.max(0.))) - }, - _ => lopn, - }; - GenericMaxLength::LengthOrPercentageOrNone(result) - }, - _ => animated, - } - } -} - -impl ToAnimatedValue for ComputedMozLength { - type AnimatedValue = Self; - - #[inline] - fn to_animated_value(self) -> Self { - self - } - - #[inline] - fn from_animated_value(animated: Self::AnimatedValue) -> Self { - use values::computed::{Length, LengthOrPercentageOrAuto, Percentage}; - use values::generics::length::MozLength as GenericMozLength; - match animated { - GenericMozLength::LengthOrPercentageOrAuto(lopa) => { - let result = match lopa { - LengthOrPercentageOrAuto::Length(px) => { - LengthOrPercentageOrAuto::Length(Length::new(px.px().max(0.))) - }, - LengthOrPercentageOrAuto::Percentage(percentage) => { - LengthOrPercentageOrAuto::Percentage(Percentage(percentage.0.max(0.))) - }, - _ => lopa, - }; - GenericMozLength::LengthOrPercentageOrAuto(result) - }, - _ => animated, - } - } -} - impl ToAnimatedZero for Au { #[inline] fn to_animated_zero(&self) -> Result { From 707bd841a82c001c77fb77904c297aa4d11dfe70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 23:18:18 +0000 Subject: [PATCH 23/32] style: Move various font-related animation code to its own file. Similarly, the code is not trivial and doesn't really need to be in mako. Differential Revision: https://phabricator.services.mozilla.com/D10842 --- .../helpers/animated_properties.mako.rs | 147 ----------------- components/style/values/animated/font.rs | 156 ++++++++++++++++++ components/style/values/animated/mod.rs | 1 + 3 files changed, 157 insertions(+), 147 deletions(-) create mode 100644 components/style/values/animated/font.rs diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 85c588502c5..0e39495afcf 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -17,7 +17,6 @@ use itertools::{EitherOrBoth, Itertools}; use num_traits::Zero; use properties::{CSSWideKeyword, PropertyDeclaration}; use properties::longhands; -use properties::longhands::font_weight::computed_value::T as FontWeight; use properties::longhands::visibility::computed_value::T as Visibility; use properties::{LonghandId, ShorthandId}; use servo_arc::Arc; @@ -46,8 +45,6 @@ use values::computed::transform::Scale as ComputedScale; use values::computed::url::ComputedUrl; use values::generics::transform::{self, Rotate, Translate, Scale, Transform, TransformOperation}; use values::distance::{ComputeSquaredDistance, SquaredDistance}; -use values::generics::font::{FontSettings as GenericFontSettings, FontTag, VariationValue}; -use values::computed::font::FontVariationSettings; use values::generics::effects::Filter; use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; @@ -844,150 +841,6 @@ impl ToAnimatedZero for Visibility { } } -impl ToAnimatedZero for FontWeight { - #[inline] - fn to_animated_zero(&self) -> Result { - Ok(FontWeight::normal()) - } -} - -/// -impl Animate for FontVariationSettings { - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - FontSettingTagIter::new(self, other)? - .map(|r| r.and_then(|(st, ot)| st.animate(&ot, procedure))) - .collect::, ()>>() - .map(|v| GenericFontSettings(v.into_boxed_slice())) - } -} - -impl ComputeSquaredDistance for FontVariationSettings { - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - FontSettingTagIter::new(self, other)? - .map(|r| r.and_then(|(st, ot)| st.compute_squared_distance(&ot))) - .sum() - } -} - -impl ToAnimatedZero for FontVariationSettings { - #[inline] - fn to_animated_zero(&self) -> Result { - Err(()) - } -} - -type ComputedVariationValue = VariationValue; - -// FIXME: Could do a rename, this is only used for font variations. -struct FontSettingTagIterState<'a> { - tags: Vec<(&'a ComputedVariationValue)>, - index: usize, - prev_tag: FontTag, -} - -impl<'a> FontSettingTagIterState<'a> { - fn new(tags: Vec<<&'a ComputedVariationValue>) -> FontSettingTagIterState<'a> { - FontSettingTagIterState { - index: tags.len(), - tags, - prev_tag: FontTag(0), - } - } -} - -/// Iterator for font-variation-settings tag lists -/// -/// [CSS fonts level 4](https://drafts.csswg.org/css-fonts-4/#descdef-font-face-font-variation-settings) -/// defines the animation of font-variation-settings as follows: -/// -/// Two declarations of font-feature-settings[sic] can be animated between if -/// they are "like". "Like" declarations are ones where the same set of -/// properties appear (in any order). Because succesive[sic] duplicate -/// properties are applied instead of prior duplicate properties, two -/// declarations can be "like" even if they have differing number of -/// properties. If two declarations are "like" then animation occurs pairwise -/// between corresponding values in the declarations. -/// -/// In other words if we have the following lists: -/// -/// "wght" 1.4, "wdth" 5, "wght" 2 -/// "wdth" 8, "wght" 4, "wdth" 10 -/// -/// We should animate between: -/// -/// "wdth" 5, "wght" 2 -/// "wght" 4, "wdth" 10 -/// -/// This iterator supports this by sorting the two lists, then iterating them in -/// reverse, and skipping entries with repeated tag names. It will return -/// Some(Err()) if it reaches the end of one list before the other, or if the -/// tag names do not match. -/// -/// For the above example, this iterator would return: -/// -/// Some(Ok("wght" 2, "wght" 4)) -/// Some(Ok("wdth" 5, "wdth" 10)) -/// None -/// -struct FontSettingTagIter<'a> { - a_state: FontSettingTagIterState<'a>, - b_state: FontSettingTagIterState<'a>, -} - -impl<'a> FontSettingTagIter<'a> { - fn new( - a_settings: &'a FontVariationSettings, - b_settings: &'a FontVariationSettings, - ) -> Result, ()> { - if a_settings.0.is_empty() || b_settings.0.is_empty() { - return Err(()); - } - fn as_new_sorted_tags(tags: &[ComputedVariationValue]) -> Vec<<&ComputedVariationValue> { - use std::iter::FromIterator; - let mut sorted_tags = Vec::from_iter(tags.iter()); - sorted_tags.sort_by_key(|k| k.tag.0); - sorted_tags - }; - - Ok(FontSettingTagIter { - a_state: FontSettingTagIterState::new(as_new_sorted_tags(&a_settings.0)), - b_state: FontSettingTagIterState::new(as_new_sorted_tags(&b_settings.0)), - }) - } - - fn next_tag(state: &mut FontSettingTagIterState<'a>) -> Option<<&'a ComputedVariationValue> { - if state.index == 0 { - return None; - } - - state.index -= 1; - let tag = state.tags[state.index]; - if tag.tag == state.prev_tag { - FontSettingTagIter::next_tag(state) - } else { - state.prev_tag = tag.tag; - Some(tag) - } - } -} - -impl<'a> Iterator for FontSettingTagIter<'a> { - type Item = Result<(&'a ComputedVariationValue, &'a ComputedVariationValue), ()>; - - fn next(&mut self) -> Option> { - match ( - FontSettingTagIter::next_tag(&mut self.a_state), - FontSettingTagIter::next_tag(&mut self.b_state), - ) { - (Some(at), Some(bt)) if at.tag == bt.tag => Some(Ok((at, bt))), - (None, None) => None, - _ => Some(Err(())), // Mismatch number of unique tags or tag names. - } - } -} - /// impl Animate for ClipRect { #[inline] diff --git a/components/style/values/animated/font.rs b/components/style/values/animated/font.rs new file mode 100644 index 00000000000..26c88a2d3f1 --- /dev/null +++ b/components/style/values/animated/font.rs @@ -0,0 +1,156 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Animation implementation for various font-related types. + +use values::computed::Number; +use values::computed::font::{FontWeight, FontVariationSettings}; +use values::distance::{ComputeSquaredDistance, SquaredDistance}; +use values::generics::font::{FontSettings as GenericFontSettings, FontTag, VariationValue}; +use super::{Animate, Procedure, ToAnimatedZero}; + +impl ToAnimatedZero for FontWeight { + #[inline] + fn to_animated_zero(&self) -> Result { + Ok(FontWeight::normal()) + } +} + +/// +impl Animate for FontVariationSettings { + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + FontSettingTagIter::new(self, other)? + .map(|r| r.and_then(|(st, ot)| st.animate(&ot, procedure))) + .collect::, ()>>() + .map(|v| GenericFontSettings(v.into_boxed_slice())) + } +} + +impl ComputeSquaredDistance for FontVariationSettings { + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + FontSettingTagIter::new(self, other)? + .map(|r| r.and_then(|(st, ot)| st.compute_squared_distance(&ot))) + .sum() + } +} + +impl ToAnimatedZero for FontVariationSettings { + #[inline] + fn to_animated_zero(&self) -> Result { + Err(()) + } +} + +type ComputedVariationValue = VariationValue; + +// FIXME: Could do a rename, this is only used for font variations. +struct FontSettingTagIterState<'a> { + tags: Vec<(&'a ComputedVariationValue)>, + index: usize, + prev_tag: FontTag, +} + +impl<'a> FontSettingTagIterState<'a> { + fn new(tags: Vec<&'a ComputedVariationValue>) -> FontSettingTagIterState<'a> { + FontSettingTagIterState { + index: tags.len(), + tags, + prev_tag: FontTag(0), + } + } +} + +/// Iterator for font-variation-settings tag lists +/// +/// [CSS fonts level 4](https://drafts.csswg.org/css-fonts-4/#descdef-font-face-font-variation-settings) +/// defines the animation of font-variation-settings as follows: +/// +/// Two declarations of font-feature-settings[sic] can be animated between if +/// they are "like". "Like" declarations are ones where the same set of +/// properties appear (in any order). Because succesive[sic] duplicate +/// properties are applied instead of prior duplicate properties, two +/// declarations can be "like" even if they have differing number of +/// properties. If two declarations are "like" then animation occurs pairwise +/// between corresponding values in the declarations. +/// +/// In other words if we have the following lists: +/// +/// "wght" 1.4, "wdth" 5, "wght" 2 +/// "wdth" 8, "wght" 4, "wdth" 10 +/// +/// We should animate between: +/// +/// "wdth" 5, "wght" 2 +/// "wght" 4, "wdth" 10 +/// +/// This iterator supports this by sorting the two lists, then iterating them in +/// reverse, and skipping entries with repeated tag names. It will return +/// Some(Err()) if it reaches the end of one list before the other, or if the +/// tag names do not match. +/// +/// For the above example, this iterator would return: +/// +/// Some(Ok("wght" 2, "wght" 4)) +/// Some(Ok("wdth" 5, "wdth" 10)) +/// None +/// +struct FontSettingTagIter<'a> { + a_state: FontSettingTagIterState<'a>, + b_state: FontSettingTagIterState<'a>, +} + +impl<'a> FontSettingTagIter<'a> { + fn new( + a_settings: &'a FontVariationSettings, + b_settings: &'a FontVariationSettings, + ) -> Result, ()> { + if a_settings.0.is_empty() || b_settings.0.is_empty() { + return Err(()); + } + + fn as_new_sorted_tags(tags: &[ComputedVariationValue]) -> Vec<&ComputedVariationValue> { + use std::iter::FromIterator; + let mut sorted_tags = Vec::from_iter(tags.iter()); + sorted_tags.sort_by_key(|k| k.tag.0); + sorted_tags + }; + + Ok(FontSettingTagIter { + a_state: FontSettingTagIterState::new(as_new_sorted_tags(&a_settings.0)), + b_state: FontSettingTagIterState::new(as_new_sorted_tags(&b_settings.0)), + }) + } + + fn next_tag(state: &mut FontSettingTagIterState<'a>) -> Option<&'a ComputedVariationValue> { + if state.index == 0 { + return None; + } + + state.index -= 1; + let tag = state.tags[state.index]; + if tag.tag == state.prev_tag { + FontSettingTagIter::next_tag(state) + } else { + state.prev_tag = tag.tag; + Some(tag) + } + } +} + +impl<'a> Iterator for FontSettingTagIter<'a> { + type Item = Result<(&'a ComputedVariationValue, &'a ComputedVariationValue), ()>; + + fn next(&mut self) -> Option> { + match ( + FontSettingTagIter::next_tag(&mut self.a_state), + FontSettingTagIter::next_tag(&mut self.b_state), + ) { + (Some(at), Some(bt)) if at.tag == bt.tag => Some(Ok((at, bt))), + (None, None) => None, + _ => Some(Err(())), // Mismatch number of unique tags or tag names. + } + } +} diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 7d781f19a5d..5c90a1a1fd3 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -20,6 +20,7 @@ use values::computed::url::ComputedUrl; pub mod color; pub mod effects; +mod font; mod length; /// The category a property falls into for ordering purposes. From 8b49ef813fe787852b47cddb6bcae8dc6808d10c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 14:01:19 +0100 Subject: [PATCH 24/32] style: Remove nscsspropertyid_is_{animatable,transitionable}. There are better ways, plus the existing code didn't handle aliases at all (not that it needed to, but it's better if it does). Differential Revision: https://phabricator.services.mozilla.com/D10838 --- components/style/properties/data.py | 2 ++ .../helpers/animated_properties.mako.rs | 27 ------------------- .../style/properties/properties.mako.rs | 14 ++++++++++ 3 files changed, 16 insertions(+), 27 deletions(-) diff --git a/components/style/properties/data.py b/components/style/properties/data.py index 38b5273db26..5b07f9c326a 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -432,6 +432,8 @@ class Alias(object): self.original = original self.enabled_in = original.enabled_in self.servo_pref = original.servo_pref + self.animatable = original.animatable + self.transitionable = original.transitionable self.gecko_pref = gecko_pref self.allowed_in_page_rule = original.allowed_in_page_rule self.allowed_in_keyframe_block = original.allowed_in_keyframe_block diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 0e39495afcf..1936bacc3c4 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -50,20 +50,6 @@ use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint} use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; use void::{self, Void}; - -/// Returns true if this nsCSSPropertyID is one of the animatable properties. -#[cfg(feature = "gecko")] -pub fn nscsspropertyid_is_animatable(property: nsCSSPropertyID) -> bool { - match property { - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.animatable: - ${prop.nscsspropertyid()} => true, - % endif - % endfor - _ => false - } -} - /// Convert nsCSSPropertyID to TransitionProperty #[cfg(feature = "gecko")] #[allow(non_upper_case_globals)] @@ -90,19 +76,6 @@ impl From for TransitionProperty { } } -/// Returns true if this nsCSSPropertyID is one of the transitionable properties. -#[cfg(feature = "gecko")] -pub fn nscsspropertyid_is_transitionable(property: nsCSSPropertyID) -> bool { - match property { - % for prop in data.longhands + data.shorthands_except_all(): - % if prop.transitionable: - ${prop.nscsspropertyid()} => true, - % endif - % endfor - _ => false - } -} - /// An animated property interpolation between two computed values for that /// property. #[derive(Clone, Debug, PartialEq)] diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 6676e956e38..33220ebfd60 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -471,6 +471,20 @@ impl NonCustomPropertyId { MAP[self.0] } + /// Returns whether this property is transitionable. + #[inline] + pub fn is_transitionable(self) -> bool { + ${static_non_custom_property_id_set("TRANSITIONABLE", lambda p: p.transitionable)} + TRANSITIONABLE.contains(self) + } + + /// Returns whether this property is animatable. + #[inline] + pub fn is_animatable(self) -> bool { + ${static_non_custom_property_id_set("ANIMATABLE", lambda p: p.animatable)} + ANIMATABLE.contains(self) + } + #[inline] fn enabled_for_all_content(self) -> bool { ${static_non_custom_property_id_set( From c88a483322b7108b32ce9807a0cf2c1a4c79ed19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 14:56:21 +0100 Subject: [PATCH 25/32] style: Move animation of svg-related bits outside of animated_properties. Being in mako is unnecessary, and makes it harder to debug and such. Differential Revision: https://phabricator.services.mozilla.com/D10843 --- .../helpers/animated_properties.mako.rs | 206 +---------------- components/style/values/animated/mod.rs | 1 + components/style/values/animated/svg.rs | 213 ++++++++++++++++++ 3 files changed, 215 insertions(+), 205 deletions(-) create mode 100644 components/style/values/animated/svg.rs diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 1936bacc3c4..79287e178d3 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -27,14 +27,12 @@ use hash::FxHashMap; use super::ComputedValues; use values::CSSFloat; use values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; -use values::animated::color::Color as AnimatedColor; use values::animated::effects::Filter as AnimatedFilter; #[cfg(feature = "gecko")] use values::computed::TransitionProperty; use values::computed::Angle; use values::computed::{ClipRect, Context}; use values::computed::{Length, LengthOrPercentage}; -use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, Percentage}; -use values::computed::length::NonNegativeLengthOrPercentage; +use values::computed::{Number, Percentage}; use values::computed::ToComputedValue; use values::computed::transform::{DirectionVector, Matrix, Matrix3D}; use values::computed::transform::TransformOperation as ComputedTransformOperation; @@ -42,12 +40,9 @@ use values::computed::transform::Transform as ComputedTransform; use values::computed::transform::Rotate as ComputedRotate; use values::computed::transform::Translate as ComputedTranslate; use values::computed::transform::Scale as ComputedScale; -use values::computed::url::ComputedUrl; use values::generics::transform::{self, Rotate, Translate, Scale, Transform, TransformOperation}; use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::effects::Filter; -use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; -use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; use void::{self, Void}; /// Convert nsCSSPropertyID to TransitionProperty @@ -2535,205 +2530,6 @@ impl ComputeSquaredDistance for ComputedTransform { } } -/// Animated SVGPaint -pub type IntermediateSVGPaint = SVGPaint; - -/// Animated SVGPaintKind -pub type IntermediateSVGPaintKind = SVGPaintKind; - -impl ToAnimatedZero for IntermediateSVGPaint { - #[inline] - fn to_animated_zero(&self) -> Result { - Ok(IntermediateSVGPaint { - kind: self.kind.to_animated_zero()?, - fallback: self.fallback.and_then(|v| v.to_animated_zero().ok()), - }) - } -} - -impl From for NumberOrPercentage { - fn from(lop: NonNegativeLengthOrPercentage) -> NumberOrPercentage { - lop.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(num: NonNegativeNumber) -> NumberOrPercentage { - num.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(lop: LengthOrPercentage) -> NumberOrPercentage { - match lop { - LengthOrPercentage::Length(len) => NumberOrPercentage::Number(len.px()), - LengthOrPercentage::Percentage(p) => NumberOrPercentage::Percentage(p), - LengthOrPercentage::Calc(_) => { - panic!("We dont't expected calc interpolation for SvgLengthOrPercentageOrNumber"); - }, - } - } -} - -impl From for NumberOrPercentage { - fn from(num: Number) -> NumberOrPercentage { - NumberOrPercentage::Number(num) - } -} - -fn convert_to_number_or_percentage( - from: SvgLengthOrPercentageOrNumber) - -> NumberOrPercentage - where LengthOrPercentageType: Into, - NumberType: Into -{ - match from { - SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { - lop.into() - } - SvgLengthOrPercentageOrNumber::Number(num) => { - num.into() - } - } -} - -fn convert_from_number_or_percentage( - from: NumberOrPercentage) - -> SvgLengthOrPercentageOrNumber - where LengthOrPercentageType: From, - NumberType: From -{ - match from { - NumberOrPercentage::Number(num) => - SvgLengthOrPercentageOrNumber::Number(num.into()), - NumberOrPercentage::Percentage(p) => - SvgLengthOrPercentageOrNumber::LengthOrPercentage( - (LengthOrPercentage::Percentage(p)).into()) - } -} - -impl Animate for SvgLengthOrPercentageOrNumber -where - L: Animate + From + Into + Copy, - N: Animate + From + Into, - LengthOrPercentage: From, - Self: Copy, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - if self.has_calc() || other.has_calc() { - // TODO: We need to treat calc value. - // https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 - return Err(()); - } - - let this = convert_to_number_or_percentage(*self); - let other = convert_to_number_or_percentage(*other); - - match (this, other) { - ( - NumberOrPercentage::Number(ref this), - NumberOrPercentage::Number(ref other), - ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Number(this.animate(other, procedure)?) - )) - }, - ( - NumberOrPercentage::Percentage(ref this), - NumberOrPercentage::Percentage(ref other), - ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Percentage(this.animate(other, procedure)?) - )) - }, - _ => Err(()), - } - } -} - -impl Animate for SVGLength -where - L: Animate + Clone, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - match (self, other) { - (&SVGLength::Length(ref this), &SVGLength::Length(ref other)) => { - Ok(SVGLength::Length(this.animate(other, procedure)?)) - }, - _ => Err(()), - } - } -} - -/// -impl Animate for SVGStrokeDashArray -where - L: Clone + Animate, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - if matches!(procedure, Procedure::Add | Procedure::Accumulate { .. }) { - // Non-additive. - return Err(()); - } - match (self, other) { - (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => { - Ok(SVGStrokeDashArray::Values(this.animate_repeatable_list(other, procedure)?)) - }, - _ => Err(()), - } - } -} - -impl ComputeSquaredDistance for SVGStrokeDashArray -where - L: ComputeSquaredDistance, -{ - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - match (self, other) { - (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => { - this.squared_distance_repeatable_list(other) - }, - _ => Err(()), - } - } -} - -impl ToAnimatedZero for SVGStrokeDashArray -where - L: ToAnimatedZero, -{ - #[inline] - fn to_animated_zero(&self) -> Result { - match *self { - SVGStrokeDashArray::Values(ref values) => { - Ok(SVGStrokeDashArray::Values( - values.iter().map(ToAnimatedZero::to_animated_zero).collect::, _>>()?, - )) - } - SVGStrokeDashArray::ContextValue => Ok(SVGStrokeDashArray::ContextValue), - } - } -} - -impl Animate for SVGOpacity -where - O: Animate + Clone, -{ - #[inline] - fn animate(&self, other: &Self, procedure: Procedure) -> Result { - match (self, other) { - (&SVGOpacity::Opacity(ref this), &SVGOpacity::Opacity(ref other)) => { - Ok(SVGOpacity::Opacity(this.animate(other, procedure)?)) - }, - _ => Err(()), - } - } -} - <% FILTER_FUNCTIONS = [ 'Blur', 'Brightness', 'Contrast', 'Grayscale', 'HueRotate', 'Invert', 'Opacity', 'Saturate', diff --git a/components/style/values/animated/mod.rs b/components/style/values/animated/mod.rs index 5c90a1a1fd3..3fe369c4e91 100644 --- a/components/style/values/animated/mod.rs +++ b/components/style/values/animated/mod.rs @@ -22,6 +22,7 @@ pub mod color; pub mod effects; mod font; mod length; +mod svg; /// The category a property falls into for ordering purposes. /// diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs new file mode 100644 index 00000000000..49641a8e376 --- /dev/null +++ b/components/style/values/animated/svg.rs @@ -0,0 +1,213 @@ +/* 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 http://mozilla.org/MPL/2.0/. */ + +//! Animation implementations for various SVG-related types. + +use properties::animated_properties::ListAnimation; +use values::animated::color::Color as AnimatedColor; +use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, LengthOrPercentage}; +use values::computed::url::ComputedUrl; +use values::computed::length::NonNegativeLengthOrPercentage; +use values::distance::{ComputeSquaredDistance, SquaredDistance}; +use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; +use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; +use super::{Animate, Procedure, ToAnimatedZero}; + +/// Animated SVGPaint. +pub type IntermediateSVGPaint = SVGPaint; + +impl ToAnimatedZero for IntermediateSVGPaint { + #[inline] + fn to_animated_zero(&self) -> Result { + Ok(IntermediateSVGPaint { + kind: self.kind.to_animated_zero()?, + fallback: self.fallback.and_then(|v| v.to_animated_zero().ok()), + }) + } +} + +impl From for NumberOrPercentage { + fn from(lop: NonNegativeLengthOrPercentage) -> NumberOrPercentage { + lop.0.into() + } +} + +impl From for NumberOrPercentage { + fn from(num: NonNegativeNumber) -> NumberOrPercentage { + num.0.into() + } +} + +impl From for NumberOrPercentage { + fn from(lop: LengthOrPercentage) -> NumberOrPercentage { + match lop { + LengthOrPercentage::Length(len) => NumberOrPercentage::Number(len.px()), + LengthOrPercentage::Percentage(p) => NumberOrPercentage::Percentage(p), + LengthOrPercentage::Calc(_) => { + panic!("We dont't expected calc interpolation for SvgLengthOrPercentageOrNumber"); + }, + } + } +} + +impl From for NumberOrPercentage { + fn from(num: Number) -> NumberOrPercentage { + NumberOrPercentage::Number(num) + } +} + +fn convert_to_number_or_percentage( + from: SvgLengthOrPercentageOrNumber, +) -> NumberOrPercentage +where + LengthOrPercentageType: Into, + NumberType: Into, +{ + match from { + SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { + lop.into() + } + SvgLengthOrPercentageOrNumber::Number(num) => { + num.into() + } + } +} + +fn convert_from_number_or_percentage( + from: NumberOrPercentage, +) -> SvgLengthOrPercentageOrNumber +where + LengthOrPercentageType: From, + NumberType: From, +{ + match from { + NumberOrPercentage::Number(num) => + SvgLengthOrPercentageOrNumber::Number(num.into()), + NumberOrPercentage::Percentage(p) => + SvgLengthOrPercentageOrNumber::LengthOrPercentage( + (LengthOrPercentage::Percentage(p)).into()) + } +} + +impl Animate for SvgLengthOrPercentageOrNumber +where + L: Animate + From + Into + Copy, + N: Animate + From + Into, + LengthOrPercentage: From, + Self: Copy, +{ + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + if self.has_calc() || other.has_calc() { + // TODO: We need to treat calc value. + // https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 + return Err(()); + } + + let this = convert_to_number_or_percentage(*self); + let other = convert_to_number_or_percentage(*other); + + match (this, other) { + ( + NumberOrPercentage::Number(ref this), + NumberOrPercentage::Number(ref other), + ) => { + Ok(convert_from_number_or_percentage( + NumberOrPercentage::Number(this.animate(other, procedure)?) + )) + }, + ( + NumberOrPercentage::Percentage(ref this), + NumberOrPercentage::Percentage(ref other), + ) => { + Ok(convert_from_number_or_percentage( + NumberOrPercentage::Percentage(this.animate(other, procedure)?) + )) + }, + _ => Err(()), + } + } +} + +impl Animate for SVGLength +where + L: Animate + Clone, +{ + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + match (self, other) { + (&SVGLength::Length(ref this), &SVGLength::Length(ref other)) => { + Ok(SVGLength::Length(this.animate(other, procedure)?)) + }, + _ => Err(()), + } + } +} + +/// +impl Animate for SVGStrokeDashArray +where + L: Clone + Animate, +{ + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + if matches!(procedure, Procedure::Add | Procedure::Accumulate { .. }) { + // Non-additive. + return Err(()); + } + match (self, other) { + (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => { + Ok(SVGStrokeDashArray::Values(this.animate_repeatable_list(other, procedure)?)) + }, + _ => Err(()), + } + } +} + +impl ComputeSquaredDistance for SVGStrokeDashArray +where + L: ComputeSquaredDistance, +{ + #[inline] + fn compute_squared_distance(&self, other: &Self) -> Result { + match (self, other) { + (&SVGStrokeDashArray::Values(ref this), &SVGStrokeDashArray::Values(ref other)) => { + this.squared_distance_repeatable_list(other) + }, + _ => Err(()), + } + } +} + +impl ToAnimatedZero for SVGStrokeDashArray +where + L: ToAnimatedZero, +{ + #[inline] + fn to_animated_zero(&self) -> Result { + match *self { + SVGStrokeDashArray::Values(ref values) => { + Ok(SVGStrokeDashArray::Values( + values.iter().map(ToAnimatedZero::to_animated_zero).collect::, _>>()?, + )) + } + SVGStrokeDashArray::ContextValue => Ok(SVGStrokeDashArray::ContextValue), + } + } +} + +impl Animate for SVGOpacity +where + O: Animate + Clone, +{ + #[inline] + fn animate(&self, other: &Self, procedure: Procedure) -> Result { + match (self, other) { + (&SVGOpacity::Opacity(ref this), &SVGOpacity::Opacity(ref other)) => { + Ok(SVGOpacity::Opacity(this.animate(other, procedure)?)) + }, + _ => Err(()), + } + } +} From 5af6abfb784c57ea7d0a81563a98fed8e46f3461 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sun, 4 Nov 2018 15:48:08 +0100 Subject: [PATCH 26/32] style: Simplify the SVG animation code. It's overly generic for no good reason. Differential Revision: https://phabricator.services.mozilla.com/D10844 --- components/style/properties/gecko.mako.rs | 23 ++-- .../longhands/inherited_svg.mako.rs | 7 +- components/style/values/animated/svg.rs | 112 +++++------------- components/style/values/computed/svg.rs | 20 ++-- components/style/values/generics/svg.rs | 57 +-------- 5 files changed, 63 insertions(+), 156 deletions(-) diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 59beaea0a80..d51a43166b9 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -561,17 +561,24 @@ def set_gecko_property(ffi_name, expr): return SVGLength::ContextValue; } let length = match self.gecko.${gecko_ffi_name}.as_value() { - CoordDataValue::Factor(number) => - SvgLengthOrPercentageOrNumber::Number(number), - CoordDataValue::Coord(coord) => + CoordDataValue::Factor(number) => { + SvgLengthOrPercentageOrNumber::Number(number) + }, + CoordDataValue::Coord(coord) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Length(Au(coord).into())), - CoordDataValue::Percent(p) => + LengthOrPercentage::Length(Au(coord).into()) + ) + }, + CoordDataValue::Percent(p) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Percentage(Percentage(p))), - CoordDataValue::Calc(calc) => + LengthOrPercentage::Percentage(Percentage(p)) + ) + }, + CoordDataValue::Calc(calc) => { SvgLengthOrPercentageOrNumber::LengthOrPercentage( - LengthOrPercentage::Calc(calc.into())), + LengthOrPercentage::Calc(calc.into()) + ) + }, _ => unreachable!("Unexpected coordinate in ${ident}"), }; SVGLength::Length(length.into()) diff --git a/components/style/properties/longhands/inherited_svg.mako.rs b/components/style/properties/longhands/inherited_svg.mako.rs index 3cdc5e76569..4ecad6e39fd 100644 --- a/components/style/properties/longhands/inherited_svg.mako.rs +++ b/components/style/properties/longhands/inherited_svg.mako.rs @@ -85,7 +85,7 @@ ${helpers.predefined_type( ${helpers.predefined_type( "stroke-width", "SVGWidth", - "::values::computed::NonNegativeLength::new(1.).into()", + "computed::SVGWidth::one()", products="gecko", animation_value_type="::values::computed::SVGWidth", spec="https://www.w3.org/TR/SVG2/painting.html#StrokeWidth", @@ -135,8 +135,9 @@ ${helpers.predefined_type( )} ${helpers.predefined_type( - "stroke-dashoffset", "SVGLength", - "Au(0).into()", + "stroke-dashoffset", + "SVGLength", + "computed::SVGLength::zero()", products="gecko", animation_value_type="ComputedValue", spec="https://www.w3.org/TR/SVG2/painting.html#StrokeDashing", diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 49641a8e376..0c4e3be7166 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -6,12 +6,11 @@ use properties::animated_properties::ListAnimation; use values::animated::color::Color as AnimatedColor; -use values::computed::{NonNegativeNumber, Number, NumberOrPercentage, LengthOrPercentage}; +use values::computed::{Number, NumberOrPercentage, LengthOrPercentage}; use values::computed::url::ComputedUrl; -use values::computed::length::NonNegativeLengthOrPercentage; use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; -use values::generics::svg::{SVGPaintKind, SVGStrokeDashArray, SVGOpacity}; +use values::generics::svg::{SVGStrokeDashArray, SVGOpacity}; use super::{Animate, Procedure, ToAnimatedZero}; /// Animated SVGPaint. @@ -27,102 +26,44 @@ impl ToAnimatedZero for IntermediateSVGPaint { } } -impl From for NumberOrPercentage { - fn from(lop: NonNegativeLengthOrPercentage) -> NumberOrPercentage { - lop.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(num: NonNegativeNumber) -> NumberOrPercentage { - num.0.into() - } -} - -impl From for NumberOrPercentage { - fn from(lop: LengthOrPercentage) -> NumberOrPercentage { - match lop { - LengthOrPercentage::Length(len) => NumberOrPercentage::Number(len.px()), - LengthOrPercentage::Percentage(p) => NumberOrPercentage::Percentage(p), - LengthOrPercentage::Calc(_) => { - panic!("We dont't expected calc interpolation for SvgLengthOrPercentageOrNumber"); - }, +// FIXME: We need to handle calc here properly, see +// https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 +fn to_number_or_percentage( + value: &SvgLengthOrPercentageOrNumber, +) -> Result { + Ok(match *value { + SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref l) => { + match *l { + LengthOrPercentage::Length(ref l) => NumberOrPercentage::Number(l.px()), + LengthOrPercentage::Percentage(ref p) => NumberOrPercentage::Percentage(*p), + LengthOrPercentage::Calc(..) => return Err(()), + } } - } + SvgLengthOrPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n), + }) } -impl From for NumberOrPercentage { - fn from(num: Number) -> NumberOrPercentage { - NumberOrPercentage::Number(num) - } -} - -fn convert_to_number_or_percentage( - from: SvgLengthOrPercentageOrNumber, -) -> NumberOrPercentage -where - LengthOrPercentageType: Into, - NumberType: Into, -{ - match from { - SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { - lop.into() - } - SvgLengthOrPercentageOrNumber::Number(num) => { - num.into() - } - } -} - -fn convert_from_number_or_percentage( - from: NumberOrPercentage, -) -> SvgLengthOrPercentageOrNumber -where - LengthOrPercentageType: From, - NumberType: From, -{ - match from { - NumberOrPercentage::Number(num) => - SvgLengthOrPercentageOrNumber::Number(num.into()), - NumberOrPercentage::Percentage(p) => - SvgLengthOrPercentageOrNumber::LengthOrPercentage( - (LengthOrPercentage::Percentage(p)).into()) - } -} - -impl Animate for SvgLengthOrPercentageOrNumber -where - L: Animate + From + Into + Copy, - N: Animate + From + Into, - LengthOrPercentage: From, - Self: Copy, -{ +impl Animate for SvgLengthOrPercentageOrNumber { #[inline] fn animate(&self, other: &Self, procedure: Procedure) -> Result { - if self.has_calc() || other.has_calc() { - // TODO: We need to treat calc value. - // https://bugzilla.mozilla.org/show_bug.cgi?id=1386967 - return Err(()); - } - - let this = convert_to_number_or_percentage(*self); - let other = convert_to_number_or_percentage(*other); + let this = to_number_or_percentage(self)?; + let other = to_number_or_percentage(other)?; match (this, other) { ( NumberOrPercentage::Number(ref this), NumberOrPercentage::Number(ref other), ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Number(this.animate(other, procedure)?) + Ok(SvgLengthOrPercentageOrNumber::Number( + this.animate(other, procedure)? )) }, ( NumberOrPercentage::Percentage(ref this), NumberOrPercentage::Percentage(ref other), ) => { - Ok(convert_from_number_or_percentage( - NumberOrPercentage::Percentage(this.animate(other, procedure)?) + Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage( + LengthOrPercentage::Percentage(this.animate(other, procedure)?) )) }, _ => Err(()), @@ -130,6 +71,13 @@ where } } +impl ComputeSquaredDistance for SvgLengthOrPercentageOrNumber { + fn compute_squared_distance(&self, other: &Self) -> Result { + to_number_or_percentage(self)? + .compute_squared_distance(&to_number_or_percentage(other)?) + } +} + impl Animate for SVGLength where L: Animate + Clone, diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index ab9bbd18b4e..11917b01779 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -4,9 +4,8 @@ //! Computed types for SVG properties. -use app_units::Au; use values::RGBA; -use values::computed::{LengthOrPercentage, NonNegativeLength}; +use values::computed::LengthOrPercentage; use values::computed::{NonNegativeLengthOrPercentage, NonNegativeNumber, Number}; use values::computed::Opacity; use values::computed::color::Color; @@ -50,10 +49,11 @@ pub type SvgLengthOrPercentageOrNumber = /// | | | context-value pub type SVGLength = generic::SVGLength; -impl From for SVGLength { - fn from(length: Au) -> Self { +impl SVGLength { + /// `0px` + pub fn zero() -> Self { generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage( - length.into(), + LengthOrPercentage::zero() )) } } @@ -63,6 +63,8 @@ impl From for SVGLength { pub type NonNegativeSvgLengthOrPercentageOrNumber = generic::SvgLengthOrPercentageOrNumber; +// FIXME(emilio): This is really hacky, and can go away with a bit of work on +// the clone_stroke_width code in gecko.mako.rs. impl Into for SvgLengthOrPercentageOrNumber { fn into(self) -> NonNegativeSvgLengthOrPercentageOrNumber { match self { @@ -79,10 +81,12 @@ impl Into for SvgLengthOrPercentageOrN /// An non-negative wrapper of SVGLength. pub type SVGWidth = generic::SVGLength; -impl From for SVGWidth { - fn from(length: NonNegativeLength) -> Self { +impl SVGWidth { + /// `1px`. + pub fn one() -> Self { + use values::generics::NonNegative; generic::SVGLength::Length(generic::SvgLengthOrPercentageOrNumber::LengthOrPercentage( - length.into(), + NonNegative(LengthOrPercentage::one()) )) } } diff --git a/components/style/values/generics/svg.rs b/components/style/values/generics/svg.rs index 8b8fa786e8f..2dd81da5eda 100644 --- a/components/style/values/generics/svg.rs +++ b/components/style/values/generics/svg.rs @@ -8,9 +8,6 @@ use cssparser::Parser; use parser::{Parse, ParserContext}; use style_traits::{ParseError, StyleParseErrorKind}; use values::{Either, None_}; -use values::computed::NumberOrPercentage; -use values::computed::length::LengthOrPercentage; -use values::distance::{ComputeSquaredDistance, SquaredDistance}; /// An SVG paint value /// @@ -152,54 +149,6 @@ pub enum SvgLengthOrPercentageOrNumber { Number(Number), } -impl ComputeSquaredDistance for SvgLengthOrPercentageOrNumber -where - L: ComputeSquaredDistance + Copy + Into, - N: ComputeSquaredDistance + Copy + Into, -{ - #[inline] - fn compute_squared_distance(&self, other: &Self) -> Result { - match (self, other) { - ( - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref from), - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(ref to), - ) => from.compute_squared_distance(to), - ( - &SvgLengthOrPercentageOrNumber::Number(ref from), - &SvgLengthOrPercentageOrNumber::Number(ref to), - ) => from.compute_squared_distance(to), - ( - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(from), - &SvgLengthOrPercentageOrNumber::Number(to), - ) => from.into().compute_squared_distance(&to.into()), - ( - &SvgLengthOrPercentageOrNumber::Number(from), - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(to), - ) => from.into().compute_squared_distance(&to.into()), - } - } -} - -impl - SvgLengthOrPercentageOrNumber -where - LengthOrPercentage: From, - LengthOrPercentageType: Copy, -{ - /// return true if this struct has calc value. - pub fn has_calc(&self) -> bool { - match self { - &SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop) => { - match LengthOrPercentage::from(lop) { - LengthOrPercentage::Calc(_) => true, - _ => false, - } - }, - _ => false, - } - } -} - /// Parsing the SvgLengthOrPercentageOrNumber. At first, we need to parse number /// since prevent converting to the length. impl Parse @@ -213,10 +162,8 @@ impl Parse return Ok(SvgLengthOrPercentageOrNumber::Number(num)); } - if let Ok(lop) = input.try(|i| LengthOrPercentageType::parse(context, i)) { - return Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop)); - } - Err(input.new_custom_error(StyleParseErrorKind::UnspecifiedError)) + let lop = LengthOrPercentageType::parse(context, input)?; + Ok(SvgLengthOrPercentageOrNumber::LengthOrPercentage(lop)) } } From b7da1bac888b7464164cf7223528beb25826f627 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 10:39:46 +0000 Subject: [PATCH 27/32] style: Implement the env() function with hardcoded zeros for safe-area-inset. Intent to Implement and Ship: https://groups.google.com/d/msg/mozilla.dev.platform/EVKyR1B87T0/_l-_qK8SAAAJ Differential Revision: https://phabricator.services.mozilla.com/D9609 --- components/malloc_size_of/Cargo.toml | 2 +- components/selectors/Cargo.toml | 2 +- components/style/Cargo.toml | 2 +- components/style/custom_properties.rs | 370 +++++++++++++----- components/style/gecko/media_queries.rs | 11 + components/style/properties/cascade.rs | 6 +- .../style/properties/declaration_block.rs | 24 +- .../helpers/animated_properties.mako.rs | 1 + .../style/properties/properties.mako.rs | 60 +-- components/style_traits/Cargo.toml | 2 +- tests/unit/style/Cargo.toml | 2 +- 11 files changed, 346 insertions(+), 136 deletions(-) diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index fff432e3fbc..252185fd0c6 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -26,7 +26,7 @@ servo = [ [dependencies] app_units = "0.7" -cssparser = "0.24.0" +cssparser = "0.25" euclid = "0.19" hashglobe = { path = "../hashglobe" } hyper = { version = "0.12", optional = true } diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index f7a0a6a85ce..86d4a718606 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -22,7 +22,7 @@ bench = [] [dependencies] bitflags = "1.0" matches = "0.1" -cssparser = "0.24.0" +cssparser = "0.25" log = "0.4" fxhash = "0.2" phf = "0.7.18" diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index b3ca19a34d7..15e41751603 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -31,7 +31,7 @@ atomic_refcell = "0.1" bitflags = "1.0" byteorder = "1.0" cfg-if = "0.1.0" -cssparser = "0.24.0" +cssparser = "0.25" new_debug_unreachable = "1.0" encoding_rs = {version = "0.7", optional = true} euclid = "0.19" diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 331d2a74342..344e38af8d1 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -21,6 +21,59 @@ use std::fmt::{self, Write}; use std::hash::Hash; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; +/// The environment from which to get `env` function values. +/// +/// TODO(emilio): If this becomes a bit more complex we should probably move it +/// to the `media_queries` module, or something. +pub struct CssEnvironment; + +struct EnvironmentVariable { + name: Atom, + value: VariableValue, +} + +macro_rules! make_variable { + ($name:expr, $value: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, + } + }, + } + }}; +} + +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"), + ]; +} + +impl CssEnvironment { + #[inline] + fn get(&self, name: &Atom) -> Option<&VariableValue> { + let var = ENVIRONMENT_VARIABLES.iter().find(|var| var.name == *name)?; + Some(&var.value) + } +} + /// A custom property name is just an `Atom`. /// /// Note that this does not include the `--` prefix @@ -48,6 +101,12 @@ pub struct VariableValue { first_token_type: TokenSerializationType, last_token_type: TokenSerializationType, + /// Whether a variable value has a reference to an environment variable. + /// + /// If this is the case, we need to perform variable substitution on the + /// value. + references_environment: bool, + /// Custom property names in var() functions. references: PrecomputedHashSet, } @@ -216,6 +275,14 @@ where } } +/// A struct holding information about the external references to that a custom +/// property value may have. +#[derive(Default)] +struct VarOrEnvReferences { + custom_property_references: PrecomputedHashSet, + references_environment: bool, +} + impl VariableValue { fn empty() -> Self { Self { @@ -223,6 +290,7 @@ impl VariableValue { last_token_type: TokenSerializationType::nothing(), first_token_type: TokenSerializationType::nothing(), references: PrecomputedHashSet::default(), + references_environment: false, } } @@ -273,7 +341,7 @@ impl VariableValue { /// Parse a custom property value. pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result, ParseError<'i>> { - let mut references = PrecomputedHashSet::default(); + let mut references = VarOrEnvReferences::default(); let (first_token_type, css, last_token_type) = parse_self_contained_declaration_value(input, Some(&mut references))?; @@ -282,7 +350,8 @@ impl VariableValue { css: css.into_owned(), first_token_type, last_token_type, - references, + references: references.custom_property_references, + references_environment: references.references_environment, })) } } @@ -297,7 +366,7 @@ pub fn parse_non_custom_with_var<'i, 't>( fn parse_self_contained_declaration_value<'i, 't>( input: &mut Parser<'i, 't>, - references: Option<&mut PrecomputedHashSet>, + references: Option<&mut VarOrEnvReferences>, ) -> Result<(TokenSerializationType, Cow<'i, str>, TokenSerializationType), ParseError<'i>> { let start_position = input.position(); let mut missing_closing_characters = String::new(); @@ -317,7 +386,7 @@ fn parse_self_contained_declaration_value<'i, 't>( /// fn parse_declaration_value<'i, 't>( input: &mut Parser<'i, 't>, - references: Option<&mut PrecomputedHashSet>, + references: Option<&mut VarOrEnvReferences>, missing_closing_characters: &mut String, ) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> { input.parse_until_before(Delimiter::Bang | Delimiter::Semicolon, |input| { @@ -334,7 +403,7 @@ fn parse_declaration_value<'i, 't>( /// invalid at the top level fn parse_declaration_value_block<'i, 't>( input: &mut Parser<'i, 't>, - mut references: Option<&mut PrecomputedHashSet>, + mut references: Option<&mut VarOrEnvReferences>, missing_closing_characters: &mut String, ) -> Result<(TokenSerializationType, TokenSerializationType), ParseError<'i>> { let mut token_start = input.position(); @@ -407,6 +476,12 @@ fn parse_declaration_value_block<'i, 't>( parse_var_function(input, references.as_mut().map(|r| &mut **r)) })?; input.reset(&args_start); + } else if name.eq_ignore_ascii_case("env") { + let args_start = input.state(); + input.parse_nested_block(|input| { + parse_env_function(input, references.as_mut().map(|r| &mut **r)) + })?; + input.reset(&args_start); } nested!(); check_closed!(")"); @@ -468,29 +543,48 @@ fn parse_declaration_value_block<'i, 't>( } } +fn parse_fallback<'i, 't>(input: &mut Parser<'i, 't>) -> Result<(), ParseError<'i>> { + // Exclude `!` and `;` at the top level + // https://drafts.csswg.org/css-syntax/#typedef-declaration-value + input.parse_until_before(Delimiter::Bang | Delimiter::Semicolon, |input| { + // At least one non-comment token. + input.next_including_whitespace()?; + // Skip until the end. + while let Ok(_) = input.next_including_whitespace_and_comments() {} + Ok(()) + }) +} + // If the var function is valid, return Ok((custom_property_name, fallback)) fn parse_var_function<'i, 't>( input: &mut Parser<'i, 't>, - references: Option<&mut PrecomputedHashSet>, + references: Option<&mut VarOrEnvReferences>, ) -> Result<(), ParseError<'i>> { let name = input.expect_ident_cloned()?; - let name: Result<_, ParseError> = parse_name(&name).map_err(|()| { + let name = parse_name(&name).map_err(|()| { input.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name.clone())) - }); - let name = name?; + })?; if input.try(|input| input.expect_comma()).is_ok() { - // Exclude `!` and `;` at the top level - // https://drafts.csswg.org/css-syntax/#typedef-declaration-value - input.parse_until_before(Delimiter::Bang | Delimiter::Semicolon, |input| { - // At least one non-comment token. - input.next_including_whitespace()?; - // Skip until the end. - while let Ok(_) = input.next_including_whitespace_and_comments() {} - Ok(()) - })?; + parse_fallback(input)?; } if let Some(refs) = references { - refs.insert(Atom::from(name)); + refs.custom_property_references.insert(Atom::from(name)); + } + Ok(()) +} + +fn parse_env_function<'i, 't>( + input: &mut Parser<'i, 't>, + references: Option<&mut VarOrEnvReferences>, +) -> Result<(), ParseError<'i>> { + // TODO(emilio): This should be per spec, but no other + // browser does that, see https://github.com/w3c/csswg-drafts/issues/3262. + input.expect_ident()?; + if input.try(|input| input.expect_comma()).is_ok() { + parse_fallback(input)?; + } + if let Some(references) = references { + references.references_environment = true; } Ok(()) } @@ -502,25 +596,26 @@ pub struct CustomPropertiesBuilder<'a> { may_have_cycles: bool, custom_properties: Option, inherited: Option<&'a Arc>, + environment: &'a CssEnvironment, } impl<'a> CustomPropertiesBuilder<'a> { /// Create a new builder, inheriting from a given custom properties map. - pub fn new(inherited: Option<&'a Arc>) -> Self { + pub fn new( + inherited: Option<&'a Arc>, + environment: &'a CssEnvironment, + ) -> Self { Self { seen: PrecomputedHashSet::default(), may_have_cycles: false, custom_properties: None, inherited, + environment, } } /// Cascade a given custom property declaration. - pub fn cascade( - &mut self, - name: &'a Name, - specified_value: &CustomDeclarationValue, - ) { + pub fn cascade(&mut self, name: &'a Name, specified_value: &CustomDeclarationValue) { let was_already_present = !self.seen.insert(name); if was_already_present { return; @@ -540,8 +635,31 @@ impl<'a> CustomPropertiesBuilder<'a> { let map = self.custom_properties.as_mut().unwrap(); match *specified_value { CustomDeclarationValue::Value(ref unparsed_value) => { - self.may_have_cycles |= !unparsed_value.references.is_empty(); - map.insert(name.clone(), (*unparsed_value).clone()); + let has_references = !unparsed_value.references.is_empty(); + self.may_have_cycles |= has_references; + + // If the variable value has no references and it has an + // environment variable here, perform substitution here instead + // of forcing a full traversal in `substitute_all` afterwards. + let value = if !has_references && unparsed_value.references_environment { + let invalid = Default::default(); // Irrelevant, since there are no references. + let result = substitute_references_in_value( + unparsed_value, + &map, + &invalid, + &self.environment, + ); + match result { + Ok(new_value) => Arc::new(new_value), + Err(..) => { + map.remove(name); + return; + }, + } + } else { + (*unparsed_value).clone() + }; + map.insert(name.clone(), value); }, CustomDeclarationValue::CSSWideKeyword(keyword) => match keyword { CSSWideKeyword::Initial => { @@ -553,11 +671,7 @@ impl<'a> CustomPropertiesBuilder<'a> { } } - fn value_may_affect_style( - &self, - name: &Name, - value: &CustomDeclarationValue, - ) -> bool { + fn value_may_affect_style(&self, name: &Name, value: &CustomDeclarationValue) -> bool { match *value { CustomDeclarationValue::CSSWideKeyword(CSSWideKeyword::Unset) | CustomDeclarationValue::CSSWideKeyword(CSSWideKeyword::Inherit) => { @@ -596,8 +710,8 @@ impl<'a> CustomPropertiesBuilder<'a> { /// Returns the final map of applicable custom properties. /// - /// If there was any specified property, we've created a new map and now we need - /// to remove any potential cycles, and wrap it in an arc. + /// If there was any specified property, we've created a new map and now we + /// need to remove any potential cycles, and wrap it in an arc. /// /// Otherwise, just use the inherited custom properties map. pub fn build(mut self) -> Option> { @@ -605,9 +719,8 @@ impl<'a> CustomPropertiesBuilder<'a> { Some(m) => m, None => return self.inherited.cloned(), }; - if self.may_have_cycles { - substitute_all(&mut map); + substitute_all(&mut map, self.environment); } Some(Arc::new(map)) } @@ -616,7 +729,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) { +fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: &CssEnvironment) { // The cycle dependencies removal in this function is a variant // of Tarjan's algorithm. It is mostly based on the pseudo-code // listed in @@ -664,6 +777,8 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { map: &'a mut CustomPropertiesMap, /// The set of invalid custom properties. invalid: &'a mut PrecomputedHashSet, + /// The environment to substitute `env()` variables. + environment: &'a CssEnvironment, } /// This function combines the traversal for cycle removal and value @@ -686,11 +801,23 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { /// * There is no such variable at all. fn traverse<'a>(name: Name, context: &mut Context<'a>) -> Option { // Some shortcut checks. - let (name, value) = if let Some(value) = context.map.get(&name) { - // This variable has been resolved. Return the signal value. - if value.references.is_empty() || context.invalid.contains(&name) { + let (name, value) = { + let value = context.map.get(&name)?; + + // Nothing to resolve. + if value.references.is_empty() { + debug_assert!( + !value.references_environment, + "Should've been handled earlier" + ); return None; } + + // This variable has already been resolved. + if context.invalid.contains(&name) { + return None; + } + // Whether this variable has been visited in this traversal. let key; match context.index_map.entry(name) { @@ -702,12 +829,10 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { entry.insert(context.count); }, } + // Hold a strong reference to the value so that we don't // need to keep reference to context.map. (key, value.clone()) - } else { - // The variable doesn't exist at all. - return None; }; // Add new entry to the information table. @@ -793,29 +918,20 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { // 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 mut computed_value = ComputedValue::empty(); - let mut input = ParserInput::new(&value.css); - let mut input = Parser::new(&mut input); - let mut position = (input.position(), value.first_token_type); - let result = substitute_block( - &mut input, - &mut position, - &mut computed_value, - &mut |name, partial_computed_value| { - if let Some(value) = context.map.get(name) { - if !context.invalid.contains(name) { - partial_computed_value.push_variable(value); - return Ok(value.last_token_type); - } - } - Err(()) - }, + let result = substitute_references_in_value( + &value, + &context.map, + &context.invalid, + &context.environment, ); - if let Ok(last_token_type) = result { - computed_value.push_from(position, &input, last_token_type); - context.map.insert(name, Arc::new(computed_value)); - } else { - context.invalid.insert(name); + + match result { + Ok(computed_value) => { + context.map.insert(name, Arc::new(computed_value)); + }, + Err(..) => { + context.invalid.insert(name); + }, } // All resolved, so return the signal value. @@ -834,6 +950,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { var_info: SmallVec::new(), map: custom_properties_map, invalid: &mut invalid, + environment, }; traverse(name, &mut context); } @@ -841,25 +958,51 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap) { custom_properties_map.remove_set(&invalid); } +/// Replace `var()` and `env()` functions in a pre-existing variable value. +fn substitute_references_in_value<'i>( + value: &'i VariableValue, + custom_properties: &CustomPropertiesMap, + invalid_custom_properties: &PrecomputedHashSet, + environment: &CssEnvironment, +) -> Result> { + debug_assert!(!value.references.is_empty() || value.references_environment); + + let mut input = ParserInput::new(&value.css); + let mut input = Parser::new(&mut input); + let mut position = (input.position(), value.first_token_type); + let mut computed_value = ComputedValue::empty(); + + let last_token_type = substitute_block( + &mut input, + &mut position, + &mut computed_value, + custom_properties, + invalid_custom_properties, + environment, + )?; + + computed_value.push_from(position, &input, last_token_type); + Ok(computed_value) +} + /// Replace `var()` functions in an arbitrary bit of input. /// -/// The `substitute_one` callback is called for each `var()` function in `input`. -/// If the variable has its initial value, -/// the callback should return `Err(())` and leave `partial_computed_value` unchanged. +/// If the variable has its initial value, the callback should return `Err(())` +/// and leave `partial_computed_value` unchanged. +/// /// Otherwise, it should push the value of the variable (with its own `var()` functions replaced) /// to `partial_computed_value` and return `Ok(last_token_type of what was pushed)` /// /// Return `Err(())` if `input` is invalid at computed-value time. /// or `Ok(last_token_type that was pushed to partial_computed_value)` otherwise. -fn substitute_block<'i, 't, F>( +fn substitute_block<'i, 't>( input: &mut Parser<'i, 't>, position: &mut (SourcePosition, TokenSerializationType), partial_computed_value: &mut ComputedValue, - substitute_one: &mut F, -) -> Result> -where - F: FnMut(&Name, &mut ComputedValue) -> Result, -{ + custom_properties: &CustomPropertiesMap, + invalid_custom_properties: &PrecomputedHashSet, + env: &CssEnvironment, +) -> Result> { let mut last_token_type = TokenSerializationType::nothing(); let mut set_position_at_next_iteration = false; loop { @@ -883,27 +1026,49 @@ where Err(..) => break, }; match token { - Token::Function(ref name) if name.eq_ignore_ascii_case("var") => { + Token::Function(ref name) + if name.eq_ignore_ascii_case("var") || name.eq_ignore_ascii_case("env") => + { + let is_env = name.eq_ignore_ascii_case("env"); + partial_computed_value.push( input.slice(position.0..before_this_token), position.1, last_token_type, ); input.parse_nested_block(|input| { - // parse_var_function() ensures neither .unwrap() will fail. - let name = input.expect_ident_cloned().unwrap(); - let name = Atom::from(parse_name(&name).unwrap()); + // parse_var_function() / parse_env_function() ensure neither .unwrap() will fail. + let name = { + let name = input.expect_ident().unwrap(); + if is_env { + Atom::from(&**name) + } else { + Atom::from(parse_name(&name).unwrap()) + } + }; - if let Ok(last) = substitute_one(&name, partial_computed_value) { - last_token_type = last; + let value = if is_env { + env.get(&name) + } else { + if invalid_custom_properties.contains(&name) { + None + } else { + custom_properties.get(&name).map(|v| &**v) + } + }; + + if let Some(v) = value { + last_token_type = v.last_token_type; + partial_computed_value.push_variable(v); // Skip over the fallback, as `parse_nested_block` would return `Err` - // if we don’t consume all of `input`. + // if we don't consume all of `input`. // FIXME: Add a specialized method to cssparser to do this with less work. - while let Ok(_) = input.next() {} + while input.next().is_ok() {} } else { input.expect_comma()?; let after_comma = input.state(); - let first_token_type = input.next_including_whitespace_and_comments() + let first_token_type = input + .next_including_whitespace_and_comments() // parse_var_function() ensures that .unwrap() will not fail. .unwrap() .serialization_type(); @@ -913,23 +1078,31 @@ where input, &mut position, partial_computed_value, - substitute_one, + custom_properties, + invalid_custom_properties, + env, )?; partial_computed_value.push_from(position, input, last_token_type); } Ok(()) })?; set_position_at_next_iteration = true - }, - + } Token::Function(_) | Token::ParenthesisBlock | Token::CurlyBracketBlock | Token::SquareBracketBlock => { input.parse_nested_block(|input| { - substitute_block(input, position, partial_computed_value, substitute_one) + substitute_block( + input, + position, + partial_computed_value, + custom_properties, + invalid_custom_properties, + env, + ) })?; - // It’s the same type for CloseCurlyBracket and CloseSquareBracket. + // It's the same type for CloseCurlyBracket and CloseSquareBracket. last_token_type = Token::CloseParenthesis.serialization_type(); }, @@ -945,29 +1118,32 @@ where Ok(last_token_type) } -/// Replace `var()` functions for a non-custom property. +/// Replace `var()` and `env()` functions for a non-custom property. +/// /// Return `Err(())` for invalid at computed time. pub fn substitute<'i>( input: &'i str, first_token_type: TokenSerializationType, computed_values_map: Option<&Arc>, + env: &CssEnvironment, ) -> Result> { let mut substituted = ComputedValue::empty(); let mut input = ParserInput::new(input); let mut input = Parser::new(&mut input); let mut position = (input.position(), first_token_type); + let invalid = PrecomputedHashSet::default(); + let empty_map = CustomPropertiesMap::new(); + let custom_properties = match computed_values_map { + Some(m) => &**m, + None => &empty_map, + }; let last_token_type = substitute_block( &mut input, &mut position, &mut substituted, - &mut |name, substituted| { - if let Some(value) = computed_values_map.and_then(|map| map.get(name)) { - substituted.push_variable(value); - Ok(value.last_token_type) - } else { - Err(()) - } - }, + &custom_properties, + &invalid, + env, )?; substituted.push_from(position, &input, last_token_type); Ok(substituted.css) diff --git a/components/style/gecko/media_queries.rs b/components/style/gecko/media_queries.rs index d95c8df7028..ae14d332f89 100644 --- a/components/style/gecko/media_queries.rs +++ b/components/style/gecko/media_queries.rs @@ -7,6 +7,7 @@ use app_units::AU_PER_PX; use app_units::Au; use cssparser::RGBA; +use custom_properties::CssEnvironment; use euclid::Size2D; use euclid::TypedScale; use gecko::values::{convert_nscolor_to_rgba, convert_rgba_to_nscolor}; @@ -52,6 +53,9 @@ pub struct Device { /// Whether any styles computed in the document relied on the viewport size /// by using vw/vh/vmin/vmax units. used_viewport_size: AtomicBool, + /// The CssEnvironment object responsible of getting CSS environment + /// variables. + environment: CssEnvironment, } impl fmt::Debug for Device { @@ -87,9 +91,16 @@ impl Device { body_text_color: AtomicUsize::new(unsafe { &*pres_context }.mDefaultColor as usize), used_root_font_size: AtomicBool::new(false), used_viewport_size: AtomicBool::new(false), + environment: CssEnvironment, } } + /// Get the relevant environment to resolve `env()` functions. + #[inline] + pub fn environment(&self) -> &CssEnvironment { + &self.environment + } + /// Tells the device that a new viewport rule has been found, and stores the /// relevant viewport constraints. pub fn account_for_viewport_rule(&mut self, _constraints: &ViewportConstraints) { diff --git a/components/style/properties/cascade.rs b/components/style/properties/cascade.rs index 82c8e1501d7..c37a4327e9f 100644 --- a/components/style/properties/cascade.rs +++ b/components/style/properties/cascade.rs @@ -243,7 +243,10 @@ where let mut declarations = SmallVec::<[(&_, CascadeLevel); 32]>::new(); let custom_properties = { - let mut builder = CustomPropertiesBuilder::new(inherited_style.custom_properties()); + let mut builder = CustomPropertiesBuilder::new( + inherited_style.custom_properties(), + device.environment(), + ); for (declaration, cascade_level) in iter_declarations() { declarations.push((declaration, cascade_level)); @@ -420,6 +423,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(), )) } diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index aa8e93bd9c7..99dee046112 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -9,7 +9,7 @@ use context::QuirksMode; use cssparser::{DeclarationListParser, parse_important, ParserInput, CowRcStr}; use cssparser::{Parser, AtRuleParser, DeclarationParser, Delimiter, ParseErrorKind}; -use custom_properties::CustomPropertiesBuilder; +use custom_properties::{CustomPropertiesBuilder, CssEnvironment}; use error_reporting::{ParseErrorReporter, ContextualParseError}; use itertools::Itertools; use parser::ParserContext; @@ -760,13 +760,19 @@ 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()) + block.cascade_custom_properties(cv.custom_properties(), &env) } else { cv.custom_properties().cloned() } @@ -790,6 +796,7 @@ impl PropertyDeclarationBlock { declaration.id, custom_properties.as_ref(), QuirksMode::NoQuirks, + &env, ).to_css(dest) }, (ref d, _) => d.to_css(dest), @@ -835,17 +842,24 @@ impl PropertyDeclarationBlock { &self, context: &Context, ) -> Option> { - self.cascade_custom_properties(context.style().custom_properties()) + self.cascade_custom_properties( + context.style().custom_properties(), + context.device().environment(), + ) } /// Returns a custom properties map which is the result of cascading custom /// properties in this declaration block along with the given custom /// properties. - pub fn cascade_custom_properties( + fn cascade_custom_properties( &self, inherited_custom_properties: Option<&Arc<::custom_properties::CustomPropertiesMap>>, + environment: &CssEnvironment, ) -> Option> { - let mut builder = CustomPropertiesBuilder::new(inherited_custom_properties); + let mut builder = CustomPropertiesBuilder::new( + inherited_custom_properties, + environment, + ); 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 79287e178d3..58c727ca416 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -519,6 +519,7 @@ impl AnimationValue { declaration.id, custom_properties, context.quirks_mode, + context.device().environment(), ) }; return AnimationValue::from_declaration( diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index 33220ebfd60..8080d0c14f0 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -1536,10 +1536,14 @@ impl UnparsedValue { longhand_id: LonghandId, custom_properties: Option<<&Arc<::custom_properties::CustomPropertiesMap>>, quirks_mode: QuirksMode, + environment: &::custom_properties::CssEnvironment, ) -> PropertyDeclaration { - ::custom_properties::substitute(&self.css, self.first_token_type, custom_properties) - .ok() - .and_then(|css| { + ::custom_properties::substitute( + &self.css, + self.first_token_type, + custom_properties, + environment, + ).ok().and_then(|css| { // As of this writing, only the base URL is used for property // values. // @@ -2214,34 +2218,33 @@ impl PropertyDeclaration { WideKeywordDeclaration { id, keyword }, ) }).or_else(|()| { - input.look_for_var_functions(); + input.look_for_var_or_env_functions(); input.parse_entirely(|input| id.parse_value(context, input)) .or_else(|err| { while let Ok(_) = input.next() {} // Look for var() after the error. - if input.seen_var_functions() { - input.reset(&start); - let (first_token_type, css) = - ::custom_properties::parse_non_custom_with_var(input).map_err(|e| { - StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - e, - ) - })?; - Ok(PropertyDeclaration::WithVariables(VariableDeclaration { - id, - value: Arc::new(UnparsedValue { + if !input.seen_var_or_env_functions() { + return Err(StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + err, + )); + } + input.reset(&start); + let (first_token_type, css) = + ::custom_properties::parse_non_custom_with_var(input).map_err(|e| { + StyleParseErrorKind::new_invalid( + non_custom_id.unwrap().name(), + e, + ) + })?; + Ok(PropertyDeclaration::WithVariables(VariableDeclaration { + id, + value: Arc::new(UnparsedValue { css: css.into_owned(), first_token_type: first_token_type, url_data: context.url_data.clone(), from_shorthand: None, - }), - })) - } else { - Err(StyleParseErrorKind::new_invalid( - non_custom_id.unwrap().name(), - err, - )) - } + }), + })) }) }).map(|declaration| { declarations.push(declaration) @@ -2264,12 +2267,13 @@ impl PropertyDeclaration { } } } else { - input.look_for_var_functions(); - // Not using parse_entirely here: each ${shorthand.ident}::parse_into function - // needs to do so *before* pushing to `declarations`. + input.look_for_var_or_env_functions(); + // Not using parse_entirely here: each + // ${shorthand.ident}::parse_into function needs to do so + // *before* pushing to `declarations`. id.parse_into(declarations, context, input).or_else(|err| { while let Ok(_) = input.next() {} // Look for var() after the error. - if !input.seen_var_functions() { + if !input.seen_var_or_env_functions() { return Err(StyleParseErrorKind::new_invalid( non_custom_id.unwrap().name(), err, diff --git a/components/style_traits/Cargo.toml b/components/style_traits/Cargo.toml index b9a9b3717b4..c51831a9bcf 100644 --- a/components/style_traits/Cargo.toml +++ b/components/style_traits/Cargo.toml @@ -15,7 +15,7 @@ gecko = [] [dependencies] app_units = "0.7" -cssparser = "0.24.0" +cssparser = "0.25" bitflags = "1.0" euclid = "0.19" malloc_size_of = { path = "../malloc_size_of" } diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 8057a95c0ba..65937bbac7e 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -12,7 +12,7 @@ doctest = false [dependencies] byteorder = "1.0" app_units = "0.7" -cssparser = "0.24.0" +cssparser = "0.25" euclid = "0.19" html5ever = "0.22" parking_lot = "0.6" From 99f9d845558de784c171274fcb4592d7e686883a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 10:42:12 +0000 Subject: [PATCH 28/32] style: Simplify invalid custom property handling. It's a bit useless to keep a set of invalid properties if we're going to use them just to reject lookups into another key. This makes it more consistent with the cascade / no-references code, and should not change behavior. Differential Revision: https://phabricator.services.mozilla.com/D9632 --- components/style/custom_properties.rs | 73 +++++---------------------- 1 file changed, 12 insertions(+), 61 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 344e38af8d1..6d7a198e047 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -230,18 +230,6 @@ where self.index.remove(index); self.values.remove(key) } - - fn remove_set(&mut self, set: &::hash::HashSet) - where - S: ::std::hash::BuildHasher, - { - if set.is_empty() { - return; - } - self.index.retain(|key| !set.contains(key)); - self.values.retain(|key, _| !set.contains(key)); - debug_assert_eq!(self.values.len(), self.index.len()); - } } /// An iterator for OrderedMap. @@ -642,11 +630,9 @@ impl<'a> CustomPropertiesBuilder<'a> { // environment variable here, perform substitution here instead // of forcing a full traversal in `substitute_all` afterwards. let value = if !has_references && unparsed_value.references_environment { - let invalid = Default::default(); // Irrelevant, since there are no references. let result = substitute_references_in_value( unparsed_value, &map, - &invalid, &self.environment, ); match result { @@ -735,18 +721,6 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: // listed in // https://en.wikipedia.org/w/index.php? // title=Tarjan%27s_strongly_connected_components_algorithm&oldid=801728495 - // - // FIXME This function currently does at least one addref to names - // for each variable regardless whether it has reference. Each - // variable with any reference would have an additional addref. - // There is another addref for each reference. - // Strictly speaking, these addrefs are not necessary, because we - // don't add/remove entry from custom properties map, and thus keys - // should be alive in the whole process until we start removing - // invalids. However, there is no safe way for us to prove this to - // the compiler. We may be able to fix this issue at some point if - // the standard library can provide some kind of hashmap wrapper - // with frozen keys. /// Struct recording necessary information for each variable. struct VarInfo { @@ -775,8 +749,6 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: /// all unfinished strong connected components. stack: SmallVec<[usize; 5]>, map: &'a mut CustomPropertiesMap, - /// The set of invalid custom properties. - invalid: &'a mut PrecomputedHashSet, /// The environment to substitute `env()` variables. environment: &'a CssEnvironment, } @@ -790,9 +762,9 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: /// in one of the following states: /// * It is still in context.stack, which means it is part of an /// potentially incomplete dependency circle. - /// * It has been added into the invalid set. It can be either that - /// the substitution failed, or it is inside a dependency circle. - /// When this function put a variable into the invalid set because + /// * It has been removed from the map. It can be either that the + /// substitution failed, or it is inside a dependency circle. + /// When this function removes a variable from the map because /// of dependency circle, it would put all variables in the same /// strong connected component to the set together. /// * It doesn't have any reference, because either this variable @@ -813,11 +785,6 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: return None; } - // This variable has already been resolved. - if context.invalid.contains(&name) { - return None; - } - // Whether this variable has been visited in this traversal. let key; match context.index_map.entry(name) { @@ -875,8 +842,9 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: if lowlink != index { // This variable is in a loop, but it is not the root of // this strong connected component. We simply return for - // now, and the root would add it into the invalid set. - // This cannot be added into the invalid set here, because + // now, and the root would remove it from the map. + // + // This cannot be removed from the map here, because // otherwise the shortcut check at the beginning of this // function would return the wrong value. return Some(index); @@ -903,15 +871,14 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: break; } // Anything here is in a loop which can traverse to the - // variable we are handling, so we should add it into - // the invalid set. We should never visit the variable - // again so it's safe to just take the name away. - context.invalid.insert(var_name); + // variable we are handling, so remove it from the map, it's invalid + // at computed-value time. + context.map.remove(&var_name); in_loop = true; } if in_loop { // This variable is in loop. Resolve to invalid. - context.invalid.insert(name); + context.map.remove(&name); return None; } @@ -921,7 +888,6 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: let result = substitute_references_in_value( &value, &context.map, - &context.invalid, &context.environment, ); @@ -930,7 +896,7 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: context.map.insert(name, Arc::new(computed_value)); }, Err(..) => { - context.invalid.insert(name); + context.map.remove(&name); }, } @@ -941,7 +907,6 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: // We have to clone the names so that we can mutably borrow the map // in the context we create for traversal. let names = custom_properties_map.index.clone(); - let mut invalid = PrecomputedHashSet::default(); for name in names.into_iter() { let mut context = Context { count: 0, @@ -949,20 +914,16 @@ fn substitute_all(custom_properties_map: &mut CustomPropertiesMap, environment: stack: SmallVec::new(), var_info: SmallVec::new(), map: custom_properties_map, - invalid: &mut invalid, environment, }; traverse(name, &mut context); } - - custom_properties_map.remove_set(&invalid); } /// Replace `var()` and `env()` functions in a pre-existing variable value. fn substitute_references_in_value<'i>( value: &'i VariableValue, custom_properties: &CustomPropertiesMap, - invalid_custom_properties: &PrecomputedHashSet, environment: &CssEnvironment, ) -> Result> { debug_assert!(!value.references.is_empty() || value.references_environment); @@ -977,7 +938,6 @@ fn substitute_references_in_value<'i>( &mut position, &mut computed_value, custom_properties, - invalid_custom_properties, environment, )?; @@ -1000,7 +960,6 @@ fn substitute_block<'i, 't>( position: &mut (SourcePosition, TokenSerializationType), partial_computed_value: &mut ComputedValue, custom_properties: &CustomPropertiesMap, - invalid_custom_properties: &PrecomputedHashSet, env: &CssEnvironment, ) -> Result> { let mut last_token_type = TokenSerializationType::nothing(); @@ -1050,11 +1009,7 @@ fn substitute_block<'i, 't>( let value = if is_env { env.get(&name) } else { - if invalid_custom_properties.contains(&name) { - None - } else { - custom_properties.get(&name).map(|v| &**v) - } + custom_properties.get(&name).map(|v| &**v) }; if let Some(v) = value { @@ -1079,7 +1034,6 @@ fn substitute_block<'i, 't>( &mut position, partial_computed_value, custom_properties, - invalid_custom_properties, env, )?; partial_computed_value.push_from(position, input, last_token_type); @@ -1098,7 +1052,6 @@ fn substitute_block<'i, 't>( position, partial_computed_value, custom_properties, - invalid_custom_properties, env, ) })?; @@ -1131,7 +1084,6 @@ pub fn substitute<'i>( let mut input = ParserInput::new(input); let mut input = Parser::new(&mut input); let mut position = (input.position(), first_token_type); - let invalid = PrecomputedHashSet::default(); let empty_map = CustomPropertiesMap::new(); let custom_properties = match computed_values_map { Some(m) => &**m, @@ -1142,7 +1094,6 @@ pub fn substitute<'i>( &mut position, &mut substituted, &custom_properties, - &invalid, env, )?; substituted.push_from(position, &input, last_token_type); From 29f56919e21a38e543a4bcf8a510fa1027c1d4ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 12:19:23 +0100 Subject: [PATCH 29/32] Update remaining references to cssparser 0.24. --- Cargo.lock | 22 +++++++++---------- components/canvas/Cargo.toml | 2 +- components/canvas_traits/Cargo.toml | 2 +- components/script/Cargo.toml | 2 +- components/script_layout_interface/Cargo.toml | 2 +- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cabae5e8e81..a06a8096172 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -329,7 +329,7 @@ dependencies = [ "azure 0.34.0 (git+https://github.com/servo/rust-azure)", "canvas_traits 0.0.1", "compositing 0.0.1", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "fnv 1.0.5 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -348,7 +348,7 @@ dependencies = [ name = "canvas_traits" version = "0.0.1" dependencies = [ - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "gleam 0.6.4 (registry+https://github.com/rust-lang/crates.io-index)", "ipc-channel 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -717,7 +717,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [[package]] name = "cssparser" -version = "0.24.0" +version = "0.25.0" source = "registry+https://github.com/rust-lang/crates.io-index" dependencies = [ "cssparser-macros 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2200,7 +2200,7 @@ name = "malloc_size_of" version = "0.0.1" dependencies = [ "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "hashglobe 0.1.0", "hyper 0.12.12 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3180,7 +3180,7 @@ dependencies = [ "chrono 0.4.2 (registry+https://github.com/rust-lang/crates.io-index)", "cmake 0.1.29 (registry+https://github.com/rust-lang/crates.io-index)", "cookie 0.11.0 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "deny_public_fields 0.0.1", "devtools_traits 0.0.1", "dom_struct 0.0.1", @@ -3265,7 +3265,7 @@ dependencies = [ "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)", "canvas_traits 0.0.1", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "gfx_traits 0.0.1", "html5ever 0.22.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3350,7 +3350,7 @@ name = "selectors" version = "0.20.0" dependencies = [ "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "fxhash 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.5 (registry+https://github.com/rust-lang/crates.io-index)", "matches 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3785,7 +3785,7 @@ dependencies = [ "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "encoding_rs 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "fallible 0.0.1", @@ -3848,7 +3848,7 @@ version = "0.0.1" dependencies = [ "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "byteorder 1.2.1 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "html5ever 0.22.3 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)", @@ -3871,7 +3871,7 @@ version = "0.0.1" dependencies = [ "app_units 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)", "bitflags 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)", - "cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)", + "cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)", "euclid 0.19.0 (registry+https://github.com/rust-lang/crates.io-index)", "malloc_size_of 0.0.1", "malloc_size_of_derive 0.0.1", @@ -4803,7 +4803,7 @@ dependencies = [ "checksum crossbeam-epoch 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "9c90f1474584f38e270b5b613e898c8c328aa4f3dea85e0a27ac2e642f009416" "checksum crossbeam-utils 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "2760899e32a1d58d5abb31129f8fae5de75220bc2176e77ff7c627ae45c918d9" "checksum crossbeam-utils 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "677d453a17e8bd2b913fa38e8b9cf04bcdbb5be790aa294f2389661d72036015" -"checksum cssparser 0.24.0 (registry+https://github.com/rust-lang/crates.io-index)" = "495beddc39b1987b8e9f029354eccbd5ef88eb5f1cd24badb764dce338acf2e0" +"checksum cssparser 0.25.0 (registry+https://github.com/rust-lang/crates.io-index)" = "730363a45c4e248d4f21d3e5c1156d1a9cdec0855056c0d9539e814bc59865c3" "checksum cssparser-macros 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "f3a5383ae18dbfdeb569ed62019f5bddb2a95cd2d3833313c475a0d014777805" "checksum darling 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "2a78af487e4eb8f4421a1770687b328af6bb4494ca93435210678c6eea875c11" "checksum darling_core 0.4.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b315f49c7b6db3708bca6e6913c194581a44ec619b7a39e131d4dd63733a3698" diff --git a/components/canvas/Cargo.toml b/components/canvas/Cargo.toml index 27d87c9cf99..1ccbe5f9bd8 100644 --- a/components/canvas/Cargo.toml +++ b/components/canvas/Cargo.toml @@ -16,7 +16,7 @@ webgl_backtrace = ["canvas_traits/webgl_backtrace"] azure = {git = "https://github.com/servo/rust-azure"} canvas_traits = {path = "../canvas_traits"} compositing = {path = "../compositing"} -cssparser = "0.24" +cssparser = "0.25" euclid = "0.19" fnv = "1.0" gleam = "0.6.4" diff --git a/components/canvas_traits/Cargo.toml b/components/canvas_traits/Cargo.toml index 187b46d617e..edfb382d395 100644 --- a/components/canvas_traits/Cargo.toml +++ b/components/canvas_traits/Cargo.toml @@ -13,7 +13,7 @@ path = "lib.rs" webgl_backtrace = [] [dependencies] -cssparser = "0.24.0" +cssparser = "0.25" euclid = "0.19" ipc-channel = "0.11" gleam = "0.6" diff --git a/components/script/Cargo.toml b/components/script/Cargo.toml index 614f9fcded1..f5d712dd795 100644 --- a/components/script/Cargo.toml +++ b/components/script/Cargo.toml @@ -39,7 +39,7 @@ canvas_traits = {path = "../canvas_traits"} caseless = "0.2" cookie = "0.11" chrono = "0.4" -cssparser = "0.24" +cssparser = "0.25" deny_public_fields = {path = "../deny_public_fields"} devtools_traits = {path = "../devtools_traits"} dom_struct = {path = "../dom_struct"} diff --git a/components/script_layout_interface/Cargo.toml b/components/script_layout_interface/Cargo.toml index 820148f3b65..3dee509e5df 100644 --- a/components/script_layout_interface/Cargo.toml +++ b/components/script_layout_interface/Cargo.toml @@ -13,7 +13,7 @@ path = "lib.rs" app_units = "0.7" atomic_refcell = "0.1" canvas_traits = {path = "../canvas_traits"} -cssparser = "0.24" +cssparser = "0.25" euclid = "0.19" gfx_traits = {path = "../gfx_traits"} html5ever = "0.22" From 8560c8dd5a59b09c10c55ee64eab2f04e6461259 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 12:20:26 +0100 Subject: [PATCH 30/32] Fix tidy issues. --- components/style/values/animated/font.rs | 2 +- components/style/values/animated/length.rs | 2 +- components/style/values/animated/svg.rs | 2 +- components/style/values/computed/svg.rs | 4 +--- 4 files changed, 4 insertions(+), 6 deletions(-) diff --git a/components/style/values/animated/font.rs b/components/style/values/animated/font.rs index 26c88a2d3f1..5a85bc14b18 100644 --- a/components/style/values/animated/font.rs +++ b/components/style/values/animated/font.rs @@ -4,11 +4,11 @@ //! Animation implementation for various font-related types. +use super::{Animate, Procedure, ToAnimatedZero}; use values::computed::Number; use values::computed::font::{FontWeight, FontVariationSettings}; use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::font::{FontSettings as GenericFontSettings, FontTag, VariationValue}; -use super::{Animate, Procedure, ToAnimatedZero}; impl ToAnimatedZero for FontWeight { #[inline] diff --git a/components/style/values/animated/length.rs b/components/style/values/animated/length.rs index f6bbaa6df38..f655ec33a2d 100644 --- a/components/style/values/animated/length.rs +++ b/components/style/values/animated/length.rs @@ -4,11 +4,11 @@ //! Animation implementation for various length-related types. +use super::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; use values::computed::MaxLength as ComputedMaxLength; use values::computed::MozLength as ComputedMozLength; use values::computed::Percentage; use values::computed::length::{Length, CalcLengthOrPercentage, LengthOrPercentageOrNone, LengthOrPercentageOrAuto}; -use super::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; /// impl Animate for CalcLengthOrPercentage { diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index 0c4e3be7166..df8c43368a1 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -5,13 +5,13 @@ //! Animation implementations for various SVG-related types. use properties::animated_properties::ListAnimation; +use super::{Animate, Procedure, ToAnimatedZero}; use values::animated::color::Color as AnimatedColor; use values::computed::{Number, NumberOrPercentage, LengthOrPercentage}; use values::computed::url::ComputedUrl; use values::distance::{ComputeSquaredDistance, SquaredDistance}; use values::generics::svg::{SVGLength, SvgLengthOrPercentageOrNumber, SVGPaint}; use values::generics::svg::{SVGStrokeDashArray, SVGOpacity}; -use super::{Animate, Procedure, ToAnimatedZero}; /// Animated SVGPaint. pub type IntermediateSVGPaint = SVGPaint; diff --git a/components/style/values/computed/svg.rs b/components/style/values/computed/svg.rs index 11917b01779..b41c59082ed 100644 --- a/components/style/values/computed/svg.rs +++ b/components/style/values/computed/svg.rs @@ -5,9 +5,7 @@ //! Computed types for SVG properties. use values::RGBA; -use values::computed::LengthOrPercentage; -use values::computed::{NonNegativeLengthOrPercentage, NonNegativeNumber, Number}; -use values::computed::Opacity; +use values::computed::{NonNegativeLengthOrPercentage, NonNegativeNumber, Number, LengthOrPercentage, Opacity}; use values::computed::color::Color; use values::computed::url::ComputedUrl; use values::generics::svg as generic; From 64e70e28fce1b2dd345f9c68e8094d4e3d8ffce4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 12:36:14 +0100 Subject: [PATCH 31/32] style: Add the safe area constant names as atoms. --- components/atoms/static_atoms.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/atoms/static_atoms.txt b/components/atoms/static_atoms.txt index d3e7bf26f11..081c8c314cd 100644 --- a/components/atoms/static_atoms.txt +++ b/components/atoms/static_atoms.txt @@ -70,6 +70,10 @@ resize right rtl sans-serif +safe-area-inset-top +safe-area-inset-bottom +safe-area-inset-left +safe-area-inset-right scan screen scroll-position From ac6f9215883db2d322c4b08eaeaf7464843515de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 5 Nov 2018 12:39:54 +0100 Subject: [PATCH 32/32] style: Fix servo build. --- components/style/custom_properties.rs | 1 + .../properties/helpers/animated_properties.mako.rs | 3 ++- components/style/servo/media_queries.rs | 11 +++++++++++ tests/unit/style/custom_properties.rs | 5 +++-- 4 files changed, 17 insertions(+), 3 deletions(-) diff --git a/components/style/custom_properties.rs b/components/style/custom_properties.rs index 6d7a198e047..574031453e5 100644 --- a/components/style/custom_properties.rs +++ b/components/style/custom_properties.rs @@ -25,6 +25,7 @@ use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; /// /// TODO(emilio): If this becomes a bit more complex we should probably move it /// to the `media_queries` module, or something. +#[derive(Debug, MallocSizeOf)] pub struct CssEnvironment; struct EnvironmentVariable { diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 58c727ca416..ba1a6444d42 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -18,7 +18,7 @@ use num_traits::Zero; use properties::{CSSWideKeyword, PropertyDeclaration}; use properties::longhands; use properties::longhands::visibility::computed_value::T as Visibility; -use properties::{LonghandId, ShorthandId}; +use properties::LonghandId; use servo_arc::Arc; use smallvec::SmallVec; use std::{cmp, ptr}; @@ -50,6 +50,7 @@ use void::{self, Void}; #[allow(non_upper_case_globals)] impl From for TransitionProperty { fn from(property: nsCSSPropertyID) -> TransitionProperty { + use properties::ShorthandId; match property { % for prop in data.longhands: ${prop.nscsspropertyid()} => { diff --git a/components/style/servo/media_queries.rs b/components/style/servo/media_queries.rs index 98b1ec7ae6f..bae0aa3978b 100644 --- a/components/style/servo/media_queries.rs +++ b/components/style/servo/media_queries.rs @@ -6,6 +6,7 @@ use app_units::Au; use cssparser::RGBA; +use custom_properties::CssEnvironment; use euclid::{Size2D, TypedScale, TypedSize2D}; use media_queries::MediaType; use media_queries::media_feature::{AllowsRanges, ParsingRequirements}; @@ -49,6 +50,9 @@ pub struct Device { /// Whether any styles computed in the document relied on the viewport size. #[ignore_malloc_size_of = "Pure stack type"] used_viewport_units: AtomicBool, + /// The CssEnvironment object responsible of getting CSS environment + /// variables. + environment: CssEnvironment, } impl Device { @@ -66,9 +70,16 @@ impl Device { root_font_size: AtomicIsize::new(FontSize::medium().size().0 as isize), used_root_font_size: AtomicBool::new(false), used_viewport_units: AtomicBool::new(false), + environment: CssEnvironment, } } + /// Get the relevant environment to resolve `env()` functions. + #[inline] + pub fn environment(&self) -> &CssEnvironment { + &self.environment + } + /// Return the default computed values for this device. pub fn default_computed_values(&self) -> &ComputedValues { // FIXME(bz): This isn't really right, but it's no more wrong diff --git a/tests/unit/style/custom_properties.rs b/tests/unit/style/custom_properties.rs index dfb0c825627..aeefe6e19b9 100644 --- a/tests/unit/style/custom_properties.rs +++ b/tests/unit/style/custom_properties.rs @@ -4,7 +4,7 @@ use cssparser::{Parser, ParserInput}; use servo_arc::Arc; -use style::custom_properties::{Name, SpecifiedValue, CustomPropertiesMap, CustomPropertiesBuilder}; +use style::custom_properties::{Name, SpecifiedValue, CustomPropertiesMap, CustomPropertiesBuilder, CssEnvironment}; use style::properties::CustomDeclarationValue; use test::{self, Bencher}; @@ -18,7 +18,8 @@ fn cascade( (Name::from(name), SpecifiedValue::parse(&mut parser).unwrap()) }).collect::>(); - let mut builder = CustomPropertiesBuilder::new(inherited); + let env = CssEnvironment; + let mut builder = CustomPropertiesBuilder::new(inherited, &env); for &(ref name, ref val) in &values { builder.cascade(name, &CustomDeclarationValue::Value(val.clone()));