From: Stephen Pegoraro <spegoraro@tutive.com>
To: emacs-devel@gnu.org
Subject: Re: HiDPI support for wave style underlines
Date: Sat, 29 Jul 2017 17:12:20 +0800 [thread overview]
Message-ID: <CAPzg8ER=5jU6jwhy2uRX-8eGFjnCE3jm8TUkCbPEL9qZcsCeKg@mail.gmail.com> (raw)
In-Reply-To: <CAPzg8ETxJYr8Gw0Pag5zpWqA7aap_kduwuGvJaxAT5yvPhNd6w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3002 bytes --]
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 <spegoraro@tutive.com> 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 <eliz@gnu.org> wrote:
>>> From: Stephen Pegoraro <spegoraro@tutive.com>
>>> 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.
[-- Attachment #2: 0001-Implement-HiDPI-support-for-wave-style-underlines (1).patch --]
[-- Type: text/x-patch, Size: 3415 bytes --]
From 7d657b57e63587293b8ee753fe10971c1b0d1d55 Mon Sep 17 00:00:00 2001
From: Stephen Pegoraro <spegoraro@tutive.com>
Date: Sat, 29 Jul 2017 11:12:03 +0800
Subject: [PATCH] Implement HiDPI support for wave style underlines
* src/xterm.h (x_get_scale_factor): New function declaration.
* src/xterm.c (x_draw_underwave): Compute height, length and thickness
based on scale factor.
(x_get_scale_factor): New function.
---
src/xterm.c | 27 ++++++++++++++++++++++-----
src/xterm.h | 8 ++++++++
2 files changed, 30 insertions(+), 5 deletions(-)
diff --git a/src/xterm.c b/src/xterm.c
index a214cd8103..3c0440379b 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -23,9 +23,7 @@ along with GNU Emacs. If not, see <http://www.gnu.org/licenses/>. */
#include <config.h>
#include <stdio.h>
#include <stdlib.h>
-#ifdef USE_CAIRO
#include <math.h>
-#endif
#include "lisp.h"
#include "blockinput.h"
@@ -3475,6 +3473,21 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
s->background_filled_p = true;
}
+struct x_display_scale x_get_scale_factor(Display *disp)
+{
+ int base_res = 96;
+ struct x_display_scale scale = { 1, 1 };
+ struct x_display_info * dpyinfo = x_display_info_for_display (disp);
+
+ if (!dpyinfo)
+ return scale;
+
+ scale.x = floor(dpyinfo->resx / base_res);
+ scale.y = floor(dpyinfo->resy / base_res);
+
+ return scale;
+}
+
/*
Draw a wavy line under S. The wave fills wave_height pixels from y0.
@@ -3485,11 +3498,13 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
wave_height = 3 | * * * *
*/
-
static void
x_draw_underwave (struct glyph_string *s)
{
- int wave_height = 3, wave_length = 2;
+ /* Adjust for scale/HiDPI. */
+ struct x_display_scale scale = x_get_scale_factor (s->display);
+ int wave_height = 3 * scale.y, wave_length = 2 * scale.x, thickness = scale.y;
+
#ifdef USE_CAIRO
x_draw_horizontal_wave (s->f, s->gc, s->x, s->ybase - wave_height + 3,
s->width, wave_height, wave_length);
@@ -3501,7 +3516,7 @@ x_draw_underwave (struct glyph_string *s)
dx = wave_length;
dy = wave_height - 1;
x0 = s->x;
- y0 = s->ybase - wave_height + 3;
+ y0 = s->ybase + wave_height / 2;
width = s->width;
xmax = x0 + width;
@@ -3535,6 +3550,8 @@ x_draw_underwave (struct glyph_string *s)
while (x1 <= xmax)
{
+ XSetLineAttributes (s->display, s->gc, thickness, LineSolid, CapButt,
+ JoinRound);
XDrawLine (s->display, FRAME_X_DRAWABLE (s->f), s->gc, x1, y1, x2, y2);
x1 = x2, y1 = y2;
x2 += dx, y2 = y0 + odd*dy;
diff --git a/src/xterm.h b/src/xterm.h
index 803feda99f..ffd19dcb8f 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -172,6 +172,13 @@ Status x_parse_color (struct frame *f, const char *color_name,
XColor *color);
\f
+struct x_display_scale
+{
+ int x;
+ int y;
+};
+
+\f
/* For each X display, we have a structure that records
information about it. */
@@ -489,6 +496,7 @@ extern bool use_xim;
/* This is a chain of structures for all the X displays currently in use. */
extern struct x_display_info *x_display_list;
+extern struct x_display_scale x_get_scale_factor(Display *);
extern struct x_display_info *x_display_info_for_display (Display *);
extern struct frame *x_top_window_to_frame (struct x_display_info *, int);
extern struct x_display_info *x_term_init (Lisp_Object, char *, char *);
--
2.13.3
next prev parent reply other threads:[~2017-07-29 9:12 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-29 3:29 HiDPI support for wave style underlines Stephen Pegoraro
2017-07-29 7:13 ` Eli Zaretskii
[not found] ` <CAPzg8ETxJYr8Gw0Pag5zpWqA7aap_kduwuGvJaxAT5yvPhNd6w@mail.gmail.com>
2017-07-29 9:12 ` Stephen Pegoraro [this message]
2017-08-18 8:17 ` Eli Zaretskii
2017-08-21 12:20 ` Yuri D'Elia
2017-08-21 12:32 ` Yuri D'Elia
2017-08-21 14:49 ` Eli Zaretskii
2017-08-27 4:26 ` Stephen Pegoraro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAPzg8ER=5jU6jwhy2uRX-8eGFjnCE3jm8TUkCbPEL9qZcsCeKg@mail.gmail.com' \
--to=spegoraro@tutive.com \
--cc=emacs-devel@gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.