From 05881b5ab4bf6f18d09763978ee5924a5b0dd895 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 18 Jan 2019 19:32:27 +0000 Subject: [PATCH] style: Represent the percentage in LengthPercentage with something other than an option. Not the prettiest, but it will work, and LengthPercentage will be 12 bytes which is pretty good (we could do better if wanted I guess): * Au(i32) length; * f32 percentage; * AllowedNumericType(u8) clamping_mode; * bool has_percentage; * bool was_calc; This will allow me to start moving C++ stuff to use this representation. Differential Revision: https://phabricator.services.mozilla.com/D16929 --- components/style/cbindgen.toml | 1 + components/style/gecko/conversions.rs | 7 +- components/style/gecko/values.rs | 6 +- .../gecko_bindings/sugar/ns_css_value.rs | 6 +- components/style/values/animated/length.rs | 7 +- components/style/values/animated/svg.rs | 18 +++-- components/style/values/computed/length.rs | 75 +++++++++++-------- 7 files changed, 68 insertions(+), 52 deletions(-) diff --git a/components/style/cbindgen.toml b/components/style/cbindgen.toml index fecb0b2dc3f..25004b2def6 100644 --- a/components/style/cbindgen.toml +++ b/components/style/cbindgen.toml @@ -68,5 +68,6 @@ include = [ "OverflowClipBox", "Resize", "Overflow", + "LengthPercentage", ] item_types = ["enums", "structs", "typedefs"] diff --git a/components/style/gecko/conversions.rs b/components/style/gecko/conversions.rs index 345e199a9a6..365e53f6251 100644 --- a/components/style/gecko/conversions.rs +++ b/components/style/gecko/conversions.rs @@ -37,14 +37,13 @@ impl From for nsStyleCoord_CalcValue { fn from(other: LengthPercentage) -> nsStyleCoord_CalcValue { debug_assert!( other.was_calc || - other.percentage.is_none() || + !other.has_percentage || other.unclamped_length() == Length::zero() ); - let has_percentage = other.percentage.is_some(); nsStyleCoord_CalcValue { mLength: other.unclamped_length().to_i32_au(), - mPercent: other.percentage.map_or(0., |p| p.0), - mHasPercent: has_percentage, + mPercent: other.percentage(), + mHasPercent: other.has_percentage, } } } diff --git a/components/style/gecko/values.rs b/components/style/gecko/values.rs index 2288db104a4..42177bf0858 100644 --- a/components/style/gecko/values.rs +++ b/components/style/gecko/values.rs @@ -151,9 +151,9 @@ impl GeckoStyleCoordConvertible for LengthPercentage { if self.was_calc { return coord.set_value(CoordDataValue::Calc((*self).into())); } - debug_assert!(self.percentage.is_none() || self.unclamped_length() == Length::zero()); - if let Some(p) = self.percentage { - return coord.set_value(CoordDataValue::Percent(p.0)); + debug_assert!(!self.has_percentage || self.unclamped_length() == Length::zero()); + if self.has_percentage { + return coord.set_value(CoordDataValue::Percent(self.percentage())); } coord.set_value(CoordDataValue::Coord(self.unclamped_length().to_i32_au())) } diff --git a/components/style/gecko_bindings/sugar/ns_css_value.rs b/components/style/gecko_bindings/sugar/ns_css_value.rs index 73657af3e35..ef971c669ad 100644 --- a/components/style/gecko_bindings/sugar/ns_css_value.rs +++ b/components/style/gecko_bindings/sugar/ns_css_value.rs @@ -72,9 +72,9 @@ impl nsCSSValue { if lp.was_calc { return bindings::Gecko_CSSValue_SetCalc(self, lp.into()); } - debug_assert!(lp.percentage.is_none() || lp.unclamped_length() == Length::zero()); - if let Some(p) = lp.percentage { - return self.set_percentage(p.0); + debug_assert!(!lp.has_percentage || lp.unclamped_length() == Length::zero()); + if lp.has_percentage { + return self.set_percentage(lp.percentage()); } self.set_px(lp.unclamped_length().px()); } diff --git a/components/style/values/animated/length.rs b/components/style/values/animated/length.rs index 672c4117b00..569d9888862 100644 --- a/components/style/values/animated/length.rs +++ b/components/style/values/animated/length.rs @@ -26,10 +26,13 @@ impl Animate for LengthPercentage { let length = self .unclamped_length() .animate(&other.unclamped_length(), procedure)?; - let percentage = animate_percentage_half(self.percentage, other.percentage)?; + let percentage = animate_percentage_half( + self.specified_percentage(), + other.specified_percentage(), + )?; let is_calc = self.was_calc || other.was_calc || - self.percentage.is_some() != other.percentage.is_some(); + self.has_percentage != other.has_percentage; Ok(Self::with_clamping_mode( length, percentage, diff --git a/components/style/values/animated/svg.rs b/components/style/values/animated/svg.rs index af627bba784..947e4135ae3 100644 --- a/components/style/values/animated/svg.rs +++ b/components/style/values/animated/svg.rs @@ -32,14 +32,16 @@ fn to_number_or_percentage( value: &SvgLengthPercentageOrNumber, ) -> Result { Ok(match *value { - SvgLengthPercentageOrNumber::LengthPercentage(ref l) => match l.percentage { - Some(p) => { - if l.unclamped_length().px() != 0. { - return Err(()); - } - NumberOrPercentage::Percentage(p) - }, - None => NumberOrPercentage::Number(l.length().px()), + SvgLengthPercentageOrNumber::LengthPercentage(ref l) => { + match l.specified_percentage() { + Some(p) => { + if l.unclamped_length().px() != 0. { + return Err(()); + } + NumberOrPercentage::Percentage(p) + }, + None => NumberOrPercentage::Number(l.length().px()), + } }, SvgLengthPercentageOrNumber::Number(ref n) => NumberOrPercentage::Number(*n), }) diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs index daf14ea19e3..012c2e076f2 100644 --- a/components/style/values/computed/length.rs +++ b/components/style/values/computed/length.rs @@ -77,7 +77,10 @@ pub struct LengthPercentage { #[animation(constant)] pub clamping_mode: AllowedNumericType, length: Length, - pub percentage: Option, + percentage: Percentage, + /// Whether we specified a percentage or not. + #[animation(constant)] + pub has_percentage: bool, /// Whether this was from a calc() expression. This is needed because right /// now we don't treat calc() the same way as non-calc everywhere, but /// that's a bug in most cases. @@ -99,7 +102,8 @@ pub struct LengthPercentage { // like calc(0px + 5%) and such. impl PartialEq for LengthPercentage { fn eq(&self, other: &Self) -> bool { - self.length == other.length && self.percentage == other.percentage + self.length == other.length && self.percentage == other.percentage && + self.has_percentage == other.has_percentage } } @@ -111,8 +115,7 @@ impl ComputeSquaredDistance for LengthPercentage { Ok(self .unclamped_length() .compute_squared_distance(&other.unclamped_length())? + - self.percentage() - .compute_squared_distance(&other.percentage())?) + self.percentage.compute_squared_distance(&other.percentage)?) } } @@ -144,7 +147,8 @@ impl LengthPercentage { Self { clamping_mode, length, - percentage, + percentage: percentage.unwrap_or_default(), + has_percentage: percentage.is_some(), was_calc, } } @@ -154,7 +158,7 @@ impl LengthPercentage { /// Panics in debug mode if a percentage is present in the expression. #[inline] pub fn length(&self) -> CSSPixelLength { - debug_assert!(self.percentage.is_none()); + debug_assert!(!self.has_percentage); self.length_component() } @@ -173,22 +177,32 @@ impl LengthPercentage { /// Return the percentage value as CSSFloat. #[inline] pub fn percentage(&self) -> CSSFloat { - self.percentage.map_or(0., |p| p.0) + self.percentage.0 + } + + /// Return the specified percentage if any. + #[inline] + pub fn specified_percentage(&self) -> Option { + if self.has_percentage { + Some(self.percentage) + } else { + None + } } /// Returns the percentage component if this could be represented as a /// non-calc percentage. pub fn as_percentage(&self) -> Option { - if self.length.px() != 0. { + if !self.has_percentage || self.length.px() != 0. { return None; } - let p = self.percentage?; - if self.clamping_mode.clamp(p.0) != p.0 { + if self.clamping_mode.clamp(self.percentage.0) != self.percentage.0 { + debug_assert!(self.was_calc); return None; } - Some(p) + Some(self.percentage) } /// Convert the computed value into used value. @@ -201,14 +215,11 @@ impl LengthPercentage { /// the height property), they apply whenever a calc() expression contains /// percentages. pub fn maybe_to_pixel_length(&self, container_len: Option) -> Option { - match (container_len, self.percentage) { - (Some(len), Some(percent)) => { - let pixel = self.length.px() + len.scale_by(percent.0).to_f32_px(); - Some(Length::new(self.clamping_mode.clamp(pixel))) - }, - (_, None) => Some(self.length()), - _ => None, + if self.has_percentage { + let length = self.unclamped_length().px() + container_len?.scale_by(self.percentage.0).to_f32_px(); + return Some(Length::new(self.clamping_mode.clamp(length))); } + Some(self.length()) } } @@ -262,12 +273,12 @@ impl specified::CalcLengthPercentage { } } - LengthPercentage { - clamping_mode: self.clamping_mode, - length: Length::new(length.min(f32::MAX).max(f32::MIN)), - percentage: self.percentage, - was_calc: true, - } + LengthPercentage::with_clamping_mode( + Length::new(length.min(f32::MAX).max(f32::MIN)), + self.percentage, + self.clamping_mode, + /* was_calc = */ true, + ) } /// Compute font-size or line-height taking into account text-zoom if necessary. @@ -322,7 +333,7 @@ impl ToComputedValue for specified::CalcLengthPercentage { specified::CalcLengthPercentage { clamping_mode: computed.clamping_mode, absolute: Some(AbsoluteLength::from_computed_value(&computed.length)), - percentage: computed.percentage, + percentage: computed.specified_percentage(), ..Default::default() } } @@ -344,7 +355,7 @@ impl LengthPercentage { /// Returns true if the computed value is absolute 0 or 0%. #[inline] pub fn is_definitely_zero(&self) -> bool { - self.unclamped_length().px() == 0.0 && self.percentage.map_or(true, |p| p.0 == 0.0) + self.unclamped_length().px() == 0.0 && self.percentage.0 == 0.0 } // CSSFloat doesn't implement Hash, so does CSSPixelLength. Therefore, we still use Au as the @@ -353,7 +364,7 @@ impl LengthPercentage { pub fn to_hash_key(&self) -> (Au, NotNan) { ( Au::from(self.unclamped_length()), - NotNan::new(self.percentage()).unwrap(), + NotNan::new(self.percentage.0).unwrap(), ) } @@ -376,14 +387,14 @@ impl LengthPercentage { if self.was_calc { return Self::with_clamping_mode( self.length, - self.percentage, + self.specified_percentage(), AllowedNumericType::NonNegative, self.was_calc, ); } - debug_assert!(self.percentage.is_none() || self.unclamped_length() == Length::zero()); - if let Some(p) = self.percentage { + debug_assert!(!self.has_percentage || self.unclamped_length() == Length::zero()); + if let Some(p) = self.specified_percentage() { return Self::with_clamping_mode( Length::zero(), Some(p.clamp_to_non_negative()), @@ -420,8 +431,8 @@ impl ToComputedValue for specified::LengthPercentage { return specified::LengthPercentage::Percentage(p); } - let percentage = computed.percentage; - if percentage.is_none() && computed.clamping_mode.clamp(length.px()) == length.px() { + if !computed.has_percentage && + computed.clamping_mode.clamp(length.px()) == length.px() { return specified::LengthPercentage::Length(ToComputedValue::from_computed_value( &length, ));