Skip to content

Commit

Permalink
Auto merge of #15065 - Manishearth:box-calclop, r=heycam
Browse files Browse the repository at this point in the history 
Use Box<CalcLengthOrPercentage> in specified values to avoid bloating inline sizes

For #15061

CalcLOP is a large struct, and gets used quite often. While #15063 reduces its size a bit,
it will still be much larger than any of the other variants in the `specified::Length*` types,
so it will still bloat sizes, especially for specified values that contain many lengths.

This change boxes it in the length types, so that it just takes one word.

r? @heycam

<!-- 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/15065)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jan 17, 2017
2 parents a73fd01 + f59557d commit f010fb5
Show file tree
Hide file tree
Showing 20 changed files with 280 additions and 249 deletions.
8 changes: 4 additions & 4 deletions components/script/dom/element.rs
View file
Expand Up @@ -480,7 +480,7 @@ impl LayoutElementHelpers for LayoutJS<Element> {
hints.push(from_declaration(
PropertyDeclaration::BorderSpacing(DeclaredValue::Value(
border_spacing::SpecifiedValue {
horizontal: width_value,
horizontal: width_value.clone(),
vertical: width_value,
}))));
}
Expand Down Expand Up @@ -625,11 +625,11 @@ impl LayoutElementHelpers for LayoutJS<Element> {
let width_value = specified::BorderWidth::from_length(
specified::Length::Absolute(Au::from_px(border as i32)));
hints.push(from_declaration(
PropertyDeclaration::BorderTopWidth(DeclaredValue::Value(width_value))));
PropertyDeclaration::BorderTopWidth(DeclaredValue::Value(width_value.clone()))));
hints.push(from_declaration(
PropertyDeclaration::BorderLeftWidth(DeclaredValue::Value(width_value))));
PropertyDeclaration::BorderLeftWidth(DeclaredValue::Value(width_value.clone()))));
hints.push(from_declaration(
PropertyDeclaration::BorderBottomWidth(DeclaredValue::Value(width_value))));
PropertyDeclaration::BorderBottomWidth(DeclaredValue::Value(width_value.clone()))));
hints.push(from_declaration(
PropertyDeclaration::BorderRightWidth(DeclaredValue::Value(width_value))));
}
Expand Down
6 changes: 3 additions & 3 deletions components/style/properties/longhand/border.mako.rs
View file
Expand Up @@ -427,7 +427,7 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box",
impl ToCss for computed_value::SingleComputedValue {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match *self {
computed_value::SingleComputedValue::LengthOrPercentage(len) => len.to_css(dest),
computed_value::SingleComputedValue::LengthOrPercentage(ref len) => len.to_css(dest),
computed_value::SingleComputedValue::Number(number) => number.to_css(dest),
computed_value::SingleComputedValue::Auto => dest.write_str("auto"),
}
Expand All @@ -436,7 +436,7 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box",
impl ToCss for SingleSpecifiedValue {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match *self {
SingleSpecifiedValue::LengthOrPercentage(len) => len.to_css(dest),
SingleSpecifiedValue::LengthOrPercentage(ref len) => len.to_css(dest),
SingleSpecifiedValue::Number(number) => number.to_css(dest),
SingleSpecifiedValue::Auto => dest.write_str("auto"),
}
Expand All @@ -449,7 +449,7 @@ ${helpers.single_keyword("-moz-float-edge", "content-box margin-box",
#[inline]
fn to_computed_value(&self, context: &Context) -> computed_value::SingleComputedValue {
match *self {
SingleSpecifiedValue::LengthOrPercentage(len) => {
SingleSpecifiedValue::LengthOrPercentage(ref len) => {
computed_value::SingleComputedValue::LengthOrPercentage(
len.to_computed_value(context))
},
Expand Down
40 changes: 20 additions & 20 deletions components/style/properties/longhand/box.mako.rs
View file
Expand Up @@ -258,15 +258,15 @@ ${helpers.single_keyword("-moz-top-layer", "none top",
impl HasViewportPercentage for SpecifiedValue {
fn has_viewport_percentage(&self) -> bool {
match *self {
SpecifiedValue::LengthOrPercentage(length) => length.has_viewport_percentage(),
SpecifiedValue::LengthOrPercentage(ref length) => length.has_viewport_percentage(),
_ => false
}
}
}

/// The `vertical-align` value.
#[allow(non_camel_case_types)]
#[derive(Debug, Clone, PartialEq, Copy)]
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub enum SpecifiedValue {
% for keyword in vertical_align_keywords:
Expand All @@ -281,7 +281,7 @@ ${helpers.single_keyword("-moz-top-layer", "none top",
% for keyword in vertical_align_keywords:
SpecifiedValue::${to_rust_ident(keyword)} => dest.write_str("${keyword}"),
% endfor
SpecifiedValue::LengthOrPercentage(value) => value.to_css(dest),
SpecifiedValue::LengthOrPercentage(ref value) => value.to_css(dest),
}
}
}
Expand Down Expand Up @@ -324,7 +324,7 @@ ${helpers.single_keyword("-moz-top-layer", "none top",
% for keyword in vertical_align_keywords:
T::${to_rust_ident(keyword)} => dest.write_str("${keyword}"),
% endfor
T::LengthOrPercentage(value) => value.to_css(dest),
T::LengthOrPercentage(ref value) => value.to_css(dest),
}
}
}
Expand All @@ -347,7 +347,7 @@ ${helpers.single_keyword("-moz-top-layer", "none top",
computed_value::T::${to_rust_ident(keyword)}
}
% endfor
SpecifiedValue::LengthOrPercentage(value) =>
SpecifiedValue::LengthOrPercentage(ref value) =>
computed_value::T::LengthOrPercentage(value.to_computed_value(context)),
}
}
Expand Down Expand Up @@ -954,7 +954,7 @@ ${helpers.single_keyword("animation-fill-mode",
impl HasViewportPercentage for SpecifiedValue {
fn has_viewport_percentage(&self) -> bool {
match *self {
SpecifiedValue::Repeat(length) => length.has_viewport_percentage(),
SpecifiedValue::Repeat(ref length) => length.has_viewport_percentage(),
_ => false
}
}
Expand All @@ -968,7 +968,7 @@ ${helpers.single_keyword("animation-fill-mode",
pub struct T(pub Option<LengthOrPercentage>);
}

#[derive(Debug, Clone, Copy, PartialEq)]
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub enum SpecifiedValue {
None,
Expand All @@ -979,7 +979,7 @@ ${helpers.single_keyword("animation-fill-mode",
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match self.0 {
None => dest.write_str("none"),
Some(l) => {
Some(ref l) => {
try!(dest.write_str("repeat("));
try!(l.to_css(dest));
dest.write_str(")")
Expand Down Expand Up @@ -1012,7 +1012,7 @@ ${helpers.single_keyword("animation-fill-mode",
fn to_computed_value(&self, context: &Context) -> computed_value::T {
match *self {
SpecifiedValue::None => computed_value::T(None),
SpecifiedValue::Repeat(l) =>
SpecifiedValue::Repeat(ref l) =>
computed_value::T(Some(l.to_computed_value(context))),
}
}
Expand Down Expand Up @@ -1167,12 +1167,12 @@ ${helpers.single_keyword("animation-fill-mode",
impl HasViewportPercentage for SpecifiedOperation {
fn has_viewport_percentage(&self) -> bool {
match *self {
SpecifiedOperation::Translate(_, l1, l2, l3) => {
SpecifiedOperation::Translate(_, ref l1, ref l2, ref l3) => {
l1.has_viewport_percentage() ||
l2.has_viewport_percentage() ||
l3.has_viewport_percentage()
},
SpecifiedOperation::Perspective(length) => length.has_viewport_percentage(),
SpecifiedOperation::Perspective(ref length) => length.has_viewport_percentage(),
_ => false
}
}
Expand All @@ -1183,13 +1183,13 @@ ${helpers.single_keyword("animation-fill-mode",
match *self {
// todo(gw): implement serialization for transform
// types other than translate.
SpecifiedOperation::Matrix(_m) => {
SpecifiedOperation::Matrix(..) => {
Ok(())
}
SpecifiedOperation::Skew(_sx, _sy) => {
SpecifiedOperation::Skew(..) => {
Ok(())
}
SpecifiedOperation::Translate(kind, tx, ty, tz) => {
SpecifiedOperation::Translate(kind, ref tx, ref ty, ref tz) => {
match kind {
TranslateKind::Translate => {
try!(dest.write_str("translate("));
Expand Down Expand Up @@ -1224,13 +1224,13 @@ ${helpers.single_keyword("animation-fill-mode",
}
}
}
SpecifiedOperation::Scale(_sx, _sy, _sz) => {
SpecifiedOperation::Scale(..) => {
Ok(())
}
SpecifiedOperation::Rotate(_ax, _ay, _az, _angle) => {
SpecifiedOperation::Rotate(..) => {
Ok(())
}
SpecifiedOperation::Perspective(_p) => {
SpecifiedOperation::Perspective(_) => {
Ok(())
}
}
Expand Down Expand Up @@ -1525,7 +1525,7 @@ ${helpers.single_keyword("animation-fill-mode",
SpecifiedOperation::Skew(theta_x, theta_y) => {
result.push(computed_value::ComputedOperation::Skew(theta_x, theta_y));
}
SpecifiedOperation::Perspective(d) => {
SpecifiedOperation::Perspective(ref d) => {
result.push(computed_value::ComputedOperation::Perspective(d.to_computed_value(context)));
}
};
Expand Down Expand Up @@ -1674,7 +1674,7 @@ ${helpers.predefined_type("perspective",
}
}

#[derive(Clone, Copy, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SpecifiedValue {
horizontal: LengthOrPercentage,
Expand Down Expand Up @@ -1788,7 +1788,7 @@ ${helpers.single_keyword("transform-style",
}
}

#[derive(Clone, Copy, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SpecifiedValue {
horizontal: LengthOrPercentage,
Expand Down
32 changes: 15 additions & 17 deletions components/style/properties/longhand/effects.mako.rs
View file
Expand Up @@ -150,13 +150,13 @@ ${helpers.predefined_type("opacity",
impl HasViewportPercentage for SpecifiedClipRect {
fn has_viewport_percentage(&self) -> bool {
self.top.has_viewport_percentage() ||
self.right.map_or(false, |x| x.has_viewport_percentage()) ||
self.bottom.map_or(false, |x| x.has_viewport_percentage()) ||
self.right.as_ref().map_or(false, |x| x.has_viewport_percentage()) ||
self.bottom.as_ref().map_or(false, |x| x.has_viewport_percentage()) ||
self.left.has_viewport_percentage()
}
}

#[derive(Clone, Debug, PartialEq, Copy)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SpecifiedClipRect {
pub top: specified::Length,
Expand All @@ -167,12 +167,11 @@ ${helpers.predefined_type("opacity",

impl HasViewportPercentage for SpecifiedValue {
fn has_viewport_percentage(&self) -> bool {
let &SpecifiedValue(clip) = self;
clip.map_or(false, |x| x.has_viewport_percentage())
self.0.as_ref().map_or(false, |x| x.has_viewport_percentage())
}
}

#[derive(Clone, Debug, PartialEq, Copy)]
#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
pub struct SpecifiedValue(Option<SpecifiedClipRect>);

Expand All @@ -183,14 +182,14 @@ ${helpers.predefined_type("opacity",
try!(self.top.to_css(dest));
try!(dest.write_str(", "));

if let Some(right) = self.right {
if let Some(ref right) = self.right {
try!(right.to_css(dest));
try!(dest.write_str(", "));
} else {
try!(dest.write_str("auto, "));
}

if let Some(bottom) = self.bottom {
if let Some(ref bottom) = self.bottom {
try!(bottom.to_css(dest));
try!(dest.write_str(", "));
} else {
Expand Down Expand Up @@ -224,10 +223,10 @@ ${helpers.predefined_type("opacity",

#[inline]
fn to_computed_value(&self, context: &Context) -> computed_value::T {
computed_value::T(self.0.map(|value| computed_value::ClipRect {
computed_value::T(self.0.as_ref().map(|value| computed_value::ClipRect {
top: value.top.to_computed_value(context),
right: value.right.map(|right| right.to_computed_value(context)),
bottom: value.bottom.map(|bottom| bottom.to_computed_value(context)),
right: value.right.as_ref().map(|right| right.to_computed_value(context)),
bottom: value.bottom.as_ref().map(|bottom| bottom.to_computed_value(context)),
left: value.left.to_computed_value(context),
}))
}
Expand Down Expand Up @@ -302,8 +301,7 @@ ${helpers.predefined_type("opacity",

impl HasViewportPercentage for SpecifiedValue {
fn has_viewport_percentage(&self) -> bool {
let &SpecifiedValue(ref vec) = self;
vec.iter().any(|ref x| x.has_viewport_percentage())
self.0.iter().any(|ref x| x.has_viewport_percentage())
}
}

Expand All @@ -314,7 +312,7 @@ ${helpers.predefined_type("opacity",
impl HasViewportPercentage for SpecifiedFilter {
fn has_viewport_percentage(&self) -> bool {
match *self {
SpecifiedFilter::Blur(length) => length.has_viewport_percentage(),
SpecifiedFilter::Blur(ref length) => length.has_viewport_percentage(),
_ => false
}
}
Expand Down Expand Up @@ -439,7 +437,7 @@ ${helpers.predefined_type("opacity",
impl ToCss for computed_value::Filter {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match *self {
computed_value::Filter::Blur(value) => {
computed_value::Filter::Blur(ref value) => {
try!(dest.write_str("blur("));
try!(value.to_css(dest));
try!(dest.write_str(")"));
Expand Down Expand Up @@ -477,7 +475,7 @@ ${helpers.predefined_type("opacity",
impl ToCss for SpecifiedFilter {
fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
match *self {
SpecifiedFilter::Blur(value) => {
SpecifiedFilter::Blur(ref value) => {
try!(dest.write_str("blur("));
try!(value.to_css(dest));
try!(dest.write_str(")"));
Expand Down Expand Up @@ -567,7 +565,7 @@ ${helpers.predefined_type("opacity",
fn to_computed_value(&self, context: &Context) -> computed_value::T {
computed_value::T{ filters: self.0.iter().map(|value| {
match *value {
SpecifiedFilter::Blur(factor) =>
SpecifiedFilter::Blur(ref factor) =>
computed_value::Filter::Blur(factor.to_computed_value(context)),
SpecifiedFilter::Brightness(factor) => computed_value::Filter::Brightness(factor),
SpecifiedFilter::Contrast(factor) => computed_value::Filter::Contrast(factor),
Expand Down
7 changes: 3 additions & 4 deletions components/style/properties/longhand/font.mako.rs
View file
Expand Up @@ -309,8 +309,7 @@ ${helpers.single_keyword("font-variant-caps",

impl HasViewportPercentage for SpecifiedValue {
fn has_viewport_percentage(&self) -> bool {
let &SpecifiedValue(length) = self;
return length.has_viewport_percentage()
return self.0.has_viewport_percentage()
}
}

Expand Down Expand Up @@ -340,13 +339,13 @@ ${helpers.single_keyword("font-variant-caps",
LengthOrPercentage::Length(Length::ServoCharacterWidth(value)) => {
value.to_computed_value(context.inherited_style().get_font().clone_font_size())
}
LengthOrPercentage::Length(l) => {
LengthOrPercentage::Length(ref l) => {
l.to_computed_value(context)
}
LengthOrPercentage::Percentage(Percentage(value)) => {
context.inherited_style().get_font().clone_font_size().scale_by(value)
}
LengthOrPercentage::Calc(calc) => {
LengthOrPercentage::Calc(ref calc) => {
let calc = calc.to_computed_value(context);
calc.length() + context.inherited_style().get_font().clone_font_size()
.scale_by(calc.percentage())
Expand Down
19 changes: 12 additions & 7 deletions components/style/properties/longhand/inherited_table.mako.rs
View file
Expand Up @@ -107,21 +107,26 @@ ${helpers.single_keyword("caption-side", "top bottom",
}

pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
let mut lengths = [ None, None ];
for i in 0..2 {
match specified::Length::parse_non_negative(input) {
Err(()) => break,
Ok(length) => lengths[i] = Some(length),
let mut first = None;
let mut second = None;
match specified::Length::parse_non_negative(input) {
Err(()) => (),
Ok(length) => {
first = Some(length);
match specified::Length::parse_non_negative(input) {
Err(()) => (),
Ok(length) => second = Some(length),
}
}
}
if input.next().is_ok() {
return Err(())
}
match (lengths[0], lengths[1]) {
match (first, second) {
(None, None) => Err(()),
(Some(length), None) => {
Ok(SpecifiedValue {
horizontal: length,
horizontal: length.clone(),
vertical: length,
})
}
Expand Down

0 comments on commit f010fb5

Please sign in to comment.