[Wayland-bugs] [Bug 786694] wayland: fix fontconfig monitoring

[gnome-settings-daemon (GNOME Bugzilla)]

https://bugzilla.gnome.org/show_bug.cgi?id=786694

Bastien Nocera <bugzilla at hadess.net> changed:

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

--- Comment #12 from Bastien Nocera <bugzilla at hadess.net> ---
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
time
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
that?

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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/wayland-bugs/attachments/20180222/2f97a307/attachment.html>