Make TransitionProperty treat all properties that are not transitionable as unsupported

Currently properties that are discretely animated cannot be
transitioned. Now that TransitionProperty should only be used for
transitions, we can redefine it to treat non-transitionable properties
as unsupported. This should allow us to simplify the code and make it
more self-documenting (e.g. making TransitionProperty actually relate to
transitions).
This commit is contained in:
Brian Birtles 2017-06-15 10:19:48 +09:00
parent 5ce7b1cc55
commit e74f7792f5
3 changed files with 47 additions and 46 deletions

View file

@ -1040,7 +1040,6 @@ impl<'le> TElement for GeckoElement<'le> {
after_change_style: &ComputedValues) after_change_style: &ComputedValues)
-> bool { -> bool {
use gecko_bindings::structs::nsCSSPropertyID; use gecko_bindings::structs::nsCSSPropertyID;
use properties::{PropertyId, animated_properties};
use std::collections::HashSet; use std::collections::HashSet;
debug_assert!(self.might_need_transitions_update(Some(before_change_style), debug_assert!(self.might_need_transitions_update(Some(before_change_style),
@ -1075,6 +1074,8 @@ impl<'le> TElement for GeckoElement<'le> {
continue; continue;
} }
let transition_property: TransitionProperty = property.into();
let mut property_check_helper = |property: &TransitionProperty| -> bool { let mut property_check_helper = |property: &TransitionProperty| -> bool {
if self.needs_transitions_update_per_property(property, if self.needs_transitions_update_per_property(property,
combined_duration, combined_duration,
@ -1091,26 +1092,25 @@ impl<'le> TElement for GeckoElement<'le> {
} }
false false
}; };
if property == nsCSSPropertyID::eCSSPropertyExtra_all_properties {
if TransitionProperty::any(property_check_helper) { match transition_property {
return true; TransitionProperty::All => {
} if TransitionProperty::any(property_check_helper) {
} else { return true;
let is_shorthand = PropertyId::from_nscsspropertyid(property).ok().map_or(false, |p| { }
p.as_shorthand().is_ok() },
}); TransitionProperty::Unsupported(_) => { },
if is_shorthand { ref shorthand if shorthand.is_shorthand() => {
let shorthand: TransitionProperty = property.into();
if shorthand.longhands().iter().any(|p| property_check_helper(p)) { if shorthand.longhands().iter().any(|p| property_check_helper(p)) {
return true; return true;
} }
} else { },
if animated_properties::nscsspropertyid_is_animatable(property) && ref longhand => {
property_check_helper(&property.into()) { if property_check_helper(longhand) {
return true; return true;
} }
} },
} };
} }
// Check if we have to cancel the running transition because this is not a matching // Check if we have to cancel the running transition because this is not a matching
@ -1133,11 +1133,6 @@ impl<'le> TElement for GeckoElement<'le> {
// |property| should be an animatable longhand // |property| should be an animatable longhand
let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap(); let animatable_longhand = AnimatableLonghand::from_transition_property(property).unwrap();
// We don't allow transitions on properties that are not interpolable.
if animatable_longhand.is_discrete() {
return false;
}
if existing_transitions.contains_key(property) { if existing_transitions.contains_key(property) {
// If there is an existing transition, update only if the end value differs. // If there is an existing transition, update only if the end value differs.
// If the end value has not changed, we should leave the currently running // If the end value has not changed, we should leave the currently running

View file

@ -194,12 +194,15 @@ class Longhand(object):
self.animation_value_type = animation_value_type self.animation_value_type = animation_value_type
self.animatable = animation_value_type != "none" self.animatable = animation_value_type != "none"
self.transitionable = animation_value_type != "none" \
and animation_value_type != "discrete"
self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \ self.is_animatable_with_computed_value = animation_value_type == "ComputedValue" \
or animation_value_type == "discrete" or animation_value_type == "discrete"
if self.logical: if self.logical:
# Logical properties will be animatable (i.e. the animation type is # Logical properties will be animatable (i.e. the animation type is
# discrete). For now, it is still non-animatable. # discrete). For now, it is still non-animatable.
self.animatable = False self.animatable = False
self.transitionable = False
self.animation_type = None self.animation_type = None
# NB: Animatable implies clone because a property animation requires a # NB: Animatable implies clone because a property animation requires a
# copy of the computed value. # copy of the computed value.
@ -242,7 +245,16 @@ class Shorthand(object):
break break
return animatable return animatable
def get_transitionable(self):
transitionable = False
for sub in self.sub_properties:
if sub.transitionable:
transitionable = True
break
return transitionable
animatable = property(get_animatable) animatable = property(get_animatable)
transitionable = property(get_transitionable)
class Method(object): class Method(object):

View file

@ -95,13 +95,7 @@ impl AnimatableLonghand {
pub fn from_transition_property(transition_property: &TransitionProperty) -> Option<Self> { pub fn from_transition_property(transition_property: &TransitionProperty) -> Option<Self> {
match *transition_property { match *transition_property {
% for prop in data.longhands: % for prop in data.longhands:
<% % if prop.transitionable and prop.animatable:
# TODO: Later in this patch series, once we introduce the 'transitionable'
# definition, we will need to make the below test:
#
# if prop.transitionable and prop.animatable:
%>
% if prop.animatable:
TransitionProperty::${prop.camel_case} TransitionProperty::${prop.camel_case}
=> Some(AnimatableLonghand::${prop.camel_case}), => Some(AnimatableLonghand::${prop.camel_case}),
% endif % endif
@ -169,22 +163,22 @@ impl<'a> From<AnimatableLonghand> for PropertyDeclarationId<'a> {
} }
} }
/// A given transition property, that is either `All`, an animatable longhand property, /// A given transition property, that is either `All`, a transitionable longhand property,
/// a shorthand with at least one animatable longhand component, or an unsupported property. /// a shorthand with at least one transitionable longhand component, or an unsupported property.
// NB: This needs to be here because it needs all the longhands generated // NB: This needs to be here because it needs all the longhands generated
// beforehand. // beforehand.
#[derive(Clone, Debug, PartialEq, Eq, Hash)] #[derive(Clone, Debug, PartialEq, Eq, Hash)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))] #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub enum TransitionProperty { pub enum TransitionProperty {
/// All, any animatable property changing should generate a transition. /// All, any transitionable property changing should generate a transition.
All, All,
% for prop in data.longhands + data.shorthands_except_all(): % for prop in data.longhands + data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
/// ${prop.name} /// ${prop.name}
${prop.camel_case}, ${prop.camel_case},
% endif % endif
% endfor % endfor
/// Unrecognized property which could be any non-animatable, custom property, or /// Unrecognized property which could be any non-transitionable, custom property, or
/// unknown property. /// unknown property.
Unsupported(Atom) Unsupported(Atom)
} }
@ -197,17 +191,17 @@ impl TransitionProperty {
/// Iterates over each longhand property. /// Iterates over each longhand property.
pub fn each<F: FnMut(&TransitionProperty) -> ()>(mut cb: F) { pub fn each<F: FnMut(&TransitionProperty) -> ()>(mut cb: F) {
% for prop in data.longhands: % for prop in data.longhands:
% if prop.animatable: % if prop.transitionable:
cb(&TransitionProperty::${prop.camel_case}); cb(&TransitionProperty::${prop.camel_case});
% endif % endif
% endfor % endfor
} }
/// Iterates over every property that is not TransitionProperty::All, stopping and returning /// Iterates over every longhand property that is not TransitionProperty::All, stopping and
/// true when the provided callback returns true for the first time. /// returning true when the provided callback returns true for the first time.
pub fn any<F: FnMut(&TransitionProperty) -> bool>(mut cb: F) -> bool { pub fn any<F: FnMut(&TransitionProperty) -> bool>(mut cb: F) -> bool {
% for prop in data.longhands: % for prop in data.longhands:
% if prop.animatable: % if prop.transitionable:
if cb(&TransitionProperty::${prop.camel_case}) { if cb(&TransitionProperty::${prop.camel_case}) {
return true; return true;
} }
@ -222,7 +216,7 @@ impl TransitionProperty {
(match_ignore_ascii_case! { &ident, (match_ignore_ascii_case! { &ident,
"all" => Ok(TransitionProperty::All), "all" => Ok(TransitionProperty::All),
% for prop in data.longhands + data.shorthands_except_all(): % for prop in data.longhands + data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
"${prop.name}" => Ok(TransitionProperty::${prop.camel_case}), "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
% endif % endif
% endfor % endfor
@ -236,13 +230,13 @@ impl TransitionProperty {
}).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into()) }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
} }
/// Return animatable longhands of this shorthand TransitionProperty, except for "all". /// Return transitionable longhands of this shorthand TransitionProperty, except for "all".
pub fn longhands(&self) -> &'static [TransitionProperty] { pub fn longhands(&self) -> &'static [TransitionProperty] {
% for prop in data.shorthands_except_all(): % for prop in data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
static ${prop.ident.upper()}: &'static [TransitionProperty] = &[ static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
% for sub in prop.sub_properties: % for sub in prop.sub_properties:
% if sub.animatable: % if sub.transitionable:
TransitionProperty::${sub.camel_case}, TransitionProperty::${sub.camel_case},
% endif % endif
% endfor % endfor
@ -251,7 +245,7 @@ impl TransitionProperty {
% endfor % endfor
match *self { match *self {
% for prop in data.shorthands_except_all(): % for prop in data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
TransitionProperty::${prop.camel_case} => ${prop.ident.upper()}, TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
% endif % endif
% endfor % endfor
@ -263,7 +257,7 @@ impl TransitionProperty {
pub fn is_shorthand(&self) -> bool { pub fn is_shorthand(&self) -> bool {
match *self { match *self {
% for prop in data.shorthands_except_all(): % for prop in data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
TransitionProperty::${prop.camel_case} => true, TransitionProperty::${prop.camel_case} => true,
% endif % endif
% endfor % endfor
@ -292,7 +286,7 @@ impl ToCss for TransitionProperty {
match *self { match *self {
TransitionProperty::All => dest.write_str("all"), TransitionProperty::All => dest.write_str("all"),
% for prop in data.longhands + data.shorthands_except_all(): % for prop in data.longhands + data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"), TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
% endif % endif
% endfor % endfor
@ -312,7 +306,7 @@ impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID { fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID {
match *transition_property { match *transition_property {
% for prop in data.longhands + data.shorthands_except_all(): % for prop in data.longhands + data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
TransitionProperty::${prop.camel_case} TransitionProperty::${prop.camel_case}
=> ${helpers.to_nscsspropertyid(prop.ident)}, => ${helpers.to_nscsspropertyid(prop.ident)},
% endif % endif
@ -330,7 +324,7 @@ impl From<nsCSSPropertyID> for TransitionProperty {
fn from(property: nsCSSPropertyID) -> TransitionProperty { fn from(property: nsCSSPropertyID) -> TransitionProperty {
match property { match property {
% for prop in data.longhands + data.shorthands_except_all(): % for prop in data.longhands + data.shorthands_except_all():
% if prop.animatable: % if prop.transitionable:
${helpers.to_nscsspropertyid(prop.ident)} ${helpers.to_nscsspropertyid(prop.ident)}
=> TransitionProperty::${prop.camel_case}, => TransitionProperty::${prop.camel_case},
% else: % else: