diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index ee0c134b269..00a7dcdb2c4 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -882,6 +882,7 @@ impl<'le> GeckoElement<'le> { .expect("AnimationValue not found in ElementTransitions"); let property = end_value.id(); + debug_assert!(!property.is_logical()); map.insert(property, end_value.clone_arc()); } map @@ -896,6 +897,7 @@ impl<'le> GeckoElement<'le> { existing_transitions: &FnvHashMap>, ) -> bool { use values::animated::{Animate, Procedure}; + debug_assert!(!longhand_id.is_logical()); // If there is an existing transition, update only if the end value // differs. @@ -1657,6 +1659,8 @@ impl<'le> TElement for GeckoElement<'le> { let transition_property: TransitionProperty = property.into(); let mut property_check_helper = |property: LonghandId| -> bool { + let property = + property.to_physical(after_change_style.writing_mode); transitions_to_keep.insert(property); self.needs_transitions_update_per_property( property, diff --git a/components/style/properties/data.py b/components/style/properties/data.py index c33eeb7b39c..ea0516d803e 100644 --- a/components/style/properties/data.py +++ b/components/style/properties/data.py @@ -223,12 +223,6 @@ class Longhand(object): and animation_value_type != "discrete" self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \ or animation_value_type == "discrete" - if self.logical: - # Logical properties will be animatable (i.e. the animation type is - # discrete). For now, it is still non-animatable. - self.animatable = False - self.transitionable = False - self.animation_value_type = None # See compute_damage for the various values this can take self.servo_restyle_damage = servo_restyle_damage diff --git a/components/style/properties/declaration_block.rs b/components/style/properties/declaration_block.rs index 29277eef1d8..7f6fc15bde9 100644 --- a/components/style/properties/declaration_block.rs +++ b/components/style/properties/declaration_block.rs @@ -131,12 +131,15 @@ impl<'a> DoubleEndedIterator for DeclarationImportanceIterator<'a> { } /// Iterator over `PropertyDeclaration` for Importance::Normal. +/// +/// TODO(emilio): This should be replaced by `impl Trait`, returning a +/// filter()ed iterator when available instead, and all the boilerplate below +/// should go. pub struct NormalDeclarationIterator<'a>(DeclarationImportanceIterator<'a>); impl<'a> NormalDeclarationIterator<'a> { - /// Constructor. #[inline] - pub fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self { + fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self { NormalDeclarationIterator( DeclarationImportanceIterator::new(declarations, important) ) @@ -146,6 +149,7 @@ impl<'a> NormalDeclarationIterator<'a> { impl<'a> Iterator for NormalDeclarationIterator<'a> { type Item = &'a PropertyDeclaration; + #[inline] fn next(&mut self) -> Option { loop { let (decl, importance) = self.0.iter.next()?; @@ -155,11 +159,24 @@ impl<'a> Iterator for NormalDeclarationIterator<'a> { } } + #[inline] fn size_hint(&self) -> (usize, Option) { self.0.iter.size_hint() } } +impl<'a> DoubleEndedIterator for NormalDeclarationIterator<'a> { + #[inline] + fn next_back(&mut self) -> Option { + loop { + let (decl, importance) = self.0.iter.next_back()?; + if !importance { + return Some(decl); + } + } + } +} + /// Iterator for AnimationValue to be generated from PropertyDeclarationBlock. pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> { iter: NormalDeclarationIterator<'a>, diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index a51860f9a19..d00f1947d86 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -5,7 +5,7 @@ <%namespace name="helpers" file="/helpers.mako.rs" /> <% - from data import to_idl_name, SYSTEM_FONT_LONGHANDS + from data import to_idl_name, SYSTEM_FONT_LONGHANDS, to_camel_case from itertools import groupby %> @@ -114,7 +114,7 @@ pub fn nscsspropertyid_is_transitionable(property: nsCSSPropertyID) -> bool { #[cfg_attr(feature = "servo", derive(MallocSizeOf))] pub enum AnimatedProperty { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: <% value_type = "longhands::{}::computed_value::T".format(prop.ident) if not prop.is_animatable_with_computed_value: @@ -131,7 +131,7 @@ impl AnimatedProperty { pub fn name(&self) -> &'static str { match *self { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: AnimatedProperty::${prop.camel_case}(..) => "${prop.name}", % endif % endfor @@ -143,7 +143,7 @@ impl AnimatedProperty { pub fn does_animate(&self) -> bool { match *self { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: AnimatedProperty::${prop.camel_case}(ref from, ref to) => from != to, % endif % endfor @@ -154,7 +154,7 @@ impl AnimatedProperty { pub fn has_the_same_end_value_as(&self, other: &Self) -> bool { match (self, other) { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: (&AnimatedProperty::${prop.camel_case}(_, ref this_end_value), &AnimatedProperty::${prop.camel_case}(_, ref other_end_value)) => { this_end_value == other_end_value @@ -173,7 +173,7 @@ impl AnimatedProperty { { match *self { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: AnimatedProperty::${prop.camel_case}(ref from, ref to) => { // https://drafts.csswg.org/web-animations/#discrete-animation-type % if prop.animation_value_type == "discrete": @@ -203,9 +203,12 @@ impl AnimatedProperty { old_style: &ComputedValues, new_style: &ComputedValues, ) -> Option { + // FIXME(emilio): Handle the case where old_style and new_style's + // writing mode differ. + let property = property.to_physical(new_style.writing_mode); Some(match property { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: LonghandId::${prop.camel_case} => { let old_computed = old_style.clone_${prop.ident}(); let new_computed = new_style.clone_${prop.ident}(); @@ -254,11 +257,10 @@ unsafe impl HasSimpleFFI for AnimationValueMap {} #[repr(u16)] pub enum AnimationValue { % for prop in data.longhands: - % if prop.animatable: /// `${prop.name}` + % if prop.animatable and not prop.logical: ${prop.camel_case}(${prop.animated_type()}), % else: - /// `${prop.name}` (not animatable) ${prop.camel_case}(Void), % endif % endfor @@ -267,8 +269,11 @@ pub enum AnimationValue { <% animated = [] unanimated = [] + animated_with_logical = [] for prop in data.longhands: if prop.animatable: + animated_with_logical.append(prop) + if prop.animatable and not prop.logical: animated.append(prop) else: unanimated.append(prop) @@ -370,7 +375,7 @@ impl AnimationValue { let id = unsafe { *(self as *const _ as *const LonghandId) }; debug_assert_eq!(id, match *self { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: AnimationValue::${prop.camel_case}(..) => LonghandId::${prop.camel_case}, % else: AnimationValue::${prop.camel_case}(void) => void::unreachable(void), @@ -444,17 +449,18 @@ impl AnimationValue { %> let animatable = match *decl { - % for (specified_ty, ty, boxed, to_animated, inherit, system), props in groupby(animated, key=keyfunc): + % for (specified_ty, ty, boxed, to_animated, inherit, system), props in groupby(animated_with_logical, key=keyfunc): ${" |\n".join("PropertyDeclaration::{}(ref value)".format(prop.camel_case) for prop in props)} => { let decl_repr = unsafe { &*(decl as *const _ as *const PropertyDeclarationVariantRepr<${specified_ty}>) }; + let longhand_id = unsafe { + *(&decl_repr.tag as *const u16 as *const LonghandId) + }; % if inherit: context.for_non_inherited_property = None; % else: - context.for_non_inherited_property = unsafe { - Some(*(&decl_repr.tag as *const u16 as *const LonghandId)) - }; + context.for_non_inherited_property = Some(longhand_id); % endif % if system: if let Some(sf) = value.get_system() { @@ -475,7 +481,7 @@ impl AnimationValue { ptr::write( &mut out as *mut _ as *mut AnimationValueVariantRepr<${ty}>, AnimationValueVariantRepr { - tag: decl_repr.tag, + tag: longhand_id.to_physical(context.builder.writing_mode) as u16, value, }, ); @@ -492,24 +498,40 @@ impl AnimationValue { LonghandId::${prop.camel_case} => { let style_struct = match declaration.keyword { % if not prop.style_struct.inherited: - CSSWideKeyword::Unset | + CSSWideKeyword::Unset | % endif CSSWideKeyword::Initial => { initial.get_${prop.style_struct.name_lower}() }, % if prop.style_struct.inherited: - CSSWideKeyword::Unset | + CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { context.builder .get_parent_${prop.style_struct.name_lower}() }, }; - let computed = style_struct.clone_${prop.ident}(); + let computed = style_struct + % if prop.logical: + .clone_${prop.ident}(context.builder.writing_mode); + % else: + .clone_${prop.ident}(); + % endif + % if not prop.is_animatable_with_computed_value: let computed = computed.to_animated_value(); % endif - AnimationValue::${prop.camel_case}(computed) + + % if prop.logical: + let wm = context.builder.writing_mode; + <%helpers:logical_setter_helper name="${prop.name}"> + <%def name="inner(physical_ident)"> + AnimationValue::${to_camel_case(physical_ident)}(computed) + + + % else: + AnimationValue::${prop.camel_case}(computed) + % endif }, % endif % endfor @@ -548,9 +570,10 @@ impl AnimationValue { property: LonghandId, style: &ComputedValues, ) -> Option { + let property = property.to_physical(style.writing_mode); Some(match property { % for prop in data.longhands: - % if prop.animatable: + % if prop.animatable and not prop.logical: LonghandId::${prop.camel_case} => { let computed = style.clone_${prop.ident}(); AnimationValue::${prop.camel_case}( @@ -656,7 +679,7 @@ impl ToAnimatedZero for AnimationValue { fn to_animated_zero(&self) -> Result { match *self { % for prop in data.longhands: - % if prop.animatable and prop.animation_value_type != "discrete": + % if prop.animatable and not prop.logical and prop.animation_value_type != "discrete": AnimationValue::${prop.camel_case}(ref base) => { Ok(AnimationValue::${prop.camel_case}(base.to_animated_zero()?)) }, @@ -2924,43 +2947,67 @@ impl ToAnimatedZero for AnimatedFilter { } } -/// A comparator to sort PropertyIds such that longhands are sorted before 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]. +/// 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. -/// -/// Example orderings that result from this: -/// -/// margin-left, margin -/// -/// and: -/// -/// border-top-color, border-color, border-top, border +/// 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 { - match (a.as_shorthand(), b.as_shorthand()) { - // Within shorthands, sort by the number of subproperties, then by IDL name. - (Ok(a), Ok(b)) => { - let subprop_count_a = a.longhands().count(); - let subprop_count_b = b.longhands().count(); - subprop_count_a - .cmp(&subprop_count_b) - .then_with(|| { - get_idl_name_sort_order(a).cmp(&get_idl_name_sort_order(b)) - }) - }, + let a_category = PropertyCategory::of(a); + let b_category = PropertyCategory::of(b); - // Longhands go before shorthands. - (Ok(_), Err(_)) => cmp::Ordering::Greater, - (Err(_), Ok(_)) => cmp::Ordering::Less, - - // Both are longhands or custom properties in which case they don't overlap and should - // sort equally. - _ => cmp::Ordering::Equal, + if a_category != b_category { + 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)) + }); + } + + cmp::Ordering::Equal } fn get_idl_name_sort_order(shorthand: ShorthandId) -> u32 {