css: Don't got to the selector tree for change
authorMatthias Clasen <mclasen@redhat.com>
Wed, 15 Jan 2020 13:03:09 +0000 (08:03 -0500)
committerMatthias Clasen <mclasen@redhat.com>
Thu, 16 Jan 2020 22:20:45 +0000 (17:20 -0500)
The tree is optimized for mimizing the decisions, and is built ahead-of-time.
That prevents us from taking advantage of the information in the matcher when
collecting changes.

So, instead do what we used to do for verification: Use the selector tree
for finding the superset matches, then just walk the rulesets to collect
the changes.

Since we are now recomputing the change masks much less frequently, this
slightly less optimized way of computing them is not a problem, and will
let us compute better results in the future, by improving the superset
matcher to be more precise.

gtk/gtkcssprovider.c

index 3713b83529e8601f8671f697c14c89950a530508..59573ebb341217c4cb9349c7bfd4db327c6a2ec4 100644 (file)
@@ -425,61 +425,40 @@ verify_tree_match_results (GtkCssProvider *provider,
 #endif
 }
 
-static void
-verify_tree_get_change_results (GtkCssProvider *provider,
-                               const GtkCssMatcher *matcher,
-                               GtkCssChange change)
+/* Compute the change flags for a given matcher.
+ * We assume that name, id and classes stay unchanged, and look for all rules
+ * that can be matched if anything else changes. We collect the change masks
+ * from the selectors of these rules.
+ */
+static GtkCssChange
+compute_change (GtkCssProvider      *provider,
+                const GtkCssMatcher *matcher)
 {
-#ifdef VERIFY_TREE
-  {
-    GtkCssProviderPrivate *priv = gtk_css_provider_get_instance_private (provider);
-    GtkCssChange verify_change = 0;
-    GPtrArray *tree_rules;
-    int i;
-
-    tree_rules = _gtk_css_selector_tree_match_all (priv->tree, matcher);
-    if (tree_rules)
-      {
-        verify_tree_match_results (provider, matcher, tree_rules);
-
-        for (i = tree_rules->len - 1; i >= 0; i--)
-          {
-           GtkCssRuleset *ruleset;
-
-            ruleset = tree_rules->pdata[i];
-
-            verify_change |= _gtk_css_selector_get_change (ruleset->selector);
-          }
-
-        g_ptr_array_free (tree_rules, TRUE);
-      }
-
-    if (change != verify_change)
-      {
-       GString *s;
-
-       s = g_string_new ("");
-       g_string_append (s, "expected change ");
-        gtk_css_change_print (verify_change, s);
-        g_string_append (s, ", but it was ");
-        gtk_css_change_print (change, s);
-       if ((change & ~verify_change) != 0)
-          {
-           g_string_append (s, ", unexpectedly set: ");
-            gtk_css_change_print (change & ~verify_change, s);
-          }
-       if ((~change & verify_change) != 0)
-          {
-           g_string_append_printf (s, ", unexpectedly not set: ");
-            gtk_css_change_print (~change & verify_change, s);
-          }
-       g_warning (s->str);
-       g_string_free (s, TRUE);
-      }
-  }
-#endif
-}
+  GtkCssProviderPrivate *priv = gtk_css_provider_get_instance_private (provider);
+  GtkCssChange change = 0;
+  GPtrArray *tree_rules;
+  int i;
+  GtkCssMatcher change_matcher;
+
+  _gtk_css_matcher_superset_init (&change_matcher, matcher, GTK_CSS_CHANGE_NAME | GTK_CSS_CHANGE_CLASS);
 
+  tree_rules = _gtk_css_selector_tree_match_all (priv->tree, &change_matcher);
+  if (tree_rules)
+    {
+      for (i = tree_rules->len - 1; i >= 0; i--)
+        {
+         GtkCssRuleset *ruleset;
+
+          ruleset = tree_rules->pdata[i];
+
+          change |= _gtk_css_selector_get_change (ruleset->selector);
+        }
+
+      g_ptr_array_free (tree_rules, TRUE);
+   }
+
+  return change;
+}
 
 static GtkCssValue *
 gtk_css_style_provider_get_color (GtkStyleProvider *provider,
@@ -551,14 +530,7 @@ gtk_css_style_provider_lookup (GtkStyleProvider    *provider,
     }
 
   if (change)
-    {
-      GtkCssMatcher change_matcher;
-
-      _gtk_css_matcher_superset_init (&change_matcher, matcher, GTK_CSS_CHANGE_NAME | GTK_CSS_CHANGE_CLASS);
-
-      *change = _gtk_css_selector_tree_get_change_all (priv->tree, &change_matcher);
-      verify_tree_get_change_results (css_provider, &change_matcher, *change);
-    }
+    *change = compute_change (css_provider, matcher);
 }
 
 static void