1131978 - Gtk3 - need to adjust internal DPI according to the GDK_SCALE factor

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect)

Description

[Martin Stránský [:stransky] (ni? me)]

Follower from Bug 1097897 comment 6:

> Right now the Gtk3 scaling (GDK_SCALE) is applied on top of the DPI scaling.
> so for instance system DPI 150 and GDK_SCALE = 2 results to 4x scaling
> factor in GTK3. Is that intended or should be those two settings concurrent?

Yes, that is what I meant.  If the system reports DPI=192 and GDK_SCALE=2,
then GDK will adjust the DPI by the scale and report 96.  Gecko usually works
in device pixels, so we undo GDK's adjustment.

I don't see that behavior in the patch here, but GTK3 can be addressed
separately.  The patch is the right kind of thing for GTK2.  For GTK3, it
always uses the same scale on all monitors, with X11, so GetDPIScale() can
include the scale factor from monitor 0.

Comment 1

[Martin Stránský [:stransky] (ni? me)]

Actually it looks like the Gtk3 apps behaves the same way as Firefox now. DPI scale is independent to GDK_SCALE and DPI=192 and GDK_SCALE=2 results to 4x scale factor. But I'll investigate it later when I have some HiDPI system available.

Comment 2

[Andrew Comminos [:acomminos]]

I work on a HiDPI system, I'll whip up a patch for this.

Comment 3

[Andrew Comminos [:acomminos]]

Attachment: Use GDK's scale factor for screen calculations where available, fix coordinate mixup.

This patch uses GDK's reported monitor scale (instead of the value calculated based on DPI) where available, and fixes an issue with the wrong unit used for screen widget calculation.

Comment 4

[Karl Tomlinson (:karlt)]

Comment on attachment 8624483 [details] [diff] [review]
Use GDK's scale factor for screen calculations where available, fix coordinate mixup.

The ScreenForNativeWidget() change looks good.

IIRC nsScreenGtk::GetDPIScale() should be consistent with
nsWindow::GetDefaultScaleInternal(), but neither of those are doing quite what
I think we want.

What we'd like to do is consider both GDK's scale factor and the system DPI,
so that we get the desired scaled regardless of which mechanism the system is
using for scaling.  Assuming GetDPIScale() would return the same value for all
monitors (I don't know what would happen on Wayland), that can be done in
nsWindow::GetDefaultScaleInternal with

 GdkScaleFactor() * gfxPlatformGtk::GetDPIScale();

and in nsScreenGtk::GetDPIScale() with

 GetGtkMonitorScaleFactor() * gfxPlatformGtk::GetDPIScale()

Does that make sense?

Comment 5

[Andrew Comminos [:acomminos]]

Yes, thanks- so we'll be using the composite scale factor (GdkScaleFactor() * gfxPlatformGtk::GetDPIScale()) for everything except the initial translation from GDK coordinates.

Comment 6

[Andrew Comminos [:acomminos]]

Attachment: Acknowledge GDK's scale factor in scale calculation, fix coordinate mixup.

Updated to use composite scale factor.

Comment 7

[Karl Tomlinson (:karlt)]

Comment on attachment 8624807 [details] [diff] [review]
Acknowledge GDK's scale factor in scale calculation, fix coordinate mixup.

(In reply to Andrew Comminos [:acomminos] from comment #5)
> Yes, thanks- so we'll be using the composite scale factor (GdkScaleFactor()
> * gfxPlatformGtk::GetDPIScale()) for everything except the initial
> translation from GDK coordinates.

Yes, (GdkScaleFactor() * gfxPlatformGtk::GetDPIScale() or override) converts between CSS pixels and device (X11) pixels, while GdkScaleFactor() converts between GDK units and device pixels.

>     gdk_window_get_origin(GDK_WINDOW(aWidget), &x, &y);
>-    rv = ScreenForRectPix(x, y, width, height, outScreen);
>+    rv = ScreenForRect(x, y, width, height, outScreen);

However, please remove this change from this patch and the "fix coordinate
mixup" part of the comment, because the existing code is currently correct for GTK2 and system DPI scaling through gfxPlatformGtk::GetDPIScale(), and this change will break that.

The right fix to include GDK's scale factor here is to convert from GDK coords
to device coords with GdkScaleFactor().  (The ratio between the units of
ScreenForRect and ScreenForRectPix() is (GdkScaleFactor() *
gfxPlatformGtk::GetDPIScale() or override).

Comment 8

[Andrew Comminos [:acomminos]]

Attachment: Acknowledge GDK's scale factor in scale calculation. Carry r=karlt

Removed ScreenForRect coordinate changes.

Comment 9

[Andrew Comminos [:acomminos]]

Skipping try run, try server doesn't run a GDK version that supports scale factors.

Comment 10

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d3d1ddf6a0cf

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d3d1ddf6a0cf