Auto merge of #16770 - nox:POSITION-DO-YOU-EVEN, r=Wafflespeanut,nox

Refactor Position

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16770)
<!-- Reviewable:end -->

components/layout/display_list_builder.rs

@@ -1008,9 +1008,9 @@ impl FragmentDisplayListBuilding for Fragment {
             let horiz_position = *get_cyclic(&background.background_position_x.0, index);
             let vert_position = *get_cyclic(&background.background_position_y.0, index);
             // Use `background-position` to get the offset.
-            let horizontal_position = model::specified(horiz_position.0,
+            let horizontal_position = model::specified(horiz_position,
                                                        bounds.size.width - image_size.width);
-            let vertical_position = model::specified(vert_position.0,
+            let vertical_position = model::specified(vert_position,
                                                      bounds.size.height - image_size.height);
 
             // The anchor position for this background, based on both the background-attachment
@@ -1167,8 +1167,8 @@ impl FragmentDisplayListBuilding for Fragment {
                                repeating: bool,
                                style: &ServoComputedValues)
                                -> display_list::RadialGradient {
-        let center = Point2D::new(specified(center.horizontal.0, bounds.size.width),
-                                  specified(center.vertical.0, bounds.size.height));
+        let center = Point2D::new(specified(center.horizontal, bounds.size.width),
+                                  specified(center.vertical, bounds.size.height));
         let radius = match *shape {
             EndingShape::Circle(LengthOrKeyword::Length(length))
                 => Size2D::new(length, length),

components/style/gecko/conversions.rs

@@ -307,8 +307,8 @@ impl nsStyleImage {
                     },
                 }
                 unsafe {
-                    (*gecko_gradient).mBgPosX.set(position.horizontal.0);
-                    (*gecko_gradient).mBgPosY.set(position.vertical.0);
+                    (*gecko_gradient).mBgPosX.set(position.horizontal);
+                    (*gecko_gradient).mBgPosY.set(position.vertical);
                 }
 
                 gecko_gradient
@@ -372,7 +372,6 @@ pub mod basic_shape {
     use values::computed::position;
     use values::generics::BorderRadiusSize as GenericBorderRadiusSize;
     use values::generics::basic_shape::FillRule;
-    use values::generics::position::{HorizontalPosition, VerticalPosition};
 
     // using Borrow so that we can have a non-moving .into()
     impl<T: Borrow<StyleBasicShape>> From<T> for BasicShape {
@@ -483,8 +482,8 @@ pub mod basic_shape {
     impl From<position::Position> for structs::Position {
         fn from(other: position::Position) -> Self {
             structs::Position {
-                mXPosition: other.horizontal.0.into(),
-                mYPosition: other.vertical.0.into()
+                mXPosition: other.horizontal.into(),
+                mYPosition: other.vertical.into()
             }
         }
     }
@@ -501,8 +500,8 @@ pub mod basic_shape {
         fn from(other: T) -> Self {
             let other = other.borrow();
             position::Position {
-                horizontal: HorizontalPosition(other.mXPosition.into()),
-                vertical: VerticalPosition(other.mYPosition.into()),
+                horizontal: other.mXPosition.into(),
+                vertical: other.mYPosition.into(),
             }
         }
     }

components/style/properties/gecko.mako.rs

@@ -367,17 +367,16 @@ fn color_to_nscolor_zero_currentcolor(color: Color) -> structs::nscolor {
 <%def name="impl_position(ident, gecko_ffi_name, need_clone=False)">
     #[allow(non_snake_case)]
     pub fn set_${ident}(&mut self, v: longhands::${ident}::computed_value::T) {
-        ${set_gecko_property("%s.mXPosition" % gecko_ffi_name, "v.horizontal.0.into()")}
-        ${set_gecko_property("%s.mYPosition" % gecko_ffi_name, "v.vertical.0.into()")}
+        ${set_gecko_property("%s.mXPosition" % gecko_ffi_name, "v.horizontal.into()")}
+        ${set_gecko_property("%s.mYPosition" % gecko_ffi_name, "v.vertical.into()")}
     }
     <%call expr="impl_simple_copy(ident, gecko_ffi_name)"></%call>
     % if need_clone:
         #[allow(non_snake_case)]
         pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
-            use values::generics::position::{HorizontalPosition, Position, VerticalPosition};
-            Position {
-                horizontal: HorizontalPosition(self.gecko.${gecko_ffi_name}.mXPosition.into()),
-                vertical: VerticalPosition(self.gecko.${gecko_ffi_name}.mYPosition.into()),
+            longhands::${ident}::computed_value::T {
+                horizontal: self.gecko.${gecko_ffi_name}.mXPosition.into(),
+                vertical: self.gecko.${gecko_ffi_name}.mYPosition.into(),
             }
         }
     % endif
@@ -2040,8 +2039,8 @@ fn static_assert() {
         for (gecko, servo) in self.gecko.mScrollSnapCoordinate
                                .iter_mut()
                                .zip(v) {
-            gecko.mXPosition = servo.horizontal.0.into();
-            gecko.mYPosition = servo.vertical.0.into();
+            gecko.mXPosition = servo.horizontal.into();
+            gecko.mYPosition = servo.vertical.into();
         }
     }
 
@@ -2726,12 +2725,12 @@ fn static_assert() {
         }
     </%self:simple_image_array_property>
 
-    % for orientation in [("x", "Horizontal"), ("y", "Vertical")]:
-    pub fn copy_${shorthand}_position_${orientation[0]}_from(&mut self, other: &Self) {
+    % for orientation in ["x", "y"]:
+    pub fn copy_${shorthand}_position_${orientation}_from(&mut self, other: &Self) {
         use gecko_bindings::structs::nsStyleImageLayers_LayerType as LayerType;
 
-        self.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count
-            = cmp::min(1, other.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count);
+        self.gecko.${image_layers_field}.mPosition${orientation.upper()}Count
+            = cmp::min(1, other.gecko.${image_layers_field}.mPosition${orientation.upper()}Count);
         self.gecko.${image_layers_field}.mLayers.mFirstElement.mPosition =
             other.gecko.${image_layers_field}.mLayers.mFirstElement.mPosition;
         unsafe {
@@ -2742,20 +2741,19 @@ fn static_assert() {
 
         for (layer, other) in self.gecko.${image_layers_field}.mLayers.iter_mut()
                                   .zip(other.gecko.${image_layers_field}.mLayers.iter()) {
-            layer.mPosition.m${orientation[0].upper()}Position
-                = other.mPosition.m${orientation[0].upper()}Position;
+            layer.mPosition.m${orientation.upper()}Position
+                = other.mPosition.m${orientation.upper()}Position;
         }
-        self.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count
-               = other.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count;
+        self.gecko.${image_layers_field}.mPosition${orientation.upper()}Count
+               = other.gecko.${image_layers_field}.mPosition${orientation.upper()}Count;
     }
 
-    pub fn clone_${shorthand}_position_${orientation[0]}(&self)
-        -> longhands::${shorthand}_position_${orientation[0]}::computed_value::T {
-        use values::generics::position::${orientation[1]}Position;
-        longhands::${shorthand}_position_${orientation[0]}::computed_value::T(
+    pub fn clone_${shorthand}_position_${orientation}(&self)
+        -> longhands::${shorthand}_position_${orientation}::computed_value::T {
+        longhands::${shorthand}_position_${orientation}::computed_value::T(
             self.gecko.${image_layers_field}.mLayers.iter()
-                .take(self.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count as usize)
-                .map(|position| ${orientation[1]}Position(position.mPosition.m${orientation[0].upper()}Position.into()))
+                .take(self.gecko.${image_layers_field}.mPosition${orientation.upper()}Count as usize)
+                .map(|position| position.mPosition.m${orientation.upper()}Position.into())
                 .collect()
         )
     }
@@ -2778,7 +2776,7 @@ fn static_assert() {
         self.gecko.${image_layers_field}.mPosition${orientation[0].upper()}Count = v.len() as u32;
         for (servo, geckolayer) in v.zip(self.gecko.${image_layers_field}
                                                            .mLayers.iter_mut()) {
-            geckolayer.mPosition.m${orientation[0].upper()}Position = servo.0.into();
+            geckolayer.mPosition.m${orientation[0].upper()}Position = servo.into();
         }
     }
     % endfor

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

@@ -41,7 +41,6 @@ use values::computed::{Angle, LengthOrPercentageOrAuto, LengthOrPercentageOrNone
 use values::computed::{BorderRadiusSize, ClipRect};
 use values::computed::{CalcLengthOrPercentage, Context, LengthOrPercentage};
 use values::computed::{MaxLength, MinLength};
-use values::computed::position::{HorizontalPosition, VerticalPosition};
 use values::computed::ToComputedValue;
 use values::generics::position as generic_position;
 
@@ -655,6 +654,7 @@ pub trait Animatable: Sized {
 /// https://drafts.csswg.org/css-transitions/#animtype-repeatable-list
 pub trait RepeatableListAnimatable: Animatable {}
 
+impl RepeatableListAnimatable for LengthOrPercentage {}
 impl RepeatableListAnimatable for Either<f32, LengthOrPercentage> {}
 
 impl<T: RepeatableListAnimatable> Animatable for SmallVec<[T; 1]> {
@@ -1336,46 +1336,6 @@ impl<H: Animatable, V: Animatable> Animatable for generic_position::Position<H,
 impl<H, V> RepeatableListAnimatable for generic_position::Position<H, V>
     where H: RepeatableListAnimatable, V: RepeatableListAnimatable {}
 
-/// https://drafts.csswg.org/css-transitions/#animtype-simple-list
-impl Animatable for HorizontalPosition {
-    #[inline]
-    fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-        self.0.interpolate(&other.0, progress).map(generic_position::HorizontalPosition)
-    }
-
-    #[inline]
-    fn compute_distance(&self, other: &Self) -> Result<f64, ()> {
-        self.0.compute_distance(&other.0)
-    }
-
-    #[inline]
-    fn compute_squared_distance(&self, other: &Self) -> Result<f64, ()> {
-        self.0.compute_squared_distance(&other.0)
-    }
-}
-
-impl RepeatableListAnimatable for HorizontalPosition {}
-
-/// https://drafts.csswg.org/css-transitions/#animtype-simple-list
-impl Animatable for VerticalPosition {
-    #[inline]
-    fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-        self.0.interpolate(&other.0, progress).map(generic_position::VerticalPosition)
-    }
-
-    #[inline]
-    fn compute_distance(&self, other: &Self) -> Result<f64, ()> {
-        self.0.compute_distance(&other.0)
-    }
-
-    #[inline]
-    fn compute_squared_distance(&self, other: &Self) -> Result<f64, ()> {
-        self.0.compute_squared_distance(&other.0)
-    }
-}
-
-impl RepeatableListAnimatable for VerticalPosition {}
-
 /// https://drafts.csswg.org/css-transitions/#animtype-rect
 impl Animatable for ClipRect {
     #[inline]

components/style/properties/longhand/background.mako.rs

@@ -20,38 +20,13 @@ ${helpers.predefined_type("background-image", "LayerImage",
     animation_value_type="none",
     has_uncacheable_values="True" if product == "gecko" else "False")}
 
-<%helpers:predefined_type name="background-position-x" type="position::HorizontalPosition"
-                          initial_value="computed::position::HorizontalPosition::zero()"
-                          initial_specified_value="specified::position::HorizontalPosition::left()"
-                          spec="https://drafts.csswg.org/css-backgrounds-4/#propdef-background-position-x"
-                          animation_value_type="ComputedValue" vector="True" delegate_animate="True">
-    #[inline]
-    /// Get the initial value for horizontal position.
-    pub fn get_initial_position_value() -> SpecifiedValue {
-        use values::generics::position::{HorizontalPosition, PositionValue};
-        use values::specified::{LengthOrPercentage, Percentage};
-        HorizontalPosition(PositionValue {
-            keyword: None,
-            position: Some(LengthOrPercentage::Percentage(Percentage(0.0))),
-        })
-    }
-</%helpers:predefined_type>
-
-<%helpers:predefined_type name="background-position-y" type="position::VerticalPosition"
-                          initial_value="computed::position::VerticalPosition::zero()"
-                          initial_specified_value="specified::position::VerticalPosition::top()"
-                          spec="https://drafts.csswg.org/css-backgrounds-4/#propdef-background-position-y"
-                          animation_value_type="ComputedValue" vector="True" delegate_animate="True">
-    /// Get the initial value for vertical position.
-    pub fn get_initial_position_value() -> SpecifiedValue {
-        use values::generics::position::{VerticalPosition, PositionValue};
-        use values::specified::{LengthOrPercentage, Percentage};
-        VerticalPosition(PositionValue {
-            keyword: None,
-            position: Some(LengthOrPercentage::Percentage(Percentage(0.0))),
-        })
-    }
-</%helpers:predefined_type>
+% for (axis, direction, initial) in [("x", "Horizontal", "left"), ("y", "Vertical", "top")]:
+    ${helpers.predefined_type("background-position-" + axis, "position::" + direction + "Position",
+                              initial_value="computed::LengthOrPercentage::zero()",
+                              initial_specified_value="SpecifiedValue::initial_specified_value()",
+                              spec="https://drafts.csswg.org/css-backgrounds-4/#propdef-background-position-" + axis,
+                              animation_value_type="ComputedValue", vector=True, delegate_animate=True)}
+% endfor
 
 <%helpers:vector_longhand name="background-repeat" animation_value_type="none"
                           spec="https://drafts.csswg.org/css-backgrounds/#the-background-repeat">

components/style/properties/longhand/box.mako.rs

@@ -2143,11 +2143,11 @@ ${helpers.predefined_type("perspective",
                           animation_value_type="ComputedValue")}
 
 ${helpers.predefined_type("perspective-origin",
-                          "position::OriginPosition",
-                          "computed::position::OriginPosition::center()",
+                          "position::Position",
+                          "computed::position::Position::center()",
                           boxed="True",
                           extra_prefixes="moz webkit",
-                          spec="https://drafts.csswg.org/css-transforms/#perspective-origin-property",
+                          spec="https://drafts.csswg.org/css-transforms-2/#perspective-origin-property",
                           animation_value_type="ComputedValue")}
 
 ${helpers.single_keyword("backface-visibility",

components/style/properties/longhand/svg.mako.rs

@@ -90,49 +90,14 @@ ${helpers.single_keyword("mask-mode",
     }
 </%helpers:vector_longhand>
 
-<%helpers:vector_longhand name="mask-position-x" products="gecko"
-                          animation_value_type="ComputedValue" extra_prefixes="webkit"
-                          spec="https://drafts.fxtf.org/css-masking/#propdef-mask-position">
-    pub use properties::longhands::background_position_x::single_value::get_initial_value;
-    pub use properties::longhands::background_position_x::single_value::get_initial_position_value;
-    pub use properties::longhands::background_position_x::single_value::get_initial_specified_value;
-    pub use properties::longhands::background_position_x::single_value::parse;
-    pub use properties::longhands::background_position_x::single_value::SpecifiedValue;
-    pub use properties::longhands::background_position_x::single_value::computed_value;
-    use properties::animated_properties::{Animatable, RepeatableListAnimatable};
-    use properties::longhands::mask_position_x::computed_value::T as MaskPositionX;
-
-    impl Animatable for MaskPositionX {
-        #[inline]
-        fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-            Ok(MaskPositionX(try!(self.0.interpolate(&other.0, progress))))
-        }
-    }
-
-    impl RepeatableListAnimatable for MaskPositionX {}
-</%helpers:vector_longhand>
-
-<%helpers:vector_longhand name="mask-position-y" products="gecko"
-                          animation_value_type="ComputedValue" extra_prefixes="webkit"
-                          spec="https://drafts.fxtf.org/css-masking/#propdef-mask-position">
-    pub use properties::longhands::background_position_y::single_value::get_initial_value;
-    pub use properties::longhands::background_position_y::single_value::get_initial_position_value;
-    pub use properties::longhands::background_position_y::single_value::get_initial_specified_value;
-    pub use properties::longhands::background_position_y::single_value::parse;
-    pub use properties::longhands::background_position_y::single_value::SpecifiedValue;
-    pub use properties::longhands::background_position_y::single_value::computed_value;
-    use properties::animated_properties::{Animatable, RepeatableListAnimatable};
-    use properties::longhands::mask_position_y::computed_value::T as MaskPositionY;
-
-    impl Animatable for MaskPositionY {
-        #[inline]
-        fn interpolate(&self, other: &Self, progress: f64) -> Result<Self, ()> {
-            Ok(MaskPositionY(try!(self.0.interpolate(&other.0, progress))))
-        }
-    }
-
-    impl RepeatableListAnimatable for MaskPositionY {}
-</%helpers:vector_longhand>
+% for (axis, direction) in [("x", "Horizontal"), ("y", "Vertical")]:
+    ${helpers.predefined_type("mask-position-" + axis, "position::" + direction + "Position",
+                              products="gecko", extra_prefixes="webkit",
+                              initial_value="computed::LengthOrPercentage::zero()",
+                              initial_specified_value="specified::PositionComponent::Center",
+                              spec="https://drafts.fxtf.org/css-masking/#propdef-mask-position",
+                              animation_value_type="ComputedValue", vector=True, delegate_animate=True)}
+% endfor
 
 ${helpers.single_keyword("mask-clip",
                          "border-box content-box padding-box",