servo: Merge #13902 - Prefer auto-generation for some keyword props (from Wafflespeanut:keyword); r=emilio
authorRavi Shankar <wafflespeanut@gmail.com>
Wed, 26 Oct 2016 02:27:13 -0500
changeset 339990 c6aa23dda265e80a79979cafd8fe0bb98fbf101d
parent 339989 93271bea02075acf5fda2c65cf8d6c576fa3de69
child 339991 86e6ad90db5558310ed7c7b03cb954202a89dc81
push id31307
push usergszorc@mozilla.com
push dateSat, 04 Feb 2017 00:59:06 +0000
treeherdermozilla-central@94079d43835f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
servo: Merge #13902 - Prefer auto-generation for some keyword props (from Wafflespeanut:keyword); r=emilio <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [x] `./mach build-geckolib` does not report any errors <!-- Either: --> - [x] These changes do not require tests because it's a refactor <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> r? @Manishearth or @emilio Source-Repo: https://github.com/servo/servo Source-Revision: 3c16dde1f268ffab3e2b220fda7e5be299d005b4
servo/components/gfx/paint_context.rs
servo/components/layout/webrender_helpers.rs
servo/components/style/properties/data.py
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/properties/longhand/inherited_box.mako.rs
--- a/servo/components/gfx/paint_context.rs
+++ b/servo/components/gfx/paint_context.rs
@@ -218,17 +218,17 @@ impl<'a> PaintContext<'a> {
                                                 image_info.height as AzFloat));
         let dest_rect = self.to_nearest_azure_rect(bounds);
 
         // TODO(pcwalton): According to CSS-IMAGES-3 ยง 5.3, nearest-neighbor interpolation is a
         // conforming implementation of `crisp-edges`, but it is not the best we could do.
         // Something like Scale2x would be ideal.
         let draw_surface_filter = match image_rendering {
             image_rendering::T::auto => Filter::Linear,
-            image_rendering::T::crispedges | image_rendering::T::pixelated => Filter::Point,
+            image_rendering::T::crisp_edges | image_rendering::T::pixelated => Filter::Point,
         };
 
         let draw_surface_options = DrawSurfaceOptions::new(draw_surface_filter, true);
         let draw_options = DrawOptions::new(1.0, CompositionOp::Over, AntialiasMode::None);
 
         // Fast path: No need to create a pattern.
         if bounds.size == *stretch_size && *tile_spacing == Size2D::zero() {
             draw_target_ref.draw_surface(azure_surface,
--- a/servo/components/layout/webrender_helpers.rs
+++ b/servo/components/layout/webrender_helpers.rs
@@ -209,17 +209,17 @@ impl ToBlendMode for mix_blend_mode::T {
 
 trait ToImageRendering {
     fn to_image_rendering(&self) -> webrender_traits::ImageRendering;
 }
 
 impl ToImageRendering for image_rendering::T {
     fn to_image_rendering(&self) -> webrender_traits::ImageRendering {
         match *self {
-            image_rendering::T::crispedges => webrender_traits::ImageRendering::CrispEdges,
+            image_rendering::T::crisp_edges => webrender_traits::ImageRendering::CrispEdges,
             image_rendering::T::auto => webrender_traits::ImageRendering::Auto,
             image_rendering::T::pixelated => webrender_traits::ImageRendering::Pixelated,
         }
     }
 }
 
 trait ToFilterOps {
     fn to_filter_ops(&self) -> Vec<webrender_traits::FilterOp>;
--- a/servo/components/style/properties/data.py
+++ b/servo/components/style/properties/data.py
@@ -13,76 +13,81 @@ def to_rust_ident(name):
 
 
 def to_camel_case(ident):
     return re.sub("(^|_|-)([a-z])", lambda m: m.group(2).upper(), ident.strip("_").strip("-"))
 
 
 class Keyword(object):
     def __init__(self, name, values, gecko_constant_prefix=None,
-                 gecko_enum_prefix=None,
+                 gecko_enum_prefix=None, custom_consts=None,
                  extra_gecko_values=None, extra_servo_values=None):
         self.name = name
         self.values = values.split()
         if gecko_constant_prefix and gecko_enum_prefix:
             raise TypeError("Only one of gecko_constant_prefix and gecko_enum_prefix can be specified")
         self.gecko_constant_prefix = gecko_constant_prefix or \
             "NS_STYLE_" + self.name.upper().replace("-", "_")
         self.gecko_enum_prefix = gecko_enum_prefix
         self.extra_gecko_values = (extra_gecko_values or "").split()
         self.extra_servo_values = (extra_servo_values or "").split()
+        self.consts_map = {} if custom_consts is None else custom_consts
 
     def gecko_values(self):
         return self.values + self.extra_gecko_values
 
     def servo_values(self):
         return self.values + self.extra_servo_values
 
     def values_for(self, product):
         if product == "gecko":
             return self.gecko_values()
         elif product == "servo":
             return self.servo_values()
         else:
             raise Exception("Bad product: " + product)
 
     def gecko_constant(self, value):
+        moz_stripped = value.replace("-moz-", '')
+        parts = moz_stripped.split('-')
         if self.gecko_enum_prefix:
-            parts = value.replace("-moz-", "").split("-")
             parts = [p.title() for p in parts]
             return self.gecko_enum_prefix + "::" + "".join(parts)
         else:
-            return self.gecko_constant_prefix + "_" + value.replace("-moz-", "").replace("-", "_").upper()
+            mapped = self.consts_map.get(value)
+            suffix = mapped if mapped else moz_stripped.replace("-", "_")
+            return self.gecko_constant_prefix + "_" + suffix.upper()
 
     def needs_cast(self):
         return self.gecko_enum_prefix is None
 
     def maybe_cast(self, type_str):
         return "as " + type_str if self.needs_cast() else ""
 
 
 class Longhand(object):
     def __init__(self, style_struct, name, animatable=None, derived_from=None, keyword=None,
                  predefined_type=None, custom_cascade=False, experimental=False, internal=False,
                  need_clone=False, need_index=False, gecko_ffi_name=None, depend_on_viewport_size=False,
-                 allowed_in_keyframe_block=True, complex_color=False):
+                 allowed_in_keyframe_block=True, complex_color=False, cast_type='u8'):
         self.name = name
         self.keyword = keyword
         self.predefined_type = predefined_type
         self.ident = to_rust_ident(name)
         self.camel_case = to_camel_case(self.ident)
         self.style_struct = style_struct
         self.experimental = ("layout.%s.enabled" % name) if experimental else None
         self.custom_cascade = custom_cascade
         self.internal = internal
         self.need_index = need_index
         self.gecko_ffi_name = gecko_ffi_name or "m" + self.camel_case
         self.depend_on_viewport_size = depend_on_viewport_size
         self.derived_from = (derived_from or "").split()
         self.complex_color = complex_color
+        self.cast_type = cast_type
 
         # https://drafts.csswg.org/css-animations/#keyframes
         # > The <declaration-list> inside of <keyframe-block> accepts any CSS property
         # > except those defined in this specification,
         # > but does accept the `animation-play-state` property and interprets it specially.
         self.allowed_in_keyframe_block = allowed_in_keyframe_block \
             and allowed_in_keyframe_block != "False"
 
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -485,63 +485,70 @@ impl Debug for ${style_struct.gecko_stru
         "LengthOrPercentage": impl_style_coord,
         "LengthOrPercentageOrAuto": impl_style_coord,
         "LengthOrPercentageOrNone": impl_style_coord,
         "Number": impl_simple,
         "Opacity": impl_simple,
         "CSSColor": impl_color,
     }
 
-    def predefined_type_method(longhand):
+    def longhand_method(longhand):
         args = dict(ident=longhand.ident, gecko_ffi_name=longhand.gecko_ffi_name,
                     need_clone=longhand.need_clone)
-        method = predefined_types[longhand.predefined_type]
 
-        # additional type-specific arguments
-        if longhand.predefined_type in ["CSSColor"]:
-            args.update(complex_color=longhand.complex_color)
+        # get the method and pass additional keyword or type-specific arguments
+        if longhand.keyword:
+            method = impl_keyword
+            args.update(keyword=longhand.keyword)
+            if "font" in longhand.ident:
+                args.update(cast_type=longhand.cast_type)
+        else:
+            method = predefined_types[longhand.predefined_type]
+            if longhand.predefined_type in ["CSSColor"]:
+                args.update(complex_color=longhand.complex_color)
 
         method(**args)
 
-    keyword_longhands = [x for x in longhands if x.keyword and x.name not in force_stub]
-    predefined_longhands = [x for x in longhands
-                            if x.predefined_type in predefined_types and x.name not in force_stub]
-    stub_longhands = [x for x in longhands if x not in keyword_longhands + predefined_longhands]
+    picked_longhands, stub_longhands = [], []
+    for x in longhands:
+        if (x.keyword or x.predefined_type in predefined_types) and x.name not in force_stub:
+            picked_longhands.append(x)
+        else:
+            stub_longhands.append(x)
 
     # If one of the longhands is not handled
     # by either:
     # - being a keyword
     # - being a predefined longhand
     # - being a longhand with manual glue code (i.e. in skip_longhands)
     # - being generated as a stub
     #
     # then we raise an error here.
     #
     # If you hit this error, please add `product="servo"` to the longhand.
     # In case the longhand is used in a shorthand, add it to the force_stub
     # list above.
+
     for stub in stub_longhands:
        if stub.name not in force_stub:
            raise Exception("Don't know what to do with longhand %s in style struct %s"
                            % (stub.name,style_struct. gecko_struct_name))
 %>
 impl ${style_struct.gecko_struct_name} {
     /*
      * Manually-Implemented Methods.
      */
     ${caller.body().strip()}
 
     /*
      * Auto-Generated Methods.
      */
     <%
-    for longhand in keyword_longhands:
-        impl_keyword(longhand.ident, longhand.gecko_ffi_name, longhand.keyword, longhand.need_clone)
-    for longhand in predefined_longhands:
-        predefined_type_method(longhand)
+    for longhand in picked_longhands:
+        longhand_method(longhand)
     %>
 
     /*
      * Stubs.
      */
     % for longhand in stub_longhands:
     #[allow(non_snake_case)]
     pub fn set_${longhand.ident}(&mut self, _: longhands::${longhand.ident}::computed_value::T) {
@@ -753,17 +760,17 @@ fn static_assert() {
     % endfor
 
     pub fn outline_has_nonzero_width(&self) -> bool {
         self.gecko.mActualOutlineWidth != 0
     }
 </%self:impl_trait>
 
 <%self:impl_trait style_struct_name="Font"
-    skip_longhands="font-family font-kerning font-stretch font-style font-size font-weight"
+    skip_longhands="font-family font-size font-weight"
     skip_additionals="*">
 
     pub fn set_font_family(&mut self, v: longhands::font_family::computed_value::T) {
         use properties::longhands::font_family::computed_value::FontFamily;
         use gecko_bindings::structs::FontFamilyType;
 
         let list = &mut self.gecko.mFont.fontlist;
         unsafe { Gecko_FontFamilyList_Clear(list); }
@@ -786,47 +793,31 @@ fn static_assert() {
             }
         }
     }
 
     pub fn copy_font_family_from(&mut self, other: &Self) {
         unsafe { Gecko_CopyFontFamilyFrom(&mut self.gecko.mFont, &other.gecko.mFont); }
     }
 
-    <%call expr="impl_keyword('font_style', 'mFont.style',
-        data.longhands_by_name['font-style'].keyword, need_clone=False)"></%call>
-
     // FIXME(bholley): Gecko has two different sizes, one of which (mSize) is the
     // actual computed size, and the other of which (mFont.size) is the 'display
     // size' which takes font zooming into account. We don't handle font zooming yet.
     pub fn set_font_size(&mut self, v: longhands::font_size::computed_value::T) {
         self.gecko.mFont.size = v.0;
         self.gecko.mSize = v.0;
     }
     pub fn copy_font_size_from(&mut self, other: &Self) {
         self.gecko.mFont.size = other.gecko.mFont.size;
         self.gecko.mSize = other.gecko.mSize;
     }
     pub fn clone_font_size(&self) -> longhands::font_size::computed_value::T {
         Au(self.gecko.mSize)
     }
 
-    <% kerning_keyword = Keyword("font-kerning", "auto normal none",
-                                 gecko_constant_prefix='NS_FONT_KERNING') %>
-
-    ${impl_keyword('font_kerning', 'mFont.kerning', kerning_keyword, need_clone=False)}
-
-    <% stretch_keyword = Keyword("font-stretch",
-                                 "normal ultra-condensed extra-condensed condensed " +
-                                 "semi-condensed semi-expanded expanded " +
-                                 "extra-expanded ultra-expanded",
-                                 gecko_constant_prefix='NS_FONT_STRETCH') %>
-
-    ${impl_keyword('font_stretch', 'mFont.stretch', stretch_keyword, need_clone=False, cast_type='i16')}
-
     pub fn set_font_weight(&mut self, v: longhands::font_weight::computed_value::T) {
         self.gecko.mFont.weight = v as u16;
     }
     ${impl_simple_copy('font_weight', 'mFont.weight')}
 
     pub fn clone_font_weight(&self) -> longhands::font_weight::computed_value::T {
         debug_assert!(self.gecko.mFont.weight >= 100);
         debug_assert!(self.gecko.mFont.weight <= 900);
@@ -1629,27 +1620,16 @@ fn static_assert() {
     pub fn copy_border_spacing_from(&mut self, other: &Self) {
         self.gecko.mBorderSpacingCol = other.gecko.mBorderSpacingCol;
         self.gecko.mBorderSpacingRow = other.gecko.mBorderSpacingRow;
     }
 
 </%self:impl_trait>
 
 
-<%self:impl_trait style_struct_name="InheritedBox"
-                  skip_longhands="image-rendering">
-
-    <% render_keyword = Keyword("image-rendering",
-                                "auto optimizequality optimizespeed crispedges") %>
-
-    ${impl_keyword('image_rendering', 'mImageRendering', render_keyword, need_clone=False)}
-
-</%self:impl_trait>
-
-
 <%self:impl_trait style_struct_name="InheritedText"
                   skip_longhands="text-align text-emphasis-style text-shadow line-height letter-spacing word-spacing">
 
     <% text_align_keyword = Keyword("text-align", "start end left right center justify -moz-center -moz-left " +
                                                   "-moz-right match-parent") %>
     ${impl_keyword('text_align', 'mTextAlign', text_align_keyword, need_clone=False)}
 
     pub fn set_text_shadow(&mut self, v: longhands::text_shadow::computed_value::T) {
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -307,16 +307,17 @@
     </%call>
 </%def>
 
 <%def name="single_keyword_computed(name, values, vector=False, **kwargs)">
     <%
         keyword_kwargs = {a: kwargs.pop(a, None) for a in [
             'gecko_constant_prefix', 'gecko_enum_prefix',
             'extra_gecko_values', 'extra_servo_values',
+            'custom_consts',
         ]}
     %>
 
     <%def name="inner_body()">
         pub use self::computed_value::T as SpecifiedValue;
         pub mod computed_value {
             define_css_keyword_enum! { T:
                 % for value in data.longhands_by_name[name].keyword.values_for(product):
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -118,17 +118,19 @@
         Ok(FontFamily::FamilyName(Atom::from(value)))
     }
 </%helpers:longhand>
 
 
 ${helpers.single_keyword("font-style",
                          "normal italic oblique",
                          gecko_constant_prefix="NS_FONT_STYLE",
+                         gecko_ffi_name="mFont.style",
                          animatable=False)}
+
 ${helpers.single_keyword("font-variant",
                          "normal small-caps",
                          animatable=False)}
 
 <%helpers:longhand name="font-weight" need_clone="True" animatable="True">
     use cssparser::ToCss;
     use std::fmt;
     use values::NoViewportPercentage;
@@ -342,21 +344,26 @@
     }
 </%helpers:longhand>
 
 // FIXME: This prop should be animatable
 ${helpers.single_keyword("font-stretch",
                          "normal ultra-condensed extra-condensed condensed \
                           semi-condensed semi-expanded expanded extra-expanded \
                           ultra-expanded",
+                         gecko_ffi_name="mFont.stretch",
+                         gecko_constant_prefix="NS_FONT_STRETCH",
+                         cast_type='i16',
                          animatable=False)}
 
 ${helpers.single_keyword("font-kerning",
                          "auto none normal",
                          products="gecko",
+                         gecko_ffi_name="mFont.kerning",
+                         gecko_constant_prefix="NS_FONT_KERNING",
                          animatable=False)}
 
 ${helpers.single_keyword("font-variant-position",
                          "normal sub super",
                          products="gecko",
                          gecko_ffi_name="mFont.variantPosition",
                          gecko_constant_prefix="NS_FONT_VARIANT_POSITION",
                          animatable=False)}
--- a/servo/components/style/properties/longhand/inherited_box.mako.rs
+++ b/servo/components/style/properties/longhand/inherited_box.mako.rs
@@ -35,83 +35,25 @@
                          animatable=False)}
 
 // CSS Color Module Level 4
 // https://drafts.csswg.org/css-color/
 ${helpers.single_keyword("color-adjust",
                          "economy exact", products="gecko",
                          animatable=False)}
 
-<%helpers:longhand name="image-rendering" animatable="False">
-    pub mod computed_value {
-        use cssparser::ToCss;
-        use std::fmt;
-
-        #[allow(non_camel_case_types)]
-        #[derive(Copy, Clone, Debug, PartialEq)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
-        pub enum T {
-            auto,
-            crispedges,
-            % if product == "gecko":
-                optimizequality,
-                optimizespeed,
-            % else:
-                pixelated,  // firefox doesn't support it (https://bugzilla.mozilla.org/show_bug.cgi?id=856337)
-            % endif
-        }
-
-        impl ToCss for T {
-            fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                match *self {
-                    T::auto => dest.write_str("auto"),
-                    T::crispedges => dest.write_str("crisp-edges"),
-                    % if product == "gecko":
-                        T::optimizequality => dest.write_str("optimizeQuality"),
-                        T::optimizespeed => dest.write_str("optimizeSpeed"),
-                    % else:
-                        T::pixelated => dest.write_str("pixelated"),
-                    % endif
-                }
-            }
-        }
-    }
-
-    use values::NoViewportPercentage;
-    impl NoViewportPercentage for SpecifiedValue {}
-
-    pub type SpecifiedValue = computed_value::T;
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T::auto
-    }
-
-    pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
-        // According to to CSS-IMAGES-3, `optimizespeed` and `optimizequality` are synonyms for
-        // `auto`.
-        match_ignore_ascii_case! {
-            try!(input.expect_ident()),
-            "auto" => Ok(computed_value::T::auto),
-            "crisp-edges" => Ok(computed_value::T::crispedges),
-            % if product == "gecko":
-                "optimizequality" => Ok(computed_value::T::optimizequality),
-                "optimizespeed" => Ok(computed_value::T::optimizespeed),
-            % else:
-                "optimizequality" => Ok(computed_value::T::auto),
-                "optimizespeed" => Ok(computed_value::T::auto),
-                "pixelated" => Ok(computed_value::T::pixelated),
-            % endif
-            _ => Err(())
-        }
-    }
-
-    use values::computed::ComputedValueAsSpecified;
-    impl ComputedValueAsSpecified for SpecifiedValue { }
-</%helpers:longhand>
+<% image_rendering_custom_consts = { "crisp-edges": "CRISPEDGES" } %>
+// According to to CSS-IMAGES-3, `optimizespeed` and `optimizequality` are synonyms for `auto`
+// And, firefox doesn't support `pixelated` yet (https://bugzilla.mozilla.org/show_bug.cgi?id=856337)
+${helpers.single_keyword("image-rendering",
+                         "auto crisp-edges",
+                         extra_gecko_values="optimizespeed optimizequality",
+                         extra_servo_values="pixelated",
+                         custom_consts=image_rendering_custom_consts,
+                         animatable=False)}
 
 // Used in the bottom-up flow construction traversal to avoid constructing flows for
 // descendants of nodes with `display: none`.
 <%helpers:longhand name="-servo-under-display-none"
                    derived_from="display"
                    products="servo"
                    animatable="False">
     use cssparser::ToCss;