From 819ebc57104f2f2a705e91c2d727b04918f6f595 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Sat, 12 Aug 2023 00:10:23 +0200 Subject: [PATCH] style: Introduce Optional to represent optional values in the style system cross-fade() was kinda doing this in its own way with PercentOrNone, but since now we have more use-cases for this we should probably make this a slightly more general solution. I added some convenience APIs, but they're unused as of this patch so let me know if you want them gone. Differential Revision: https://phabricator.services.mozilla.com/D144831 --- components/style/values/computed/image.rs | 2 - components/style/values/generics/image.rs | 17 +----- components/style/values/generics/mod.rs | 71 ++++++++++++++++++++++ components/style/values/specified/image.rs | 39 ++++++------ 4 files changed, 90 insertions(+), 39 deletions(-) diff --git a/components/style/values/computed/image.rs b/components/style/values/computed/image.rs index 980017b2822..49145b0bfd8 100644 --- a/components/style/values/computed/image.rs +++ b/components/style/values/computed/image.rs @@ -47,8 +47,6 @@ pub type Gradient = generic::GenericGradient< /// Computed values for CSS cross-fade /// pub type CrossFade = generic::CrossFade; -/// A computed percentage or nothing. -pub type PercentOrNone = generic::PercentOrNone; /// A computed radial gradient ending shape. pub type EndingShape = generic::GenericEndingShape; diff --git a/components/style/values/generics/image.rs b/components/style/values/generics/image.rs index dd504a07d24..d5e830810a8 100644 --- a/components/style/values/generics/image.rs +++ b/components/style/values/generics/image.rs @@ -8,6 +8,7 @@ use crate::custom_properties; use crate::values::generics::position::PositionComponent; +use crate::values::generics::Optional; use crate::values::serialize_atom_identifier; use crate::Atom; use crate::Zero; @@ -71,20 +72,6 @@ pub struct GenericCrossFade { pub elements: crate::OwnedSlice>, } -/// A ` | none` value. Represents optional percentage values -/// assosicated with cross-fade images. -#[derive( - Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem, ToCss, -)] -#[repr(C, u8)] -pub enum PercentOrNone { - /// `none` variant. - #[css(skip)] - None, - /// A percentage variant. - Percent(Percentage), -} - /// An optional percent and a cross fade image. #[derive( Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem, ToCss, @@ -92,7 +79,7 @@ pub enum PercentOrNone { #[repr(C)] pub struct GenericCrossFadeElement { /// The percent of the final image that `image` will be. - pub percent: PercentOrNone, + pub percent: Optional, /// A color or image that will be blended when cross-fade is /// evaluated. pub image: GenericCrossFadeImage, diff --git a/components/style/values/generics/mod.rs b/components/style/values/generics/mod.rs index b15d4f01887..8c10085af18 100644 --- a/components/style/values/generics/mod.rs +++ b/components/style/values/generics/mod.rs @@ -310,3 +310,74 @@ impl ClipRectOrAuto { } pub use page::PageSize; + +/// An optional value, much like `Option`, but with a defined struct layout +/// to be able to use it from C++ as well. +/// +/// Note that this is relatively inefficient, struct-layout-wise, as you have +/// one byte for the tag, but padding to the alignment of T. If you have +/// multiple optional values and care about struct compactness, you might be +/// better off "coalescing" the combinations into a parent enum. But that +/// shouldn't matter for most use cases. +#[allow(missing_docs)] +#[derive( + Animate, + Clone, + ComputeSquaredDistance, + Copy, + Debug, + MallocSizeOf, + Parse, + PartialEq, + SpecifiedValueInfo, + ToAnimatedValue, + ToAnimatedZero, + ToComputedValue, + ToCss, + ToResolvedValue, + ToShmem, +)] +#[repr(C, u8)] +pub enum Optional { + #[css(skip)] + None, + Some(T), +} + +impl Optional { + /// Returns whether this value is present. + pub fn is_some(&self) -> bool { + matches!(*self, Self::Some(..)) + } + + /// Returns whether this value is not present. + pub fn is_none(&self) -> bool { + matches!(*self, Self::None) + } + + /// Turns this Optional<> into a regular rust Option<>. + pub fn into_rust(self) -> Option { + match self { + Self::Some(v) => Some(v), + Self::None => None, + } + } + + /// Return a reference to the containing value, if any, as a plain rust + /// Option<>. + pub fn as_ref(&self) -> Option<&T> { + match *self { + Self::Some(ref v) => Some(v), + Self::None => None, + } + } +} + +impl From> for Optional { + fn from(rust: Option) -> Self { + match rust { + Some(t) => Self::Some(t), + None => Self::None, + } + } +} diff --git a/components/style/values/specified/image.rs b/components/style/values/specified/image.rs index f4cf417a044..ec516b24e72 100644 --- a/components/style/values/specified/image.rs +++ b/components/style/values/specified/image.rs @@ -60,8 +60,6 @@ pub type CrossFade = generic::CrossFade; pub type CrossFadeElement = generic::CrossFadeElement; /// CrossFadeImage = image | color pub type CrossFadeImage = generic::CrossFadeImage; -/// A specified percentage or nothing. -pub type PercentOrNone = generic::PercentOrNone; /// `image-set()` pub type ImageSet = generic::ImageSet; @@ -315,6 +313,16 @@ impl CrossFade { } impl CrossFadeElement { + fn parse_percentage<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Option { + // We clamp our values here as this is the way that Safari and Chrome's + // implementation handle out-of-bounds percentages but whether or not + // this behavior follows the specification is still being discussed. + // See: + input.try_parse(|input| Percentage::parse_non_negative(context, input)) + .ok() + .map(|p| p.clamp_to_hundred()) + } + /// = ? && [ | ] fn parse<'i, 't>( context: &ParserContext, @@ -322,14 +330,17 @@ impl CrossFadeElement { cors_mode: CorsMode, ) -> Result> { // Try and parse a leading percent sign. - let mut percent = PercentOrNone::parse_or_none(context, input); + let mut percent = Self::parse_percentage(context, input); // Parse the image let image = CrossFadeImage::parse(context, input, cors_mode)?; // Try and parse a trailing percent sign. - if percent == PercentOrNone::None { - percent = PercentOrNone::parse_or_none(context, input); + if percent.is_none() { + percent = Self::parse_percentage(context, input); } - Ok(Self { percent, image }) + Ok(Self { + percent: percent.into(), + image, + }) } } @@ -351,22 +362,6 @@ impl CrossFadeImage { } } -impl PercentOrNone { - fn parse_or_none<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Self { - // We clamp our values here as this is the way that Safari and - // Chrome's implementation handle out-of-bounds percentages - // but whether or not this behavior follows the specification - // is still being discussed. See: - // - if let Ok(percent) = input.try_parse(|input| Percentage::parse_non_negative(context, input)) - { - Self::Percent(percent.clamp_to_hundred()) - } else { - Self::None - } - } -} - impl ImageSet { fn parse<'i, 't>( context: &ParserContext,