Bug 1457678: Make the font canvas code not mess with the rule tree. r=xidorn,hiro
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 03 May 2018 19:24:05 +0200
changeset 472974 32192f022b8455c322d592e9785359a7c9496c6a
parent 472973 d0990dff4fc4eabd59b09cf6f9ff5c0e0f64fe9c
child 472992 b917e7e80a51f27410b9fde249c72ae49803a163
push id1728
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:12:27 +0000
treeherdermozilla-release@c296fde26f5f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersxidorn, hiro
bugs1457678
milestone61.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1457678: Make the font canvas code not mess with the rule tree. r=xidorn,hiro Inserting a lot of rules in the root level is super-inefficient. This fixes it by not doing it. It gives the root rule node to the style, but that's fine, since it's useless. All this code-path is already pretty messy. MozReview-Commit-ID: GoGHI4YJbKr
servo/components/style/stylist.rs
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -1550,37 +1550,35 @@ impl Stylist {
         guards: &StylesheetGuards,
         parent_style: &ComputedValues,
         declarations: Arc<Locked<PropertyDeclarationBlock>>,
     ) -> Arc<ComputedValues>
     where
         E: TElement,
     {
         use font_metrics::get_metrics_provider_for_product;
-        use std::iter;
 
-        // FIXME(emilio): Why do we even need the rule node? We should probably
-        // just avoid allocating it and calling `apply_declarations` directly,
-        // maybe...
-        let rule_node = self.rule_tree.insert_ordered_rules(iter::once((
-            StyleSource::from_declarations(declarations),
-            CascadeLevel::StyleAttributeNormal,
-        )));
+        let block = declarations.read_with(guards.author);
+        let iter_declarations = || {
+            block.declaration_importance_iter().map(|(declaration, importance)| {
+                debug_assert!(!importance.important());
+                (declaration, CascadeLevel::StyleAttributeNormal)
+            })
+        };
 
-        // This currently ignores visited styles.  It appears to be used for
-        // font styles in <canvas> via Servo_StyleSet_ResolveForDeclarations.
-        // It is unclear if visited styles are meaningful for this case.
         let metrics = get_metrics_provider_for_product();
 
-        // FIXME(emilio): the pseudo bit looks quite dubious!
-        properties::cascade::<E>(
+        // We don't bother inserting these declarations in the rule tree, since
+        // it'd be quite useless and slow.
+        properties::apply_declarations::<E, _, _>(
             &self.device,
             /* pseudo = */ None,
-            &rule_node,
+            self.rule_tree.root(),
             guards,
+            iter_declarations,
             Some(parent_style),
             Some(parent_style),
             Some(parent_style),
             None,
             &metrics,
             CascadeFlags::empty(),
             self.quirks_mode,
             /* rule_cache = */ None,