Skip to content

Commit

Permalink
Cherry-pick 277537@main (51f2745). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=272676

    [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

Canonical link: https://commits.webkit.org/274313.194@webkitglib/2.44
  • Loading branch information
gwhitney authored and aperezdc committed May 2, 2024
1 parent bd536df commit 95b1c08
Show file tree
Hide file tree
Showing 10 changed files with 45 additions and 10 deletions.
6 changes: 6 additions & 0 deletions LayoutTests/fast/box-sizing/247980-expected.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
<!DOCTYPE html>
<html>
<body style="font-size:96px;">
<hr style="width:1in;height:1in;">
</body>
</html>
14 changes: 14 additions & 0 deletions LayoutTests/fast/box-sizing/247980.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<html>
<body style="font-size:96px;">
<hr style="width:1em;height:1em;">
<!-- The box produced by this hr element should be identical in size
to one with a width and height of 1in in CSS units, since 1em is
defined as the nominal font size, which has been set to 96px. And
that dimension 96px is in turn required by the CSS standard to equal
1in. Also, the box should in either case measure exactly one inch on
screen, but it's not clear to me how to test that in WebKit's testing
framework.
-->
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -784,7 +784,7 @@ AccessibilityObjectAtspi::TextAttributes AccessibilityObjectAtspi::textAttribute
}

addAttributeIfNeeded("family-name"_s, style.fontCascade().firstFamily());
addAttributeIfNeeded("size"_s, makeString(std::round(style.computedFontSize() * 72 / WebCore::screenDPI()), "pt"));
addAttributeIfNeeded("size"_s, makeString(std::round(style.computedFontSize() * 72 / WebCore::fontDPI()), "pt"));
addAttributeIfNeeded("weight"_s, makeString(static_cast<float>(style.fontCascade().weight())));
addAttributeIfNeeded("style"_s, style.fontCascade().italic() ? "italic"_s : "normal"_s);
addAttributeIfNeeded("strikethrough"_s, style.textDecorationLine() & TextDecorationLine::LineThrough ? "true"_s : "false"_s);
Expand Down
3 changes: 2 additions & 1 deletion Source/WebCore/platform/PlatformScreen.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ WEBCORE_EXPORT DestinationColorSpace screenColorSpace(Widget* = nullptr);
bool screenHasInvertedColors();

#if USE(GLIB)
double screenDPI();
double fontDPI(); // dpi to use for font scaling
double screenDPI(PlatformDisplayID); // dpi of the display device, corrected for device scaling
#endif

FloatRect screenRect(Widget*);
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/platform/ScreenProperties.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ struct ScreenData {
#endif
#if PLATFORM(GTK) || (PLATFORM(WPE) && ENABLE(WPE_PLATFORM))
IntSize screenSize; // In millimeters.
double dpi;
double dpi; // Already corrected for device scaling.
#endif

#if PLATFORM(MAC) || PLATFORM(IOS_FAMILY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ auto SystemFontDatabase::platformSystemFontShorthandInfo(FontShorthand) -> Syste
int size = pango_font_description_get_size(pangoDescription) / PANGO_SCALE;
// If the size of the font is in points, we need to convert it to pixels.
if (!pango_font_description_get_size_is_absolute(pangoDescription))
size = size * (screenDPI() / 72.0);
size = size * (fontDPI() / 72.0);

SystemFontShorthandInfo result { AtomString::fromLatin1(pango_font_description_get_family(pangoDescription)), static_cast<float>(size), normalWeightValue() };
pango_font_description_free(pangoDescription);
Expand Down
9 changes: 8 additions & 1 deletion Source/WebCore/platform/gtk/PlatformScreenGtk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ bool screenHasInvertedColors()
return false;
}

double screenDPI()
double fontDPI()
{
static GtkSettings* gtkSettings = gtk_settings_get_default();
if (gtkSettings) {
Expand All @@ -97,6 +97,13 @@ double screenDPI()
return data ? data->dpi : 96.;
}

double screenDPI(PlatformDisplayID screendisplayID)
{
auto* data = screenData(screendisplayID);
return data ? data->dpi : 96.;
}


FloatRect screenRect(Widget* widget)
{
if (auto* data = screenData(widgetDisplayID(widget)))
Expand Down
10 changes: 8 additions & 2 deletions Source/WebCore/platform/wpe/PlatformScreenWPE.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,23 @@ bool screenHasInvertedColors()
return false;
}

double screenDPI()
double screenDPI(PlatformDisplayID screendisplayID)
{
#if ENABLE(WPE_PLATFORM)
auto* data = screenData(primaryScreenDisplayID());
auto* data = screenData(screendisplayID);
return data ? data->dpi : 96.;
#else
notImplemented();
return 96;
#endif
}

double fontDPI()
{
// In WPE, there is no notion of font scaling separate from device DPI.
return screenDPI(primaryScreenDisplayID());
}

FloatRect screenRect(Widget* widget)
{
#if ENABLE(WPE_PLATFORM)
Expand Down
3 changes: 2 additions & 1 deletion Source/WebKit/UIProcess/API/glib/WebKitProtocolHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,8 @@ void WebKitProtocolHandler::handleGPU(WebKitURISchemeRequest* request)
addTableRow(displayObject, "Screen work area"_s, makeString(rect.x(), ',', rect.y(), ' ', rect.width(), 'x', rect.height()));
addTableRow(displayObject, "Depth"_s, String::number(screenDepth(nullptr)));
addTableRow(displayObject, "Bits per color component"_s, String::number(screenDepthPerComponent(nullptr)));
addTableRow(displayObject, "DPI"_s, String::number(screenDPI()));
addTableRow(displayObject, "Font Scaling DPI"_s, String::number(fontDPI()));
addTableRow(displayObject, "Screen DPI"_s, String::number(screenDPI(displayID.value_or(primaryScreenDisplayID()))));

if (displayID) {
if (auto* displayLink = page.process().processPool().displayLinks().existingDisplayLinkForDisplay(*displayID)) {
Expand Down
4 changes: 2 additions & 2 deletions Source/WebKit/UIProcess/API/glib/WebKitSettings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3835,7 +3835,7 @@ void webkit_settings_set_enable_back_forward_navigation_gestures(WebKitSettings*
*/
guint32 webkit_settings_font_size_to_points(guint32 pixels)
{
return std::round(pixels * 72 / WebCore::screenDPI());
return std::round(pixels * 72 / WebCore::fontDPI());
}

/**
Expand All @@ -3855,7 +3855,7 @@ guint32 webkit_settings_font_size_to_points(guint32 pixels)
*/
guint32 webkit_settings_font_size_to_pixels(guint32 points)
{
return std::round(points * WebCore::screenDPI() / 72);
return std::round(points * WebCore::fontDPI() / 72);
}
#endif // PLATFORM(GTK)

Expand Down

0 comments on commit 95b1c08

Please sign in to comment.