emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: HiDPI support for wave style underlines


From: Eli Zaretskii
Subject: Re: HiDPI support for wave style underlines
Date: Sat, 29 Jul 2017 10:13:07 +0300

> From: Stephen Pegoraro <address@hidden>
> 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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]