210543 2020-04-15 05:44:13 -0700 [GTK] Make PlatformScreen::screenDPI() GTK4-ready 2020-04-15 07:54:58 -0700 1 1 1 Unclassified WebKit WebKitGTK WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED P2 Normal --- 210100 1 csaavedra csaavedra aperez bugs-noreply oldest_to_newest 1641748 0 csaavedra 2020-04-15 05:44:13 -0700 [GTK] Make PlatformScreen::screenDPI() GTK4-ready 1641749 1 396521 csaavedra 2020-04-15 05:46:28 -0700 Created attachment 396521 Patch 1641762 2 396521 aperez 2020-04-15 06:37:13 -0700 Comment on attachment 396521 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 🙆‍♂️️ 1641781 3 396521 csaavedra 2020-04-15 07:13:45 -0700 Comment on attachment 396521 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. 1641782 4 396530 csaavedra 2020-04-15 07:15:25 -0700 Created attachment 396530 Patch for landing 1641790 5 ews-feeder 2020-04-15 07:54:57 -0700 Committed r260127: <https://trac.webkit.org/changeset/260127> All reviewed patches have been landed. Closing bug and clearing flags on attachment 396530. 396521 2020-04-15 05:46:28 -0700 2020-04-15 07:15:14 -0700 Patch bug-210543-20200415154626.patch text/plain 3377 csaavedra U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwMDcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2EwMDQ4M2QyMDU4OGY5 YmE1ZDcxYzFjYzk2NjQ5MzVmZjAwY2ZhOS4uZTNiYzMwMzRkZjcwMjlhYTVkNWQwNzQ0NTM2MDg3 ZjA5NGU3Nzc3MyAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMwIEBACisyMDIwLTA0LTE1ICBDbGF1 ZGlvIFNhYXZlZHJhICA8Y3NhYXZlZHJhQGlnYWxpYS5jb20+CisKKyAgICAgICAgW0dUS10gTWFr ZSBQbGF0Zm9ybVNjcmVlbjo6c2NyZWVuRFBJKCkgR1RLNC1yZWFkeQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEwNTQzCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgTm8gbmV3IHRlc3RzIG5lZWRlZC4KKwor ICAgICAgICBUaGlzIG1ldGhvZCBpcyB1c2luZyBkZXByZWNhdGVkIGFuZCByZW1vdmVkIEFQSXMK KyAgICAgICAgZnJvbSBHREsuIEd1YXJkIHRoZSByZW1vdmVkIEFQSSB1c2FnZSBzbyB0aGF0IGl0 J3Mgb25seQorICAgICAgICB1c2VkIGluIEdUSzMgYW5kIHVwZGF0ZSB0byB1c2UgdGhlIHJlcGxh Y2VtZW50IEFQSXMgb3RoZXJ3aXNlLgorCisgICAgICAgIEFsc28sIG1ha2UgaXQgdG8gYWxzbyB1 c2UgdGhlIGd0ay14ZnQtZHBpIEd0a1NldHRpbmdzIHByb3BlcnR5LgorICAgICAgICBUaGlzIG1l dGhvZCBpcyBtb3N0bHkgdXNlZCBpbiByZXNwb25zZSB0byBhIGNoYW5nZSBpbiB0aGlzCisgICAg ICAgIHByb3BlcnR5LCBzbyBpZ25vcmluZyBpdHMgdmFsdWUgZG9lc24ndCBzZWVtIGEgZ29vZCBp ZGVhLgorCisgICAgICAgIFRoZSBmb2xsb3dpbmcgcHJpb3JpdHkgaXMgdXNlZDoKKworICAgICAg ICAxLiAoR1RLMyBvbmx5KSBxdWVyeSBnZGtfc2NyZWVuX2dldF9yZXNvbHV0aW9uKCkuCisgICAg ICAgIDIuIFVzZSB0aGUgR3RrU2V0dGluZ3M6Omd0ay14ZnQtZHBpIHByb3BlcnR5LgorICAgICAg ICAzLiBDYWxjdWxhdGUgdGhlIGFjdHVhbCBEUEkgZnJvbSB0aGUgbW9uaXRvciAwJ3MgcHJvcGVy dGllcy4KKyAgICAgICAgNC4gSWYgbm9uZSBvZiB0aGVzZSBzdWNjZWVkLCB1c2UgdGhlIGRlZmF1 bHQgRFBJLCA5Ni4KKworICAgICAgICAqIHBsYXRmb3JtL2d0ay9QbGF0Zm9ybVNjcmVlbkd0ay5j cHA6CisgICAgICAgIChXZWJDb3JlOjpzY3JlZW5EUEkpOgorCiAyMDIwLTA0LTE0ICBDbGF1ZGlv IFNhYXZlZHJhICA8Y3NhYXZlZHJhQGlnYWxpYS5jb20+CiAKICAgICAgICAgW0dUS10gQWRhcHQg dG8gR2RrVmlzdWFsIGRlcHJlY2F0aW9uIGFuZCByZW1vdmFsCmRpZmYgLS1naXQgYS9Tb3VyY2Uv V2ViQ29yZS9wbGF0Zm9ybS9ndGsvUGxhdGZvcm1TY3JlZW5HdGsuY3BwIGIvU291cmNlL1dlYkNv cmUvcGxhdGZvcm0vZ3RrL1BsYXRmb3JtU2NyZWVuR3RrLmNwcAppbmRleCAxM2E4NzhiNDI0ZjA5 YTk2YzNlZTE1MmYzOTNiYzBmNDY1YzgxNzlkLi4yMDk2NzQ2NDA3YmMwOTdhNmMyYzY3ZjZiNTcy YWUzYTdiZjc4OWIxIDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9ndGsvUGxh dGZvcm1TY3JlZW5HdGsuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9QbGF0 Zm9ybVNjcmVlbkd0ay5jcHAKQEAgLTkyLDIxICs5MiwzOSBAQCBib29sIHNjcmVlbkhhc0ludmVy dGVkQ29sb3JzKCkKIGRvdWJsZSBzY3JlZW5EUEkoKQogewogICAgIHN0YXRpYyBjb25zdCBkb3Vi bGUgZGVmYXVsdERwaSA9IDk2OworI2lmICFVU0UoR1RLNCkKICAgICBHZGtTY3JlZW4qIHNjcmVl biA9IGdka19zY3JlZW5fZ2V0X2RlZmF1bHQoKTsKLSAgICBpZiAoIXNjcmVlbikKLSAgICAgICAg cmV0dXJuIGRlZmF1bHREcGk7CisgICAgaWYgKHNjcmVlbikgeworICAgICAgICBkb3VibGUgZHBp ID0gZ2RrX3NjcmVlbl9nZXRfcmVzb2x1dGlvbihzY3JlZW4pOworICAgICAgICBpZiAoZHBpICE9 IC0xKQorICAgICAgICAgICAgcmV0dXJuIGRwaTsKKyAgICB9CisjZW5kaWYKIAotICAgIGRvdWJs ZSBkcGkgPSBnZGtfc2NyZWVuX2dldF9yZXNvbHV0aW9uKHNjcmVlbik7Ci0gICAgaWYgKGRwaSAh PSAtMSkKLSAgICAgICAgcmV0dXJuIGRwaTsKKyAgICBzdGF0aWMgR3RrU2V0dGluZ3MqIGd0a1Nl dHRpbmdzID0gZ3RrX3NldHRpbmdzX2dldF9kZWZhdWx0KCk7CisgICAgaWYgKGd0a1NldHRpbmdz KSB7CisgICAgICAgIGludCBndGtYZnREcGk7CisgICAgICAgIGdfb2JqZWN0X2dldChndGtTZXR0 aW5ncywgImd0ay14ZnQtZHBpIiwgJmd0a1hmdERwaSwgTlVMTCk7CisgICAgICAgIHJldHVybiBn dGtYZnREcGkgLyAxMDI0LjA7CisgICAgfQogCiAgICAgc3RhdGljIGRvdWJsZSBjYWNoZWREcGkg PSAwOwogICAgIGlmIChjYWNoZWREcGkpCiAgICAgICAgIHJldHVybiBjYWNoZWREcGk7CiAKICAg ICBzdGF0aWMgY29uc3QgZG91YmxlIG1pbGxpbWV0cmVzUGVySW5jaCA9IDI1LjQ7Ci0gICAgZG91 YmxlIGRpYWdvbmFsSW5QaXhlbHMgPSBzdGQ6Omh5cG90KGdka19zY3JlZW5fZ2V0X3dpZHRoKHNj cmVlbiksIGdka19zY3JlZW5fZ2V0X2hlaWdodChzY3JlZW4pKTsKLSAgICBkb3VibGUgZGlhZ29u YWxJbkluY2hlcyA9IHN0ZDo6aHlwb3QoZ2RrX3NjcmVlbl9nZXRfd2lkdGhfbW0oc2NyZWVuKSwg Z2RrX3NjcmVlbl9nZXRfaGVpZ2h0X21tKHNjcmVlbikpIC8gbWlsbGltZXRyZXNQZXJJbmNoOwor CisgICAgR2RrRGlzcGxheSogZGlzcGxheSA9IGdka19kaXNwbGF5X2dldF9kZWZhdWx0KCk7Cisg ICAgaWYgKCFkaXNwbGF5KQorICAgICAgICByZXR1cm4gZGVmYXVsdERwaTsKKyAgICBHZGtNb25p dG9yKiBtb25pdG9yID0gZ2RrX2Rpc3BsYXlfZ2V0X21vbml0b3IoZGlzcGxheSwgMCk7CisgICAg aWYgKCFtb25pdG9yKQorICAgICAgICByZXR1cm4gZGVmYXVsdERwaTsKKworICAgIEdka1JlY3Rh bmdsZSBnZW9tZXRyeTsKKyAgICBnZGtfbW9uaXRvcl9nZXRfZ2VvbWV0cnkobW9uaXRvciwgJmdl b21ldHJ5KTsKKyAgICBkb3VibGUgZGlhZ29uYWxJblBpeGVscyA9IHN0ZDo6aHlwb3QoZ2VvbWV0 cnkud2lkdGgsIGdlb21ldHJ5LmhlaWdodCk7CisgICAgZG91YmxlIGRpYWdvbmFsSW5JbmNoZXMg PSBzdGQ6Omh5cG90KGdka19tb25pdG9yX2dldF93aWR0aF9tbShtb25pdG9yKSwgZ2RrX21vbml0 b3JfZ2V0X2hlaWdodF9tbShtb25pdG9yKSkgLyBtaWxsaW1ldHJlc1BlckluY2g7CiAgICAgY2Fj aGVkRHBpID0gZGlhZ29uYWxJblBpeGVscyAvIGRpYWdvbmFsSW5JbmNoZXM7CiAKICAgICByZXR1 cm4gY2FjaGVkRHBpOwo= 396530 2020-04-15 07:15:25 -0700 2020-04-15 07:54:58 -0700 Patch for landing bug-210543-20200415171521.patch text/plain 3388 csaavedra U3VidmVyc2lvbiBSZXZpc2lvbjogMjYwMDcyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggN2EwMDQ4M2QyMDU4OGY5 YmE1ZDcxYzFjYzk2NjQ5MzVmZjAwY2ZhOS4uMzM2ZjYzNjM5OWNhZjljMjg4NGViYTUxZDY0OGM3 ZTMwMTMyODBiNiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDMwIEBACisyMDIwLTA0LTE1ICBDbGF1 ZGlvIFNhYXZlZHJhICA8Y3NhYXZlZHJhQGlnYWxpYS5jb20+CisKKyAgICAgICAgW0dUS10gTWFr ZSBQbGF0Zm9ybVNjcmVlbjo6c2NyZWVuRFBJKCkgR1RLNC1yZWFkeQorICAgICAgICBodHRwczov L2J1Z3Mud2Via2l0Lm9yZy9zaG93X2J1Zy5jZ2k/aWQ9MjEwNTQzCisKKyAgICAgICAgUmV2aWV3 ZWQgYnkgQWRyaWFuIFBlcmV6IGRlIENhc3Ryby4KKworICAgICAgICBObyBuZXcgdGVzdHMgbmVl ZGVkLgorCisgICAgICAgIFRoaXMgbWV0aG9kIGlzIHVzaW5nIGRlcHJlY2F0ZWQgYW5kIHJlbW92 ZWQgQVBJcworICAgICAgICBmcm9tIEdESy4gR3VhcmQgdGhlIHJlbW92ZWQgQVBJIHVzYWdlIHNv IHRoYXQgaXQncyBvbmx5CisgICAgICAgIHVzZWQgaW4gR1RLMyBhbmQgdXBkYXRlIHRvIHVzZSB0 aGUgcmVwbGFjZW1lbnQgQVBJcyBvdGhlcndpc2UuCisKKyAgICAgICAgQWxzbywgbWFrZSBpdCB0 byBhbHNvIHVzZSB0aGUgZ3RrLXhmdC1kcGkgR3RrU2V0dGluZ3MgcHJvcGVydHkuCisgICAgICAg IFRoaXMgbWV0aG9kIGlzIG1vc3RseSB1c2VkIGluIHJlc3BvbnNlIHRvIGEgY2hhbmdlIGluIHRo aXMKKyAgICAgICAgcHJvcGVydHksIHNvIGlnbm9yaW5nIGl0cyB2YWx1ZSBkb2Vzbid0IHNlZW0g YSBnb29kIGlkZWEuCisKKyAgICAgICAgVGhlIGZvbGxvd2luZyBwcmlvcml0eSBpcyB1c2VkOgor CisgICAgICAgIDEuIChHVEszIG9ubHkpIHF1ZXJ5IGdka19zY3JlZW5fZ2V0X3Jlc29sdXRpb24o KS4KKyAgICAgICAgMi4gVXNlIHRoZSBHdGtTZXR0aW5nczo6Z3RrLXhmdC1kcGkgcHJvcGVydHku CisgICAgICAgIDMuIENhbGN1bGF0ZSB0aGUgYWN0dWFsIERQSSBmcm9tIHRoZSBtb25pdG9yIDAn cyBwcm9wZXJ0aWVzLgorICAgICAgICA0LiBJZiBub25lIG9mIHRoZXNlIHN1Y2NlZWQsIHVzZSB0 aGUgZGVmYXVsdCBEUEksIDk2LgorCisgICAgICAgICogcGxhdGZvcm0vZ3RrL1BsYXRmb3JtU2Ny ZWVuR3RrLmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OnNjcmVlbkRQSSk6CisKIDIwMjAtMDQtMTQg IENsYXVkaW8gU2FhdmVkcmEgIDxjc2FhdmVkcmFAaWdhbGlhLmNvbT4KIAogICAgICAgICBbR1RL XSBBZGFwdCB0byBHZGtWaXN1YWwgZGVwcmVjYXRpb24gYW5kIHJlbW92YWwKZGlmZiAtLWdpdCBh L1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL2d0ay9QbGF0Zm9ybVNjcmVlbkd0ay5jcHAgYi9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9ndGsvUGxhdGZvcm1TY3JlZW5HdGsuY3BwCmluZGV4IDEzYTg3 OGI0MjRmMDlhOTZjM2VlMTUyZjM5M2JjMGY0NjVjODE3OWQuLjIxMmQwZTlhMWRlZjEyMzNmOTEw ODJiNTk2NjVhZTc0ODNkZjE2ZGQgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L2d0ay9QbGF0Zm9ybVNjcmVlbkd0ay5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v Z3RrL1BsYXRmb3JtU2NyZWVuR3RrLmNwcApAQCAtOTIsMjEgKzkyLDM5IEBAIGJvb2wgc2NyZWVu SGFzSW52ZXJ0ZWRDb2xvcnMoKQogZG91YmxlIHNjcmVlbkRQSSgpCiB7CiAgICAgc3RhdGljIGNv bnN0IGRvdWJsZSBkZWZhdWx0RHBpID0gOTY7CisjaWYgIVVTRShHVEs0KQogICAgIEdka1NjcmVl biogc2NyZWVuID0gZ2RrX3NjcmVlbl9nZXRfZGVmYXVsdCgpOwotICAgIGlmICghc2NyZWVuKQot ICAgICAgICByZXR1cm4gZGVmYXVsdERwaTsKKyAgICBpZiAoc2NyZWVuKSB7CisgICAgICAgIGRv dWJsZSBkcGkgPSBnZGtfc2NyZWVuX2dldF9yZXNvbHV0aW9uKHNjcmVlbik7CisgICAgICAgIGlm IChkcGkgIT0gLTEpCisgICAgICAgICAgICByZXR1cm4gZHBpOworICAgIH0KKyNlbmRpZgogCi0g ICAgZG91YmxlIGRwaSA9IGdka19zY3JlZW5fZ2V0X3Jlc29sdXRpb24oc2NyZWVuKTsKLSAgICBp ZiAoZHBpICE9IC0xKQotICAgICAgICByZXR1cm4gZHBpOworICAgIHN0YXRpYyBHdGtTZXR0aW5n cyogZ3RrU2V0dGluZ3MgPSBndGtfc2V0dGluZ3NfZ2V0X2RlZmF1bHQoKTsKKyAgICBpZiAoZ3Rr U2V0dGluZ3MpIHsKKyAgICAgICAgaW50IGd0a1hmdERwaTsKKyAgICAgICAgZ19vYmplY3RfZ2V0 KGd0a1NldHRpbmdzLCAiZ3RrLXhmdC1kcGkiLCAmZ3RrWGZ0RHBpLCBudWxscHRyKTsKKyAgICAg ICAgcmV0dXJuIGd0a1hmdERwaSAvIDEwMjQuMDsKKyAgICB9CiAKICAgICBzdGF0aWMgZG91Ymxl IGNhY2hlZERwaSA9IDA7CiAgICAgaWYgKGNhY2hlZERwaSkKICAgICAgICAgcmV0dXJuIGNhY2hl ZERwaTsKIAogICAgIHN0YXRpYyBjb25zdCBkb3VibGUgbWlsbGltZXRyZXNQZXJJbmNoID0gMjUu NDsKLSAgICBkb3VibGUgZGlhZ29uYWxJblBpeGVscyA9IHN0ZDo6aHlwb3QoZ2RrX3NjcmVlbl9n ZXRfd2lkdGgoc2NyZWVuKSwgZ2RrX3NjcmVlbl9nZXRfaGVpZ2h0KHNjcmVlbikpOwotICAgIGRv dWJsZSBkaWFnb25hbEluSW5jaGVzID0gc3RkOjpoeXBvdChnZGtfc2NyZWVuX2dldF93aWR0aF9t bShzY3JlZW4pLCBnZGtfc2NyZWVuX2dldF9oZWlnaHRfbW0oc2NyZWVuKSkgLyBtaWxsaW1ldHJl c1BlckluY2g7CisKKyAgICBHZGtEaXNwbGF5KiBkaXNwbGF5ID0gZ2RrX2Rpc3BsYXlfZ2V0X2Rl ZmF1bHQoKTsKKyAgICBpZiAoIWRpc3BsYXkpCisgICAgICAgIHJldHVybiBkZWZhdWx0RHBpOwor ICAgIEdka01vbml0b3IqIG1vbml0b3IgPSBnZGtfZGlzcGxheV9nZXRfbW9uaXRvcihkaXNwbGF5 LCAwKTsKKyAgICBpZiAoIW1vbml0b3IpCisgICAgICAgIHJldHVybiBkZWZhdWx0RHBpOworCisg ICAgR2RrUmVjdGFuZ2xlIGdlb21ldHJ5OworICAgIGdka19tb25pdG9yX2dldF9nZW9tZXRyeSht b25pdG9yLCAmZ2VvbWV0cnkpOworICAgIGRvdWJsZSBkaWFnb25hbEluUGl4ZWxzID0gc3RkOjpo eXBvdChnZW9tZXRyeS53aWR0aCwgZ2VvbWV0cnkuaGVpZ2h0KTsKKyAgICBkb3VibGUgZGlhZ29u YWxJbkluY2hlcyA9IHN0ZDo6aHlwb3QoZ2RrX21vbml0b3JfZ2V0X3dpZHRoX21tKG1vbml0b3Ip LCBnZGtfbW9uaXRvcl9nZXRfaGVpZ2h0X21tKG1vbml0b3IpKSAvIG1pbGxpbWV0cmVzUGVySW5j aDsKICAgICBjYWNoZWREcGkgPSBkaWFnb25hbEluUGl4ZWxzIC8gZGlhZ29uYWxJbkluY2hlczsK IAogICAgIHJldHVybiBjYWNoZWREcGk7Cg==