From b1d9987ccdc006b454d7098018238c97c893714e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Thu, 20 Jul 2017 02:36:15 -0700 Subject: [PATCH] servo: Merge #17788 - style: Cleanup the cascade a good bit (from emilio:clean-cascade); r=heycam Was about the time. Source-Repo: https://github.com/servo/servo Source-Revision: f594ae58a68479af958989aae369a2bfee2b2246 --HG-- extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear extra : subtree_revision : 0c6942502e978107dabbbda199194c1501d9992f --- servo/components/style/gecko/conversions.rs | 19 +- servo/components/style/gecko/media_queries.rs | 4 +- .../components/style/properties/gecko.mako.rs | 6 +- .../style/properties/helpers.mako.rs | 88 ++++----- .../helpers/animated_properties.mako.rs | 18 +- .../style/properties/longhand/box.mako.rs | 6 +- .../style/properties/longhand/color.mako.rs | 4 +- .../style/properties/longhand/font.mako.rs | 95 ++++++---- .../properties/longhand/inherited_box.mako.rs | 4 +- .../longhand/inherited_text.mako.rs | 4 +- .../style/properties/longhand/text.mako.rs | 4 +- .../style/properties/properties.mako.rs | 175 +++++++++++++----- servo/components/style/servo/media_queries.rs | 4 +- servo/components/style/style_adjuster.rs | 9 +- .../style/stylesheets/viewport_rule.rs | 4 +- servo/components/style/values/computed/mod.rs | 46 +++-- .../style/values/specified/color.rs | 6 +- .../style/values/specified/length.rs | 6 +- servo/ports/geckolib/glue.rs | 28 +-- servo/tests/unit/style/parsing/mod.rs | 4 +- 20 files changed, 286 insertions(+), 248 deletions(-) diff --git a/servo/components/style/gecko/conversions.rs b/servo/components/style/gecko/conversions.rs index ea4e4588ee0d..2426bef42dc0 100644 --- a/servo/components/style/gecko/conversions.rs +++ b/servo/components/style/gecko/conversions.rs @@ -138,7 +138,7 @@ impl Angle { impl nsStyleImage { /// Set a given Servo `Image` value into this `nsStyleImage`. - pub fn set(&mut self, image: Image, cacheable: &mut bool) { + pub fn set(&mut self, image: Image) { match image { GenericImage::Gradient(gradient) => { self.set_gradient(gradient) @@ -146,14 +146,6 @@ impl nsStyleImage { GenericImage::Url(ref url) => { unsafe { Gecko_SetLayerImageImageValue(self, url.image_value.clone().unwrap().get()); - // We unfortunately must make any url() value uncacheable, since - // the applicable declarations cache is not per document, but - // global, and the imgRequestProxy objects we store in the style - // structs don't like to be tracked by more than one document. - // - // FIXME(emilio): With the scoped TLS thing this is no longer - // true, remove this line in a follow-up! - *cacheable = false; } }, GenericImage::Rect(ref image_rect) => { @@ -161,15 +153,6 @@ impl nsStyleImage { Gecko_SetLayerImageImageValue(self, image_rect.url.image_value.clone().unwrap().get()); Gecko_InitializeImageCropRect(self); - // We unfortunately must make any url() value uncacheable, since - // the applicable declarations cache is not per document, but - // global, and the imgRequestProxy objects we store in the style - // structs don't like to be tracked by more than one document. - // - // FIXME(emilio): With the scoped TLS thing this is no longer - // true, remove this line in a follow-up! - *cacheable = false; - // Set CropRect let ref mut rect = *self.mCropRect.mPtr; image_rect.top.to_gecko_style_coord(&mut rect.data_at_mut(0)); diff --git a/servo/components/style/gecko/media_queries.rs b/servo/components/style/gecko/media_queries.rs index 573cd0160058..922a5df637fb 100644 --- a/servo/components/style/gecko/media_queries.rs +++ b/servo/components/style/gecko/media_queries.rs @@ -624,9 +624,7 @@ impl Expression { // em units are relative to the initial font-size. let context = computed::Context { is_root_element: false, - device: device, - inherited_style: default_values, - style: StyleBuilder::for_derived_style(device, default_values, None), + builder: StyleBuilder::for_derived_style(device, default_values, None, None), font_metrics_provider: &provider, cached_system_font: None, in_media_query: true, diff --git a/servo/components/style/properties/gecko.mako.rs b/servo/components/style/properties/gecko.mako.rs index cae2495be2e8..9a626f98c747 100644 --- a/servo/components/style/properties/gecko.mako.rs +++ b/servo/components/style/properties/gecko.mako.rs @@ -1189,7 +1189,7 @@ fn static_assert() { } if let Either::Second(image) = image { - self.gecko.mBorderImageSource.set(image, &mut false) + self.gecko.mBorderImageSource.set(image); } } @@ -3423,7 +3423,7 @@ fn static_assert() { } #[allow(unused_variables)] - pub fn set_${shorthand}_image(&mut self, images: I, cacheable: &mut bool) + pub fn set_${shorthand}_image(&mut self, images: I) where I: IntoIterator, I::IntoIter: ExactSizeIterator { @@ -3446,7 +3446,7 @@ fn static_assert() { for (image, geckoimage) in images.zip(self.gecko.${image_layers_field} .mLayers.iter_mut()) { if let Either::Second(image) = image { - geckoimage.mImage.set(image, cacheable) + geckoimage.mImage.set(image) } } } diff --git a/servo/components/style/properties/helpers.mako.rs b/servo/components/style/properties/helpers.mako.rs index f29841cc893b..442b9894f4f6 100644 --- a/servo/components/style/properties/helpers.mako.rs +++ b/servo/components/style/properties/helpers.mako.rs @@ -263,6 +263,7 @@ fn to_computed_value(&self, context: &Context) -> computed_value::T { computed_value::T(self.compute_iter(context).collect()) } + #[inline] fn from_computed_value(computed: &computed_value::T) -> Self { SpecifiedValue(computed.0.iter() @@ -316,12 +317,11 @@ use Atom; ${caller.body()} #[allow(unused_variables)] - pub fn cascade_property(declaration: &PropertyDeclaration, - inherited_style: &ComputedValues, - default_style: &ComputedValues, - context: &mut computed::Context, - cacheable: &mut bool, - cascade_info: &mut Option<<&mut CascadeInfo>) { + pub fn cascade_property( + declaration: &PropertyDeclaration, + context: &mut computed::Context, + cascade_info: &mut Option<<&mut CascadeInfo>, + ) { let value = match *declaration { PropertyDeclaration::${property.camel_case}(ref value) => { DeclaredValue::Value(value) @@ -338,18 +338,8 @@ % if not property.derived_from: if let Some(ref mut cascade_info) = *cascade_info { - cascade_info.on_cascade_property(&declaration, - &value); + cascade_info.on_cascade_property(&declaration, &value); } - % if property.logical: - let wm = context.style.writing_mode; - % endif - <% - maybe_wm = ", wm" if property.logical else "" - maybe_cacheable = ", cacheable" if property.has_uncacheable_values == "True" else "" - props_need_device = "content list_style_type".split() if product == "gecko" else [] - maybe_device = ", context.device" if property.ident in props_need_device else "" - %> match value { DeclaredValue::Value(ref specified_value) => { % if property.ident in SYSTEM_FONT_LONGHANDS and product == "gecko": @@ -358,30 +348,33 @@ } % endif % if property.is_vector: - // In the case of a vector property we want to pass down - // an iterator so that this can be computed without allocation + // In the case of a vector property we want to pass + // down an iterator so that this can be computed + // without allocation // - // However, computing requires a context, but the style struct - // being mutated is on the context. We temporarily remove it, - // mutate it, and then put it back. Vector longhands cannot - // touch their own style struct whilst computing, else this will panic. - let mut s = context.mutate_style().take_${data.current_style_struct.name_lower}(); + // However, computing requires a context, but the + // style struct being mutated is on the context. We + // temporarily remove it, mutate it, and then put it + // back. Vector longhands cannot touch their own + // style struct whilst computing, else this will + // panic. + let mut s = + context.builder.take_${data.current_style_struct.name_lower}(); { let iter = specified_value.compute_iter(context); - s.set_${property.ident}(iter ${maybe_cacheable}); + s.set_${property.ident}(iter); } - context.mutate_style().put_${data.current_style_struct.name_lower}(s); + context.builder.put_${data.current_style_struct.name_lower}(s); % else: let computed = specified_value.to_computed_value(context); - % if property.ident == "font_size": - longhands::font_size::cascade_specified_font_size(context, - specified_value, - computed, - inherited_style.get_font()); + % if property.ident == "font_size": + longhands::font_size::cascade_specified_font_size( + context, + &specified_value, + computed, + ); % else: - context.mutate_style().mutate_${data.current_style_struct.name_lower}() - .set_${property.ident}(computed ${maybe_device} - ${maybe_cacheable} ${maybe_wm}); + context.builder.set_${property.ident}(computed) % endif % endif } @@ -394,41 +387,24 @@ % if property.ident == "font_size": longhands::font_size::cascade_initial_font_size(context); % else: - // We assume that it's faster to use copy_*_from rather than - // set_*(get_initial_value()); - let initial_struct = default_style - .get_${data.current_style_struct.name_lower}(); - context.mutate_style().mutate_${data.current_style_struct.name_lower}() - .copy_${property.ident}_from(initial_struct ${maybe_wm}); + context.builder.reset_${property.ident}(); % endif }, % if data.current_style_struct.inherited: CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { - // This is a bit slow, but this is rare so it shouldn't - // matter. - // - // FIXME: is it still? - *cacheable = false; - let inherited_struct = - inherited_style.get_${data.current_style_struct.name_lower}(); - % if property.ident == "font_size": - longhands::font_size::cascade_inherit_font_size(context, inherited_struct); + longhands::font_size::cascade_inherit_font_size(context); % else: - context.mutate_style().mutate_${data.current_style_struct.name_lower}() - .copy_${property.ident}_from(inherited_struct ${maybe_wm}); + context.builder.inherit_${property.ident}(); % endif } } } % if property.custom_cascade: - cascade_property_custom(declaration, - inherited_style, - context, - cacheable); + cascade_property_custom(declaration, context); % endif % else: // Do not allow stylesheets to set derived properties. @@ -1108,7 +1084,7 @@ % else: if let ${length_type}::ExtremumLength(..) = computed { <% is_height = "true" if "height" in name else "false" %> - if ${is_height} != context.style().writing_mode.is_vertical() { + if ${is_height} != context.builder.writing_mode.is_vertical() { return get_initial_value() } } diff --git a/servo/components/style/properties/helpers/animated_properties.mako.rs b/servo/components/style/properties/helpers/animated_properties.mako.rs index c216201dafc8..b32dd84fa3f2 100644 --- a/servo/components/style/properties/helpers/animated_properties.mako.rs +++ b/servo/components/style/properties/helpers/animated_properties.mako.rs @@ -455,12 +455,7 @@ impl AnimatedProperty { let value: longhands::${prop.ident}::computed_value::T = ToAnimatedValue::from_animated_value(value); % endif - <% method = "style.mutate_" + prop.style_struct.ident.strip("_") + "().set_" + prop.ident %> - % if prop.has_uncacheable_values is "True": - ${method}(value, &mut false); - % else: - ${method}(value); - % endif + style.mutate_${prop.style_struct.name_lower}().set_${prop.ident}(value); } % endif % endfor @@ -562,9 +557,12 @@ impl AnimationValue { } } - /// Construct an AnimationValue from a property declaration - pub fn from_declaration(decl: &PropertyDeclaration, context: &mut Context, - initial: &ComputedValues) -> Option { + /// Construct an AnimationValue from a property declaration. + pub fn from_declaration( + decl: &PropertyDeclaration, + context: &mut Context, + initial: &ComputedValues + ) -> Option { use properties::LonghandId; match *decl { @@ -606,7 +604,7 @@ impl AnimationValue { CSSWideKeyword::Unset | % endif CSSWideKeyword::Inherit => { - let inherit_struct = context.inherited_style + let inherit_struct = context.inherited_style() .get_${prop.style_struct.name_lower}(); inherit_struct.clone_${prop.ident}() }, diff --git a/servo/components/style/properties/longhand/box.mako.rs b/servo/components/style/properties/longhand/box.mako.rs index f2c3c8d517e7..54b79481c81f 100644 --- a/servo/components/style/properties/longhand/box.mako.rs +++ b/servo/components/style/properties/longhand/box.mako.rs @@ -181,9 +181,7 @@ % if product == "servo": fn cascade_property_custom(_declaration: &PropertyDeclaration, - _inherited_style: &ComputedValues, - context: &mut computed::Context, - _cacheable: &mut bool) { + context: &mut computed::Context) { longhands::_servo_display_for_hypothetical_box::derive_from_display(context); longhands::_servo_text_decorations_in_effect::derive_from_display(context); longhands::_servo_under_display_none::derive_from_display(context); @@ -302,7 +300,7 @@ ${helpers.single_keyword("position", "static absolute relative fixed", #[inline] pub fn derive_from_display(context: &mut Context) { let d = context.style().get_box().clone_display(); - context.mutate_style().mutate_box().set__servo_display_for_hypothetical_box(d); + context.builder.set__servo_display_for_hypothetical_box(d); } diff --git a/servo/components/style/properties/longhand/color.mako.rs b/servo/components/style/properties/longhand/color.mako.rs index e3e8221eb8da..6fdd33156d5d 100644 --- a/servo/components/style/properties/longhand/color.mako.rs +++ b/servo/components/style/properties/longhand/color.mako.rs @@ -21,7 +21,7 @@ #[inline] fn to_computed_value(&self, context: &Context) -> computed_value::T { self.0.to_computed_value(context) - .to_rgba(context.inherited_style.get_color().clone_color()) + .to_rgba(context.inherited_style().get_color().clone_color()) } #[inline] @@ -109,7 +109,7 @@ fn to_computed_value(&self, cx: &Context) -> Self::ComputedValue { unsafe { Gecko_GetLookAndFeelSystemColor(*self as i32, - cx.device.pres_context()) + cx.device().pres_context()) } } diff --git a/servo/components/style/properties/longhand/font.mako.rs b/servo/components/style/properties/longhand/font.mako.rs index d5a0180208ed..7543238ee76e 100644 --- a/servo/components/style/properties/longhand/font.mako.rs +++ b/servo/components/style/properties/longhand/font.mako.rs @@ -562,7 +562,6 @@ ${helpers.single_keyword_system("font-variant-caps", allow_quirks="True" spec="https://drafts.csswg.org/css-fonts/#propdef-font-size"> use app_units::Au; use properties::longhands::system_font::SystemFont; - use properties::style_structs::Font; use std::fmt; use style_traits::{HasViewportPercentage, ToCss}; use values::FONT_MEDIUM_PX; @@ -906,11 +905,10 @@ ${helpers.single_keyword_system("font-variant-caps", #[allow(unused_mut)] pub fn cascade_specified_font_size(context: &mut Context, - specified_value: &SpecifiedValue, - mut computed: Au, - parent: &Font) { + specified_value: &SpecifiedValue, + mut computed: Au) { if let SpecifiedValue::Keyword(kw, fraction) = *specified_value { - context.mutate_style().font_size_keyword = Some((kw, fraction)); + context.builder.font_size_keyword = Some((kw, fraction)); } else if let Some(ratio) = specified_value.as_font_ratio() { // In case a font-size-relative value was applied to a keyword // value, we must preserve this fact in case the generic font family @@ -918,13 +916,13 @@ ${helpers.single_keyword_system("font-variant-caps", // recomputed from the base size for the keyword and the relative size. // // See bug 1355707 - if let Some((kw, fraction)) = context.inherited_style().font_computation_data.font_size_keyword { - context.mutate_style().font_size_keyword = Some((kw, fraction * ratio)); + if let Some((kw, fraction)) = context.builder.inherited_style().font_computation_data.font_size_keyword { + context.builder.font_size_keyword = Some((kw, fraction * ratio)); } else { - context.mutate_style().font_size_keyword = None; + context.builder.font_size_keyword = None; } } else { - context.mutate_style().font_size_keyword = None; + context.builder.font_size_keyword = None; } // we could use clone_language and clone_font_family() here but that's @@ -933,61 +931,76 @@ ${helpers.single_keyword_system("font-variant-caps", use gecko_bindings::structs::nsIAtom; // if the language or generic changed, we need to recalculate // the font size from the stored font-size origin information. - if context.style().get_font().gecko().mLanguage.raw::() != - context.inherited_style().get_font().gecko().mLanguage.raw::() || - context.style().get_font().gecko().mGenericID != - context.inherited_style().get_font().gecko().mGenericID { - if let Some((kw, ratio)) = context.style().font_size_keyword { + if context.builder.get_font().gecko().mLanguage.raw::() != + context.builder.inherited_style().get_font().gecko().mLanguage.raw::() || + context.builder.get_font().gecko().mGenericID != + context.builder.inherited_style().get_font().gecko().mGenericID { + if let Some((kw, ratio)) = context.builder.font_size_keyword { computed = kw.to_computed_value(context).scale_by(ratio); } } % endif + let device = context.builder.device; + let mut font = context.builder.take_font(); let parent_unconstrained = { - let (style, device) = context.mutate_style_with_device(); - - style.mutate_font().apply_font_size(computed, parent, device) + let parent_style = context.builder.inherited_style(); + let parent_font = parent_style.get_font(); + font.apply_font_size(computed, parent_font, device) }; - + context.builder.put_font(font); if let Some(parent) = parent_unconstrained { - let new_unconstrained = specified_value - .to_computed_value_against(context, FontBaseSize::Custom(parent)); - context.mutate_style() + let new_unconstrained = + specified_value + .to_computed_value_against(context, FontBaseSize::Custom(parent)); + context.builder .mutate_font() .apply_unconstrained_font_size(new_unconstrained); } } - pub fn cascade_inherit_font_size(context: &mut Context, parent: &Font) { - // If inheriting, we must recompute font-size in case of language changes - // using the font_size_keyword. We also need to do this to handle - // mathml scriptlevel changes - let kw_inherited_size = context.style().font_size_keyword.map(|(kw, ratio)| { + /// FIXME(emilio): This is very complex. Also, it should move to + /// StyleBuilder. + pub fn cascade_inherit_font_size(context: &mut Context) { + // If inheriting, we must recompute font-size in case of language + // changes using the font_size_keyword. We also need to do this to + // handle mathml scriptlevel changes + let kw_inherited_size = context.builder.font_size_keyword.map(|(kw, ratio)| { SpecifiedValue::Keyword(kw, ratio).to_computed_value(context) }); - let parent_kw = context.inherited_style.font_computation_data.font_size_keyword; - let (style, device) = context.mutate_style_with_device(); - let used_kw = style.mutate_font() - .inherit_font_size_from(parent, kw_inherited_size, device); - if used_kw { - style.font_size_keyword = parent_kw; - } else { - style.font_size_keyword = None; - } + let parent_kw; + let device = context.builder.device; + let mut font = context.builder.take_font(); + let used_kw = { + let parent_style = context.builder.inherited_style(); + let parent_font = parent_style.get_font(); + parent_kw = parent_style.font_computation_data.font_size_keyword; + + font.inherit_font_size_from(parent_font, kw_inherited_size, device) + }; + context.builder.put_font(font); + context.builder.font_size_keyword = + if used_kw { parent_kw } else { None }; } + /// Cascade the initial value for the `font-size` property. + /// + /// FIXME(emilio): This is the only function that is outside of the + /// `StyleBuilder`, and should really move inside! + /// + /// Can we move the font stuff there? pub fn cascade_initial_font_size(context: &mut Context) { // font-size's default ("medium") does not always // compute to the same value and depends on the font let computed = longhands::font_size::get_initial_specified_value() .to_computed_value(context); - let (style, _device) = context.mutate_style_with_device(); - style.mutate_font().set_font_size(computed); + context.builder.mutate_font().set_font_size(computed); % if product == "gecko": - style.mutate_font().fixup_font_min_size(_device); + let device = context.builder.device; + context.builder.mutate_font().fixup_font_min_size(device); % endif - style.font_size_keyword = Some((Default::default(), 1.)); + context.builder.font_size_keyword = Some((Default::default(), 1.)); } @@ -2414,8 +2427,8 @@ ${helpers.single_keyword("-moz-math-variant", bindings::Gecko_nsFont_InitSystem( &mut system, id as i32, - cx.style.get_font().gecko(), - cx.device.pres_context() + cx.style().get_font().gecko(), + cx.device().pres_context() ) } let family = system.fontlist.mFontlist.iter().map(|font| { diff --git a/servo/components/style/properties/longhand/inherited_box.mako.rs b/servo/components/style/properties/longhand/inherited_box.mako.rs index 56a11c0ce133..2a8bff1a1206 100644 --- a/servo/components/style/properties/longhand/inherited_box.mako.rs +++ b/servo/components/style/properties/longhand/inherited_box.mako.rs @@ -285,8 +285,8 @@ ${helpers.single_keyword("image-rendering", use super::display::computed_value::T as Display; if context.style().get_box().clone_display() == Display::none { - context.mutate_style().mutate_inheritedbox() - .set__servo_under_display_none(SpecifiedValue(true)); + context.builder + .set__servo_under_display_none(SpecifiedValue(true)); } } diff --git a/servo/components/style/properties/longhand/inherited_text.mako.rs b/servo/components/style/properties/longhand/inherited_text.mako.rs index a544c4a97369..f114344ae525 100644 --- a/servo/components/style/properties/longhand/inherited_text.mako.rs +++ b/servo/components/style/properties/longhand/inherited_text.mako.rs @@ -350,13 +350,13 @@ ${helpers.predefined_type("word-spacing", #[inline] pub fn derive_from_text_decoration(context: &mut Context) { let derived = derive(context); - context.mutate_style().mutate_inheritedtext().set__servo_text_decorations_in_effect(derived); + context.builder.set__servo_text_decorations_in_effect(derived); } #[inline] pub fn derive_from_display(context: &mut Context) { let derived = derive(context); - context.mutate_style().mutate_inheritedtext().set__servo_text_decorations_in_effect(derived); + context.builder.set__servo_text_decorations_in_effect(derived); } diff --git a/servo/components/style/properties/longhand/text.mako.rs b/servo/components/style/properties/longhand/text.mako.rs index 3be23f1681e3..721fd2ad6f13 100644 --- a/servo/components/style/properties/longhand/text.mako.rs +++ b/servo/components/style/properties/longhand/text.mako.rs @@ -249,9 +249,7 @@ ${helpers.single_keyword("unicode-bidi", % if product == "servo": fn cascade_property_custom(_declaration: &PropertyDeclaration, - _inherited_style: &ComputedValues, - context: &mut computed::Context, - _cacheable: &mut bool) { + context: &mut computed::Context) { longhands::_servo_text_decorations_in_effect::derive_from_text_decoration(context); } % endif diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index cbaa62b7fc42..5db0ec71ed40 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -2453,12 +2453,31 @@ impl<'a, T: 'a> ops::Deref for StyleStructRef<'a, T> { /// actually cloning them, until we either build the style, or mutate the /// inherited value. pub struct StyleBuilder<'a> { - device: &'a Device, + /// The device we're using to compute style. + /// + /// This provides access to viewport unit ratios, etc. + pub device: &'a Device, + + /// The style we're inheriting from. + /// + /// This is effectively + /// `parent_style.unwrap_or(device.default_computed_values())`. + inherited_style: &'a ComputedValues, + + /// The style we're getting reset structs from. + reset_style: &'a ComputedValues, + + /// The style we're inheriting from explicitly, or none if we're the root of + /// a subtree. parent_style: Option<<&'a ComputedValues>, + + /// The pseudo-element this style will represent. pseudo: Option<<&'a PseudoElement>, + /// The rule node representing the ordered list of rules matched for this /// node. rules: Option, + custom_properties: Option>, /// The writing mode flags. /// @@ -2482,7 +2501,6 @@ impl<'a> StyleBuilder<'a> { fn new( device: &'a Device, parent_style: Option<<&'a ComputedValues>, - reset_style: &'a ComputedValues, pseudo: Option<<&'a PseudoElement>, cascade_flags: CascadeFlags, rules: Option, @@ -2492,6 +2510,7 @@ impl<'a> StyleBuilder<'a> { flags: ComputedValueFlags, visited_style: Option>, ) -> Self { + let reset_style = device.default_computed_values(); let inherited_style = parent_style.unwrap_or(reset_style); let reset_style = if cascade_flags.contains(INHERIT_ALL) { inherited_style @@ -2502,6 +2521,8 @@ impl<'a> StyleBuilder<'a> { StyleBuilder { device, parent_style, + inherited_style, + reset_style, pseudo, rules, custom_properties, @@ -2523,18 +2544,87 @@ impl<'a> StyleBuilder<'a> { /// order to create a derived style. pub fn for_derived_style( device: &'a Device, - s: &'a ComputedValues, + style_to_derive_from: &'a ComputedValues, + parent_style: Option<<&'a ComputedValues>, pseudo: Option<<&'a PseudoElement>, ) -> Self { - Self::for_inheritance(device, s, s, pseudo) + let reset_style = device.default_computed_values(); + let inherited_style = parent_style.unwrap_or(reset_style); + StyleBuilder { + device, + parent_style, + inherited_style, + reset_style, + pseudo, + rules: None, // FIXME(emilio): Dubious... + custom_properties: style_to_derive_from.custom_properties(), + writing_mode: style_to_derive_from.writing_mode, + font_size_keyword: style_to_derive_from.font_computation_data.font_size_keyword, + flags: style_to_derive_from.flags, + visited_style: style_to_derive_from.clone_visited_style(), + % for style_struct in data.active_style_structs(): + ${style_struct.ident}: StyleStructRef::Borrowed( + style_to_derive_from.${style_struct.name_lower}_arc() + ), + % endfor + } } + % for property in data.longhands: + % if property.ident != "font_size": + /// Inherit `${property.ident}` from our parent style. + #[allow(non_snake_case)] + pub fn inherit_${property.ident}(&mut self) { + let inherited_struct = + self.inherited_style.get_${property.style_struct.name_lower}(); + self.${property.style_struct.ident}.mutate() + .copy_${property.ident}_from( + inherited_struct, + % if property.logical: + self.writing_mode, + % endif + ); + } + + /// Reset `${property.ident}` to the initial value. + #[allow(non_snake_case)] + pub fn reset_${property.ident}(&mut self) { + let reset_struct = self.reset_style.get_${property.style_struct.name_lower}(); + self.${property.style_struct.ident}.mutate() + .copy_${property.ident}_from( + reset_struct, + % if property.logical: + self.writing_mode, + % endif + ); + } + + % if not property.is_vector: + /// Set the `${property.ident}` to the computed value `value`. + #[allow(non_snake_case)] + pub fn set_${property.ident}( + &mut self, + value: longhands::${property.ident}::computed_value::T + ) { + self.${property.style_struct.ident}.mutate() + .set_${property.ident}( + value, + % if property.logical: + self.writing_mode, + % elif product == "gecko" and property.ident in ["content", "list_style_type"]: + self.device, + % endif + ); + } + % endif + % endif + % endfor + /// Inherits style from the parent element, accounting for the default /// computed values that need to be provided as well. pub fn for_inheritance( device: &'a Device, parent: &'a ComputedValues, - reset: &'a ComputedValues, pseudo: Option<<&'a PseudoElement>, ) -> Self { // FIXME(emilio): This Some(parent) here is inconsistent with what we @@ -2543,7 +2633,6 @@ impl<'a> StyleBuilder<'a> { Self::new( device, Some(parent), - reset, pseudo, CascadeFlags::empty(), /* rules = */ None, @@ -2555,6 +2644,15 @@ impl<'a> StyleBuilder<'a> { ) } + /// Returns the style we're inheriting from. + pub fn inherited_style(&self) -> &'a ComputedValues { + self.inherited_style + } + + /// Returns the style we're getting reset properties from. + pub fn default_style(&self) -> &'a ComputedValues { + self.reset_style + } % for style_struct in data.active_style_structs(): /// Gets an immutable view of the current `${style_struct.name}` style. @@ -2585,9 +2683,9 @@ impl<'a> StyleBuilder<'a> { } /// Reset the current `${style_struct.name}` style to its default value. - pub fn reset_${style_struct.name_lower}(&mut self, default: &'a ComputedValuesInner) { + pub fn reset_${style_struct.name_lower}_struct(&mut self) { self.${style_struct.ident} = - StyleStructRef::Borrowed(default.${style_struct.name_lower}_arc()); + StyleStructRef::Borrowed(self.reset_style.${style_struct.name_lower}_arc()); } % endfor @@ -2685,10 +2783,7 @@ mod lazy_static_module { /// A per-longhand function that performs the CSS cascade for that longhand. pub type CascadePropertyFn = extern "Rust" fn(declaration: &PropertyDeclaration, - inherited_style: &ComputedValues, - default_style: &ComputedValues, context: &mut computed::Context, - cacheable: &mut bool, cascade_info: &mut Option<<&mut CascadeInfo>); /// A per-longhand array of functions to perform the CSS cascade on each of @@ -2831,7 +2926,6 @@ where } }; - let default_style = device.default_computed_values(); let inherited_custom_properties = inherited_style.custom_properties(); let mut custom_properties = None; let mut seen_custom = HashSet::new(); @@ -2849,15 +2943,12 @@ where let mut context = computed::Context { is_root_element: flags.contains(IS_ROOT_ELEMENT), - device: device, - inherited_style: inherited_style, // We'd really like to own the rules here to avoid refcount traffic, but // animation's usage of `apply_declarations` make this tricky. See bug // 1375525. - style: StyleBuilder::new( + builder: StyleBuilder::new( device, parent_style, - device.default_computed_values(), pseudo, flags, Some(rules.clone()), @@ -2883,10 +2974,6 @@ where // Set computed values, overwriting earlier declarations for the same // property. - // - // NB: The cacheable boolean is not used right now, but will be once we - // start caching computed values in the rule nodes. - let mut cacheable = true; let mut seen = LonghandIdSet::new(); // Declaration blocks are stored in increasing precedence order, we want @@ -2909,7 +2996,7 @@ where PropertyDeclaration::WithVariables(id, ref unparsed) => { Cow::Owned(unparsed.substitute_variables( id, - &context.style.custom_properties, + &context.builder.custom_properties, context.quirks_mode )) } @@ -2983,15 +3070,12 @@ where let discriminant = longhand_id as usize; (CASCADE_PROPERTY[discriminant])(&*declaration, - inherited_style, - default_style, &mut context, - &mut cacheable, &mut cascade_info); } % if category_to_cascade_now == "early": - let writing_mode = get_writing_mode(context.style.get_inheritedbox()); - context.style.writing_mode = writing_mode; + let writing_mode = get_writing_mode(context.builder.get_inheritedbox()); + context.builder.writing_mode = writing_mode; let mut _skip_font_family = false; @@ -3027,13 +3111,14 @@ where // which Gecko just does regular cascading with. Do the same. // This can only happen in the case where the language changed but the family did not if generic != structs::kGenericFont_NONE { - let gecko_font = context.style.mutate_font().gecko_mut(); + let pres_context = context.builder.device.pres_context(); + let gecko_font = context.builder.mutate_font().gecko_mut(); gecko_font.mGenericID = generic; unsafe { bindings::Gecko_nsStyleFont_PrefillDefaultForGeneric( gecko_font, - context.device.pres_context(), - generic + pres_context, + generic, ); } } @@ -3056,13 +3141,11 @@ where let discriminant = LonghandId::FontFamily as usize; (CASCADE_PROPERTY[discriminant])(declaration, - inherited_style, - default_style, &mut context, - &mut cacheable, &mut cascade_info); % if product == "gecko": - context.style.mutate_font().fixup_none_generic(context.device); + let device = context.builder.device; + context.builder.mutate_font().fixup_none_generic(device); % endif } } @@ -3070,10 +3153,7 @@ where if let Some(ref declaration) = font_size { let discriminant = LonghandId::FontSize as usize; (CASCADE_PROPERTY[discriminant])(declaration, - inherited_style, - default_style, &mut context, - &mut cacheable, &mut cascade_info); % if product == "gecko": // Font size must be explicitly inherited to handle lang changes and @@ -3087,33 +3167,26 @@ where LonghandId::FontSize, CSSWideKeyword::Inherit); (CASCADE_PROPERTY[discriminant])(&size, - inherited_style, - default_style, &mut context, - &mut cacheable, &mut cascade_info); % endif } % endif % endfor - let mut style = context.style; + let mut builder = context.builder; { - StyleAdjuster::new(&mut style) - .adjust( - layout_parent_style, - context.device.default_computed_values(), - flags - ); + StyleAdjuster::new(&mut builder) + .adjust(layout_parent_style, flags); } % if product == "gecko": - if let Some(ref mut bg) = style.get_background_if_mutated() { + if let Some(ref mut bg) = builder.get_background_if_mutated() { bg.fill_arrays(); } - if let Some(ref mut svg) = style.get_svg_if_mutated() { + if let Some(ref mut svg) = builder.get_svg_if_mutated() { svg.fill_arrays(); } % endif @@ -3123,11 +3196,11 @@ where seen.contains(LonghandId::FontWeight) || seen.contains(LonghandId::FontStretch) || seen.contains(LonghandId::FontFamily) { - style.mutate_font().compute_font_hash(); + builder.mutate_font().compute_font_hash(); } % endif - style.build() + builder.build() } /// See StyleAdjuster::adjust_for_border_width. @@ -3136,7 +3209,7 @@ pub fn adjust_border_width(style: &mut StyleBuilder) { // Like calling to_computed_value, which wouldn't type check. if style.get_border().clone_border_${side}_style().none_or_hidden() && style.get_border().border_${side}_has_nonzero_width() { - style.mutate_border().set_border_${side}_width(Au(0)); + style.set_border_${side}_width(Au(0)); } % endfor } diff --git a/servo/components/style/servo/media_queries.rs b/servo/components/style/servo/media_queries.rs index 3570fb1d66b3..4c56860d2cd8 100644 --- a/servo/components/style/servo/media_queries.rs +++ b/servo/components/style/servo/media_queries.rs @@ -229,9 +229,7 @@ impl Range { // em units are relative to the initial font-size. let context = computed::Context { is_root_element: false, - device: device, - inherited_style: default_values, - style: StyleBuilder::for_derived_style(device, default_values, None), + builder: StyleBuilder::for_derived_style(device, default_values, None, None), // Servo doesn't support font metrics // A real provider will be needed here once we do; since // ch units can exist in media queries. diff --git a/servo/components/style/style_adjuster.rs b/servo/components/style/style_adjuster.rs index 805a7063cb1b..b8e234085405 100644 --- a/servo/components/style/style_adjuster.rs +++ b/servo/components/style/style_adjuster.rs @@ -387,7 +387,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { #[cfg(feature = "gecko")] fn adjust_for_ruby(&mut self, layout_parent_style: &ComputedValues, - default_computed_values: &'b ComputedValues, flags: CascadeFlags) { use properties::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP; use properties::computed_value_flags::SHOULD_SUPPRESS_LINEBREAK; @@ -408,8 +407,8 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { // how border and padding should be handled for ruby level container, // and suppressing them here make it easier for layout to handle. if self_display.is_ruby_level_container() { - self.style.reset_border(default_computed_values); - self.style.reset_padding(default_computed_values); + self.style.reset_border_struct(); + self.style.reset_padding_struct(); } // Force bidi isolation on all internal ruby boxes and ruby container // per spec https://drafts.csswg.org/css-ruby-1/#bidi @@ -432,7 +431,6 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { /// `nsStyleContext::ApplyStyleFixups`. pub fn adjust(&mut self, layout_parent_style: &ComputedValues, - _default_computed_values: &'b ComputedValues, flags: CascadeFlags) { #[cfg(feature = "gecko")] { @@ -459,8 +457,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> { self.adjust_for_text_decoration_lines(layout_parent_style); #[cfg(feature = "gecko")] { - self.adjust_for_ruby(layout_parent_style, - _default_computed_values, flags); + self.adjust_for_ruby(layout_parent_style, flags); } } } diff --git a/servo/components/style/stylesheets/viewport_rule.rs b/servo/components/style/stylesheets/viewport_rule.rs index 965929f79b18..015eb33a4f66 100644 --- a/servo/components/style/stylesheets/viewport_rule.rs +++ b/servo/components/style/stylesheets/viewport_rule.rs @@ -704,9 +704,7 @@ impl MaybeNew for ViewportConstraints { let context = Context { is_root_element: false, - device: device, - inherited_style: default_values, - style: StyleBuilder::for_derived_style(device, default_values, None), + builder: StyleBuilder::for_derived_style(device, default_values, None, None), font_metrics_provider: &provider, cached_system_font: None, in_media_query: false, diff --git a/servo/components/style/values/computed/mod.rs b/servo/components/style/values/computed/mod.rs index 329a83b2d0bb..b8a1f1bcb8ac 100644 --- a/servo/components/style/values/computed/mod.rs +++ b/servo/components/style/values/computed/mod.rs @@ -68,16 +68,10 @@ pub struct Context<'a> { /// Whether the current element is the root element. pub is_root_element: bool, - /// The Device holds the viewport and other external state. - pub device: &'a Device, - - /// The style we're inheriting from. - pub inherited_style: &'a ComputedValues, - /// Values accessed through this need to be in the properties "computed /// early": color, text-decoration, font-size, display, position, float, /// border-*-style, outline-style, font-family, writing-mode... - pub style: StyleBuilder<'a>, + pub builder: StyleBuilder<'a>, /// A cached computed system font value, for use by gecko. /// @@ -105,18 +99,34 @@ pub struct Context<'a> { impl<'a> Context<'a> { /// Whether the current element is the root element. - pub fn is_root_element(&self) -> bool { self.is_root_element } + pub fn is_root_element(&self) -> bool { + self.is_root_element + } + + /// The current device. + pub fn device(&self) -> &Device { + self.builder.device + } + /// The current viewport size. - pub fn viewport_size(&self) -> Size2D { self.device.au_viewport_size() } + pub fn viewport_size(&self) -> Size2D { + self.builder.device.au_viewport_size() + } + /// The style we're inheriting from. - pub fn inherited_style(&self) -> &ComputedValues { &self.inherited_style } - /// The current style. Note that only "eager" properties should be accessed - /// from here, see the comment in the member. - pub fn style(&self) -> &StyleBuilder { &self.style } - /// A mutable reference to the current style. - pub fn mutate_style(&mut self) -> &mut StyleBuilder<'a> { &mut self.style } - /// Get a mutable reference to the current style as well as the device - pub fn mutate_style_with_device(&mut self) -> (&mut StyleBuilder<'a>, &Device) { (&mut self.style, &self.device) } + pub fn inherited_style(&self) -> &ComputedValues { + self.builder.inherited_style() + } + + /// The default computed style we're getting our reset style from. + pub fn default_style(&self) -> &ComputedValues { + self.builder.default_style() + } + + /// The current style. + pub fn style(&self) -> &StyleBuilder { + &self.builder + } } /// An iterator over a slice of computed values @@ -395,7 +405,7 @@ impl ToComputedValue for specified::JustifyItems { // If the inherited value of `justify-items` includes the `legacy` keyword, `auto` computes // to the inherited value. if self.0 == align::ALIGN_AUTO { - let inherited = context.inherited_style.get_position().clone_justify_items(); + let inherited = context.inherited_style().get_position().clone_justify_items(); if inherited.0.contains(align::ALIGN_LEGACY) { return inherited } diff --git a/servo/components/style/values/specified/color.rs b/servo/components/style/values/specified/color.rs index 46539ed10fb6..c5705ce40270 100644 --- a/servo/components/style/values/specified/color.rs +++ b/servo/components/style/values/specified/color.rs @@ -254,7 +254,7 @@ impl ToComputedValue for Color { #[cfg(feature = "gecko")] Color::Special(special) => { use self::gecko::SpecialColorKeyword as Keyword; - let pres_context = _context.device.pres_context(); + let pres_context = _context.device().pres_context(); convert_nscolor_to_computedcolor(match special { Keyword::MozDefaultColor => pres_context.mDefaultColor, Keyword::MozDefaultBackgroundColor => pres_context.mBackgroundColor, @@ -268,7 +268,7 @@ impl ToComputedValue for Color { use dom::TElement; use gecko::wrapper::GeckoElement; use gecko_bindings::bindings::Gecko_GetBody; - let pres_context = _context.device.pres_context(); + let pres_context = _context.device().pres_context(); let body = unsafe { Gecko_GetBody(pres_context) }; @@ -316,7 +316,7 @@ impl ToComputedValue for RGBAColor { fn to_computed_value(&self, context: &Context) -> RGBA { self.0.to_computed_value(context) - .to_rgba(context.style.get_color().clone_color()) + .to_rgba(context.style().get_color().clone_color()) } fn from_computed_value(computed: &RGBA) -> Self { diff --git a/servo/components/style/values/specified/length.rs b/servo/components/style/values/specified/length.rs index 711d4167a37d..3ec01f065455 100644 --- a/servo/components/style/values/specified/length.rs +++ b/servo/components/style/values/specified/length.rs @@ -107,7 +107,7 @@ impl FontRelativeLength { reference_font_size, context.style().writing_mode, context.in_media_query, - context.device) + context.device()) } let reference_font_size = base_size.resolve(context); @@ -158,7 +158,7 @@ impl FontRelativeLength { if context.is_root_element { reference_font_size.scale_by(length) } else { - context.device.root_font_size().scale_by(length) + context.device().root_font_size().scale_by(length) } } } @@ -362,7 +362,7 @@ impl PhysicalLength { const MM_PER_INCH: f32 = 25.4; let physical_inch = unsafe { - bindings::Gecko_GetAppUnitsPerPhysicalInch(context.device.pres_context()) + bindings::Gecko_GetAppUnitsPerPhysicalInch(context.device().pres_context()) }; let inch = self.0 / MM_PER_INCH; diff --git a/servo/ports/geckolib/glue.rs b/servo/ports/geckolib/glue.rs index b7c909079648..6d71fdf7a725 100644 --- a/servo/ports/geckolib/glue.rs +++ b/servo/ports/geckolib/glue.rs @@ -1689,7 +1689,6 @@ fn get_pseudo_style( StyleBuilder::for_inheritance( doc_data.stylist.device(), styles.primary(), - doc_data.default_computed_values(), Some(pseudo), ).build() })) @@ -1712,7 +1711,6 @@ pub extern "C" fn Servo_ComputedValues_Inherit( let mut style = StyleBuilder::for_inheritance( data.stylist.device(), reference, - data.default_computed_values(), Some(&pseudo) ); @@ -1727,6 +1725,7 @@ pub extern "C" fn Servo_ComputedValues_Inherit( StyleBuilder::for_derived_style( data.stylist.device(), data.default_computed_values(), + /* parent_style = */ None, Some(&pseudo), ).build() }; @@ -2886,20 +2885,21 @@ fn simulate_compute_values_failure(_: &PropertyValuePair) -> bool { false } -fn create_context<'a>(per_doc_data: &'a PerDocumentStyleDataImpl, - font_metrics_provider: &'a FontMetricsProvider, - style: &'a ComputedValues, - parent_style: Option<&'a ComputedValues>, - pseudo: Option<&'a PseudoElement>) - -> Context<'a> { - let default_values = per_doc_data.default_computed_values(); - let inherited_style = parent_style.unwrap_or(default_values); - +fn create_context<'a>( + per_doc_data: &'a PerDocumentStyleDataImpl, + font_metrics_provider: &'a FontMetricsProvider, + style: &'a ComputedValues, + parent_style: Option<&'a ComputedValues>, + pseudo: Option<&'a PseudoElement>, +) -> Context<'a> { Context { is_root_element: false, - device: per_doc_data.stylist.device(), - inherited_style: inherited_style, - style: StyleBuilder::for_derived_style(per_doc_data.stylist.device(), style, pseudo), + builder: StyleBuilder::for_derived_style( + per_doc_data.stylist.device(), + style, + parent_style, + pseudo, + ), font_metrics_provider: font_metrics_provider, cached_system_font: None, in_media_query: false, diff --git a/servo/tests/unit/style/parsing/mod.rs b/servo/tests/unit/style/parsing/mod.rs index ab1038de1d1c..bf3f1c12e085 100644 --- a/servo/tests/unit/style/parsing/mod.rs +++ b/servo/tests/unit/style/parsing/mod.rs @@ -54,9 +54,7 @@ fn assert_computed_serialization(f: F, input: &'static str, output: &st let context = Context { is_root_element: true, - device: &device, - inherited_style: initial_style, - style: StyleBuilder::for_derived_style(&device, initial_style, None), + builder: StyleBuilder::for_derived_style(&device, initial_style, None, None), cached_system_font: None, font_metrics_provider: &ServoMetricsProvider, in_media_query: false,