servo: Merge #16937 - Fix calc() clamping issues (from nox:clamp-calc); r=emilio

Fix calc() clamping issues

Source-Repo: https://github.com/servo/servo
Source-Revision: b4204b5e2d8b2981408e2c092130af40ad3170bb

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : ea4587dcbce7b4bcb1d1977a734ee0d012fdb13a

servo/components/layout/display_list_builder.rs

@@ -2803,12 +2803,13 @@ struct StopRun {
     stop_count: usize,
 }
 
-fn position_to_offset(position: LengthOrPercentage, Au(total_length): Au) -> f32 {
+fn position_to_offset(position: LengthOrPercentage, total_length: Au) -> f32 {
     match position {
-        LengthOrPercentage::Length(Au(length)) => length as f32 / total_length as f32,
+        LengthOrPercentage::Length(Au(length)) => length as f32 / total_length.0 as f32,
         LengthOrPercentage::Percentage(percentage) => percentage as f32,
-        LengthOrPercentage::Calc(calc) =>
-            calc.percentage() + (calc.length().0 as f32) / (total_length as f32),
+        LengthOrPercentage::Calc(calc) => {
+            calc.to_used_value(Some(total_length)).unwrap().0 as f32 / total_length.0 as f32
+        },
     }
 }
 

servo/components/layout/fragment.rs

@@ -1504,7 +1504,11 @@ impl Fragment {
                         result_inline
                     }
                     LengthOrPercentageOrAuto::Length(length) => length,
-                    LengthOrPercentageOrAuto::Calc(calc) => calc.length(),
+                    LengthOrPercentageOrAuto::Calc(calc) => {
+                        // TODO(nox): This is probably wrong, because it accounts neither for
+                        // clamping (not sure if necessary here) nor percentage.
+                        calc.unclamped_length()
+                    },
                 };
 
                 let size_constraint = self.size_constraint(None, Direction::Inline);
@@ -2233,8 +2237,7 @@ impl Fragment {
                     offset -= minimum_line_metrics.space_needed().scale_by(percentage)
                 }
                 vertical_align::T::LengthOrPercentage(LengthOrPercentage::Calc(formula)) => {
-                    offset -= minimum_line_metrics.space_needed().scale_by(formula.percentage()) +
-                        formula.length()
+                    offset -= formula.to_used_value(Some(minimum_line_metrics.space_needed())).unwrap()
                 }
             }
         }

servo/components/style/gecko/conversions.rs

@@ -24,7 +24,7 @@ impl From<CalcLengthOrPercentage> for nsStyleCoord_CalcValue {
     fn from(other: CalcLengthOrPercentage) -> nsStyleCoord_CalcValue {
         let has_percentage = other.percentage.is_some();
         nsStyleCoord_CalcValue {
-            mLength: other.length().0,
+            mLength: other.unclamped_length().0,
             mPercent: other.percentage.unwrap_or(0.0),
             mHasPercent: has_percentage,
         }

servo/components/style/properties/helpers/animated_properties.mako.rs

@@ -1035,7 +1035,7 @@ impl Animatable for CalcLengthOrPercentage {
             }
         }
 
-        let length = self.length().add_weighted(&other.length(), self_portion, other_portion)?;
+        let length = self.unclamped_length().add_weighted(&other.unclamped_length(), self_portion, other_portion)?;
         let percentage = add_weighted_half(self.percentage, other.percentage, self_portion, other_portion)?;
         Ok(CalcLengthOrPercentage::with_clamping_mode(length, percentage, self.clamping_mode))
     }
@@ -1047,7 +1047,7 @@ impl Animatable for CalcLengthOrPercentage {
 
     #[inline]
     fn compute_squared_distance(&self, other: &Self) -> Result<f64, ()> {
-        let length_diff = (self.length().0 - other.length().0) as f64;
+        let length_diff = (self.unclamped_length().0 - other.unclamped_length().0) as f64;
         let percentage_diff = (self.percentage() - other.percentage()) as f64;
         Ok(length_diff * length_diff + percentage_diff * percentage_diff)
     }
@@ -1112,7 +1112,7 @@ impl Animatable for LengthOrPercentage {
             (this, other) => {
                 let this: CalcLengthOrPercentage = From::from(this);
                 let other: CalcLengthOrPercentage = From::from(other);
-                let length_diff = (this.length().0 - other.length().0) as f64;
+                let length_diff = (this.unclamped_length().0 - other.unclamped_length().0) as f64;
                 let percentage_diff = (this.percentage() - other.percentage()) as f64;
                 Ok(length_diff * length_diff + percentage_diff * percentage_diff)
             }
@@ -1185,12 +1185,12 @@ impl Animatable for LengthOrPercentageOrAuto {
             (this, other) => {
                 let this: Option<CalcLengthOrPercentage> = From::from(this);
                 let other: Option<CalcLengthOrPercentage> = From::from(other);
-                if this.is_none() || other.is_none() {
-                    Err(())
-                } else {
-                    let length_diff = (this.unwrap().length().0 - other.unwrap().length().0) as f64;
-                    let percentage_diff = (this.unwrap().percentage() - other.unwrap().percentage()) as f64;
+                if let (Some(this), Some(other)) = (this, other) {
+                    let length_diff = (this.unclamped_length().0 - other.unclamped_length().0) as f64;
+                    let percentage_diff = (this.percentage() - other.percentage()) as f64;
                     Ok(length_diff * length_diff + percentage_diff * percentage_diff)
+                } else {
+                    Err(())
                 }
             }
         }

servo/components/style/properties/longhand/font.mako.rs

@@ -800,8 +800,7 @@ ${helpers.single_keyword_system("font-variant-caps",
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Calc(ref calc)) => {
                     let calc = calc.to_computed_value(context);
-                    calc.length() + base_size.resolve(context)
-                                             .scale_by(calc.percentage())
+                    calc.to_used_value(Some(base_size.resolve(context))).unwrap()
                 }
                 SpecifiedValue::Keyword(ref key, fraction) => {
                     key.to_computed_value(context).scale_by(fraction)

servo/components/style/properties/longhand/inherited_text.mako.rs

@@ -132,7 +132,8 @@
                             let calc = calc.to_computed_value(context);
                             let fr = specified::FontRelativeLength::Em(calc.percentage());
                             let fr = specified::Length::NoCalc(specified::NoCalcLength::FontRelative(fr));
-                            computed_value::T::Length(calc.length() + fr.to_computed_value(context))
+                            let length = calc.unclamped_length();
+                            computed_value::T::Length(calc.clamping_mode.clamp(length + fr.to_computed_value(context)))
                         }
                     }
                 }

servo/components/style/values/computed/length.rs

@@ -88,12 +88,21 @@ impl CalcLengthOrPercentage {
         }
     }
 
+    /// Returns this `calc()` as a `<length>`.
+    ///
+    /// Panics in debug mode if a percentage is present in the expression.
     #[inline]
-    #[allow(missing_docs)]
     pub fn length(&self) -> Au {
+        debug_assert!(self.percentage.is_none());
         self.clamping_mode.clamp(self.length)
     }
 
+    /// Returns the `<length>` component of this `calc()`, unclamped.
+    #[inline]
+    pub fn unclamped_length(&self) -> Au {
+        self.length
+    }
+
     #[inline]
     #[allow(missing_docs)]
     pub fn percentage(&self) -> CSSFloat {
@@ -245,7 +254,7 @@ impl LengthOrPercentage {
         match *self {
             Length(l) => (l, NotNaN::new(0.0).unwrap()),
             Percentage(p) => (Au(0), NotNaN::new(p).unwrap()),
-            Calc(c) => (c.length(), NotNaN::new(c.percentage()).unwrap()),
+            Calc(c) => (c.unclamped_length(), NotNaN::new(c.percentage()).unwrap()),
         }
     }
 

servo/components/style/values/specified/position.rs

@@ -241,7 +241,7 @@ impl<S: Side> ToComputedValue for PositionComponent<S> {
                     },
                     ComputedLengthOrPercentage::Calc(calc) => {
                         let p = 1. - calc.percentage.unwrap_or(0.);
-                        ComputedLengthOrPercentage::Calc(CalcLengthOrPercentage::new(-calc.length(), Some(p)))
+                        ComputedLengthOrPercentage::Calc(CalcLengthOrPercentage::new(-calc.unclamped_length(), Some(p)))
                     },
                 }
             },

servo/components/style/viewport.rs

@@ -718,8 +718,7 @@ impl MaybeNew for ViewportConstraints {
                                 Some(initial_viewport.$dimension.scale_by(value.0)),
                             LengthOrPercentageOrAuto::Auto => None,
                             LengthOrPercentageOrAuto::Calc(ref calc) => {
-                                let calc = calc.to_computed_value(&context);
-                                Some(initial_viewport.$dimension.scale_by(calc.percentage()) + calc.length())
+                                calc.to_computed_value(&context).to_used_value(Some(initial_viewport.$dimension))
                             }
                         },
                         ViewportLength::ExtendToZoom => {