Bug 210543

Created attachment 396521 [details] Patch

Comment on attachment 396521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396521&action=review There is just one small change needed before landing; the patch looks fine otherwise. > Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:99 > + if (dpi != -1) It would be good better use “if (fabs(dpi - -1.0) > DBL_MIN)” here. Comparison of floating point values is not guaranteed to work well, I guess this has been working fine because gdk_screen_get_resolution() is doing “return -1.0” with a literal, and doing “-1 == dpi” results in comparing two doubles with the same bit-level representation… but feel free to leave it as-is if you would rather leave the existing logic untouched. > Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:107 > + g_object_get(gtkSettings, "gtk-xft-dpi", &gtkXftDpi, NULL); NULL → nullptr > Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:120 > + GdkMonitor* monitor = gdk_display_get_monitor(display, 0); It's a bit sad that we cannot set the DPI per web view; it would be nicer to find out on which monitor the widget is being displayed and use the value for that. Anyway, this is as good as the existing code was, so that's okay 🙆‍♂️️

Comment on attachment 396521 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=396521&action=review >> Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:99 >> + if (dpi != -1) > > It would be good better use “if (fabs(dpi - -1.0) > DBL_MIN)” here. Comparison > of floating point values is not guaranteed to work well, I guess this has been > working fine because gdk_screen_get_resolution() is doing “return -1.0” with a > literal, and doing “-1 == dpi” results in comparing two doubles with the same > bit-level representation… but feel free to leave it as-is if you would rather > leave the existing logic untouched. I think I'm gonna stick to the -1 comparison since we know it's a literal that's used. >> Source/WebCore/platform/gtk/PlatformScreenGtk.cpp:120 >> + GdkMonitor* monitor = gdk_display_get_monitor(display, 0); > > It's a bit sad that we cannot set the DPI per web view; it would be nicer to > find out on which monitor the widget is being displayed and use the value for > that. Anyway, this is as good as the existing code was, so that's okay 🙆‍♂️️ We can check whether doing this would be feasible when we break API for GTK4, but this code will have to move somewhere else as PlatformScreen is not useful with multiple monitors.

Created attachment 396530 [details] Patch for landing

Committed r260127: <https://trac.webkit.org/changeset/260127> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396530 [details].