Skip to content

Commit

Permalink
Bug 1846516 - [css-properties-values-api] Animate custom properties i…
Browse files Browse the repository at this point in the history 
…n a discrete way. r=firefox-style-system-reviewers,emilio

This patch also updates the bug ID for a FIXME leftover from bug 1840478
to bug 1869476, since the same FIXME is added in D190758.

Co-authored-by: Frederic Wang <fred.wang@free.fr>

Depends on D191322

Differential Revision: https://phabricator.services.mozilla.com/D190758
  • Loading branch information
@zrhoffman
zrhoffman committed Dec 18, 2023
1 parent e059ff7 commit 1971ef5
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 37 deletions.
19 changes: 17 additions & 2 deletions style/properties/declaration_block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,25 @@ impl PropertyDeclarationIdSet {

/// Returns whether this set contains all longhands in the specified set.
#[inline]
pub fn contains_all(&self, longhands: &LonghandIdSet) -> bool {
pub fn contains_all_longhands(&self, longhands: &LonghandIdSet) -> bool {
self.longhands.contains_all(longhands)
}

/// Returns whether this set contains all properties in the specified set.
#[inline]
pub fn contains_all(&self, properties: &PropertyDeclarationIdSet) -> bool {
if !self.longhands.contains_all(&properties.longhands) {
return false;
}
if properties.custom.len() > self.custom.len() {
return false;
}
properties
.custom
.iter()
.all(|item| self.custom.contains(item))
}

/// Iterate over the current property declaration id set.
pub fn iter(&self) -> PropertyDeclarationIdSetIterator {
PropertyDeclarationIdSetIterator {
Expand Down Expand Up @@ -1102,7 +1117,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.contains_all(&longhands) {
if !self.property_ids.contains_all_longhands(&longhands) {
continue;
}

Expand Down
97 changes: 71 additions & 26 deletions style/properties/helpers/animated_properties.mako.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,14 @@
%>

#[cfg(feature = "gecko")] use crate::gecko_bindings::structs::nsCSSPropertyID;
use crate::custom_properties::SpecifiedValue as SpecifiedCustomPropertyValue;
use crate::properties::{
longhands::{
self, content_visibility::computed_value::T as ContentVisibility,
visibility::computed_value::T as Visibility,
},
CSSWideKeyword, LonghandId, NonCustomPropertyIterator, PropertyDeclaration,
PropertyDeclarationId,
CSSWideKeyword, CustomDeclaration, CustomDeclarationValue, LonghandId,
NonCustomPropertyIterator, PropertyDeclaration, PropertyDeclarationId,
};
use std::ptr;
use std::mem;
Expand Down Expand Up @@ -67,9 +68,6 @@ pub type AnimationValueMap = FxHashMap<OwnedPropertyDeclarationId, AnimationValu
/// An enum to represent a single computed value belonging to an animated
/// property in order to be interpolated with another one. When interpolating,
/// both values need to belong to the same property.
///
/// FIXME: We need to add a path for custom properties, but that's trivial after
/// this (is a similar path to that of PropertyDeclaration).
#[derive(Debug, MallocSizeOf)]
#[repr(u16)]
pub enum AnimationValue {
Expand All @@ -81,6 +79,9 @@ pub enum AnimationValue {
${prop.camel_case}(Void),
% endif
% endfor
/// A custom property.
/// TODO(zrhoffman, bug 1869472): The Custom variant should hold only a single struct.
Custom(crate::custom_properties::Name, SpecifiedCustomPropertyValue),
}

<%
Expand Down Expand Up @@ -152,6 +153,7 @@ impl Clone for AnimationValue {
% endif
}
% endfor
Custom(ref name, ref value) => { Custom(name.clone(), value.clone()) },
_ => unsafe { debug_unreachable!() }
}
}
Expand All @@ -162,23 +164,31 @@ impl PartialEq for AnimationValue {
fn eq(&self, other: &Self) -> bool {
use self::AnimationValue::*;

unsafe {
let this_tag = *(self as *const _ as *const u16);
let other_tag = *(other as *const _ as *const u16);
if this_tag != other_tag {
return false;
}

match *self {
% for ty, props in groupby(animated, key=lambda x: x.animated_type()):
${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => {
let other_repr =
&*(other as *const _ as *const AnimationValueVariantRepr<${ty}>);
*this == other_repr.value
}
% endfor
${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
void::unreachable(void)
match (self, other) {
(Custom(name1, value1), Custom(name2, value2)) => {
name1 == name2 && value1 == value2
},
_ => {
unsafe {
let this_tag = *(self as *const _ as *const u16);
let other_tag = *(other as *const _ as *const u16);
if this_tag != other_tag {
return false;
}

match *self {
% for ty, props in groupby(animated, key=lambda x: x.animated_type()):
${" |\n".join("{}(ref this)".format(prop.camel_case) for prop in props)} => {
let other_repr =
&*(other as *const _ as *const AnimationValueVariantRepr<${ty}>);
*this == other_repr.value
}
% endfor
${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
void::unreachable(void)
},
AnimationValue::Custom(..) => { debug_unreachable!() },
}
}
}
}
Expand All @@ -189,6 +199,10 @@ impl AnimationValue {
/// Returns the longhand id this animated value corresponds to.
#[inline]
pub fn id(&self) -> PropertyDeclarationId {
if let AnimationValue::Custom(name, _) = self {
return PropertyDeclarationId::Custom(name);
}

let id = unsafe { *(self as *const _ as *const LonghandId) };
debug_assert_eq!(id, match *self {
% for prop in data.longhands:
Expand All @@ -198,6 +212,7 @@ impl AnimationValue {
AnimationValue::${prop.camel_case}(void) => void::unreachable(void),
% endif
% endfor
AnimationValue::Custom(..) => unsafe { debug_unreachable!() },
});
PropertyDeclarationId::Longhand(id)
}
Expand Down Expand Up @@ -246,7 +261,13 @@ impl AnimationValue {
% endfor
${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
void::unreachable(void)
}
},
Custom(ref name, ref value) => {
PropertyDeclaration::Custom(CustomDeclaration {
name: name.clone(),
value: CustomDeclarationValue::Value(value.clone().into()),
})
},
}
}

Expand Down Expand Up @@ -385,6 +406,12 @@ impl AnimationValue {
initial,
)
},
PropertyDeclaration::Custom(ref declaration) => {
match &declaration.value {
CustomDeclarationValue::Value(value) => AnimationValue::Custom(declaration.name.clone(), (**value).clone()),
_ => return None,
}
},
_ => return None // non animatable properties will get included because of shorthands. ignore.
};
Some(animatable)
Expand All @@ -397,8 +424,15 @@ impl AnimationValue {
) -> Option<Self> {
let property = match property {
PropertyDeclarationId::Longhand(id) => id,
// TODO(bug 1846516): Implement this for custom properties.
PropertyDeclarationId::Custom(_) => return None,
PropertyDeclarationId::Custom(ref name) => {
// FIXME(bug 1869476): This should use a stylist to determine whether the name
// corresponds to an inherited custom property and then choose the
// inherited/non_inherited map accordingly.
let p = &style.custom_properties();
return p.inherited.as_ref().and_then(|map| map.get(*name))
.or_else(|| p.non_inherited.as_ref().and_then(|map| map.get(*name)))
.map(|value| AnimationValue::Custom((*name).clone(), (**value).clone()));
}
};

Some(match property {
Expand Down Expand Up @@ -442,6 +476,7 @@ impl AnimationValue {
AnimationValue::${prop.camel_case}(..) => unreachable!(),
% endif
% endfor
AnimationValue::Custom(..) => unreachable!(),
}
}

Expand All @@ -461,6 +496,11 @@ fn animate_discrete<T: Clone>(this: &T, other: &T, procedure: Procedure) -> Resu

impl Animate for AnimationValue {
fn animate(&self, other: &Self, procedure: Procedure) -> Result<Self, ()> {
if let AnimationValue::Custom(..) = self {
// TODO(bug 1869185): Non-universal registered property may animate in a non-discrete way.
return Ok(animate_discrete(self, other, procedure)?)
}

Ok(unsafe {
use self::AnimationValue::*;

Expand Down Expand Up @@ -495,7 +535,8 @@ impl Animate for AnimationValue {
% endfor
${" |\n".join("{}(void)".format(prop.camel_case) for prop in unanimated)} => {
void::unreachable(void)
}
},
Custom(..) => { debug_unreachable!() },
}
})
}
Expand Down Expand Up @@ -545,6 +586,10 @@ impl ToAnimatedZero for AnimationValue {
},
% endif
% endfor
AnimationValue::Custom(..) => {
// TODO(bug 1869185): For some non-universal registered custom properties, it may make sense to implement this.
Err(())
},
_ => Err(()),
}
}
Expand Down
2 changes: 1 addition & 1 deletion style/properties/properties.mako.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3254,7 +3254,7 @@ impl ComputedValues {
s
}
PropertyDeclarationId::Custom(name) => {
// FIXME(bug 1273706): This should use a stylist to determine
// FIXME(bug 1869476): This should use a stylist to determine
// whether the name corresponds to an inherited custom property
// and then choose the inherited/non_inherited map accordingly.
let p = &self.custom_properties;
Expand Down
43 changes: 35 additions & 8 deletions style/properties/property_declaration.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

//! Structs used for property declarations.

use super::{LonghandId, NonCustomPropertyId, PropertyId, ShorthandId};
use super::{LonghandId, NonCustomPropertyId, PropertyFlags, PropertyId, ShorthandId};
use crate::custom_properties::Name;
#[cfg(feature = "gecko")]
use crate::gecko_bindings::structs::{nsCSSPropertyID, AnimatedPropertyID};
use crate::gecko_bindings::structs::{nsCSSPropertyID, AnimatedPropertyID, RefPtr};
use crate::logical_geometry::WritingMode;
use crate::values::serialize_atom_name;
#[cfg(feature = "gecko")]
Expand Down Expand Up @@ -39,8 +39,7 @@ impl OwnedPropertyDeclarationId {
pub fn is_animatable(&self) -> bool {
match self {
Self::Longhand(id) => id.is_animatable(),
// TODO(bug 1846516): This should return true.
Self::Custom(_) => false,
Self::Custom(_) => true,
}
}

Expand Down Expand Up @@ -110,6 +109,15 @@ impl<'a> ToCss for PropertyDeclarationId<'a> {
}

impl<'a> PropertyDeclarationId<'a> {
/// Returns PropertyFlags for given property.
#[inline(always)]
pub fn flags(&self) -> PropertyFlags {
match self {
Self::Longhand(id) => id.flags(),
Self::Custom(_) => PropertyFlags::empty(),
}
}

/// Convert to an OwnedPropertyDeclarationId.
pub fn to_owned(&self) -> OwnedPropertyDeclarationId {
match self {
Expand Down Expand Up @@ -192,8 +200,7 @@ impl<'a> PropertyDeclarationId<'a> {
pub fn is_animatable(&self) -> bool {
match self {
PropertyDeclarationId::Longhand(id) => id.is_animatable(),
// TODO(bug 1846516): This should return true.
PropertyDeclarationId::Custom(_) => false,
PropertyDeclarationId::Custom(_) => true,
}
}

Expand All @@ -202,8 +209,8 @@ impl<'a> PropertyDeclarationId<'a> {
pub fn is_discrete_animatable(&self) -> bool {
match self {
Self::Longhand(longhand) => longhand.is_discrete_animatable(),
// TODO(bug 1846516): Implement this for custom properties.
Self::Custom(_) => false,
// TODO(bug 1846516): Refine this?
Self::Custom(_) => true,
}
}

Expand All @@ -217,4 +224,24 @@ impl<'a> PropertyDeclarationId<'a> {
PropertyDeclarationId::Custom(_) => nsCSSPropertyID::eCSSPropertyExtra_variable,
}
}

/// Convert a `PropertyDeclarationId` into an `AnimatedPropertyID`
#[cfg(feature = "gecko")]
#[inline]
pub fn to_gecko_animated_property_id(&self) -> AnimatedPropertyID {
match self {
Self::Longhand(id) => AnimatedPropertyID {
mID: id.to_nscsspropertyid(),
mCustomName: RefPtr::null(),
},
Self::Custom(name) => {
let mut property_id = AnimatedPropertyID {
mID: nsCSSPropertyID::eCSSPropertyExtra_variable,
mCustomName: RefPtr::null(),
};
property_id.mCustomName.mRawPtr = (*name).clone().into_addrefed();
property_id
},
}
}
}

0 comments on commit 1971ef5

Please sign in to comment.