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 735244 - shell forgets the DPI of screen sometimes
shell forgets the DPI of screen sometimes
Status: RESOLVED FIXED
Product: clutter
Classification: Platform
Component: x11
unspecified
Other Linux
: Normal normal
: ---
Assigned To: clutter-maint
clutter-maint
Depends on:
Blocks:
 
 
Reported: 2014-08-22 20:15 UTC by Bastian Ilsø
Modified: 2014-08-23 12:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
picture of a shell at 96dpi on a chromebook pixels (1.19 MB, image/jpeg)
2014-08-22 20:15 UTC, Bastian Ilsø
  Details
clutter-settings: Mark window-scaling-factor as fixed when set by the app (5.26 KB, patch)
2014-08-23 08:23 UTC, drago01
committed Details | Review

Description Bastian Ilsø 2014-08-22 20:15:01 UTC
Created attachment 284252 [details]
picture of a shell at 96dpi on a chromebook pixels

I seem experience change of DPI sometimes when using gnome-shell 3.13.90. The attached picture is a picture of a chromebook pixel when this happens.

I dont seem able to re-produce this problem consistently. restarting gnome-shell fixes it, however.
Comment 1 drago01 2014-08-23 07:39:01 UTC
That's not "the shell at 96dpi" that looks like clutter's scaling is active in addition to ours (Which effectively means you have "348 dpi" ... ).

That shouldn't happen if we set the scale by hand clutter should not touch it.
Comment 2 drago01 2014-08-23 08:23:25 UTC
Created attachment 284279 [details] [review]
clutter-settings: Mark window-scaling-factor as fixed when set by the app

When an application sets the scaling factor manually we should mark it as fixed
and not override it when the xsettings change. This matches GDKs behaviour.

In order for this to work we cannot use the same path when setting the value
internally so introduce a _clutter_settings_set_property_internal and use it
for that.
Comment 3 Emmanuele Bassi (:ebassi) 2014-08-23 11:47:31 UTC
Review of attachment 284279 [details] [review]:

looks okay. just some minor comment.

::: clutter/clutter-settings-private.h
@@ +12,3 @@
                                                  GKeyFile        *key_file);
 
+void    _clutter_settings_set_property_internal (ClutterSettings *settings,

no need to use a leading '_': we use visibility macros, now.

::: clutter/clutter-settings.c
@@ +378,3 @@
+                                         GValue *value)
+{
+  if (strcmp(property, "window-scaling-factor") == 0 &&

I would intern the `property` string and then just use pointer comparison, e.g.:

  property = g_intern_string (property);

  if (property == I_("window-scaling-factor")) ...

the property name is already likely interned, so this is going to be pretty fast.

@@ +382,3 @@
+    return;
+
+  g_object_set_property (G_OBJECT (self),

you can put all arguments on the same line.

@@ +386,3 @@
+                         value);
+
+  if (strcmp(property, "window-scaling-factor") == 0)

same as above.

::: clutter/gdk/clutter-backend-gdk.c
@@ +115,3 @@
 				  &val);
+	  _clutter_settings_set_property_internal (settings,
+				  CLUTTER_SETTING_PROPERTY (i),

coding style: wrong whitespace for aligning arguments.
Comment 4 drago01 2014-08-23 12:03:44 UTC
Attachment 284279 [details] pushed as cfcba18 - clutter-settings: Mark window-scaling-factor as fixed when set by the app

Pushed with suggested changes.