[GTK] Rename screenDPI() to fontDPI() where used for font scaling.

[gwhitney]

51f2745

[GTK] Rename screenDPI() to fontDPI() where used for font scaling.
https://bugs.webkit.org/show_bug.cgi?id=272676

Reviewed by Carlos Garcia Campos.

This commit lays the groundwork for addressing DPI scaling issues under
GTK, as described in the related bugzilla bug 247980, which seems to
have arisen by virtue of confusion between the physical DPI of the display
devices that WebKit is rendering on, and the GTK font scaling DPI that sets
the desired font sizes. Hence, in line with the concerns raised in the
references bug 272676, this commit renames WebCore::screenDPI() to
WebCore::fontDPI() to clarify its semantics and hopefully avoid future
similar confusions. For cases in which the underlying display density is
needed, it adds a new WebCore::screenDPI(PlatformDisplayID) function
to access that information, on a per-display basis.

This commit also creates a reftest that probes the status of the referenced
bug. Namely, the test compares a 1em box in a text size of 96px, with a
1in box. Note the test passes as the code stands, even though when I view
247980.html and 247980-expected.html in the minibrowser on my machine, the
two boxes are clearly very different in size, which is one key aspect of the
bug. Presumably, this pass occurs because the tests are run in an environment
insulated from my actual display resolution and GTK setup. Moreover, another
aspect of the bug is that when the two DPI measures referenced above agree,
both boxes should measure 1 physical inch on the screen, which they currently
do not (unless the display happens to have exactly 96 pixels per inch, low by
today's typical specs, and unlikely to happen to be the case even when device
scaling is employed). But I have no idea how such a condition could
practically be tested in the WebKit test framework.

* LayoutTests/fast/box-sizing/247980-expected.html: Added.
* LayoutTests/fast/box-sizing/247980.html: Added.
* Source/WebCore/accessibility/atspi/AccessibilityObjectTextAtspi.cpp:
(WebCore::AccessibilityObjectAtspi::textAttributes const):
* Source/WebCore/platform/PlatformScreen.h:
* Source/WebCore/platform/ScreenProperties.h:
* Source/WebCore/platform/graphics/gtk/SystemFontDatabaseGTK.cpp:
(WebCore::SystemFontDatabase::platformSystemFontShorthandInfo):
* Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:
(WebCore::fontDPI):
(WebCore::screenDPI):
* Source/WebCore/platform/wpe/PlatformScreenWPE.cpp:
(WebCore::fontDPI):
(WebCore::screenDPI):
* Source/WebKit/UIProcess/API/glib/WebKitProtocolHandler.cpp:
(WebKit::WebKitProtocolHandler::handleGPU):
* Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp:
(webkit_settings_font_size_to_points):
(webkit_settings_font_size_to_pixels):

Canonical link: https://commits.webkit.org/277537@main

41815f5

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 0c5f439)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

[bubbleguuum]

Will this PR (and the next one to follow) fix that issue that I have observed with MiniBrowser using WebKitGTK 2.44.0 on a laptop with a 17.3" 4K panel (254 physical dpi) running on i3 Window Manager.
On that setup, I use Xft.dpi: 240 (for 2.5x font scaling) and GDK_DPI_SCALE=0.5, GDK_SCALE=2 (2x GTK UI scaling).
It works fine with all my programs except WebKitGTK resulting in huge font rendering and rendering bugs (screenshot), as if Xft scaling and GTK scaling were combined.
The only combination of these 3 parameters that I found render properly (without too big fonts nor UI rendering issues) is to set Xft.dpi: 96 (with GDK_DPI_SCALE=0.5, GDK_SCALE=2), except that fonts are a bit too small (at 2x scaling instead of 2.5).
IIRC, WebKitGTK before 2.44.0 did not have this issue where Xft.dpi and GDK env variables seems to be conflicting.

[gwhitney]

Well, compare your screenshot to the ones in my issue report https://bugs.webkit.org/show_bug.cgi?id=247980 which were generated with Xft.dpi=273 (matching my physical dpi) and a couple different GDK_SCALE values. Looks like the symptoms are pretty similar. And I am now running with my proposed patches on a day-to-day basis, and the minibrowser looks just as I would like/expect. So, fingers crossed that yes it will resolve your issue once both parts are in. If you feel pretty confident you are seeing the same issue I have reported in 247980, you might want to mention that there or upvote the bug or something, since it received no attention for a year; I imagine some "metoos" might help get this patch through. And yes, I agree, something got worse in 2.44, which is what motivated me to finally attack the issue on my own. I am not quite sure what the change was in 2.44, but hopefully the renaming in this PR will help keep things straight after the real patch lands. Thanks for sharing your situation.

[bubbleguuum]

Just did that.
I really don't remember of HiDpi scaling problems in 2.42.x.

[gwhitney]

Well, my strong suspicion is that even in 2.42, you were not getting a CSS 1in dimension to be an actual physical one inch when displayed on your screen. After the patch, you should. However, there is a question I put in the bugzilla issue about the expected behavior of the GDK_xx_SCALE environment variables, which I am unclear on as I don't use them (but happy to support whatever the "proper" behavior should be).

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 7cd4155)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	❌ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	❌ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	❌ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🛠 watch
	✅ 🛠 watch-sim

[gwhitney]

OK, hopefully the revised version I just submitted addresses all of the listed concerns. Thanks for taking the time to review!

[carlosgcampos]

Looks good to me, let's just fix the wpe build.

[mcatanzaro]

Ah, you'll also want to create a new bug report for this, so that https://bugs.webkit.org/show_bug.cgi?id=247980 doesn't get closed when this lands. The title of the bug should match the title of your commit message here.

(Lastly, in the commit message, remove the period from the end of the URL so the link works.)

[webkit-early-warning-system]

EWS run on previous version of this PR (hash 08006be)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	🛠 ios	🛠 mac	🛠 wpe	🛠 wincairo
✅ 🧪 bindings	🛠 ios-sim	🛠 mac-AS-debug	🧪 wpe-wk2
✅ 🧪 webkitperl	🧪 ios-wk2	🧪 api-mac	🧪 api-wpe
	🧪 ios-wk2-wpt	🧪 mac-wk1	🛠 wpe-skia
	🧪 api-ios	🧪 mac-wk2	🛠 gtk
	✅ 🛠 tv	🧪 mac-AS-debug-wk2	🧪 gtk-wk2
	✅ 🛠 tv-sim	🧪 mac-wk2-stress	🧪 api-gtk
	🛠 watch
	🛠 watch-sim

[gwhitney]

Apologies about the typos. Rebased and corrected them. Hope all is well with this version. Thanks!

[gwhitney]

Oh, sorry, the commit message still says "the commit does not yet fix the bug," which was in reference to the original scaling issue 247980. It does of course address the concern broken out as 272676. Let me know if you need me to amend the commit message.

[mcatanzaro]

Make sure to click the Resolve Conversation button when you've responded to the review feedback.

[mcatanzaro]

This commit also creates a reftest that probes the status of the referenced
bug. Namely, the test compares a 1em box in a text size of 96px, with a
1in box. Note the test passes as the code stands, even though when I view
247980.html and 247980-expected.html in the minibrowser on my machine, the
two boxes are clearly very different in size, which is one key aspect of
the bug.

This is strange. If you're using the same build of WebKit to run both MiniBrowser and the layout tests, then I would expect the results to be consistent. Anyway, the new test is surely not hurting anything, so let's land it.

[mcatanzaro]

Maybe either (a) add </html> closing tags to the test files, or (b) remove the first two lines (pretty sure they're optional). Just looks a little strange to have an opening tag but not a close tag. Hardly a big deal.

[mcatanzaro]

Oh, sorry, the commit message still says "the commit does not yet fix the bug," which was in reference to the original scaling issue 247980. It does of course address the concern broken out as 272676. Let me know if you need me to amend the commit message.

Again not a big deal, but since you're amending the commit one last time anyway, might as well reword it.

[gwhitney]

Make sure to click the Resolve Conversation button when you've responded to the review feedback.

Oh, sorry, in some other repos the reviewer does that when they are satisfied with the response. Will go back and resolve any I believe have been taken care of.

[gwhitney]

This is strange. If you're using the same build of WebKit to run both MiniBrowser and the layout tests, then I would expect the results to be consistent. Anyway, the new test is surely not hurting anything, so let's land it.

I actually don't think it is so strange. The tests are run without a connection to any physical monitor, and I believe in an environment that is telling GTK it is running on a monitor that is physically 96 dots per inch, with no device scaling, and with font scaling to 96 dots per inch - a classic old-school vanilla "standard monitor" which I don't think is particularly common anymore in actual operation. So I don't think any features responding to different dpis or font scaling in the GTK setting are getting exercised in the testing. I don't believe that I am familiar enough with the elaborate testing mechanism to try to address that; and I think it would be a separate issue/PR anyway. If you disagree and would like me to try to delve into that situation in this PR, please let me know.

[mcatanzaro]

Ah, you're right, they are run under Xvfb. We should probably switch that to wlheadless-run at some point, but it will no doubt have the same problem. Oh well.

[gwhitney]

OK, closed the html tag in the test cases and reworded the commit message. Thanks for all of the time and guidance; sorry if this is taking extra effort because I am a new contributor. Let me know if anything else is needed.

[webkit-early-warning-system]

EWS run on current version of this PR (hash 41815f5)

Misc	iOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-skia
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 tv	🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 tv-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🛠 watch
	✅ 🛠 watch-sim

[gwhitney]

I see for the first time there is a failure of automated checks, but it's hard for me to tell if it has anything to do with what I checked in, or if it was just a transient issue, or what. Please let me know if I need to respond in any way, thanks.

[webkit-commit-queue]

Committed 277537@main (51f2745): https://commits.webkit.org/277537@main

Reviewed commits have been landed. Closing PR #26954 and removing active labels.

[aperezdc]

Backported into the 2.44 branch as commit 95b1c08