From 34c82d6e7861c8e8d919512abdabb0ab0773fa38 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 15 Jul 2016 14:17:08 +0530 Subject: [PATCH 1/4] Replace gecko_autoarray_longhand with configurable vector_longhand --- components/style/properties/helpers.mako.rs | 13 +++++++++---- .../style/properties/longhand/background.mako.rs | 4 ++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 2b16f527e91..9db3d076fae 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -37,12 +37,17 @@ // and handle the empty case correctly <%doc> To be used in cases where we have a grammar like - " [ , ]*", but only support a single value - in servo + " [ , ]*". `gecko_only` should be set + to True for cases where Servo takes a single value + and Stylo supports vector values. + + If the computed value is the same as the specified value, + setting computed_is_specified to True will introduce additional + optimizations -<%def name="gecko_autoarray_longhand(name, **kwargs)"> +<%def name="vector_longhand(name, gecko_only=False, **kwargs)"> <%call expr="longhand(name, **kwargs)"> - % if product == "gecko": + % if product == "gecko" or not gecko_only: use cssparser::ToCss; use std::fmt; diff --git a/components/style/properties/longhand/background.mako.rs b/components/style/properties/longhand/background.mako.rs index f66e8d85af1..26dbf123df8 100644 --- a/components/style/properties/longhand/background.mako.rs +++ b/components/style/properties/longhand/background.mako.rs @@ -10,7 +10,7 @@ ${helpers.predefined_type("background-color", "CSSColor", "::cssparser::Color::RGBA(::cssparser::RGBA { red: 0., green: 0., blue: 0., alpha: 0. }) /* transparent */", animatable=True)} -<%helpers:gecko_autoarray_longhand name="background-image" animatable="False"> +<%helpers:vector_longhand gecko_only="True" name="background-image" animatable="False"> use cssparser::ToCss; use std::fmt; use values::specified::Image; @@ -70,7 +70,7 @@ ${helpers.predefined_type("background-color", "CSSColor", } } } - + <%helpers:longhand name="background-position" animatable="True"> use cssparser::ToCss; From b6edae2f5e37c57245b150af349d6f7380a650cc Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 15 Jul 2016 14:42:08 +0530 Subject: [PATCH 2/4] Add allow_empty to vector_longhand --- components/style/properties/helpers.mako.rs | 52 +++++++++++++++------ 1 file changed, 38 insertions(+), 14 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 9db3d076fae..76543de7a6f 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -41,11 +41,11 @@ to True for cases where Servo takes a single value and Stylo supports vector values. - If the computed value is the same as the specified value, - setting computed_is_specified to True will introduce additional - optimizations + Setting allow_empty to False allows for cases where the vector + is empty. The grammar for these is usually "none | [ , ]*". + We assume that the default/initial value is an empty vector for these. -<%def name="vector_longhand(name, gecko_only=False, **kwargs)"> +<%def name="vector_longhand(name, gecko_only=False, allow_empty=False, **kwargs)"> <%call expr="longhand(name, **kwargs)"> % if product == "gecko" or not gecko_only: use cssparser::ToCss; @@ -68,10 +68,15 @@ impl ToCss for computed_value::T { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if !self.0.is_empty() { - try!(self.0[0].to_css(dest)); + let mut iter = self.0.iter(); + if let Some(val) = iter.next() { + try!(val.to_css(dest)); + } else { + % if allow_empty: + try!(dest.write_str("none")); + % endif } - for i in self.0.iter().skip(1) { + for i in iter { try!(dest.write_str(", ")); try!(i.to_css(dest)); } @@ -85,10 +90,15 @@ impl ToCss for SpecifiedValue { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - if !self.0.is_empty() { - try!(self.0[0].to_css(dest)) + let mut iter = self.0.iter(); + if let Some(val) = iter.next() { + try!(val.to_css(dest)); + } else { + % if allow_empty: + try!(dest.write_str("none")); + % endif } - for i in self.0.iter().skip(1) { + for i in iter { try!(dest.write_str(", ")); try!(i.to_css(dest)); } @@ -96,12 +106,26 @@ } } pub fn get_initial_value() -> computed_value::T { - computed_value::T(vec![single_value::get_initial_value()]) + % if allow_empty: + computed_value::T(vec![]) + % else: + computed_value::T(vec![single_value::get_initial_value()]) + % endif } pub fn parse(context: &ParserContext, input: &mut Parser) -> Result { - input.parse_comma_separated(|parser| { - single_value::parse(context, parser) - }).map(|ok| SpecifiedValue(ok)) + % if allow_empty: + if input.try(|input| input.expect_ident_matching("none")).is_ok() { + Ok(SpecifiedValue(Vec::new())) + } else { + input.parse_comma_separated(|parser| { + single_value::parse(context, parser) + }).map(SpecifiedValue) + } + % else: + input.parse_comma_separated(|parser| { + single_value::parse(context, parser) + }).map(SpecifiedValue) + % endif } impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; From a37eb82da8c580cd79734f9973c1d002e9872ce3 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Fri, 15 Jul 2016 15:09:19 +0530 Subject: [PATCH 3/4] Use allow_empty vector longhand for box-shadow --- components/style/properties/helpers.mako.rs | 1 + .../helpers/animated_properties.mako.rs | 2 +- .../style/properties/longhand/effects.mako.rs | 94 ++++--------------- 3 files changed, 19 insertions(+), 78 deletions(-) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 76543de7a6f..0f9a7244f3c 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -44,6 +44,7 @@ Setting allow_empty to False allows for cases where the vector is empty. The grammar for these is usually "none | [ , ]*". We assume that the default/initial value is an empty vector for these. + `initial_value` need not be defined for these. <%def name="vector_longhand(name, gecko_only=False, allow_empty=False, **kwargs)"> <%call expr="longhand(name, **kwargs)"> diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs index 1043aaf24bc..aa72a0865b5 100644 --- a/components/style/properties/helpers/animated_properties.mako.rs +++ b/components/style/properties/helpers/animated_properties.mako.rs @@ -16,7 +16,7 @@ use properties::longhands::line_height::computed_value::T as LineHeight; use properties::longhands::text_shadow::computed_value::T as TextShadowList; use properties::longhands::text_shadow::computed_value::TextShadow; use properties::longhands::box_shadow::computed_value::T as BoxShadowList; -use properties::longhands::box_shadow::computed_value::BoxShadow; +use properties::longhands::box_shadow::single_value::computed_value::T as BoxShadow; use properties::longhands::transform::computed_value::ComputedMatrix; use properties::longhands::transform::computed_value::ComputedOperation as TransformOperation; use properties::longhands::transform::computed_value::T as TransformList; diff --git a/components/style/properties/longhand/effects.mako.rs b/components/style/properties/longhand/effects.mako.rs index 4fa881c93fd..fa747076da1 100644 --- a/components/style/properties/longhand/effects.mako.rs +++ b/components/style/properties/longhand/effects.mako.rs @@ -12,18 +12,14 @@ ${helpers.predefined_type("opacity", "1.0", animatable=True)} -<%helpers:longhand name="box-shadow" animatable="True"> +<%helpers:vector_longhand name="box-shadow" allow_empty="True" animatable="True"> use cssparser::{self, ToCss}; use std::fmt; use values::LocalToCss; #[derive(Debug, Clone, PartialEq)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct SpecifiedValue(Vec); - - #[derive(Debug, Clone, PartialEq)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct SpecifiedBoxShadow { + pub struct SpecifiedValue { pub offset_x: specified::Length, pub offset_y: specified::Length, pub blur_radius: specified::Length, @@ -33,23 +29,6 @@ ${helpers.predefined_type("opacity", } impl ToCss for SpecifiedValue { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - let mut iter = self.0.iter(); - if let Some(shadow) = iter.next() { - try!(shadow.to_css(dest)); - } else { - try!(dest.write_str("none")); - return Ok(()) - } - for shadow in iter { - try!(dest.write_str(", ")); - try!(shadow.to_css(dest)); - } - Ok(()) - } - } - - impl ToCss for SpecifiedBoxShadow { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { if self.inset { try!(dest.write_str("inset ")); @@ -75,13 +54,9 @@ ${helpers.predefined_type("opacity", use std::fmt; use values::computed; - #[derive(Clone, PartialEq, Debug)] - #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct T(pub Vec); - #[derive(Clone, PartialEq, Copy, Debug)] #[cfg_attr(feature = "servo", derive(HeapSizeOf))] - pub struct BoxShadow { + pub struct T { pub offset_x: Au, pub offset_y: Au, pub blur_radius: Au, @@ -92,23 +67,6 @@ ${helpers.predefined_type("opacity", } impl ToCss for computed_value::T { - fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { - let mut iter = self.0.iter(); - if let Some(shadow) = iter.next() { - try!(shadow.to_css(dest)); - } else { - try!(dest.write_str("none")); - return Ok(()) - } - for shadow in iter { - try!(dest.write_str(", ")); - try!(shadow.to_css(dest)); - } - Ok(()) - } - } - - impl ToCss for computed_value::BoxShadow { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write { if self.inset { try!(dest.write_str("inset ")); @@ -126,44 +84,26 @@ ${helpers.predefined_type("opacity", } } - #[inline] - pub fn get_initial_value() -> computed_value::T { - computed_value::T(Vec::new()) - } - - pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result { - if input.try(|input| input.expect_ident_matching("none")).is_ok() { - Ok(SpecifiedValue(Vec::new())) - } else { - input.parse_comma_separated(parse_one_box_shadow).map(SpecifiedValue) - } - } - impl ToComputedValue for SpecifiedValue { type ComputedValue = computed_value::T; #[inline] fn to_computed_value(&self, context: &Cx) -> computed_value::T { - computed_value::T(self.0.iter().map(|value| compute_one_box_shadow(value, context)).collect()) + computed_value::T { + offset_x: self.offset_x.to_computed_value(context), + offset_y: self.offset_y.to_computed_value(context), + blur_radius: self.blur_radius.to_computed_value(context), + spread_radius: self.spread_radius.to_computed_value(context), + color: self.color + .as_ref() + .map(|color| color.parsed) + .unwrap_or(cssparser::Color::CurrentColor), + inset: self.inset, + } } } - pub fn compute_one_box_shadow(value: &SpecifiedBoxShadow, context: &Cx) - -> computed_value::BoxShadow { - computed_value::BoxShadow { - offset_x: value.offset_x.to_computed_value(context), - offset_y: value.offset_y.to_computed_value(context), - blur_radius: value.blur_radius.to_computed_value(context), - spread_radius: value.spread_radius.to_computed_value(context), - color: value.color - .as_ref() - .map(|color| color.parsed) - .unwrap_or(cssparser::Color::CurrentColor), - inset: value.inset, - } - } - - pub fn parse_one_box_shadow(input: &mut Parser) -> Result { + pub fn parse(_context: &ParserContext, input: &mut Parser) -> Result { use app_units::Au; let mut lengths = [specified::Length::Absolute(Au(0)); 4]; let mut lengths_parsed = false; @@ -213,7 +153,7 @@ ${helpers.predefined_type("opacity", return Err(()) } - Ok(SpecifiedBoxShadow { + Ok(SpecifiedValue { offset_x: lengths[0], offset_y: lengths[1], blur_radius: lengths[2], @@ -222,7 +162,7 @@ ${helpers.predefined_type("opacity", inset: inset, }) } - + // FIXME: This prop should be animatable <%helpers:longhand name="clip" animatable="False"> From e21828080bc57c672b1c8fcb89449a8d60d49451 Mon Sep 17 00:00:00 2001 From: Manish Goregaokar Date: Sat, 16 Jul 2016 11:01:03 +0530 Subject: [PATCH 4/4] Error when non-allow_empty property is empty --- components/style/properties/helpers.mako.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/components/style/properties/helpers.mako.rs b/components/style/properties/helpers.mako.rs index 0f9a7244f3c..374cd6a011e 100644 --- a/components/style/properties/helpers.mako.rs +++ b/components/style/properties/helpers.mako.rs @@ -75,6 +75,8 @@ } else { % if allow_empty: try!(dest.write_str("none")); + % else: + error!("Found empty value for property ${name}"); % endif } for i in iter { @@ -97,6 +99,8 @@ } else { % if allow_empty: try!(dest.write_str("none")); + % else: + error!("Found empty value for property ${name}"); % endif } for i in iter {