From 0ec875a0486033fa4733a46a51b61996d81c476a Mon Sep 17 00:00:00 2001 From: Zach Hoffman Date: Mon, 18 Dec 2023 09:24:07 +0000 Subject: [PATCH] Bug 1846516 - [css-properties-values-api] Use PropertyDeclarationId/PropertyDeclarationIdSet for animated properties. r=firefox-style-system-reviewers,emilio This will make possible to animate custom properties. For now, the animation code keeps only dealing with PropertyDeclarationId::Longhand, so behavior is unchanged. Co-authored-by: Frederic Wang Depends on D195972 Differential Revision: https://phabricator.services.mozilla.com/D190816 --- servo/components/style/animation.rs | 56 +++++----- servo/components/style/gecko/wrapper.rs | 42 ++++---- .../style/properties/declaration_block.rs | 80 +++++++++++--- .../helpers/animated_properties.mako.rs | 28 +++-- .../style/properties/properties.mako.rs | 10 +- .../style/properties/property_declaration.rs | 83 +++++++++++++++ .../style/stylesheets/keyframes_rule.rs | 33 +++--- servo/ports/geckolib/glue.rs | 100 ++++++++++++------ 8 files changed, 310 insertions(+), 122 deletions(-) diff --git a/servo/components/style/animation.rs b/servo/components/style/animation.rs index a0d3bfce58ffe..b865120abacad 100644 --- a/servo/components/style/animation.rs +++ b/servo/components/style/animation.rs @@ -15,8 +15,8 @@ use crate::properties::longhands::animation_fill_mode::computed_value::single_va use crate::properties::longhands::animation_play_state::computed_value::single_value::T as AnimationPlayState; use crate::properties::AnimationDeclarations; use crate::properties::{ - ComputedValues, Importance, LonghandId, LonghandIdSet, PropertyDeclarationBlock, - PropertyDeclarationId, + ComputedValues, Importance, LonghandId, PropertyDeclarationBlock, PropertyDeclarationId, + PropertyDeclarationIdSet, }; use crate::rule_tree::CascadeLevel; use crate::selector_parser::PseudoElement; @@ -51,22 +51,22 @@ pub struct PropertyAnimation { impl PropertyAnimation { /// Returns the given property longhand id. - pub fn property_id(&self) -> LonghandId { + pub fn property_id(&self) -> PropertyDeclarationId { debug_assert_eq!(self.from.id(), self.to.id()); self.from.id() } - fn from_longhand( - longhand: LonghandId, + fn from_property_declaration( + property_declaration: &PropertyDeclarationId, timing_function: TimingFunction, duration: Time, old_style: &ComputedValues, new_style: &ComputedValues, ) -> Option { // FIXME(emilio): Handle the case where old_style and new_style's writing mode differ. - let longhand = longhand.to_physical(new_style.writing_mode); - let from = AnimationValue::from_computed_values(longhand, old_style)?; - let to = AnimationValue::from_computed_values(longhand, new_style)?; + let property_declaration = property_declaration.to_physical(new_style.writing_mode); + let from = AnimationValue::from_computed_values(property_declaration, old_style)?; + let to = AnimationValue::from_computed_values(property_declaration, new_style)?; let duration = duration.seconds() as f64; if from == to || duration == 0.0 { @@ -294,7 +294,7 @@ impl ComputedKeyframe { where E: TElement, { - let mut animating_properties = LonghandIdSet::new(); + let mut animating_properties = PropertyDeclarationIdSet::default(); for property in animation.properties_changed.iter() { debug_assert!(property.is_animatable()); animating_properties.insert(property.to_physical(base_style.writing_mode)); @@ -314,7 +314,7 @@ impl ComputedKeyframe { let mut computed_steps: Vec = Vec::with_capacity(intermediate_steps.len()); for (step_index, step) in intermediate_steps.into_iter().enumerate() { let start_percentage = step.start_percentage; - let properties_changed_in_step = step.declarations.longhands().clone(); + let properties_changed_in_step = step.declarations.property_ids().clone(); let step_timing_function = step.timing_function.clone(); let step_style = step.resolve_style(element, context, base_style, resolver); let timing_function = @@ -340,9 +340,9 @@ impl ComputedKeyframe { animating_properties .iter() .zip(default_values.iter()) - .map(|(longhand, default_value)| { - if properties_changed_in_step.contains(longhand) { - AnimationValue::from_computed_values(longhand, &step_style) + .map(|(property_declaration, default_value)| { + if properties_changed_in_step.contains(property_declaration) { + AnimationValue::from_computed_values(property_declaration, &step_style) .unwrap_or_else(|| default_value.clone()) } else { default_value.clone() @@ -368,7 +368,7 @@ pub struct Animation { pub name: Atom, /// The properties that change in this animation. - properties_changed: LonghandIdSet, + properties_changed: PropertyDeclarationIdSet, /// The computed style for each keyframe of this animation. computed_steps: Vec, @@ -669,7 +669,7 @@ impl Animation { // in order to avoid doing more work. let mut add_declarations_to_map = |keyframe: &ComputedKeyframe| { for value in keyframe.values.iter() { - map.insert(value.id(), value.clone()); + map.insert(value.id().to_owned(), value.clone()); } }; if total_progress <= 0.0 { @@ -702,7 +702,7 @@ impl Animation { }; if let Ok(value) = animation.calculate_value(progress_between_keyframes) { - map.insert(value.id(), value); + map.insert(value.id().to_owned(), value); } } } @@ -1024,7 +1024,7 @@ impl ElementAnimationSet { fn start_transition_if_applicable( &mut self, context: &SharedStyleContext, - longhand_id: LonghandId, + property_declaration_id: &PropertyDeclarationId, index: usize, old_style: &ComputedValues, new_style: &Arc, @@ -1037,8 +1037,8 @@ impl ElementAnimationSet { // Only start a new transition if the style actually changes between // the old style and the new style. - let property_animation = match PropertyAnimation::from_longhand( - longhand_id, + let property_animation = match PropertyAnimation::from_property_declaration( + property_declaration_id, timing_function, duration, old_style, @@ -1078,7 +1078,9 @@ impl ElementAnimationSet { .transitions .iter_mut() .filter(|transition| transition.state == AnimationState::Running) - .find(|transition| transition.property_animation.property_id() == longhand_id) + .find(|transition| { + transition.property_animation.property_id() == *property_declaration_id + }) { // We always cancel any running transitions for the same property. old_transition.state = AnimationState::Canceled; @@ -1106,7 +1108,7 @@ impl ElementAnimationSet { Some(value) => value, None => continue, }; - map.insert(value.id(), value); + map.insert(value.id().to_owned(), value); } Some(map) @@ -1268,10 +1270,12 @@ pub fn start_transitions_if_applicable( old_style: &ComputedValues, new_style: &Arc, animation_state: &mut ElementAnimationSet, -) -> LonghandIdSet { - let mut properties_that_transition = LonghandIdSet::new(); +) -> PropertyDeclarationIdSet { + let mut properties_that_transition = PropertyDeclarationIdSet::default(); for transition in new_style.transition_properties() { - let physical_property = transition.longhand_id.to_physical(new_style.writing_mode); + let physical_property = PropertyDeclarationId::Longhand( + transition.longhand_id.to_physical(new_style.writing_mode), + ); if properties_that_transition.contains(physical_property) { continue; } @@ -1279,7 +1283,7 @@ pub fn start_transitions_if_applicable( properties_that_transition.insert(physical_property); animation_state.start_transition_if_applicable( context, - physical_property, + &physical_property, transition.index, old_style, new_style, @@ -1370,7 +1374,7 @@ pub fn maybe_start_animations( let mut new_animation = Animation { name: name.clone(), - properties_changed: keyframe_animation.properties_changed, + properties_changed: keyframe_animation.properties_changed.clone(), computed_steps, started_at, duration, diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 6b012bbace97c..b2abb4c5bdee3 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -52,9 +52,11 @@ use crate::gecko_bindings::structs::{nsINode as RawGeckoNode, Element as RawGeck use crate::global_style_data::GLOBAL_STYLE_DATA; use crate::invalidation::element::restyle_hints::RestyleHint; use crate::media_queries::Device; -use crate::properties::animated_properties::{AnimationValue, AnimationValueMap}; -use crate::properties::{ComputedValues, LonghandId}; -use crate::properties::{Importance, PropertyDeclaration, PropertyDeclarationBlock}; +use crate::properties::{ + animated_properties::{AnimationValue, AnimationValueMap}, + ComputedValues, Importance, OwnedPropertyDeclarationId, PropertyDeclaration, + PropertyDeclarationBlock, PropertyDeclarationId, PropertyDeclarationIdSet, +}; use crate::rule_tree::CascadeLevel as ServoCascadeLevel; use crate::selector_parser::{AttrValue, Lang}; use crate::shared_lock::{Locked, SharedRwLock}; @@ -812,7 +814,7 @@ impl<'le> GeckoElement<'le> { host.is_svg_element() && host.local_name() == &**local_name!("use") } - fn css_transitions_info(&self) -> FxHashMap> { + fn css_transitions_info(&self) -> FxHashMap> { use crate::gecko_bindings::bindings::Gecko_ElementTransitions_EndValueAt; use crate::gecko_bindings::bindings::Gecko_ElementTransitions_Length; @@ -824,21 +826,21 @@ impl<'le> GeckoElement<'le> { unsafe { Arc::from_raw_addrefed(Gecko_ElementTransitions_EndValueAt(self.0, i)) }; let property = end_value.id(); debug_assert!(!property.is_logical()); - map.insert(property, end_value); + map.insert(property.to_owned(), end_value); } map } fn needs_transitions_update_per_property( &self, - longhand_id: LonghandId, + property_declaration_id: PropertyDeclarationId, combined_duration_seconds: f32, before_change_style: &ComputedValues, after_change_style: &ComputedValues, - existing_transitions: &FxHashMap>, + existing_transitions: &FxHashMap>, ) -> bool { use crate::values::animated::{Animate, Procedure}; - debug_assert!(!longhand_id.is_logical()); + debug_assert!(!property_declaration_id.is_logical()); // If there is an existing transition, update only if the end value // differs. @@ -846,15 +848,17 @@ impl<'le> GeckoElement<'le> { // If the end value has not changed, we should leave the currently // running transition as-is since we don't want to interrupt its timing // function. - if let Some(ref existing) = existing_transitions.get(&longhand_id) { + if let Some(ref existing) = existing_transitions.get(&property_declaration_id.to_owned()) { let after_value = - AnimationValue::from_computed_values(longhand_id, after_change_style).unwrap(); + AnimationValue::from_computed_values(property_declaration_id, after_change_style) + .unwrap(); return ***existing != after_value; } - let from = AnimationValue::from_computed_values(longhand_id, before_change_style); - let to = AnimationValue::from_computed_values(longhand_id, after_change_style); + let from = + AnimationValue::from_computed_values(property_declaration_id, before_change_style); + let to = AnimationValue::from_computed_values(property_declaration_id, after_change_style); debug_assert_eq!(to.is_some(), from.is_some()); @@ -1517,8 +1521,6 @@ impl<'le> TElement for GeckoElement<'le> { before_change_style: &ComputedValues, after_change_style: &ComputedValues, ) -> bool { - use crate::properties::LonghandIdSet; - let after_change_ui_style = after_change_style.get_ui(); let existing_transitions = self.css_transitions_info(); @@ -1527,11 +1529,13 @@ impl<'le> TElement for GeckoElement<'le> { return !existing_transitions.is_empty(); } - let mut transitions_to_keep = LonghandIdSet::new(); + let mut transitions_to_keep = PropertyDeclarationIdSet::default(); for transition_property in after_change_style.transition_properties() { - let physical_longhand = transition_property - .longhand_id - .to_physical(after_change_style.writing_mode); + let physical_longhand = PropertyDeclarationId::Longhand( + transition_property + .longhand_id + .to_physical(after_change_style.writing_mode), + ); transitions_to_keep.insert(physical_longhand); if self.needs_transitions_update_per_property( physical_longhand, @@ -1550,7 +1554,7 @@ impl<'le> TElement for GeckoElement<'le> { // a matching transition-property value. existing_transitions .keys() - .any(|property| !transitions_to_keep.contains(*property)) + .any(|property| !transitions_to_keep.contains(property.as_borrowed())) } /// Whether there is an ElementData container. diff --git a/servo/components/style/properties/declaration_block.rs b/servo/components/style/properties/declaration_block.rs index 5e5187fd4838c..0cdf8677a8d24 100644 --- a/servo/components/style/properties/declaration_block.rs +++ b/servo/components/style/properties/declaration_block.rs @@ -12,6 +12,7 @@ use super::generated::{ SourcePropertyDeclaration, SourcePropertyDeclarationDrain, SubpropertiesVec, }; use super::property_declaration::PropertyDeclarationId; +use super::LonghandIdSetIterator; use crate::applicable_declarations::CascadePriority; use crate::context::QuirksMode; use crate::custom_properties::{self, ComputedCustomProperties, CustomPropertiesBuilder}; @@ -114,14 +115,16 @@ impl Importance { } } -#[derive(Clone, ToShmem, Default, MallocSizeOf)] -struct PropertyDeclarationIdSet { +/// A set of properties. +#[derive(Clone, Debug, ToShmem, Default, MallocSizeOf)] +pub struct PropertyDeclarationIdSet { longhands: LonghandIdSet, custom: PrecomputedHashSet, } impl PropertyDeclarationIdSet { - fn insert(&mut self, id: PropertyDeclarationId) -> bool { + /// Add the given property to the set. + pub fn insert(&mut self, id: PropertyDeclarationId) -> bool { match id { PropertyDeclarationId::Longhand(id) => { if self.longhands.contains(id) { @@ -130,18 +133,20 @@ impl PropertyDeclarationIdSet { self.longhands.insert(id); return true; }, - PropertyDeclarationId::Custom(name) => self.custom.insert(name.clone()), + PropertyDeclarationId::Custom(name) => self.custom.insert((*name).clone()), } } - fn contains(&self, id: PropertyDeclarationId) -> bool { + /// Return whether the given property is in the set. + pub fn contains(&self, id: PropertyDeclarationId) -> bool { match id { PropertyDeclarationId::Longhand(id) => self.longhands.contains(id), PropertyDeclarationId::Custom(name) => self.custom.contains(name), } } - fn remove(&mut self, id: PropertyDeclarationId) { + /// Remove the given property from the set. + pub fn remove(&mut self, id: PropertyDeclarationId) { match id { PropertyDeclarationId::Longhand(id) => self.longhands.remove(id), PropertyDeclarationId::Custom(name) => { @@ -150,10 +155,59 @@ impl PropertyDeclarationIdSet { } } - fn clear(&mut self) { + /// Remove all properties from the set. + pub fn clear(&mut self) { self.longhands.clear(); self.custom.clear(); } + + /// Returns whether the set is empty. + #[inline] + pub fn is_empty(&self) -> bool { + self.longhands.is_empty() && self.custom.is_empty() + } + /// Returns whether this set contains any reset longhand. + #[inline] + pub fn contains_any_reset(&self) -> bool { + self.longhands.contains_any_reset() + } + + /// Returns whether this set contains all longhands in the specified set. + #[inline] + pub fn contains_all(&self, longhands: &LonghandIdSet) -> bool { + self.longhands.contains_all(longhands) + } + + /// Iterate over the current property declaration id set. + pub fn iter(&self) -> PropertyDeclarationIdSetIterator { + PropertyDeclarationIdSetIterator { + longhands: self.longhands.iter(), + custom: self.custom.iter(), + } + } +} + +/// An iterator over a set of longhand ids. +pub struct PropertyDeclarationIdSetIterator<'a> { + longhands: LonghandIdSetIterator<'a>, + custom: std::collections::hash_set::Iter<'a, custom_properties::Name>, +} + +impl<'a> Iterator for PropertyDeclarationIdSetIterator<'a> { + type Item = PropertyDeclarationId<'a>; + + fn next(&mut self) -> Option { + // LonghandIdSetIterator's implementation always returns None + // after it did it once, so the code below will then continue + // to iterate over the custom properties. + match self.longhands.next() { + Some(id) => Some(PropertyDeclarationId::Longhand(id)), + None => match self.custom.next() { + Some(a) => Some(PropertyDeclarationId::Custom(a)), + None => None, + }, + } + } } /// Overridden declarations are skipped. @@ -382,11 +436,11 @@ impl PropertyDeclarationBlock { !self.declarations_importance.all_true() } - /// Returns a `LonghandIdSet` representing the properties that are changed in + /// Returns a `PropertyDeclarationIdSet` representing the properties that are changed in /// this block. #[inline] - pub fn longhands(&self) -> &LonghandIdSet { - &self.property_ids.longhands + pub fn property_ids(&self) -> &PropertyDeclarationIdSet { + &self.property_ids } /// Returns whether this block contains a declaration of a given property id. @@ -398,7 +452,7 @@ impl PropertyDeclarationBlock { /// Returns whether this block contains any reset longhand. #[inline] pub fn contains_any_reset(&self) -> bool { - self.property_ids.longhands.contains_any_reset() + self.property_ids.contains_any_reset() } /// Get a declaration for a given property. @@ -921,7 +975,7 @@ impl PropertyDeclarationBlock { let mut property_ids = PropertyDeclarationIdSet::default(); for (property, animation_value) in animation_value_map.iter() { - property_ids.longhands.insert(*property); + property_ids.insert(property.as_borrowed()); declarations.push(animation_value.uncompute()); } @@ -1048,7 +1102,7 @@ impl PropertyDeclarationBlock { // If all properties that map to shorthand are not present // in longhands, continue with the steps labeled shorthand // loop. - if !self.property_ids.longhands.contains_all(&longhands) { + if !self.property_ids.contains_all(&longhands) { continue; } diff --git a/servo/components/style/properties/helpers/animated_properties.mako.rs b/servo/components/style/properties/helpers/animated_properties.mako.rs index 3841ba209ca40..60f0171a0a62b 100644 --- a/servo/components/style/properties/helpers/animated_properties.mako.rs +++ b/servo/components/style/properties/helpers/animated_properties.mako.rs @@ -10,15 +10,19 @@ %> #[cfg(feature = "gecko")] use crate::gecko_bindings::structs::nsCSSPropertyID; -use crate::properties::{CSSWideKeyword, PropertyDeclaration, NonCustomPropertyIterator}; -use crate::properties::longhands; -use crate::properties::longhands::visibility::computed_value::T as Visibility; -use crate::properties::longhands::content_visibility::computed_value::T as ContentVisibility; -use crate::properties::LonghandId; +use crate::properties::{ + longhands::{ + self, content_visibility::computed_value::T as ContentVisibility, + visibility::computed_value::T as Visibility, + }, + CSSWideKeyword, LonghandId, NonCustomPropertyIterator, PropertyDeclaration, + PropertyDeclarationId, +}; use std::ptr; use std::mem; use fxhash::FxHashMap; use super::ComputedValues; +use crate::properties::property_declaration::OwnedPropertyDeclarationId; use crate::values::animated::{Animate, Procedure, ToAnimatedValue, ToAnimatedZero}; use crate::values::animated::effects::AnimatedFilter; #[cfg(feature = "gecko")] use crate::values::computed::TransitionProperty; @@ -58,7 +62,7 @@ impl From for TransitionProperty { /// A collection of AnimationValue that were composed on an element. /// This HashMap stores the values that are the last AnimationValue to be /// composed for each TransitionProperty. -pub type AnimationValueMap = FxHashMap; +pub type AnimationValueMap = FxHashMap; /// An enum to represent a single computed value belonging to an animated /// property in order to be interpolated with another one. When interpolating, @@ -184,7 +188,7 @@ impl PartialEq for AnimationValue { impl AnimationValue { /// Returns the longhand id this animated value corresponds to. #[inline] - pub fn id(&self) -> LonghandId { + pub fn id(&self) -> PropertyDeclarationId { let id = unsafe { *(self as *const _ as *const LonghandId) }; debug_assert_eq!(id, match *self { % for prop in data.longhands: @@ -195,7 +199,7 @@ impl AnimationValue { % endif % endfor }); - id + PropertyDeclarationId::Longhand(id) } /// Returns whether this value is interpolable with another one. @@ -388,9 +392,15 @@ impl AnimationValue { /// Get an AnimationValue for an AnimatableLonghand from a given computed values. pub fn from_computed_values( - property: LonghandId, + property: PropertyDeclarationId, style: &ComputedValues, ) -> Option { + let property = match property { + PropertyDeclarationId::Longhand(id) => id, + // TODO(bug 1846516): Implement this for custom properties. + PropertyDeclarationId::Custom(_) => return None, + }; + let property = property.to_physical(style.writing_mode); Some(match property { % for prop in data.longhands: diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index b3353c10fe8a8..c1cf0caa829d3 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -38,12 +38,14 @@ use to_shmem::impl_trivial_to_shmem; use crate::stylesheets::{CssRuleType, CssRuleTypes, Origin, UrlExtraData}; use crate::logical_geometry::{LogicalAxis, LogicalCorner, LogicalSide}; use crate::use_counters::UseCounters; -use crate::values::generics::font::LineHeight; -use crate::values::specified::length::LineHeightBase; -use crate::values::{computed, resolved, serialize_atom_name}; -use crate::values::specified::font::SystemFont; use crate::rule_tree::StrongRuleNode; use crate::str::{CssString, CssStringWriter}; +use crate::values::{ + computed, + generics::font::LineHeight, + resolved, serialize_atom_name, + specified::{font::SystemFont, length::LineHeightBase}, +}; use std::cell::Cell; use super::declaration_block::AppendableValue; use super::property_declaration::PropertyDeclarationId; diff --git a/servo/components/style/properties/property_declaration.rs b/servo/components/style/properties/property_declaration.rs index d35fc7570fc08..04bcace7425f9 100644 --- a/servo/components/style/properties/property_declaration.rs +++ b/servo/components/style/properties/property_declaration.rs @@ -6,6 +6,9 @@ use super::{LonghandId, PropertyId, ShorthandId}; use crate::custom_properties::Name; +#[cfg(feature = "gecko")] +use crate::gecko_bindings::structs::nsCSSPropertyID; +use crate::logical_geometry::WritingMode; use crate::values::serialize_atom_name; use std::{ borrow::Cow, @@ -13,6 +16,32 @@ use std::{ }; use style_traits::{CssWriter, ToCss}; +/// A PropertyDeclarationId without references, for use as a hash map key. +#[derive(Clone, Debug, PartialEq, Eq, Hash)] +pub enum OwnedPropertyDeclarationId { + /// A longhand. + Longhand(LonghandId), + /// A custom property declaration. + Custom(Name), +} + +impl OwnedPropertyDeclarationId { + /// Return whether this property is logical. + #[inline] + pub fn is_logical(&self) -> bool { + self.as_borrowed().is_logical() + } + + /// Returns the corresponding PropertyDeclarationId. + #[inline] + pub fn as_borrowed(&self) -> PropertyDeclarationId { + match self { + OwnedPropertyDeclarationId::Longhand(id) => PropertyDeclarationId::Longhand(*id), + OwnedPropertyDeclarationId::Custom(name) => PropertyDeclarationId::Custom(name), + } + } +} + /// An identifier for a given property declaration, which can be either a /// longhand or a custom property. #[derive(Clone, Copy, Debug, PartialEq)] @@ -40,6 +69,16 @@ impl<'a> ToCss for PropertyDeclarationId<'a> { } impl<'a> PropertyDeclarationId<'a> { + /// Convert to an OwnedPropertyDeclarationId. + pub fn to_owned(&self) -> OwnedPropertyDeclarationId { + match self { + PropertyDeclarationId::Longhand(id) => OwnedPropertyDeclarationId::Longhand(*id), + PropertyDeclarationId::Custom(name) => { + OwnedPropertyDeclarationId::Custom((*name).clone()) + }, + } + } + /// Whether a given declaration id is either the same as `other`, or a /// longhand of it. pub fn is_or_is_longhand_of(&self, other: &PropertyId) -> bool { @@ -83,4 +122,48 @@ impl<'a> PropertyDeclarationId<'a> { _ => None, } } + + /// Return whether this property is logical. + #[inline] + pub fn is_logical(&self) -> bool { + match self { + PropertyDeclarationId::Longhand(id) => id.is_logical(), + PropertyDeclarationId::Custom(_) => false, + } + } + + /// If this is a logical property, return the corresponding physical one in + /// the given writing mode. + /// + /// Otherwise, return unchanged. + #[inline] + pub fn to_physical(&self, wm: WritingMode) -> Self { + match self { + PropertyDeclarationId::Longhand(id) => { + PropertyDeclarationId::Longhand(id.to_physical(wm)) + }, + PropertyDeclarationId::Custom(_) => self.clone(), + } + } + + /// Returns whether this property is animatable. + #[inline] + pub fn is_animatable(&self) -> bool { + match self { + PropertyDeclarationId::Longhand(id) => id.is_animatable(), + // TODO(bug 1846516): This should return true. + PropertyDeclarationId::Custom(_) => false, + } + } + + /// Converts from a to an adequate nsCSSPropertyID, returning + /// eCSSPropertyExtra_variable for custom properties. + #[cfg(feature = "gecko")] + #[inline] + pub fn to_nscsspropertyid(self) -> nsCSSPropertyID { + match self { + PropertyDeclarationId::Longhand(id) => id.to_nscsspropertyid(), + PropertyDeclarationId::Custom(_) => nsCSSPropertyID::eCSSPropertyExtra_variable, + } + } } diff --git a/servo/components/style/stylesheets/keyframes_rule.rs b/servo/components/style/stylesheets/keyframes_rule.rs index 6e5016080e99f..96e916b553b23 100644 --- a/servo/components/style/stylesheets/keyframes_rule.rs +++ b/servo/components/style/stylesheets/keyframes_rule.rs @@ -6,12 +6,14 @@ use crate::error_reporting::ContextualParseError; use crate::parser::ParserContext; -use crate::properties::longhands::animation_composition::single_value::SpecifiedValue as SpecifiedComposition; -use crate::properties::longhands::transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction; -use crate::properties::LonghandIdSet; -use crate::properties::{Importance, PropertyDeclaration}; -use crate::properties::{LonghandId, PropertyDeclarationBlock, PropertyId}; -use crate::properties::{PropertyDeclarationId, SourcePropertyDeclaration}; +use crate::properties::{ + longhands::{ + animation_composition::single_value::SpecifiedValue as SpecifiedComposition, + transition_timing_function::single_value::SpecifiedValue as SpecifiedTimingFunction, + }, + Importance, LonghandId, PropertyDeclaration, PropertyDeclarationBlock, PropertyDeclarationId, + PropertyDeclarationIdSet, PropertyId, SourcePropertyDeclaration, +}; use crate::shared_lock::{DeepCloneParams, DeepCloneWithLock, SharedRwLock, SharedRwLockReadGuard}; use crate::shared_lock::{Locked, ToCssWithGuard}; use crate::str::CssStringWriter; @@ -407,7 +409,7 @@ pub struct KeyframesAnimation { /// The difference steps of the animation. pub steps: Vec, /// The properties that change in this animation. - pub properties_changed: LonghandIdSet, + pub properties_changed: PropertyDeclarationIdSet, /// Vendor prefix type the @keyframes has. pub vendor_prefix: Option, } @@ -416,8 +418,8 @@ pub struct KeyframesAnimation { fn get_animated_properties( keyframes: &[Arc>], guard: &SharedRwLockReadGuard, -) -> LonghandIdSet { - let mut ret = LonghandIdSet::new(); +) -> PropertyDeclarationIdSet { + let mut ret = PropertyDeclarationIdSet::default(); // NB: declarations are already deduplicated, so we don't have to check for // it here. for keyframe in keyframes { @@ -430,20 +432,17 @@ fn get_animated_properties( // be properties with !important in keyframe rules here. // See the spec issue https://github.com/w3c/csswg-drafts/issues/1824 for declaration in block.normal_declaration_iter() { - let longhand_id = match declaration.id() { - PropertyDeclarationId::Longhand(id) => id, - _ => continue, - }; + let declaration_id = declaration.id(); - if longhand_id == LonghandId::Display { + if declaration_id == PropertyDeclarationId::Longhand(LonghandId::Display) { continue; } - if !longhand_id.is_animatable() { + if !declaration_id.is_animatable() { continue; } - ret.insert(longhand_id); + ret.insert(declaration_id); } } @@ -466,7 +465,7 @@ impl KeyframesAnimation { ) -> Self { let mut result = KeyframesAnimation { steps: vec![], - properties_changed: LonghandIdSet::new(), + properties_changed: PropertyDeclarationIdSet::default(), vendor_prefix, }; diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index 02afdc2985bfd..32ec27ed99c80 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -110,12 +110,13 @@ use style::invalidation::element::restyle_hints::RestyleHint; use style::invalidation::stylesheets::RuleChangeKind; use style::media_queries::MediaList; use style::parser::{Parse, ParserContext}; -use style::properties::animated_properties::{AnimationValue, AnimationValueMap}; -use style::properties::{parse_one_declaration_into, parse_style_attribute}; -use style::properties::{ComputedValues, CountedUnknownProperty, Importance, NonCustomPropertyId}; -use style::properties::{LonghandId, LonghandIdSet, PropertyDeclarationBlock, PropertyId}; -use style::properties::{PropertyDeclarationId, ShorthandId}; -use style::properties::{SourcePropertyDeclaration, StyleBuilder}; +use style::properties::{ + animated_properties::{AnimationValue, AnimationValueMap}, + parse_one_declaration_into, parse_style_attribute, ComputedValues, CountedUnknownProperty, + Importance, LonghandId, LonghandIdSet, NonCustomPropertyId, OwnedPropertyDeclarationId, + PropertyDeclarationBlock, PropertyDeclarationId, PropertyDeclarationIdSet, PropertyId, + ShorthandId, SourcePropertyDeclaration, StyleBuilder, +}; use style::properties_and_values::registry::PropertyRegistration; use style::properties_and_values::rule::Inherits as PropertyInherits; use style::rule_cache::RuleCacheConditions; @@ -629,10 +630,13 @@ pub extern "C" fn Servo_AnimationCompose( use style::gecko_bindings::bindings::Gecko_GetPositionInSegment; use style::gecko_bindings::bindings::Gecko_GetProgressFromComputedTiming; - let property = match LonghandId::from_nscsspropertyid(css_property) { - Some(longhand) if longhand.is_animatable() => longhand, - _ => return, - }; + // TODO(bug 1846516): Caller should be able to pass custom properties. + let property = OwnedPropertyDeclarationId::Longhand( + match LonghandId::from_nscsspropertyid(css_property) { + Some(longhand) if longhand.is_animatable() => longhand, + _ => return, + }, + ); // We will need an underlying value if either of the endpoints is null... let need_underlying_value = segment.mFromValue.mServo.mRawPtr.is_null() || @@ -1127,10 +1131,12 @@ pub extern "C" fn Servo_AnimationValueMap_GetValue( value_map: &AnimationValueMap, property_id: nsCSSPropertyID, ) -> Strong { - let property = match LonghandId::from_nscsspropertyid(property_id) { - Some(longhand) => longhand, - None => return Strong::null(), - }; + // TODO(bug 1846516): Caller should be able to pass custom properties. + let property = + OwnedPropertyDeclarationId::Longhand(match LonghandId::from_nscsspropertyid(property_id) { + Some(longhand) => longhand, + Err(()) => return Strong::null(), + }); value_map .get(&property) .map_or(Strong::null(), |value| Arc::new(value.clone()).into()) @@ -1199,12 +1205,17 @@ pub extern "C" fn Servo_ComputedValues_ShouldTransition( start: &mut structs::RefPtr, end: &mut structs::RefPtr, ) -> ShouldTransitionResult { - let Some(prop) = LonghandId::from_nscsspropertyid(prop) else { - return Default::default(); - }; - if prop.is_discrete_animatable() && prop != LonghandId::Visibility { - return Default::default(); - } + // TODO(bug 1846516): Caller should be able to pass custom properties. + let prop = PropertyDeclarationId::Longhand({ + let Some(prop) = LonghandId::from_nscsspropertyid(prop) else { + return Default::default(); + }; + if prop.is_discrete_animatable() && prop != LonghandId::Visibility { + return Default::default(); + } + prop + }); + let Some(new_value) = AnimationValue::from_computed_values(prop, new) else { return Default::default(); }; @@ -1240,12 +1251,16 @@ pub extern "C" fn Servo_ComputedValues_TransitionValueMatches( prop: nsCSSPropertyID, transition_value: &AnimationValue, ) -> bool { - let Some(prop) = LonghandId::from_nscsspropertyid(prop) else { - return false; - }; - if prop.is_discrete_animatable() && prop != LonghandId::Visibility { - return false; - } + // TODO(bug 1846516): Caller should be able to pass custom properties. + let prop = PropertyDeclarationId::Longhand({ + let Some(prop) = LonghandId::from_nscsspropertyid(prop) else { + return false; + }; + if prop.is_discrete_animatable() && prop != LonghandId::Visibility { + return false; + } + prop + }); let Some(value) = AnimationValue::from_computed_values(prop, style) else { return false; }; @@ -1257,10 +1272,12 @@ pub extern "C" fn Servo_ComputedValues_ExtractAnimationValue( computed_values: &ComputedValues, property_id: nsCSSPropertyID, ) -> Strong { - let property = match LonghandId::from_nscsspropertyid(property_id) { - Some(longhand) => longhand, - None => return Strong::null(), - }; + // TODO(bug 1846516): Caller should be able to pass custom properties. + let property = + PropertyDeclarationId::Longhand(match LonghandId::from_nscsspropertyid(property_id) { + Ok(longhand) => longhand, + None => return Strong::null(), + }); match AnimationValue::from_computed_values(property, &computed_values) { Some(v) => Arc::new(v).into(), None => Strong::null(), @@ -4925,7 +4942,11 @@ pub unsafe extern "C" fn Servo_DeclarationBlock_SetPropertyToAnimationValue( animation_value: &AnimationValue, before_change_closure: DeclarationBlockMutationClosure, ) -> bool { - let non_custom_property_id = animation_value.id().into(); + let non_custom_property_id = match animation_value.id() { + PropertyDeclarationId::Longhand(id) => id.into(), + // TODO(bug 1846516): Support custom properties too? + PropertyDeclarationId::Custom(_) => return false, + }; let mut source_declarations = SourcePropertyDeclaration::with_one(animation_value.uncompute()); set_property_to_declarations( @@ -6140,6 +6161,7 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( raw_data: &PerDocumentStyleData, computed_keyframes: &mut nsTArray, ) { + // TODO(bug 1846516): Make this work for animated custom properties too. let data = raw_data.borrow(); let element = GeckoElement(element); let pseudo = PseudoElement::from_pseudo_type(pseudo_type, None); @@ -6255,7 +6277,10 @@ pub extern "C" fn Servo_GetComputedKeyframeValues( ); for value in iter { - let id = value.id(); + let id = match value.id() { + PropertyDeclarationId::Longhand(id) => id, + PropertyDeclarationId::Custom(_) => continue, + }; maybe_append_animation_value(id, Some(value)); } } @@ -6405,6 +6430,7 @@ fn fill_in_missing_keyframe_values( offset: Offset, keyframes: &mut nsTArray, ) { + // TODO(bug 1846516): Make this work for animated custom properties too. // Return early if all animated properties are already set. if longhands_at_offset.contains_all(all_properties) { return; @@ -6447,6 +6473,7 @@ pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( inherited_timing_function: &ComputedTimingFunction, keyframes: &mut nsTArray, ) -> bool { + // TODO(bug 1846516): Make this work for animated custom properties too. use style::gecko_bindings::structs::CompositeOperationOrAuto; use style::properties::longhands::animation_composition::single_value::computed_value::T as Composition; @@ -6513,7 +6540,7 @@ pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( // to represent that all properties animated by the keyframes // animation should be set to the underlying computed value for // that keyframe. - let mut seen = LonghandIdSet::new(); + let mut seen = PropertyDeclarationIdSet::default(); for property in animation.properties_changed.iter() { let property = property.to_physical(writing_mode); if seen.contains(property) { @@ -6608,7 +6635,12 @@ pub unsafe extern "C" fn Servo_StyleSet_GetKeyframesForName( let mut properties_changed = LonghandIdSet::new(); for property in animation.properties_changed.iter() { - properties_changed.insert(property.to_physical(writing_mode)); + let longhand_id = match property.to_physical(writing_mode) { + PropertyDeclarationId::Longhand(id) => id, + // TODO(zrhoffman, bug 1846516): Handle custom properties here. + PropertyDeclarationId::Custom(_) => continue, + }; + properties_changed.insert(longhand_id); } // Append property values that are missing in the initial or the final keyframes.