Bastien Nocera <> changed:

           What    |Removed                     |Added
 Attachment #368734|none                        |needs-work
             status|                            |

--- Comment #12 from Bastien Nocera <> ---
Review of attachment 368734:

1) Get rid of the array
2) Every time a timestamp change has happened, queue sending the new value for
half-a-second (or a second?), reschedule if a new timestamp arrives in that
3) Don't use g_get_real_time()

::: plugins/xsettings/gsd-xsettings-manager.c
@@ +80,3 @@
+static const gchar introspection_xml[] =
+"<node name='/org/gnome/SettingsDaemon/FontConfig'>"
+"  <interface name='org.gnome.SettingsDaemon.FontConfig'>"

Is this really the name of the interface we want to use? What about GTK+ apps
under other Wayland desktops?

@@ +284,3 @@
         GSettings         *plugin_settings;
         FcMonitor         *fontconfig_monitor;
+        GArray            *fontconfig_timestamps;

I've stared at the code for a long while, and can't figure out why you need to
send a "changed" signal for each and every fontconfig timestamp change. Why do

You could just keep the last one, and send it out when it changes, or
aggregate/coalesce the changes into one event if the changes are too close
together. You need to remember that this would wake up every single GTK+
program, so less notifications is better.

@@ +1189,3 @@
         GList       *list, *l;
         const char  *session;
+        gint64       timestamp = g_get_real_time ();

You shouldn't user g_get_real_time(), this will break if the time on the
machine is changed. Any reason why g_get_monotonic_time() can't be used?

@@ +1453,3 @@
+        manager = GNOME_XSETTINGS_MANAGER (user_data);
+        if (manager->priv->dbus_connection == NULL)

This isn't necessary, you get a connection in the arguments of the function
above, and you don't use this variable anyway.

You are receiving this mail because:
You are on the CC list for the bug.
wayland-bugs mailing list

Reply via email to