GNOME Bugzilla – Bug 705410
hi-dpi support: scale UI
Last modified: 2014-02-18 22:40:06 UTC
The UI that is drawn by gnome-shell (top bar, menus, dialogs, etc) needs to be scaled by the scale factor when on a hi-dpi display.
Where is the "scale factor" ? Env var? root window property? Something else?
Right now, it's the GDK_SCALE environment variable, but we've been talking about moving it to an xsetting or root window property.
so clutter now doubles the stage scale factor when CLUTTER_SCALE is set. It will eventually attach to the same XSetting value that we use for gtk+. unfortunately, setting CLUTTER_SCALE=2 doesn't magically make things work for the shell. Pixels get double-sized like they're supposed to, but I believe the shell still makes layout decisions based on xrandr information, so the output only shows the upper left quadrant of the screen.
Created attachment 268927 [details] [review] st: Add high dpi support Add a scale_factor property to StThemeContext that can be used to enable (integer) scaling of pixel values. -- Talked about this with Owen on IRC. Doing the handling in St's css code and adjusting the icon sizes should be enough to get high dpi working. This patches implement the former and it seems to work pretty well in my testing.
Created attachment 268929 [details] [review] layout: Set high dpi scaling factor Set the scaling factor when the dpi is above the hidpi threshold.
Review of attachment 268927 [details] [review]: ::: src/st/st-theme-context.c @@ +168,3 @@ + context->scale_factor = scale_factor; + g_object_notify (G_OBJECT (context), "scale-factor"); + g_object_thaw_notify (G_OBJECT (context)); freeze/thaw isn't useful for a single property
Review of attachment 268927 [details] [review]: ::: src/st/st-theme-context.c @@ +168,3 @@ + context->scale_factor = scale_factor; + g_object_notify (G_OBJECT (context), "scale-factor"); + g_object_thaw_notify (G_OBJECT (context)); You also don't need g_object_notify in a set_property handler. If somebody goes through g_object_set_property, it will notify automatically.
Created attachment 268942 [details] [review] st: Add high dpi support Add a scale_factor property to StThemeContext that can be used to enable (integer) scaling of pixel values.
Created attachment 268948 [details] [review] shell-global: Set high dpi scaling factor Set the scaling factor based on the xsetting. --- OK as discussed on IRC with Jasper instead of duplicating gsd code, read out the xsetting in shell-global.
Review of attachment 268948 [details] [review]: ::: src/shell-global.c @@ +749,3 @@ + +static gboolean +monitors_changed (gpointer data) This should probably be called update_scale_factor rather than monitors_changed. @@ +760,3 @@ + g_object_set (context, "scale-factor", g_value_get_int (&value), NULL); + + return TRUE; monitors-changed doesn't need a boolean return value. @@ +941,3 @@ G_CALLBACK (focus_window_changed), global); + g_signal_connect (global->gdk_screen, "monitors-changed", I'd like to have a comment about why we're connecting to the gdk_screen rather than mutter here.
Pushed with suggested changes, leaving bug open for the icon scaling issue. Attachment 268942 [details] pushed as d868e6b - st: Add high dpi support Attachment 268948 [details] pushed as ba459f4 - shell-global: Set high dpi scaling factor
(In reply to comment #9) > Created an attachment (id=268948) [details] [review] > shell-global: Set high dpi scaling factor > > Set the scaling factor based on the xsetting. > > > --- > > OK as discussed on IRC with Jasper instead of duplicating gsd code, > read out the xsetting in shell-global. OK, this seem to hang gdm (shell at gdm to be exact) at login time but not when run inside the session with --mode=gdm ... the layout.js patch works in both. I have no idea why this happens yet. Maybe we are calling it to early? But comenting out the gdk_screen_get_setting does not fix the hang either.
(In reply to comment #12) > (In reply to comment #9) > > Created an attachment (id=268948) [details] [review] [details] [review] > > shell-global: Set high dpi scaling factor > > > > Set the scaling factor based on the xsetting. > > > > > > --- > > > > OK as discussed on IRC with Jasper instead of duplicating gsd code, > > read out the xsetting in shell-global. > > OK, this seem to hang gdm (shell at gdm to be exact) at login time but not when > run inside the session with --mode=gdm ... the layout.js patch works in both. I > have no idea why this happens yet. Maybe we are calling it to early? But > comenting out the gdk_screen_get_setting does not fix the hang either. I went back to the other one to get gdm working again.
Created attachment 269201 [details] [review] altTab: Scale icons by the scale_factor This makes them look correct on high dpi screens.
Created attachment 269202 [details] [review] iconGrid: Scale icons by the scale factor This makes them look correct on high dpi screens.
Created attachment 269203 [details] [review] panel: Apply scale factor to faded icon This makes it look correct on high dpi screens.
Created attachment 269204 [details] [review] dash: Scale icons by the scale_factor This makes them look correct on high dpi screens.
Created attachment 269269 [details] [review] Here's an a bit different approach that uses GTK's scaled loaders from GtkIconTheme in order to get an icon of the proper size. --- texture-cache: require icon scale to load gicon To support HiDpi.
Created attachment 269270 [details] [review] shell-app: finish support for loading with scale
Created attachment 269271 [details] [review] altTab: don't override icon size request Instead of overriding the actor's request with the icon size, just set the new icon size on the actors, and let the default handler take the preferred size of children.
Created attachment 269272 [details] [review] altTab: account for scale factor when computing icon size
Created attachment 269273 [details] [review] iconGrid: don't force icon size to the BaseIcon Since we might get a scaled up version for HiDpi.
Created attachment 269274 [details] [review] st-icon: remove custom size request/allocate Use a layout manager instead. This has the effect of not enforcing a priv->icon_size size request, since that value is unscaled.
Created attachment 269275 [details] [review] dash: account for scale factor to determine icon size
Created attachment 269276 [details] [review] appDisplay: don't force size on folder icon actor Use a table layout instead.
(In reply to comment #18) > Created an attachment (id=269269) [details] [review] > Here's an a bit different approach that uses GTK's scaled loaders from > GtkIconTheme in order to get an icon of the proper size. > > --- > > texture-cache: require icon scale to load gicon > > To support HiDpi. OK, just tested your patches seems to work better (also fixes stuff like icons in lg).
Review of attachment 269269 [details] [review]: Looks good.
Review of attachment 269270 [details] [review]: LG.
Created attachment 269279 [details] [review] altTab: Scale thumbnails by the scale factor Fixes their size on high dpi. -- Just noticed that the window thumbnails in alt tab are small, this patch fixes them.
Review of attachment 269271 [details] [review]: LG.
Review of attachment 269272 [details] [review]: LG.
Review of attachment 269273 [details] [review]: OK.
Review of attachment 269274 [details] [review]: OK.
Review of attachment 269275 [details] [review]: OK.
Review of attachment 269276 [details] [review]: LG.
Attachment 269270 [details] pushed as 407dc74 - shell-app: finish support for loading with scale Attachment 269271 [details] pushed as ad97fc6 - altTab: don't override icon size request Attachment 269272 [details] pushed as fc719c1 - altTab: account for scale factor when computing icon size Attachment 269273 [details] pushed as 9cc1017 - iconGrid: don't force icon size to the BaseIcon Attachment 269274 [details] pushed as f543161 - st-icon: remove custom size request/allocate Attachment 269275 [details] pushed as e92d204 - dash: account for scale factor to determine icon size Attachment 269276 [details] pushed as f959caf - appDisplay: don't force size on folder icon actor Pushed all these, thanks for the review.
Review of attachment 269279 [details] [review]: Looks good other than a little coding style nit ::: js/ui/altTab.js @@ +657,3 @@ let spacing = this._items[0].child.get_theme_node().get_length('spacing'); + let scaleFactor = St.ThemeContext.get_for_stage(global.stage).scale_factor; + let thumbnail_size = THUMBNAIL_DEFAULT_SIZE * scaleFactor; Should be thumbnailSize for consistency with the rest of the file.
Pushed with coding style fix. Attachment 269279 [details] pushed as 3b7593e - altTab: Scale thumbnails by the scale factor
(In reply to comment #12) > (In reply to comment #9) > > Created an attachment (id=268948) [details] [review] [details] [review] > > shell-global: Set high dpi scaling factor > > > > Set the scaling factor based on the xsetting. > > > > > > --- > > > > OK as discussed on IRC with Jasper instead of duplicating gsd code, > > read out the xsetting in shell-global. > > OK, this seem to hang gdm (shell at gdm to be exact) at login time but not when > run inside the session with --mode=gdm ... the layout.js patch works in both. I > have no idea why this happens yet. Maybe we are calling it to early? But > comenting out the gdk_screen_get_setting does not fix the hang either. Reopening because we should figure out why this hangs gdm and stop duplicating code.
Created attachment 269629 [details] [review] hdpi: Revert hacks Revert the hacks that where added in response to a bug caused by commit ba459f4d202
Created attachment 269630 [details] [review] shell_global: Use g_signal_connect_swapped for update_scale_factor Using g_signal_connect is wrong because we get the wrong argument passed to to the callback.
Created attachment 269633 [details] [review] shell_global: Use correct agruments for in update_scale_factor Otherwise we crash ...
Review of attachment 269633 [details] [review]: Looks good.
Review of attachment 269629 [details] [review]: OK.
Attachment 269629 [details] pushed as 9e05e87 - hdpi: Revert hacks Attachment 269633 [details] pushed as cd40011 - shell_global: Use correct agruments for in update_scale_factor