Move HAS_TEXT_DECORATION_LINES setting into StyleAdjuster. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 04 Jul 2017 16:20:25 +1000
changeset 605029 554c4ce6ae3d19f78e932a03460b6479789771f9
parent 605028 352b86bbce444683c86758abdaa72cb1bd5dea39
child 605030 f169ba8d16d0d8df6eeb53b0318164aa499009f2
push id67276
push userxquan@mozilla.com
push dateThu, 06 Jul 2017 22:38:30 +0000
reviewersheycam
milestone56.0a1
Move HAS_TEXT_DECORATION_LINES setting into StyleAdjuster. r=heycam This also changes how it is computed. Originally, it is computed based on the parent style, but this commit changes it to be computed based on layout parent style, like most other things in StyleAdjuster. It is actually more correct to use layout parent style, because Gecko computes text decoration lines based on frame. Setting this flag on a style context which doesn't need it wouldn't cause any visual issue. It would only cause some wasteful computation in layout. MozReview-Commit-ID: 29PQPud3zhp
servo/components/style/properties/computed_value_flags.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/style_adjuster.rs
--- a/servo/components/style/properties/computed_value_flags.rs
+++ b/servo/components/style/properties/computed_value_flags.rs
@@ -1,50 +1,22 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 //! Misc information about a given computed style.
 
-use properties::{ComputedValues, StyleBuilder};
-
 bitflags! {
     /// Misc information about a given computed style.
     ///
     /// All flags are currently inherited for text, pseudo elements, and
     /// anonymous boxes, see StyleBuilder::for_inheritance and its callsites.
     /// If we ever want to add some flags that shouldn't inherit for them,
     /// we might want to add a function to handle this.
     pub flags ComputedValueFlags: u8 {
         /// Whether the style or any of the ancestors has a text-decoration-line
         /// property that should get propagated to descendants.
         ///
         /// text-decoration-line is a reset property, but gets propagated in the
         /// frame/box tree.
         const HAS_TEXT_DECORATION_LINES = 1 << 0,
     }
 }
-
-impl ComputedValueFlags {
-    /// Get the computed value flags for the initial style.
-    pub fn initial() -> Self {
-        Self::empty()
-    }
-
-    /// Compute the flags for this style, given the parent style.
-    pub fn compute(
-        this_style: &StyleBuilder,
-        parent_style: &ComputedValues,
-    ) -> Self {
-        let mut ret = Self::empty();
-
-        // FIXME(emilio): This feels like it wants to look at the
-        // layout_parent_style, but the way it works in Gecko means it's not
-        // needed (we'd recascade a bit more when it changes, but that's fine),
-        // and this way it simplifies the code for text styles and similar.
-        if parent_style.flags.contains(HAS_TEXT_DECORATION_LINES) ||
-           !this_style.get_text().clone_text_decoration_line().is_empty() {
-            ret.insert(HAS_TEXT_DECORATION_LINES);
-        }
-
-        ret
-    }
-}
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -120,17 +120,17 @@ impl ComputedValues {
         }
     }
 
     pub fn default_values(pres_context: RawGeckoPresContextBorrowed) -> Arc<Self> {
         Arc::new(ComputedValues {
             custom_properties: None,
             writing_mode: WritingMode::empty(), // FIXME(bz): This seems dubious
             font_computation_data: FontComputationData::default_values(),
-            flags: ComputedValueFlags::initial(),
+            flags: ComputedValueFlags::empty(),
             rules: None,
             visited_style: None,
             % for style_struct in data.style_structs:
                 ${style_struct.ident}: style_structs::${style_struct.name}::default(pres_context),
             % endfor
         })
     }
 
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2309,28 +2309,28 @@ impl<'a, T: 'a> Deref for StyleStructRef
 }
 
 /// A type used to compute a struct with minimal overhead.
 ///
 /// This allows holding references to the parent/default computed values without
 /// actually cloning them, until we either build the style, or mutate the
 /// inherited value.
 pub struct StyleBuilder<'a> {
-    /// The style we're inheriting from.
-    inherited_style: &'a ComputedValues,
     /// The rule node representing the ordered list of rules matched for this
     /// node.
     rules: Option<StrongRuleNode>,
     custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
     /// The writing mode flags.
     ///
     /// TODO(emilio): Make private.
     pub writing_mode: WritingMode,
     /// The keyword behind the current font-size property, if any.
     pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
+    /// Flags for the computed value.
+    pub flags: ComputedValueFlags,
     /// The element's style if visited, only computed if there's a relevant link
     /// for this element.  A element's "relevant link" is the element being
     /// matched if it is a link or the nearest ancestor link.
     visited_style: Option<Arc<ComputedValues>>,
     % for style_struct in data.active_style_structs():
         ${style_struct.ident}: StyleStructRef<'a, style_structs::${style_struct.name}>,
     % endfor
 }
@@ -2343,17 +2343,16 @@ impl<'a> StyleBuilder<'a> {
         rules: Option<StrongRuleNode>,
         custom_properties: Option<Arc<::custom_properties::CustomPropertiesMap>>,
         writing_mode: WritingMode,
         font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
         flags: ComputedValueFlags,
         visited_style: Option<Arc<ComputedValues>>,
     ) -> Self {
         StyleBuilder {
-            inherited_style,
             rules,
             custom_properties,
             writing_mode,
             font_size_keyword,
             flags,
             visited_style,
             % for style_struct in data.active_style_structs():
             % if style_struct.inherited:
@@ -2443,21 +2442,20 @@ impl<'a> StyleBuilder<'a> {
     pub fn in_top_layer(&self) -> bool {
         matches!(self.get_box().clone__moz_top_layer(),
                  longhands::_moz_top_layer::computed_value::T::top)
     }
 
 
     /// Turns this `StyleBuilder` into a proper `ComputedValues` instance.
     pub fn build(self) -> ComputedValues {
-        let flags = ComputedValueFlags::compute(&self, &self.inherited_style);
         ComputedValues::new(self.custom_properties,
                             self.writing_mode,
                             self.font_size_keyword,
-                            flags,
+                            self.flags,
                             self.rules,
                             self.visited_style,
                             % for style_struct in data.active_style_structs():
                             self.${style_struct.ident}.build(),
                             % endfor
         )
     }
 
@@ -2492,17 +2490,17 @@ mod lazy_static_module {
                     % endfor
                     % if style_struct.name == "Font":
                         hash: 0,
                     % endif
                 }),
             % endfor
             custom_properties: None,
             writing_mode: WritingMode::empty(),
-            flags: ComputedValueFlags::initial(),
+            flags: ComputedValueFlags::empty(),
             font_computation_data: FontComputationData::default_values(),
             rules: None,
             visited_style: None,
         };
     }
 }
 
 /// A per-longhand function that performs the CSS cascade for that longhand.
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -1,14 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-//! A struct to encapsulate all the style fixups a computed style needs in order
-//! for it to adhere to the CSS spec.
+//! A struct to encapsulate all the style fixups and flags propagations
+//! a computed style needs in order for it to adhere to the CSS spec.
 
 use app_units::Au;
 use properties::{self, CascadeFlags, ComputedValues};
 use properties::{IS_ROOT_ELEMENT, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, StyleBuilder};
 use properties::longhands::display::computed_value::T as display;
 use properties::longhands::float::computed_value::T as float;
 use properties::longhands::overflow_x::computed_value::T as overflow;
 use properties::longhands::position::computed_value::T as position;
@@ -307,16 +307,25 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
            text_align::_moz_center |
            text_align::_moz_right => {}
            _ => return,
        }
 
        self.style.mutate_inheritedtext().set_text_align(text_align::start);
     }
 
+    /// Set the HAS_TEXT_DECORATION_LINES flag based on parent style.
+    fn adjust_for_text_decoration_lines(&mut self, layout_parent_style: &ComputedValues) {
+        use properties::computed_value_flags::HAS_TEXT_DECORATION_LINES;
+        if layout_parent_style.flags.contains(HAS_TEXT_DECORATION_LINES) ||
+           !self.style.get_text().clone_text_decoration_line().is_empty() {
+            self.style.flags.insert(HAS_TEXT_DECORATION_LINES);
+        }
+    }
+
     /// Adjusts the style to account for various fixups that don't fit naturally
     /// into the cascade.
     ///
     /// When comparing to Gecko, this is similar to the work done by
     /// `nsStyleContext::ApplyStyleFixups`.
     pub fn adjust(&mut self,
                   layout_parent_style: &ComputedValues,
                   flags: CascadeFlags) {
@@ -336,10 +345,11 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
         #[cfg(feature = "servo")]
         {
             self.adjust_for_alignment(layout_parent_style);
         }
         self.adjust_for_border_width();
         self.adjust_for_outline();
         self.adjust_for_writing_mode(layout_parent_style);
+        self.adjust_for_text_decoration_lines(layout_parent_style);
     }
 }