Address review comments

This commit is contained in:
Manish Goregaokar 2016-08-05 12:14:58 +05:30
parent d1e45f78af
commit 234219cd84
4 changed files with 155 additions and 147 deletions

View file

@ -8,10 +8,13 @@
//! [basic-shape]: https://drafts.csswg.org/css-shapes/#typedef-basic-shape
use cssparser::ToCss;
use properties::shorthands::serialize_four_sides;
use std::fmt;
use values::computed::position::Position;
use values::computed::{BorderRadiusSize, LengthOrPercentage};
pub use values::specified::basic_shape::FillRule;
#[derive(Clone, PartialEq, Debug)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub enum BasicShape {
@ -24,9 +27,9 @@ pub enum BasicShape {
impl ToCss for BasicShape {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match *self {
BasicShape::Inset(rect) => rect.to_css(dest),
BasicShape::Circle(circle) => circle.to_css(dest),
BasicShape::Ellipse(e) => e.to_css(dest),
BasicShape::Inset(ref rect) => rect.to_css(dest),
BasicShape::Circle(ref circle) => circle.to_css(dest),
BasicShape::Ellipse(ref e) => e.to_css(dest),
BasicShape::Polygon(ref poly) => poly.to_css(dest),
}
}
@ -79,22 +82,23 @@ impl ToCss for Circle {
#[derive(Clone, PartialEq, Copy, Debug)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct Ellipse {
pub semiaxis_a: ShapeRadius,
pub semiaxis_b: ShapeRadius,
pub semiaxis_x: ShapeRadius,
pub semiaxis_y: ShapeRadius,
pub position: Position,
}
impl ToCss for Ellipse {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
if ShapeRadius::ClosestSide != self.semiaxis_a &&
ShapeRadius::ClosestSide != self.semiaxis_b {
try!(self.semiaxis_a.to_css(dest));
try!(dest.write_str("ellipse("));
if (self.semiaxis_x, self.semiaxis_y) != Default::default() {
try!(self.semiaxis_x.to_css(dest));
try!(dest.write_str(" "));
try!(self.semiaxis_b.to_css(dest));
try!(self.semiaxis_y.to_css(dest));
try!(dest.write_str(" "));
}
try!(dest.write_str("at "));
self.position.to_css(dest)
try!(self.position.to_css(dest));
dest.write_str(")")
}
}
@ -108,6 +112,7 @@ pub struct Polygon {
impl ToCss for Polygon {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(dest.write_str("polygon("));
let mut need_space = false;
if self.fill != Default::default() {
try!(self.fill.to_css(dest));
@ -115,14 +120,14 @@ impl ToCss for Polygon {
}
for coord in &self.coordinates {
if need_space {
try!(dest.write_str(" "));
try!(dest.write_str(", "));
}
try!(coord.0.to_css(dest));
try!(dest.write_str(" "));
try!(coord.1.to_css(dest));
need_space = true;
}
Ok(())
dest.write_str(")")
}
}
@ -157,36 +162,33 @@ impl ToCss for ShapeRadius {
pub struct BorderRadius {
pub top_left: BorderRadiusSize,
pub top_right: BorderRadiusSize,
pub bottom_left: BorderRadiusSize,
pub bottom_right: BorderRadiusSize,
pub bottom_left: BorderRadiusSize,
}
impl ToCss for BorderRadius {
// XXXManishearth: We should be producing minimal output:
// if height=width for all, we should not be printing the part after
// the slash. For any set of four values,
// we should try to reduce them to one or two. This probably should be
// a helper function somewhere, for all the parse_four_sides-like
// values
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(self.top_left.0.width.to_css(dest));
try!(dest.write_str(" "));
try!(self.top_right.0.width.to_css(dest));
try!(dest.write_str(" "));
try!(self.bottom_left.0.width.to_css(dest));
try!(dest.write_str(" "));
try!(self.bottom_right.0.width.to_css(dest));
try!(dest.write_str(" / "));
try!(self.top_left.0.height.to_css(dest));
try!(dest.write_str(" "));
try!(self.top_right.0.height.to_css(dest));
try!(dest.write_str(" "));
try!(self.bottom_left.0.height.to_css(dest));
try!(dest.write_str(" "));
try!(self.bottom_right.0.height.to_css(dest));
dest.write_str(" ")
if self.top_left.0.width == self.top_left.0.height &&
self.top_right.0.width == self.top_right.0.height &&
self.bottom_right.0.width == self.bottom_right.0.height &&
self.bottom_left.0.width == self.bottom_left.0.height {
serialize_four_sides((&self.top_left.0.width,
&self.top_right.0.width,
&self.bottom_right.0.width,
&self.bottom_left.0.width),
dest)
} else {
try!(serialize_four_sides((&self.top_left.0.width,
&self.top_right.0.width,
&self.bottom_right.0.width,
&self.bottom_left.0.width),
dest));
try!(dest.write_str(" / "));
serialize_four_sides((&self.top_left.0.height,
&self.top_right.0.height,
&self.bottom_right.0.height,
&self.bottom_left.0.height),
dest)
}
}
}
pub use values::specified::basic_shape::FillRule;

View file

@ -14,7 +14,7 @@ use std::fmt;
use values::computed::basic_shape as computed_basic_shape;
use values::computed::{Context, ToComputedValue, ComputedValueAsSpecified};
use values::specified::position::{Position, PositionComponent};
use values::specified::{BorderRadiusSize, Length, LengthOrPercentage};
use values::specified::{BorderRadiusSize, Length, LengthOrPercentage, Percentage};
#[derive(Clone, PartialEq, Debug)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
@ -27,16 +27,24 @@ pub enum BasicShape {
impl BasicShape {
pub fn parse(input: &mut Parser) -> Result<BasicShape, ()> {
if let Ok(result) = input.try(InsetRect::parse) {
Ok(BasicShape::Inset(result))
} else if let Ok(result) = input.try(Circle::parse) {
Ok(BasicShape::Circle(result))
} else if let Ok(result) = input.try(Ellipse::parse) {
Ok(BasicShape::Ellipse(result))
} else if let Ok(result) = input.try(Polygon::parse) {
Ok(BasicShape::Polygon(result))
} else {
Err(())
match_ignore_ascii_case! { try!(input.expect_function()),
"inset" => {
Ok(BasicShape::Inset(
try!(input.parse_nested_block(InsetRect::parse_function_arguments))))
},
"circle" => {
Ok(BasicShape::Circle(
try!(input.parse_nested_block(Circle::parse_function_arguments))))
},
"ellipse" => {
Ok(BasicShape::Ellipse(
try!(input.parse_nested_block(Ellipse::parse_function_arguments))))
},
"polygon" => {
Ok(BasicShape::Polygon(
try!(input.parse_nested_block(Polygon::parse_function_arguments))))
},
_ => Err(())
}
}
}
@ -81,12 +89,12 @@ impl InsetRect {
pub fn parse(input: &mut Parser) -> Result<InsetRect, ()> {
match_ignore_ascii_case! { try!(input.expect_function()),
"inset" => {
Ok(try!(input.parse_nested_block(InsetRect::parse_function)))
Ok(try!(input.parse_nested_block(InsetRect::parse_function_arguments)))
},
_ => Err(())
}
}
pub fn parse_function(input: &mut Parser) -> Result<InsetRect, ()> {
pub fn parse_function_arguments(input: &mut Parser) -> Result<InsetRect, ()> {
let (t, r, b, l) = try!(parse_four_sides(input, LengthOrPercentage::parse));
let mut rect = InsetRect {
top: t,
@ -148,18 +156,21 @@ impl Circle {
pub fn parse(input: &mut Parser) -> Result<Circle, ()> {
match_ignore_ascii_case! { try!(input.expect_function()),
"circle" => {
Ok(try!(input.parse_nested_block(Circle::parse_function)))
Ok(try!(input.parse_nested_block(Circle::parse_function_arguments)))
},
_ => Err(())
}
}
pub fn parse_function(input: &mut Parser) -> Result<Circle, ()> {
pub fn parse_function_arguments(input: &mut Parser) -> Result<Circle, ()> {
let radius = input.try(ShapeRadius::parse).ok().unwrap_or_else(Default::default);
let position = if let Ok(_) = input.try(|input| input.expect_ident_matching("at")) {
try!(Position::parse(input))
} else {
// Defaults to origin
try!(Position::new(PositionComponent::Center, PositionComponent::Center))
Position {
horizontal: LengthOrPercentage::Percentage(Percentage(0.5)),
vertical: LengthOrPercentage::Percentage(Percentage(0.5)),
}
};
Ok(Circle {
radius: radius,
@ -197,8 +208,8 @@ impl ToComputedValue for Circle {
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
/// https://drafts.csswg.org/css-shapes/#funcdef-ellipse
pub struct Ellipse {
pub semiaxis_a: ShapeRadius,
pub semiaxis_b: ShapeRadius,
pub semiaxis_x: ShapeRadius,
pub semiaxis_y: ShapeRadius,
pub position: Position,
}
@ -207,24 +218,27 @@ impl Ellipse {
pub fn parse(input: &mut Parser) -> Result<Ellipse, ()> {
match_ignore_ascii_case! { try!(input.expect_function()),
"ellipse" => {
Ok(try!(input.parse_nested_block(Ellipse::parse_function)))
Ok(try!(input.parse_nested_block(Ellipse::parse_function_arguments)))
},
_ => Err(())
}
}
pub fn parse_function(input: &mut Parser) -> Result<Ellipse, ()> {
pub fn parse_function_arguments(input: &mut Parser) -> Result<Ellipse, ()> {
let (a, b) = input.try(|input| -> Result<_, ()> {
Ok((try!(ShapeRadius::parse(input)), try!(ShapeRadius::parse(input))))
}).unwrap_or((Default::default(), Default::default()));
}).ok().unwrap_or_default();
let position = if let Ok(_) = input.try(|input| input.expect_ident_matching("at")) {
try!(Position::parse(input))
} else {
// Defaults to origin
try!(Position::new(PositionComponent::Center, PositionComponent::Center))
Position {
horizontal: LengthOrPercentage::Percentage(Percentage(0.5)),
vertical: LengthOrPercentage::Percentage(Percentage(0.5)),
}
};
Ok(Ellipse {
semiaxis_a: a,
semiaxis_b: b,
semiaxis_x: a,
semiaxis_y: b,
position: position,
})
}
@ -233,11 +247,10 @@ impl Ellipse {
impl ToCss for Ellipse {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
try!(dest.write_str("ellipse("));
if ShapeRadius::ClosestSide != self.semiaxis_a ||
ShapeRadius::ClosestSide != self.semiaxis_b {
try!(self.semiaxis_a.to_css(dest));
if (self.semiaxis_x, self.semiaxis_y) != Default::default() {
try!(self.semiaxis_x.to_css(dest));
try!(dest.write_str(" "));
try!(self.semiaxis_b.to_css(dest));
try!(self.semiaxis_y.to_css(dest));
try!(dest.write_str(" "));
}
try!(dest.write_str("at "));
@ -252,8 +265,8 @@ impl ToComputedValue for Ellipse {
#[inline]
fn to_computed_value(&self, cx: &Context) -> Self::ComputedValue {
computed_basic_shape::Ellipse {
semiaxis_a: self.semiaxis_a.to_computed_value(cx),
semiaxis_b: self.semiaxis_b.to_computed_value(cx),
semiaxis_x: self.semiaxis_x.to_computed_value(cx),
semiaxis_y: self.semiaxis_y.to_computed_value(cx),
position: self.position.to_computed_value(cx),
}
}
@ -272,25 +285,22 @@ impl Polygon {
pub fn parse(input: &mut Parser) -> Result<Polygon, ()> {
match_ignore_ascii_case! { try!(input.expect_function()),
"polygon" => {
Ok(try!(input.parse_nested_block(Polygon::parse_function)))
Ok(try!(input.parse_nested_block(Polygon::parse_function_arguments)))
},
_ => Err(())
}
}
pub fn parse_function(input: &mut Parser) -> Result<Polygon, ()> {
pub fn parse_function_arguments(input: &mut Parser) -> Result<Polygon, ()> {
let fill = input.try(|input| {
let fill = FillRule::parse(input);
// only eat the comma if there is something before it
try!(input.expect_comma());
fill
}).ok().unwrap_or_else(Default::default);
let first = (try!(LengthOrPercentage::parse(input)),
try!(LengthOrPercentage::parse(input)));
let mut buf = vec![first];
while !input.is_exhausted() {
buf.push((try!(LengthOrPercentage::parse(input)),
try!(LengthOrPercentage::parse(input))));
}
let buf = try!(input.parse_comma_separated(|input| {
Ok((try!(LengthOrPercentage::parse(input)),
try!(LengthOrPercentage::parse(input))))
}));
Ok(Polygon {
fill: fill,
coordinates: buf,
@ -308,7 +318,7 @@ impl ToCss for Polygon {
}
for coord in &self.coordinates {
if need_space {
try!(dest.write_str(" "));
try!(dest.write_str(", "));
}
try!(coord.0.to_css(dest));
try!(dest.write_str(" "));
@ -396,38 +406,32 @@ impl ToComputedValue for ShapeRadius {
pub struct BorderRadius {
pub top_left: BorderRadiusSize,
pub top_right: BorderRadiusSize,
pub bottom_left: BorderRadiusSize,
pub bottom_right: BorderRadiusSize,
pub bottom_left: BorderRadiusSize,
}
impl ToCss for BorderRadius {
// XXXManishearth: We should be producing minimal output:
// if height=width for all, we should not be printing the part after
// the slash. For any set of four values,
// we should try to reduce them to one or two. This probably should be
// a helper function somewhere, for all the parse_four_sides-like
// values
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
if self.top_left.0.width == self.top_left.0.height &&
self.top_right.0.width == self.top_right.0.height &&
self.bottom_left.0.width == self.bottom_left.0.height &&
self.bottom_right.0.width == self.bottom_right.0.height {
self.bottom_right.0.width == self.bottom_right.0.height &&
self.bottom_left.0.width == self.bottom_left.0.height {
serialize_four_sides((&self.top_left.0.width,
&self.top_right.0.width,
&self.bottom_left.0.width,
&self.bottom_right.0.width),
&self.bottom_right.0.width,
&self.bottom_left.0.width),
dest)
} else {
try!(serialize_four_sides((&self.top_left.0.width,
&self.top_right.0.width,
&self.bottom_left.0.width,
&self.bottom_right.0.width),
&self.bottom_right.0.width,
&self.bottom_left.0.width),
dest));
try!(dest.write_str(" / "));
serialize_four_sides((&self.top_left.0.height,
&self.top_right.0.height,
&self.bottom_left.0.height,
&self.bottom_right.0.height),
&self.bottom_right.0.height,
&self.bottom_left.0.height),
dest)
}
}
@ -436,38 +440,40 @@ impl ToCss for BorderRadius {
impl BorderRadius {
pub fn parse(input: &mut Parser) -> Result<BorderRadius, ()> {
let widths = try!(parse_one_set_of_border_values(input));
let mut heights = widths.clone();
if input.try(|input| input.expect_delim('/')).is_ok() {
heights = try!(parse_one_set_of_border_values(input));
}
let heights = if input.try(|input| input.expect_delim('/')).is_ok() {
try!(parse_one_set_of_border_values(input))
} else {
widths.clone()
};
Ok(BorderRadius {
top_left: BorderRadiusSize::new(widths[0], heights[0]),
top_right: BorderRadiusSize::new(widths[1], heights[1]),
bottom_left: BorderRadiusSize::new(widths[2], heights[2]),
bottom_right: BorderRadiusSize::new(widths[3], heights[3]),
bottom_right: BorderRadiusSize::new(widths[2], heights[2]),
bottom_left: BorderRadiusSize::new(widths[3], heights[3]),
})
}
}
fn parse_one_set_of_border_values(mut input: &mut Parser)
-> Result<[LengthOrPercentage; 4], ()> {
let mut count = 0;
let mut values = [LengthOrPercentage::Length(Length::Absolute(Au(0))); 4];
while count < 4 {
if let Ok(value) = input.try(LengthOrPercentage::parse) {
values[count] = value;
count += 1;
} else {
break
}
}
let a = try!(LengthOrPercentage::parse(input));
match count {
1 => Ok([values[0], values[0], values[0], values[0]]),
2 => Ok([values[0], values[1], values[0], values[1]]),
3 => Ok([values[0], values[1], values[2], values[1]]),
4 => Ok([values[0], values[1], values[2], values[3]]),
_ => Err(()),
let b = if let Ok(b) = input.try(LengthOrPercentage::parse) {
b
} else {
return Ok([a, a, a, a])
};
let c = if let Ok(c) = input.try(LengthOrPercentage::parse) {
c
} else {
return Ok([a, b, a, b])
};
if let Ok(d) = input.try(LengthOrPercentage::parse) {
Ok([a, b, c, d])
} else {
Ok([a, b, c, b])
}
}
@ -480,8 +486,8 @@ impl ToComputedValue for BorderRadius {
computed_basic_shape::BorderRadius {
top_left: self.top_left.to_computed_value(cx),
top_right: self.top_right.to_computed_value(cx),
bottom_left: self.bottom_left.to_computed_value(cx),
bottom_right: self.bottom_right.to_computed_value(cx),
bottom_left: self.bottom_left.to_computed_value(cx),
}
}
}