After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 771315 - Use g-s-d settings for drawing tablet configuration
Use g-s-d settings for drawing tablet configuration
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
unspecified
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-09-12 18:20 UTC by Carlos Garnacho
Modified: 2016-09-12 19:52 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backends: Use g-s-d settings for tablet configuration (15.80 KB, patch)
2016-09-12 18:20 UTC, Carlos Garnacho
none Details | Review
backends: Use g-s-d settings for tablet configuration (17.74 KB, patch)
2016-09-12 19:06 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2016-09-12 18:20:34 UTC
As part of the contingency plans in result of not having merged the gnome-control-center wacom redesign, we have in result a wayland backend that doesn't react to configuration changes, since mutter looks up on different schemas/paths than gnome-settings-daemon/gnome-control-center do.

I'm attaching a patch taping over this issue, making mutter look up the g-s-d/g-c-c settings for tablet/stylus/pad configuration. No explicit dependency on g-s-d is added though.

So with this patch in place, we would have mutter implementing wayland tablet support/configuration and gnome-settings-daemon handling x11 tablet configuration, both listening to the schemas as defined by g-s-d.

Ideally we do the opposite, move x11 configuration to mutter, drop gnome-settings-daemon code, use gsettings-desktop-schemas for both wayland/x11 and let gnome-control-center update the new schemas/paths, this will have to happen after 3.22 though.
Comment 1 Carlos Garnacho 2016-09-12 18:20:59 UTC
Created attachment 335382 [details] [review]
backends: Use g-s-d settings for tablet configuration

This is needed to make the wayland backend react to configuration
changes until gnome-control-center is updated to use the
gsettings-desktop-schemas settings.
Comment 2 Rui Matos 2016-09-12 18:37:02 UTC
Review of attachment 335382 [details] [review]:

looks good
Comment 3 Florian Müllner 2016-09-12 18:49:49 UTC
Review of attachment 335382 [details] [review]:

::: src/backends/meta-input-settings.c
@@ +1093,3 @@
+                              device_id);
+      settings = g_settings_new_with_path ("org.gnome.settings-daemon.peripherals.wacom",
+                                           path);

When we don't add an explicit dependency on gsd, maybe we should use g_settings_schema_source_lookup() or similar to deal gracefully with the schema not being installed?
(Not that it matters much for the GNOME case ...)

@@ +1318,3 @@
 
+  info->gsd_settings = lookup_device_gsd_settings (device, info);
+  g_signal_connect (settings, "changed",

Shouldn't this be info->gsd_settings? You already connect to settings' ::changed signal a couple of lines above ...
Comment 4 Carlos Garnacho 2016-09-12 19:06:23 UTC
Created attachment 335392 [details] [review]
backends: Use g-s-d settings for tablet configuration

This is needed to make the wayland backend react to configuration
changes until gnome-control-center is updated to use the
gsettings-desktop-schemas settings.
Comment 5 Carlos Garnacho 2016-09-12 19:07:17 UTC
(In reply to Florian Müllner from comment #3)
> Review of attachment 335382 [details] [review] [review]:
> 
> ::: src/backends/meta-input-settings.c
> @@ +1093,3 @@
> +                              device_id);
> +      settings = g_settings_new_with_path
> ("org.gnome.settings-daemon.peripherals.wacom",
> +                                           path);
> 
> When we don't add an explicit dependency on gsd, maybe we should use
> g_settings_schema_source_lookup() or similar to deal gracefully with the
> schema not being installed?
> (Not that it matters much for the GNOME case ...)

I've covered now all uses under a has_gsd_schemas() function.

> 
> @@ +1318,3 @@
>  
> +  info->gsd_settings = lookup_device_gsd_settings (device, info);
> +  g_signal_connect (settings, "changed",
> 
> Shouldn't this be info->gsd_settings? You already connect to settings'
> ::changed signal a couple of lines above ...

Oops right, fixed.
Comment 6 Florian Müllner 2016-09-12 19:17:50 UTC
Review of attachment 335392 [details] [review]:

LGTM

::: src/backends/meta-input-settings.c
@@ +1096,3 @@
+
+  if (!has_gsd_schemas ())
+    return NULL;

Mmmh - tbh, I would have expected that check ...

@@ +1106,3 @@
+      type == CLUTTER_PAD_DEVICE)
+    {
+      gchar *device_id, *path;

... here:

  schema = g_settings_schema_source_lookup (...);
  if (!schema)
    return NULL;

  device_id = ...;
  path = ...;
  settings = g_settings_new_full (schema, NULL, path);

  g_object_unref (schema);
  g_free (device_id);
  g_free (path);

Any reason for not using the schema to construct the settings object, but keeping it in a separate check?
Comment 7 Carlos Garnacho 2016-09-12 19:51:57 UTC
Pushed with the first suggestion, kept the has_gsd_schemas() check aside since it's used in multiple places.

Attachment 335392 [details] pushed as b52f304 - backends: Use g-s-d settings for tablet configuration