Open Bug 1554850 Opened 1 year ago Updated 3 months ago
Fractional DPI change not detected on Linux
Component: Untriaged → Widget: Gtk
Product: Firefox → Core
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Comment on attachment 9078711 [details] [diff] [review] dpi-dynamic-update_ff68.0.diff Adding reviewers as per https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Module_ownership
Comment on attachment 9078711 [details] [diff] [review] dpi-dynamic-update_ff68.0.diff I checking this out thanks. It's the DispatchResized() that triggers the viewport size. That shouldn't be necessary, so I'll see if I can find out what's up there.
Comment on attachment 9078711 [details] [diff] [review] dpi-dynamic-update_ff68.0.diff Thank you for this. Would you be able to [submit a patch through phabricator](https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch#Submitting_the_patch), please? That is how patches are landed now and it is usually easier to review there. >+ g_signal_connect(gtk_settings, "notify::gtk-xft-dpi", >+ G_CALLBACK(gtk_settings_xft_dpi_changed_cb), nullptr); A "notify::resolution" handler on the `GdkScreen` would be a little more consistent with the use of `gdk_screen_get_resolution()`, but I'm not aware of much difference in practice. Disconnection of the `GtkSettings` handler is a little simpler in nsWindow, but disconnection of the `GdkScreen` handler would be a little simpler in `ScreenHelperGTK`. I'm not too concerned which you use, but the ordering of signals might be relevant. On XSETTINGS changes, it looks like "notify::resolution" is signalled before "notify::gtk-xft-dpi". >+ // Font DPI has changed, so reset our cached DPI value. >+ sDPI = 0; An alternative option in gfxPlatformGtk.cpp is to just remove sDPI, and not cache results. The `GdkScreen` already caches the value for `gdk_screen_get_resolution()`. >+ // Without a call to Resize, the UI's viewport becomes too small/big. >+ LayoutDeviceIntRect bounds = window->GetBounds(); >+ window->Resize(bounds.width, bounds.height, true); Rather than doing a full `Resize()`, just `DispatchResized()`. For an explanation comment "Even though the window size in screen pixels has not changed, nsViewManager stores the dimensions in app units. DispatchResized() updates those." There is a remaining issue when reducing dpi from a larger value at Firefox startup: the hamburger menu drop-down is limited in size unnecessarily because Firefox still has the initial concept of screen size. This derives from the [use of `gfxPlatformGtk::GetFontScaleFactor()` in initializing screen data](https://searchfox.org/mozilla-central/rev/8b7aa8af652f87d39349067a5bc9c0256bf6dedc/widget/gtk/ScreenHelperGTK.cpp#151). A RefreshScreens() on dpi change resolves that. I'm pretty confident that `RefreshScreens()` before the update from nsWindow.cpp will be fine. If the order is reversed, everything may still be OK, but I'm not sure.
Attachment #9078711 - Flags: feedback?(karlt) → feedback+
You need to log in before you can comment on or make changes to this bug.