From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: Stephen Pegoraro Newsgroups: gmane.emacs.devel Subject: Re: HiDPI support for wave style underlines Date: Sat, 29 Jul 2017 17:12:20 +0800 Message-ID: References: <83wp6r3ed8.fsf@gnu.org> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="94eb2c068568eb6b210555713107" X-Trace: blaine.gmane.org 1501319588 23718 195.159.176.226 (29 Jul 2017 09:13:08 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sat, 29 Jul 2017 09:13:08 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Jul 29 11:13:02 2017 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dbNnx-0005i7-75 for ged-emacs-devel@m.gmane.org; Sat, 29 Jul 2017 11:13:01 +0200 Original-Received: from localhost ([::1]:51604 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dbNo3-0002QT-54 for ged-emacs-devel@m.gmane.org; Sat, 29 Jul 2017 05:13:07 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:38220) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dbNnL-0002QM-9t for emacs-devel@gnu.org; Sat, 29 Jul 2017 05:12:24 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dbNnJ-0006RC-MH for emacs-devel@gnu.org; Sat, 29 Jul 2017 05:12:23 -0400 Original-Received: from mail-qt0-x22c.google.com ([2607:f8b0:400d:c0d::22c]:38480) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1dbNnJ-0006QZ-Eg for emacs-devel@gnu.org; Sat, 29 Jul 2017 05:12:21 -0400 Original-Received: by mail-qt0-x22c.google.com with SMTP id t37so94148056qtg.5 for ; Sat, 29 Jul 2017 02:12:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tutive-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to; bh=Zq3gsIuYkP5EkEb6t6gDn0qGp/zNzgw67k+kIbpBFsQ=; b=xZDqRarPD2T7kLR4fEgu8jpbI80g5A3z1wEYQE+PtHNS08Ns1i1B4bTt1Zw1UaVimM tk9BmFNlnaTY3e5tIV8vbHtTQ3GSk7aC+fI5Z0AOtPVofcSP9BB4oRbJ9VlCyqoZU6T/ SoKnCUxprShukM1FgJMbrKT7gw4tBhM543IoNmaVI2QSC/u96lgCGtZOPNaiMDXkMOFY luYZJjHWx0sQ8N9ahzkQwIpzpDAHR0pfrUSeTkWGdH8SJVX3QCZbkhorBh4n8FintOBt 6iHwWD0E4fRWC0wVUPC+ldV0EUkzjM2NzxqeL/GYqs86776FFUYjNKu485beQVj3iJ7i UPCQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to; bh=Zq3gsIuYkP5EkEb6t6gDn0qGp/zNzgw67k+kIbpBFsQ=; b=SVWatdYlLs4rGgUdiGKTY/BnR0E79G1UwEUKaTLeUVSM3NO0daDfzV/h7Rg0hIrDci UIzXRWheSVmDDwAzdu0IWfUoHQ46B466QbGLD7JwIfOP/2Xk5TWpYsRV9DGQq6e/Ixsj eIOmqlf5sLb+A1ie9xPNwfY5wKQ2WgSf1pobyOdszjE9qJceN9cjv/pMKHKUar7ukyGp JJnxK5jJ3J3MJiXk4i7wp2TdP+1z3MoLudYmyfa+CmAjQsGo8FNSDkbhUuhhE3w4AfMs CsVmwp47KrEjuP8yoDksh4xOkCtSeAlZznrVG8ipZ7Rdp5VERpY51gXQZ2Kz+2xr3Yo9 qVdw== X-Gm-Message-State: AIVw112PtYQk2lSSFOi6Nil6d2Dyh5gFk1ikwttrKxrXS1Tq0kNsGGW4 9tSrzfXPQn8S06ZpmilU+e50RFFwvXCzI2M= X-Received: by 10.237.39.3 with SMTP id n3mr14678526qtd.149.1501319540682; Sat, 29 Jul 2017 02:12:20 -0700 (PDT) Original-Received: by 10.140.85.115 with HTTP; Sat, 29 Jul 2017 02:12:20 -0700 (PDT) In-Reply-To: X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 2607:f8b0:400d:c0d::22c X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: "Emacs-devel" Xref: news.gmane.org gmane.emacs.devel:217125 Archived-At: --94eb2c068568eb6b210555713107 Content-Type: text/plain; charset="UTF-8" Hi Eli, Thanks for reviewing! The patch isn't _only_ for hidpi displays but I can see how my original wording comes across that way. It provides correct scaled underlining for all displays, hidpi or otherwise. I meant to say that I have only implemented this for the Xlib side of things, no W32 or mac. Good point regarding the emacs_abort(), I honestly didn't think through that. I have modified x_get_scale_factor to return a new struct x_display_scale with x and y components which are used in the wave length and height calculation. Indentations and comment grammar have been fixed up as well. Cheers, Steve On 29 July 2017 at 15:47, Stephen Pegoraro wrote: > Hi Eli, > > Thanks for reviewing! > > The patch isn't _only_ for hidpi displays but I can see how my > original wording comes across that way. It provides correct scaled > underlining for all displays, hidpi or otherwise. I meant to say that > I have only implemented this for the Xlib side of things, no W32 or > mac. > > Good point regarding the emacs_abort(), I honestly didn't think through that. > I have modified x_get_scale_factor to return a new struct > x_display_scale with x and y components which are used in the wave > length and height calculation. > Indentations and comment grammar have been fixed up as well. > > > Cheers, > Steve > > On 29 July 2017 at 15:13, Eli Zaretskii wrote: >>> From: Stephen Pegoraro >>> Date: Sat, 29 Jul 2017 11:29:43 +0800 >>> >>> I have made an attempt at implementing scaled drawing of wave style >>> underlines for hidpi displays, X only for now. >> >> Thanks. But why do you say this is only for HiDPI displays? What >> aspects of the change are specific to that kind of display? >> >>> +static int x_get_scale_factor(Display *disp) >>> +{ >>> + struct x_display_info * dpyinfo = x_display_info_for_display (disp); >>> + if (!dpyinfo) >>> + emacs_abort (); >>> + >>> + return floor(dpyinfo->resy / 96); >>> +} >> >> The indentation here is not according to our conventions: we use the >> offset of 2, not 4. >> >> Also, why call emacs_abort here? I think it's better to return 1 >> instead. Think about this being called from a daemon, or during early >> stages of Emacs startup, when some of the stuff is not yet set up. >> >> And finally, why do you use only resy? Isn't it better to return 2 >> values, for X and Y separately, and use them accordingly in the >> calling function? >> >>> static void >>> x_draw_underwave (struct glyph_string *s) >>> { >>> - int wave_height = 3, wave_length = 2; >>> + /* Adjust for scale/HiDPI */ >> >> A comment should end with a period, and should have 2 spaces after the >> period. Like this: >> >> /* Adjust for scale/HiDPI. */ >> >>> + int scale = x_get_scale_factor (s->display); >>> + int wave_height = 3 * scale, wave_length = 2 * scale, thickness = scale; >> >> Same issue with indentation here. >> >> Thanks again for working on this. --94eb2c068568eb6b210555713107 Content-Type: text/x-patch; charset="US-ASCII"; name="0001-Implement-HiDPI-support-for-wave-style-underlines (1).patch" Content-Disposition: attachment; filename="0001-Implement-HiDPI-support-for-wave-style-underlines (1).patch" Content-Transfer-Encoding: base64 X-Attachment-Id: f_j5p2vre11 RnJvbSA3ZDY1N2I1N2U2MzU4NzI5M2I4ZWU3NTNmZTEwOTcxYzFiMGQxZDU1IE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBTdGVwaGVuIFBlZ29yYXJvIDxzcGVnb3Jhcm9AdHV0aXZlLmNv bT4KRGF0ZTogU2F0LCAyOSBKdWwgMjAxNyAxMToxMjowMyArMDgwMApTdWJqZWN0OiBbUEFUQ0hd IEltcGxlbWVudCBIaURQSSBzdXBwb3J0IGZvciB3YXZlIHN0eWxlIHVuZGVybGluZXMKCiogc3Jj L3h0ZXJtLmggKHhfZ2V0X3NjYWxlX2ZhY3Rvcik6IE5ldyBmdW5jdGlvbiBkZWNsYXJhdGlvbi4K KiBzcmMveHRlcm0uYyAoeF9kcmF3X3VuZGVyd2F2ZSk6IENvbXB1dGUgaGVpZ2h0LCBsZW5ndGgg YW5kIHRoaWNrbmVzcwpiYXNlZCBvbiBzY2FsZSBmYWN0b3IuCih4X2dldF9zY2FsZV9mYWN0b3Ip OiBOZXcgZnVuY3Rpb24uCi0tLQogc3JjL3h0ZXJtLmMgfCAyNyArKysrKysrKysrKysrKysrKysr KysrLS0tLS0KIHNyYy94dGVybS5oIHwgIDggKysrKysrKysKIDIgZmlsZXMgY2hhbmdlZCwgMzAg aW5zZXJ0aW9ucygrKSwgNSBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9zcmMveHRlcm0uYyBi L3NyYy94dGVybS5jCmluZGV4IGEyMTRjZDgxMDMuLjNjMDQ0MDM3OWIgMTAwNjQ0Ci0tLSBhL3Ny Yy94dGVybS5jCisrKyBiL3NyYy94dGVybS5jCkBAIC0yMyw5ICsyMyw3IEBAIGFsb25nIHdpdGgg R05VIEVtYWNzLiAgSWYgbm90LCBzZWUgPGh0dHA6Ly93d3cuZ251Lm9yZy9saWNlbnNlcy8+LiAg Ki8KICNpbmNsdWRlIDxjb25maWcuaD4KICNpbmNsdWRlIDxzdGRpby5oPgogI2luY2x1ZGUgPHN0 ZGxpYi5oPgotI2lmZGVmIFVTRV9DQUlSTwogI2luY2x1ZGUgPG1hdGguaD4KLSNlbmRpZgogCiAj aW5jbHVkZSAibGlzcC5oIgogI2luY2x1ZGUgImJsb2NraW5wdXQuaCIKQEAgLTM0NzUsNiArMzQ3 MywyMSBAQCB4X2RyYXdfc3RyZXRjaF9nbHlwaF9zdHJpbmcgKHN0cnVjdCBnbHlwaF9zdHJpbmcg KnMpCiAgIHMtPmJhY2tncm91bmRfZmlsbGVkX3AgPSB0cnVlOwogfQogCitzdHJ1Y3QgeF9kaXNw bGF5X3NjYWxlIHhfZ2V0X3NjYWxlX2ZhY3RvcihEaXNwbGF5ICpkaXNwKQoreworICBpbnQgYmFz ZV9yZXMgPSA5NjsKKyAgc3RydWN0IHhfZGlzcGxheV9zY2FsZSBzY2FsZSA9IHsgMSwgMSB9Owor ICBzdHJ1Y3QgeF9kaXNwbGF5X2luZm8gKiBkcHlpbmZvID0geF9kaXNwbGF5X2luZm9fZm9yX2Rp c3BsYXkgKGRpc3ApOworCisgIGlmICghZHB5aW5mbykKKyAgICByZXR1cm4gc2NhbGU7CisKKyAg c2NhbGUueCA9IGZsb29yKGRweWluZm8tPnJlc3ggLyBiYXNlX3Jlcyk7CisgIHNjYWxlLnkgPSBm bG9vcihkcHlpbmZvLT5yZXN5IC8gYmFzZV9yZXMpOworCisgIHJldHVybiBzY2FsZTsKK30KKwog LyoKICAgIERyYXcgYSB3YXZ5IGxpbmUgdW5kZXIgUy4gVGhlIHdhdmUgZmlsbHMgd2F2ZV9oZWln aHQgcGl4ZWxzIGZyb20geTAuCiAKQEAgLTM0ODUsMTEgKzM0OTgsMTMgQEAgeF9kcmF3X3N0cmV0 Y2hfZ2x5cGhfc3RyaW5nIChzdHJ1Y3QgZ2x5cGhfc3RyaW5nICpzKQogICAgIHdhdmVfaGVpZ2h0 ID0gMyAgfCAqICAgKiAgICogICAqCiAKICovCi0KIHN0YXRpYyB2b2lkCiB4X2RyYXdfdW5kZXJ3 YXZlIChzdHJ1Y3QgZ2x5cGhfc3RyaW5nICpzKQogewotICBpbnQgd2F2ZV9oZWlnaHQgPSAzLCB3 YXZlX2xlbmd0aCA9IDI7CisgIC8qIEFkanVzdCBmb3Igc2NhbGUvSGlEUEkuICAqLworICBzdHJ1 Y3QgeF9kaXNwbGF5X3NjYWxlIHNjYWxlID0geF9nZXRfc2NhbGVfZmFjdG9yIChzLT5kaXNwbGF5 KTsKKyAgaW50IHdhdmVfaGVpZ2h0ID0gMyAqIHNjYWxlLnksIHdhdmVfbGVuZ3RoID0gMiAqIHNj YWxlLngsIHRoaWNrbmVzcyA9IHNjYWxlLnk7CisKICNpZmRlZiBVU0VfQ0FJUk8KICAgeF9kcmF3 X2hvcml6b250YWxfd2F2ZSAocy0+Ziwgcy0+Z2MsIHMtPngsIHMtPnliYXNlIC0gd2F2ZV9oZWln aHQgKyAzLAogCQkJICBzLT53aWR0aCwgd2F2ZV9oZWlnaHQsIHdhdmVfbGVuZ3RoKTsKQEAgLTM1 MDEsNyArMzUxNiw3IEBAIHhfZHJhd191bmRlcndhdmUgKHN0cnVjdCBnbHlwaF9zdHJpbmcgKnMp CiAgIGR4ID0gd2F2ZV9sZW5ndGg7CiAgIGR5ID0gd2F2ZV9oZWlnaHQgLSAxOwogICB4MCA9IHMt Png7Ci0gIHkwID0gcy0+eWJhc2UgLSB3YXZlX2hlaWdodCArIDM7CisgIHkwID0gcy0+eWJhc2Ug KyB3YXZlX2hlaWdodCAvIDI7CiAgIHdpZHRoID0gcy0+d2lkdGg7CiAgIHhtYXggPSB4MCArIHdp ZHRoOwogCkBAIC0zNTM1LDYgKzM1NTAsOCBAQCB4X2RyYXdfdW5kZXJ3YXZlIChzdHJ1Y3QgZ2x5 cGhfc3RyaW5nICpzKQogCiAgIHdoaWxlICh4MSA8PSB4bWF4KQogICAgIHsKKyAgICAgIFhTZXRM aW5lQXR0cmlidXRlcyAocy0+ZGlzcGxheSwgcy0+Z2MsIHRoaWNrbmVzcywgTGluZVNvbGlkLCBD YXBCdXR0LAorICAgICAgICAgICAgICAgICAgICAgICAgICBKb2luUm91bmQpOwogICAgICAgWERy YXdMaW5lIChzLT5kaXNwbGF5LCBGUkFNRV9YX0RSQVdBQkxFIChzLT5mKSwgcy0+Z2MsIHgxLCB5 MSwgeDIsIHkyKTsKICAgICAgIHgxICA9IHgyLCB5MSA9IHkyOwogICAgICAgeDIgKz0gZHgsIHky ID0geTAgKyBvZGQqZHk7CmRpZmYgLS1naXQgYS9zcmMveHRlcm0uaCBiL3NyYy94dGVybS5oCmlu ZGV4IDgwM2ZlZGE5OWYuLmZmZDE5ZGNiOGYgMTAwNjQ0Ci0tLSBhL3NyYy94dGVybS5oCisrKyBi L3NyYy94dGVybS5oCkBAIC0xNzIsNiArMTcyLDEzIEBAIFN0YXR1cyB4X3BhcnNlX2NvbG9yIChz dHJ1Y3QgZnJhbWUgKmYsIGNvbnN0IGNoYXIgKmNvbG9yX25hbWUsCiAJCSAgICAgIFhDb2xvciAq Y29sb3IpOwogCiAMCitzdHJ1Y3QgeF9kaXNwbGF5X3NjYWxlCit7CisgIGludCB4OworICBpbnQg eTsKK307CisKKwwKIC8qIEZvciBlYWNoIFggZGlzcGxheSwgd2UgaGF2ZSBhIHN0cnVjdHVyZSB0 aGF0IHJlY29yZHMKICAgIGluZm9ybWF0aW9uIGFib3V0IGl0LiAgKi8KIApAQCAtNDg5LDYgKzQ5 Niw3IEBAIGV4dGVybiBib29sIHVzZV94aW07CiAvKiBUaGlzIGlzIGEgY2hhaW4gb2Ygc3RydWN0 dXJlcyBmb3IgYWxsIHRoZSBYIGRpc3BsYXlzIGN1cnJlbnRseSBpbiB1c2UuICAqLwogZXh0ZXJu IHN0cnVjdCB4X2Rpc3BsYXlfaW5mbyAqeF9kaXNwbGF5X2xpc3Q7CiAKK2V4dGVybiBzdHJ1Y3Qg eF9kaXNwbGF5X3NjYWxlIHhfZ2V0X3NjYWxlX2ZhY3RvcihEaXNwbGF5ICopOwogZXh0ZXJuIHN0 cnVjdCB4X2Rpc3BsYXlfaW5mbyAqeF9kaXNwbGF5X2luZm9fZm9yX2Rpc3BsYXkgKERpc3BsYXkg Kik7CiBleHRlcm4gc3RydWN0IGZyYW1lICp4X3RvcF93aW5kb3dfdG9fZnJhbWUgKHN0cnVjdCB4 X2Rpc3BsYXlfaW5mbyAqLCBpbnQpOwogZXh0ZXJuIHN0cnVjdCB4X2Rpc3BsYXlfaW5mbyAqeF90 ZXJtX2luaXQgKExpc3BfT2JqZWN0LCBjaGFyICosIGNoYXIgKik7Ci0tIAoyLjEzLjMKCg== --94eb2c068568eb6b210555713107--