* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-18 18:26 ` Eli Zaretskii
@ 2021-11-19 0:26 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 7:01 ` Eli Zaretskii
2021-11-19 1:57 ` Feng Shu
` (2 subsequent siblings)
3 siblings, 1 reply; 64+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-19 0:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: tumashu, larsi, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> Please try the patch below. I have a very limited number of cases
> with which I can test it, so I think users of CJK scripts should say
> what they think. Basically, set the new variable line-height-factor
> to something like 1.2 (more if you enlarge the non-ASCII font to make
> double-width characters take close to 2 columns), and see if you get
> the desired effect.
Thanks, it seems to work well for what it's intended to.
> If this is what people want, I will install it and document it.
As for this, I have no specific opinion, as I didn't find the old
behavior problematic in the first place. But thanks again!
> diff --git a/src/window.c b/src/window.c
> index e801ff8..a368b6d 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -5246,7 +5246,10 @@ grow_mini_window (struct window *w, int delta)
> {
> struct frame *f = XFRAME (w->frame);
> int old_height = window_body_height (w, true);
> - int min_height = FRAME_LINE_HEIGHT (f);
> + int min_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
>
> eassert (MINI_WINDOW_P (w));
>
> @@ -5279,7 +5282,11 @@ grow_mini_window (struct window *w, int delta)
> shrink_mini_window (struct window *w)
> {
> struct frame *f = XFRAME (w->frame);
> - int delta = window_body_height (w, true) - FRAME_LINE_HEIGHT (f);
> + int min_line_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
> + int delta = window_body_height (w, true) - min_line_height;
>
> eassert (MINI_WINDOW_P (w));
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index ef49297..d1f4043 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -22073,6 +22073,18 @@ append_space_for_newline (struct it *it, bool default_face_p)
> it->descent = it->override_descent;
> boff = it->override_boff;
> }
> + else
> + {
> + /* Enlarge the ascent to make the row higher by
> + line-height-factor if needed. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> + }
> if (EQ (height, Qt))
> extra_line_spacing = 0;
> else
> @@ -30587,9 +30599,9 @@ gui_produce_glyphs (struct it *it)
> /* When no suitable font is found, display this character by
> the method specified in the first extra slot of
> Vglyphless_char_display. */
> - Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
> + Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
>
> - eassert (it->what == IT_GLYPHLESS);
> + eassert (it->what == IT_GLYPHLESS);
> produce_glyphless_glyph (it, true,
> STRINGP (acronym) ? acronym : Qnil);
> goto done;
> @@ -30613,6 +30625,15 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + /* If this glyph uses the ASCII face, enlarge the ascent
> + to make the row higher by line-height-factor. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
>
> if (get_char_glyph_code (it->char_to_display, font, &char2b))
> @@ -30763,6 +30784,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> }
>
> @@ -30886,6 +30914,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> it->phys_ascent = it->ascent;
> it->phys_descent = it->descent;
> @@ -31243,6 +31278,24 @@ gui_produce_glyphs (struct it *it)
> if (it->glyph_row
> && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
> it->glyph_row->contains_overlapping_glyphs_p = true;
> + if (it->voffset == 0 && FLOATP (Vline_height_factor))
> + {
> + Lisp_Object font_object = LGSTRING_FONT (gstring);
> + struct font *gstring_font = XFONT_OBJECT (font_object);
> +
> + /* This is for when the grapheme cluster displays some
> + ligature using ASCII font: if the height is smaller
> + than line-height-factor says, enlarge the ascent. */
> + if (face == face->ascii_face
> + && face->ascii_face->font == gstring_font)
> + {
> + int enlarged_height =
> + ((FONT_BASE (gstring_font) + FONT_DESCENT (gstring_font))
> + * XFLOAT_DATA (Vline_height_factor));
> + if (metrics.ascent + metrics.descent < enlarged_height)
> + metrics.ascent = enlarged_height - metrics.descent;
> + }
> + }
> it->ascent = it->phys_ascent = metrics.ascent;
> it->descent = it->phys_descent = metrics.descent;
> }
> @@ -35581,6 +35634,16 @@ syms_of_xdisp (void)
> mini-window frame's default font's height. */);
> Vmax_mini_window_height = make_float (0.25);
>
> + DEFVAR_LISP ("line-height-factor", Vline_height_factor,
> + doc: /* Factor for enlarging the height of lines that use the default font.
> +The value should be a float number greater than 1. It determines how
> +much will Emacs enlarge the height of a screen line that shows only
> +characters displayed with the default face's font for ASCII characters.
> +This is to avoid differences in height between lines that use the
> +ASCII font and those which use non-ASCII (for example, Chinese)
> +font, which is typically higher than the ASCII one. */);
> + Vline_height_factor = Qnil;
> +
> DEFVAR_LISP ("resize-mini-windows", Vresize_mini_windows,
> doc: /* How to resize mini-windows (the minibuffer and the echo area).
> A value of nil means don't automatically resize mini-windows.
^ permalink raw reply [flat|nested] 64+ messages in thread* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 0:26 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-19 7:01 ` Eli Zaretskii
0 siblings, 0 replies; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 7:01 UTC (permalink / raw)
To: Po Lu; +Cc: tumashu, larsi, 51821
> From: Po Lu <luangruo@yahoo.com>
> Cc: tumashu@163.com, larsi@gnus.org, 51821@debbugs.gnu.org
> Date: Fri, 19 Nov 2021 08:26:57 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Please try the patch below. I have a very limited number of cases
> > with which I can test it, so I think users of CJK scripts should say
> > what they think. Basically, set the new variable line-height-factor
> > to something like 1.2 (more if you enlarge the non-ASCII font to make
> > double-width characters take close to 2 columns), and see if you get
> > the desired effect.
>
> Thanks, it seems to work well for what it's intended to.
Thanks for testing.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-18 18:26 ` Eli Zaretskii
2021-11-19 0:26 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-19 1:57 ` Feng Shu
2021-11-19 7:08 ` Eli Zaretskii
2021-11-19 7:12 ` Lars Ingebrigtsen
2021-11-19 14:48 ` Feng Shu
3 siblings, 1 reply; 64+ messages in thread
From: Feng Shu @ 2021-11-19 1:57 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, larsi, 51821
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Tue, 16 Nov 2021 18:45:16 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: tumashu@163.com, 51821@debbugs.gnu.org
>>
>> > From: Lars Ingebrigtsen <larsi@gnus.org>
>> > Cc: tumashu@163.com, 51821@debbugs.gnu.org
>> > Date: Tue, 16 Nov 2021 16:20:52 +0100
>> >
>> > > That shouldn't happen, normally. Emacs selects fonts of the same size
>> > > for a face.
>> >
>> > No, not really. Here's a trivial example:
>> >
>> >
>> > Note how the lines with jidanni are taller than the rest.
>>
>> OK, let me see what can be done about that.
>
> Please try the patch below. I have a very limited number of cases
> with which I can test it, so I think users of CJK scripts should say
> what they think. Basically, set the new variable line-height-factor
> to something like 1.2 (more if you enlarge the non-ASCII font to make
> double-width characters take close to 2 columns), and see if you get
> the desired effect.
>
> If this is what people want, I will install it and document it.
>
I think the below patch has problem, maybe it relate to font
baseline, I do not know, please see attachement.
[-- Attachment #2: test-2021-11-19_09.50.06.mkv --]
[-- Type: video/x-matroska, Size: 1227807 bytes --]
[-- Attachment #3: Type: text/plain, Size: 6098 bytes --]
> diff --git a/src/window.c b/src/window.c
> index e801ff8..a368b6d 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -5246,7 +5246,10 @@ grow_mini_window (struct window *w, int delta)
> {
> struct frame *f = XFRAME (w->frame);
> int old_height = window_body_height (w, true);
> - int min_height = FRAME_LINE_HEIGHT (f);
> + int min_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
>
> eassert (MINI_WINDOW_P (w));
>
> @@ -5279,7 +5282,11 @@ grow_mini_window (struct window *w, int delta)
> shrink_mini_window (struct window *w)
> {
> struct frame *f = XFRAME (w->frame);
> - int delta = window_body_height (w, true) - FRAME_LINE_HEIGHT (f);
> + int min_line_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
> + int delta = window_body_height (w, true) - min_line_height;
>
> eassert (MINI_WINDOW_P (w));
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index ef49297..d1f4043 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -22073,6 +22073,18 @@ append_space_for_newline (struct it *it, bool default_face_p)
> it->descent = it->override_descent;
> boff = it->override_boff;
> }
> + else
> + {
> + /* Enlarge the ascent to make the row higher by
> + line-height-factor if needed. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> + }
> if (EQ (height, Qt))
> extra_line_spacing = 0;
> else
> @@ -30587,9 +30599,9 @@ gui_produce_glyphs (struct it *it)
> /* When no suitable font is found, display this character by
> the method specified in the first extra slot of
> Vglyphless_char_display. */
> - Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
> + Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
>
> - eassert (it->what == IT_GLYPHLESS);
> + eassert (it->what == IT_GLYPHLESS);
> produce_glyphless_glyph (it, true,
> STRINGP (acronym) ? acronym : Qnil);
> goto done;
> @@ -30613,6 +30625,15 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + /* If this glyph uses the ASCII face, enlarge the ascent
> + to make the row higher by line-height-factor. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
>
> if (get_char_glyph_code (it->char_to_display, font, &char2b))
> @@ -30763,6 +30784,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> }
>
> @@ -30886,6 +30914,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> it->phys_ascent = it->ascent;
> it->phys_descent = it->descent;
> @@ -31243,6 +31278,24 @@ gui_produce_glyphs (struct it *it)
> if (it->glyph_row
> && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
> it->glyph_row->contains_overlapping_glyphs_p = true;
> + if (it->voffset == 0 && FLOATP (Vline_height_factor))
> + {
> + Lisp_Object font_object = LGSTRING_FONT (gstring);
> + struct font *gstring_font = XFONT_OBJECT (font_object);
> +
> + /* This is for when the grapheme cluster displays some
> + ligature using ASCII font: if the height is smaller
> + than line-height-factor says, enlarge the ascent. */
> + if (face == face->ascii_face
> + && face->ascii_face->font == gstring_font)
> + {
> + int enlarged_height =
> + ((FONT_BASE (gstring_font) + FONT_DESCENT (gstring_font))
> + * XFLOAT_DATA (Vline_height_factor));
> + if (metrics.ascent + metrics.descent < enlarged_height)
> + metrics.ascent = enlarged_height - metrics.descent;
> + }
> + }
> it->ascent = it->phys_ascent = metrics.ascent;
> it->descent = it->phys_descent = metrics.descent;
> }
> @@ -35581,6 +35634,16 @@ syms_of_xdisp (void)
> mini-window frame's default font's height. */);
> Vmax_mini_window_height = make_float (0.25);
>
> + DEFVAR_LISP ("line-height-factor", Vline_height_factor,
> + doc: /* Factor for enlarging the height of lines that use the default font.
> +The value should be a float number greater than 1. It determines how
> +much will Emacs enlarge the height of a screen line that shows only
> +characters displayed with the default face's font for ASCII characters.
> +This is to avoid differences in height between lines that use the
> +ASCII font and those which use non-ASCII (for example, Chinese)
> +font, which is typically higher than the ASCII one. */);
> + Vline_height_factor = Qnil;
> +
> DEFVAR_LISP ("resize-mini-windows", Vresize_mini_windows,
> doc: /* How to resize mini-windows (the minibuffer and the echo area).
> A value of nil means don't automatically resize mini-windows.
--
^ permalink raw reply [flat|nested] 64+ messages in thread* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 1:57 ` Feng Shu
@ 2021-11-19 7:08 ` Eli Zaretskii
2021-11-19 7:18 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 7:26 ` tumashu
0 siblings, 2 replies; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 7:08 UTC (permalink / raw)
To: Feng Shu; +Cc: luangruo, larsi, 51821
> From: "Feng Shu" <tumashu@163.com>
> Cc: luangruo@yahoo.com, larsi@gnus.org, 51821@debbugs.gnu.org
> Date: Fri, 19 Nov 2021 09:57:25 +0800
>
> I think the below patch has problem, maybe it relate to font
> baseline, I do not know, please see attachement.
Sorry, I cannot watch videos in that format. What exactly seems to be
the problem with the results of the patch? What did you try, exactly,
to test it, and what did you expect to happen instead of what actually
happened?
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:08 ` Eli Zaretskii
@ 2021-11-19 7:18 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 7:31 ` bug#51821: " tumashu
2021-11-19 8:06 ` Eli Zaretskii
2021-11-19 7:26 ` tumashu
1 sibling, 2 replies; 64+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-19 7:18 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Feng Shu, larsi, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> Sorry, I cannot watch videos in that format. What exactly seems to be
> the problem with the results of the patch? What did you try, exactly,
> to test it, and what did you expect to happen instead of what actually
> happened?
I think he expects the text in the smaller font to be "centered"
vertically, instead of sharing the same baseline as the Chinese font,
but I'm not sure.
Thanks.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: Re: bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:18 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-19 7:31 ` tumashu
2021-11-19 8:28 ` Eli Zaretskii
2021-11-19 8:06 ` Eli Zaretskii
1 sibling, 1 reply; 64+ messages in thread
From: tumashu @ 2021-11-19 7:31 UTC (permalink / raw)
To: Po Lu; +Cc: Lars Ingebrigtsen, 51821@debbugs.gnu.org
At 2021-11-19 15:18:16, "Po Lu" <luangruo@yahoo.com> wrote:
>Eli Zaretskii <eliz@gnu.org> writes:
>
>> Sorry, I cannot watch videos in that format. What exactly seems to be
>> the problem with the results of the patch? What did you try, exactly,
>> to test it, and what did you expect to happen instead of what actually
>> happened?
>
>I think he expects the text in the smaller font to be "centered"
>vertically, instead of sharing the same baseline as the Chinese font,
>but I'm not sure.
I do not know more about font technology, I just want to:
when I set a proper line-height-factor, and M-x,
minibuffer's height can not change again after I paste Chinese.
>
>Thanks.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:31 ` bug#51821: " tumashu
@ 2021-11-19 8:28 ` Eli Zaretskii
2021-11-19 8:45 ` tumashu
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 8:28 UTC (permalink / raw)
To: tumashu; +Cc: luangruo, larsi, 51821
> Date: Fri, 19 Nov 2021 15:31:16 +0800 (CST)
> From: tumashu <tumashu@163.com>
> Cc: "Eli Zaretskii" <eliz@gnu.org>, "Lars Ingebrigtsen" <larsi@gnus.org>,
> "51821@debbugs.gnu.org" <51821@debbugs.gnu.org>
>
> I do not know more about font technology, I just want to:
>
> when I set a proper line-height-factor, and M-x,
> minibuffer's height can not change again after I paste Chinese.
That is what's supposed to happen. If it doesn't happen, please
describe the recipe exactly.
And the value of line-height-factor is supposed to be tuned to the
fonts you use, so if with 1.5 you see a change in the height, try
making the value larger until it doesn't change.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 8:28 ` Eli Zaretskii
@ 2021-11-19 8:45 ` tumashu
0 siblings, 0 replies; 64+ messages in thread
From: tumashu @ 2021-11-19 8:45 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Po Lu, Lars Ingebrigtsen, 51821@debbugs.gnu.org
[-- Attachment #1: Type: text/plain, Size: 958 bytes --]
At 2021-11-19 16:28:23, "Eli Zaretskii" <eliz@gnu.org> wrote:
>> Date: Fri, 19 Nov 2021 15:31:16 +0800 (CST)
>> From: tumashu <tumashu@163.com>
>> Cc: "Eli Zaretskii" <eliz@gnu.org>, "Lars Ingebrigtsen" <larsi@gnus.org>,
>> "51821@debbugs.gnu.org" <51821@debbugs.gnu.org>
>>
>> I do not know more about font technology, I just want to:
>>
>> when I set a proper line-height-factor, and M-x,
>> minibuffer's height can not change again after I paste Chinese.
>
>That is what's supposed to happen. If it doesn't happen, please
>describe the recipe exactly.
I have attache a gif motion picture, it showd that minibuffer's height change again after
I paste "你好"
Every time when I M-x and paste.
>
>And the value of line-height-factor is supposed to be tuned to the
>fonts you use, so if with 1.5 you see a change in the height, try
>making the value larger until it doesn't change.
When I set it to 2.0, the problem exist too.
>
>
[-- Attachment #2: test-2021-11-19_09.50.06.gif --]
[-- Type: image/gif, Size: 13643 bytes --]
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:18 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 7:31 ` bug#51821: " tumashu
@ 2021-11-19 8:06 ` Eli Zaretskii
2021-11-19 9:33 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 8:06 UTC (permalink / raw)
To: Po Lu; +Cc: tumashu, larsi, 51821
> From: Po Lu <luangruo@yahoo.com>
> Cc: "Feng Shu" <tumashu@163.com>, larsi@gnus.org, 51821@debbugs.gnu.org
> Date: Fri, 19 Nov 2021 15:18:16 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Sorry, I cannot watch videos in that format. What exactly seems to be
> > the problem with the results of the patch? What did you try, exactly,
> > to test it, and what did you expect to happen instead of what actually
> > happened?
>
> I think he expects the text in the smaller font to be "centered"
> vertically, instead of sharing the same baseline as the Chinese font,
> but I'm not sure.
Really? That can be done, but won't it look ugly on display? Do
other GUI applications do that with mixed CJK and non-CJK fonts?
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 8:06 ` Eli Zaretskii
@ 2021-11-19 9:33 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 12:27 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-19 9:33 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: tumashu, larsi, 51821
Eli Zaretskii <eliz@gnu.org> writes:
>> I think he expects the text in the smaller font to be "centered"
>> vertically, instead of sharing the same baseline as the Chinese font,
>> but I'm not sure.
> Really? That can be done, but won't it look ugly on display?
IMO yes, but other people might have differing opinions. Would it hurt
to make this an option?
> Do other GUI applications do that with mixed CJK and non-CJK fonts?
I found out earlier that the other applications simply scale the
overlarge glyphs to fit some predefined height. In Chromium, it's
defined in CSS, but in other programs it's probably the height of the
ASCII font, or whichever font the user has configured. (Other programs
generally don't provide the ability to customize the fontset, only a
single font.)
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 9:33 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-19 12:27 ` Eli Zaretskii
2021-11-19 12:36 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 12:27 UTC (permalink / raw)
To: Po Lu; +Cc: tumashu, larsi, 51821
> From: Po Lu <luangruo@yahoo.com>
> Cc: tumashu@163.com, larsi@gnus.org, 51821@debbugs.gnu.org
> Date: Fri, 19 Nov 2021 17:33:18 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> I think he expects the text in the smaller font to be "centered"
> >> vertically, instead of sharing the same baseline as the Chinese font,
> >> but I'm not sure.
>
> > Really? That can be done, but won't it look ugly on display?
>
> IMO yes, but other people might have differing opinions. Would it hurt
> to make this an option?
I'd rather not introduce one more option to control the line height
unless it's really needed. The code there is already a "maze of
twisted passages all alike", quite hard to understand and modify, so
adding one more option could easily break something.
For now, it does sound like Feng Shu wants the baseline aligned, so
such an option is not required yet.
> > Do other GUI applications do that with mixed CJK and non-CJK fonts?
>
> I found out earlier that the other applications simply scale the
> overlarge glyphs to fit some predefined height.
Emacs doesn't scale the font glyphs, AFAIK, primarily because that
won't work with bitmapped fonts, which we still support and which some
users still use (I remember recent enough bug reports due to some
change that broke bitmapped fonts). We let the font backend do the
scaling, when we request a font of certain size, and the result is
what you see now.
But if you or someone else know how to scale font glyphs on the fly,
please show the code. The main difficulty her, as I see it, is to
decide to which size to scale, since each character can have a glyph
with different metrics.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 12:27 ` Eli Zaretskii
@ 2021-11-19 12:36 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 64+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-19 12:36 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: tumashu, larsi, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> I'd rather not introduce one more option to control the line height
> unless it's really needed. The code there is already a "maze of
> twisted passages all alike", quite hard to understand and modify, so
> adding one more option could easily break something.
I understand, thanks. But I don't understand what other problem tumashu
has, because I can't see anything specific in the recording he sent.
> For now, it does sound like Feng Shu wants the baseline aligned, so
> such an option is not required yet.
That's good to hear.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:08 ` Eli Zaretskii
2021-11-19 7:18 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-19 7:26 ` tumashu
2021-11-19 8:25 ` Eli Zaretskii
1 sibling, 1 reply; 64+ messages in thread
From: tumashu @ 2021-11-19 7:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Po Lu, Lars Ingebrigtsen, 51821
[-- Attachment #1.1: Type: text/plain, Size: 891 bytes --]
I just test like:
1. (setq line-height-factor 1.5)
2. M-x
3. yank "你好"
4. See minibuffer's height change or not.
maybe the height should split two parts, ascent and decent:
real-ascent = ascent * line-height-factor
real-decent = decent * line-height-factor
The patch seem to not change decent.
At 2021-11-19 15:08:58, "Eli Zaretskii" <eliz@gnu.org> wrote:
>> From: "Feng Shu" <tumashu@163.com>
>> Cc: luangruo@yahoo.com, larsi@gnus.org, 51821@debbugs.gnu.org
>> Date: Fri, 19 Nov 2021 09:57:25 +0800
>>
>> I think the below patch has problem, maybe it relate to font
>> baseline, I do not know, please see attachement.
>
>Sorry, I cannot watch videos in that format. What exactly seems to be
>the problem with the results of the patch? What did you try, exactly,
>to test it, and what did you expect to happen instead of what actually
>happened?
>
>
[-- Attachment #1.2: Type: text/html, Size: 1396 bytes --]
[-- Attachment #2: test-2021-11-19_09.50.06.gif --]
[-- Type: image/gif, Size: 13643 bytes --]
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:26 ` tumashu
@ 2021-11-19 8:25 ` Eli Zaretskii
2021-11-19 8:54 ` tumashu
2021-11-19 9:09 ` tumashu
0 siblings, 2 replies; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 8:25 UTC (permalink / raw)
To: tumashu; +Cc: luangruo, larsi, 51821
> Date: Fri, 19 Nov 2021 15:26:21 +0800 (CST)
> From: tumashu <tumashu@163.com>
> Cc: "Po Lu" <luangruo@yahoo.com>, "Lars Ingebrigtsen" <larsi@gnus.org>,
> 51821@debbugs.gnu.org
>
> 1. (setq line-height-factor 1.5)
> 2. M-x
> 3. yank "你好"
> 4. See minibuffer's height change or not.
Does it change just once, or does it keep changing after that?
If it changes just once, it's not a significant problem, since the
setting of line-height-factor is normally expected to be in the init
file, and so you will not see this one-time change.
> maybe the height should split two parts, ascent and decent:
>
> real-ascent = ascent * line-height-factor
> real-decent = decent * line-height-factor
>
> The patch seem to not change decent.
Why do you want to change the descent?
Anyway, if we change the descent, the baseline of ASCII characters
will be higher, which I think will look ugly on display. Are you
familiar with any other application which does that, i.e. displays
different scripts with different baseline?
You show an image of the mode line, but don't tell what is the problem
with it. Did you try displaying Chinese characters there? Or what
else is the problem with it?
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 8:25 ` Eli Zaretskii
@ 2021-11-19 8:54 ` tumashu
2021-11-19 9:09 ` tumashu
1 sibling, 0 replies; 64+ messages in thread
From: tumashu @ 2021-11-19 8:54 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Po Lu, Lars Ingebrigtsen, 51821@debbugs.gnu.org
[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]
At 2021-11-19 16:25:05, "Eli Zaretskii" <eliz@gnu.org> wrote:
>> Date: Fri, 19 Nov 2021 15:26:21 +0800 (CST)
>> From: tumashu <tumashu@163.com>
>> Cc: "Po Lu" <luangruo@yahoo.com>, "Lars Ingebrigtsen" <larsi@gnus.org>,
>> 51821@debbugs.gnu.org
>>
>> 1. (setq line-height-factor 1.5)
>> 2. M-x
>> 3. yank "你好"
>> 4. See minibuffer's height change or not.
>
>Does it change just once, or does it keep changing after that?
every time when I M-x and insert Chinese char.
>
>If it changes just once, it's not a significant problem, since the
>setting of line-height-factor is normally expected to be in the init
>file, and so you will not see this one-time change.
>
>> maybe the height should split two parts, ascent and decent:
>>
>> real-ascent = ascent * line-height-factor
>> real-decent = decent * line-height-factor
>>
>> The patch seem to not change decent.
>
>Why do you want to change the descent?
Ascii font's baseline shoud align to Chinese font's baseline, I think.
any other way to achive this, if we do not change descent? I do not know.
>
>Anyway, if we change the descent, the baseline of ASCII characters
>will be higher, which I think will look ugly on display. Are you
>familiar with any other application which does that, i.e. displays
>different scripts with different baseline?
See attach baseline.png, Chinese and Ascii align baseline in most situation.
>
>You show an image of the mode line, but don't tell what is the problem
>with it. Did you try displaying Chinese characters there? Or what
>else is the problem with it?
>
>
[-- Attachment #2: baseline.png --]
[-- Type: image/png, Size: 65077 bytes --]
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 8:25 ` Eli Zaretskii
2021-11-19 8:54 ` tumashu
@ 2021-11-19 9:09 ` tumashu
2021-11-19 12:18 ` Eli Zaretskii
1 sibling, 1 reply; 64+ messages in thread
From: tumashu @ 2021-11-19 9:09 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Po Lu, Lars Ingebrigtsen, 51821@debbugs.gnu.org
>> maybe the height should split two parts, ascent and decent:
>>
>> real-ascent = ascent * line-height-factor
>> real-decent = decent * line-height-factor
>>
>> The patch seem to not change decent.
>
>Why do you want to change the descent?
>
when we set fonts and let 1 Chinese char width = 2 * ASCII char width,
ASCII font size may different from Chinese font size, so maybe they are have different baseline.
so how to align to baseline, I do not know.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 9:09 ` tumashu
@ 2021-11-19 12:18 ` Eli Zaretskii
0 siblings, 0 replies; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 12:18 UTC (permalink / raw)
To: tumashu; +Cc: luangruo, larsi, 51821
> Date: Fri, 19 Nov 2021 17:09:31 +0800 (CST)
> From: tumashu <tumashu@163.com>
> Cc: "Po Lu" <luangruo@yahoo.com>, "Lars Ingebrigtsen" <larsi@gnus.org>,
> "51821@debbugs.gnu.org" <51821@debbugs.gnu.org>
>
> >Why do you want to change the descent?
> >
>
> when we set fonts and let 1 Chinese char width = 2 * ASCII char width,
> ASCII font size may different from Chinese font size, so maybe they are have different baseline.
>
> so how to align to baseline, I do not know.
The only way I know of to have the baseline aligned is not to change
the descent.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-18 18:26 ` Eli Zaretskii
2021-11-19 0:26 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-19 1:57 ` Feng Shu
@ 2021-11-19 7:12 ` Lars Ingebrigtsen
2021-11-19 8:04 ` Eli Zaretskii
2021-11-19 14:48 ` Feng Shu
3 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-19 7:12 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> + DEFVAR_LISP ("line-height-factor", Vline_height_factor,
> + doc: /* Factor for enlarging the height of lines that use the default font.
> +The value should be a float number greater than 1. It determines how
> +much will Emacs enlarge the height of a screen line that shows only
> +characters displayed with the default face's font for ASCII characters.
> +This is to avoid differences in height between lines that use the
> +ASCII font and those which use non-ASCII (for example, Chinese)
> +font, which is typically higher than the ASCII one. */);
Is tying this to the default font the best solution, though? Emacs (by
default) uses at least two fonts -- one monospaced and one that's
proportional, and this will only fix the first issue.
So I think the original idea (adding a line-height parameter/variable)
would fix this issue in a more general way.
Testing the patch, it also affects the height of faces with :height in
them, meaning that separator lines get much taller than they should be.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 7:12 ` Lars Ingebrigtsen
@ 2021-11-19 8:04 ` Eli Zaretskii
2021-11-20 8:26 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-19 8:04 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Fri, 19 Nov 2021 08:12:30 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > + DEFVAR_LISP ("line-height-factor", Vline_height_factor,
> > + doc: /* Factor for enlarging the height of lines that use the default font.
> > +The value should be a float number greater than 1. It determines how
> > +much will Emacs enlarge the height of a screen line that shows only
> > +characters displayed with the default face's font for ASCII characters.
> > +This is to avoid differences in height between lines that use the
> > +ASCII font and those which use non-ASCII (for example, Chinese)
> > +font, which is typically higher than the ASCII one. */);
>
> Is tying this to the default font the best solution, though? Emacs (by
> default) uses at least two fonts -- one monospaced and one that's
> proportional, and this will only fix the first issue.
No, it should fix both. Did you try that? If you did and it didn't
work, can you show a recipe from "emacs -Q" so I could investigate?
The "default font" part above is an over-simplification: it is hard to
say something accurate enough in a single short sentence. I did try
to explain it more in the rest of the doc string: this actually
affects any font which some face uses for ASCII characters.
> Testing the patch, it also affects the height of faces with :height in
> them, meaning that separator lines get much taller than they should be.
As I told you, it is currently impossible to single out such faces.
Also, I don't think we should necessarily exempt _any_ face that
specifies :height, because if that face is used to display with mixed
fonts, it will again have the same problem when both CJK fonts and
non-CJK fonts are mixed. The separator lines are thus a very special
case, and if we want to solve that, we need a more focused solution.
For example, we could not stretch the height if the face's height is
below some threshold, on the assumption that such small fonts will
never used to display human-readable text.
But first I want to hear that CJK users are happy with this, and it
for now sounds like they aren't :-(
^ permalink raw reply [flat|nested] 64+ messages in thread* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-19 8:04 ` Eli Zaretskii
@ 2021-11-20 8:26 ` Lars Ingebrigtsen
2021-11-20 8:39 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-20 8:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> No, it should fix both. Did you try that? If you did and it didn't
> work, can you show a recipe from "emacs -Q" so I could investigate?
No, I only tested in buffers with a monospaced font and then read the
doc string.
> The "default font" part above is an over-simplification: it is hard to
> say something accurate enough in a single short sentence. I did try
> to explain it more in the rest of the doc string: this actually
> affects any font which some face uses for ASCII characters.
Perhaps "default font(s)" would help.
>> Testing the patch, it also affects the height of faces with :height in
>> them, meaning that separator lines get much taller than they should be.
>
> As I told you, it is currently impossible to single out such faces.
Right, but I think I said that it should be pretty easy to carry that
information in struct face if we want to go that way.
> Also, I don't think we should necessarily exempt _any_ face that
> specifies :height, because if that face is used to display with mixed
> fonts, it will again have the same problem when both CJK fonts and
> non-CJK fonts are mixed.
Hm... Yeah, that's true. For instance, in eww in <h1> we have :height
1.3, and we probably want the CJK/non-CJK font mixture to be handled the
same way as if there isn't a :height there. OK, the :height thing
doesn't work.
> The separator lines are thus a very special
> case, and if we want to solve that, we need a more focused solution.
> For example, we could not stretch the height if the face's height is
> below some threshold, on the assumption that such small fonts will
> never used to display human-readable text.
Hm, yes, the separator lines are very special, and the :height thing is
more a hack than anything else. Perhaps we should just introduce a new
spec, like :pixel-height that allows us to specify specifically how big
the separator line should be.
Hm. Or not use faces at all for the separator lines? I don't know how
that would look, though.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 8:26 ` Lars Ingebrigtsen
@ 2021-11-20 8:39 ` Eli Zaretskii
2021-11-20 8:51 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-20 8:39 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sat, 20 Nov 2021 09:26:19 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > No, it should fix both. Did you try that? If you did and it didn't
> > work, can you show a recipe from "emacs -Q" so I could investigate?
>
> No, I only tested in buffers with a monospaced font and then read the
> doc string.
>
> > The "default font" part above is an over-simplification: it is hard to
> > say something accurate enough in a single short sentence. I did try
> > to explain it more in the rest of the doc string: this actually
> > affects any font which some face uses for ASCII characters.
>
> Perhaps "default font(s)" would help.
For now, all of this seems like a moot point, since the "main
customer" doesn't seem to want this implementation anyway.
> > The separator lines are thus a very special
> > case, and if we want to solve that, we need a more focused solution.
> > For example, we could not stretch the height if the face's height is
> > below some threshold, on the assumption that such small fonts will
> > never used to display human-readable text.
>
> Hm, yes, the separator lines are very special, and the :height thing is
> more a hack than anything else. Perhaps we should just introduce a new
> spec, like :pixel-height that allows us to specify specifically how big
> the separator line should be.
Shouldn't it be enough to check that the face's height is less than 2
pixels, and if so, not to enlarge the height?
But again, this sounds like a moot point for now.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 8:39 ` Eli Zaretskii
@ 2021-11-20 8:51 ` Lars Ingebrigtsen
2021-11-20 9:06 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-20 8:51 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> Shouldn't it be enough to check that the face's height is less than 2
> pixels, and if so, not to enlarge the height?
Perhaps, but I'm pretty sure that on this HiDPI screen, the separator
line height is about... four? pixels high. And this is with :height
0.1, anything smaller than that gives me this error:
set-face-attribute: Face height does not produce a positive integer: 0.09
> But again, this sounds like a moot point for now.
Yup. But perhaps it would make sense to allow for :pixel-height in any
case -- it'd be one less thing to worry about when figuring our options
when trying to get to a solution.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 8:51 ` Lars Ingebrigtsen
@ 2021-11-20 9:06 ` Eli Zaretskii
2021-11-20 9:27 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-20 9:06 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sat, 20 Nov 2021 09:51:39 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > Shouldn't it be enough to check that the face's height is less than 2
> > pixels, and if so, not to enlarge the height?
>
> Perhaps, but I'm pretty sure that on this HiDPI screen, the separator
> line height is about... four? pixels high.
Well, the value 2 above was just arbitrary; we could use 4 instead.
> Yup. But perhaps it would make sense to allow for :pixel-height in any
> case -- it'd be one less thing to worry about when figuring our options
> when trying to get to a solution.
Not sure I understand what you mean by "allow foe :pixel-height" here.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 9:06 ` Eli Zaretskii
@ 2021-11-20 9:27 ` Lars Ingebrigtsen
2021-11-20 9:53 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-20 9:27 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
>> Yup. But perhaps it would make sense to allow for :pixel-height in any
>> case -- it'd be one less thing to worry about when figuring our options
>> when trying to get to a solution.
>
> Not sure I understand what you mean by "allow foe :pixel-height" here.
Allow saying:
(defface separator-line
'((((type graphic) (background dark))
:pixel-height 1 :background "#505050")
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 9:27 ` Lars Ingebrigtsen
@ 2021-11-20 9:53 ` Eli Zaretskii
2021-11-20 9:58 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-20 9:53 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sat, 20 Nov 2021 10:27:26 +0100
>
> > Not sure I understand what you mean by "allow foe :pixel-height" here.
>
> Allow saying:
>
> (defface separator-line
> '((((type graphic) (background dark))
> :pixel-height 1 :background "#505050")
How is that different from :height? It uses different units, but
that's not a problem for this use case, is it?
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 9:53 ` Eli Zaretskii
@ 2021-11-20 9:58 ` Lars Ingebrigtsen
2021-11-20 10:15 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-20 9:58 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
>> Allow saying:
>>
>> (defface separator-line
>> '((((type graphic) (background dark))
>> :pixel-height 1 :background "#505050")
>
> How is that different from :height? It uses different units, but
> that's not a problem for this use case, is it?
It's a way to say "don't scale this, ever", which was a problem here,
wasn't it? (And :height 0.1 doesn't give me a single pixel on this
display -- we have no way to express that currently.)
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 9:58 ` Lars Ingebrigtsen
@ 2021-11-20 10:15 ` Eli Zaretskii
2021-11-21 8:06 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-20 10:15 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sat, 20 Nov 2021 10:58:14 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> Allow saying:
> >>
> >> (defface separator-line
> >> '((((type graphic) (background dark))
> >> :pixel-height 1 :background "#505050")
> >
> > How is that different from :height? It uses different units, but
> > that's not a problem for this use case, is it?
>
> It's a way to say "don't scale this, ever", which was a problem here,
> wasn't it? (And :height 0.1 doesn't give me a single pixel on this
> display -- we have no way to express that currently.)
No, float values are relative to the default face, which is not what
you want. I meant ":height 1" or somesuch. If that is not good
enough, the value of :height can be a function.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-20 10:15 ` Eli Zaretskii
@ 2021-11-21 8:06 ` Lars Ingebrigtsen
2021-11-21 8:18 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-21 8:06 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> No, float values are relative to the default face, which is not what
> you want. I meant ":height 1" or somesuch. If that is not good
> enough, the value of :height can be a function.
If we're going to be scaling stuff, it'd be nice if there was a setting
that was clearly non-scaleable. An integer value to :height would be
possible, but pretty error-prone, so I'd rather introduce a new keyword.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:06 ` Lars Ingebrigtsen
@ 2021-11-21 8:18 ` Eli Zaretskii
2021-11-21 8:21 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-21 8:18 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 09:06:44 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > No, float values are relative to the default face, which is not what
> > you want. I meant ":height 1" or somesuch. If that is not good
> > enough, the value of :height can be a function.
>
> If we're going to be scaling stuff, it'd be nice if there was a setting
> that was clearly non-scaleable. An integer value to :height would be
> possible, but pretty error-prone
How is something like ":height 1" error-prone?
> so I'd rather introduce a new keyword.
Adding a new face attribute is not a 5-min job, so if we already have
something that can work, it is better to use it. Especially if you
want to introduce an attribute that potentially conflicts with an
existing attribute.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:18 ` Eli Zaretskii
@ 2021-11-21 8:21 ` Lars Ingebrigtsen
2021-11-21 8:32 ` Eli Zaretskii
0 siblings, 1 reply; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-21 8:21 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
>> If we're going to be scaling stuff, it'd be nice if there was a setting
>> that was clearly non-scaleable. An integer value to :height would be
>> possible, but pretty error-prone
>
> How is something like ":height 1" error-prone?
Because people aren't expecting ":height 1" and ":height 1.0" to have
such different semantics.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:21 ` Lars Ingebrigtsen
@ 2021-11-21 8:32 ` Eli Zaretskii
2021-11-21 8:38 ` Lars Ingebrigtsen
0 siblings, 1 reply; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-21 8:32 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 09:21:25 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> >> If we're going to be scaling stuff, it'd be nice if there was a setting
> >> that was clearly non-scaleable. An integer value to :height would be
> >> possible, but pretty error-prone
> >
> > How is something like ":height 1" error-prone?
>
> Because people aren't expecting ":height 1" and ":height 1.0" to have
> such different semantics.
??? That ship has sailed long ago, in Emacs 21. Float values and
integer values have different meanings in these face attributes (and
in other similar features, like line-height, the "space :height"
display spec, etc.). Why is this suddenly a problem, and one that
requires a new face attribute at that?
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:32 ` Eli Zaretskii
@ 2021-11-21 8:38 ` Lars Ingebrigtsen
2021-11-21 10:00 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-21 14:05 ` Eli Zaretskii
0 siblings, 2 replies; 64+ messages in thread
From: Lars Ingebrigtsen @ 2021-11-21 8:38 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, tumashu, 51821
Eli Zaretskii <eliz@gnu.org> writes:
> ??? That ship has sailed long ago, in Emacs 21. Float values and
> integer values have different meanings in these face attributes (and
> in other similar features, like line-height, the "space :height"
> display spec, etc.). Why is this suddenly a problem, and one that
> requires a new face attribute at that?
I don't think we have to compound the error by extending this mis-design
to new areas, but I won't insist.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:38 ` Lars Ingebrigtsen
@ 2021-11-21 10:00 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
2021-11-21 14:05 ` Eli Zaretskii
1 sibling, 0 replies; 64+ messages in thread
From: Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2021-11-21 10:00 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: tumashu, Eli Zaretskii, 51821
Lars Ingebrigtsen <larsi@gnus.org> writes:
>> ??? That ship has sailed long ago, in Emacs 21. Float values and
>> integer values have different meanings in these face attributes (and
>> in other similar features, like line-height, the "space :height"
>> display spec, etc.). Why is this suddenly a problem, and one that
>> requires a new face attribute at that?
> I don't think we have to compound the error by extending this mis-design
> to new areas, but I won't insist.
FWIW I don't find it a mis-design. Floating point values very obviously
mean something different than an integer value, not only in Emacs, but
in most other real-world software.
Thanks.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-21 8:38 ` Lars Ingebrigtsen
2021-11-21 10:00 ` Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2021-11-21 14:05 ` Eli Zaretskii
1 sibling, 0 replies; 64+ messages in thread
From: Eli Zaretskii @ 2021-11-21 14:05 UTC (permalink / raw)
To: Lars Ingebrigtsen; +Cc: luangruo, tumashu, 51821
> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: tumashu@163.com, luangruo@yahoo.com, 51821@debbugs.gnu.org
> Date: Sun, 21 Nov 2021 09:38:06 +0100
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> > ??? That ship has sailed long ago, in Emacs 21. Float values and
> > integer values have different meanings in these face attributes (and
> > in other similar features, like line-height, the "space :height"
> > display spec, etc.). Why is this suddenly a problem, and one that
> > requires a new face attribute at that?
>
> I don't think we have to compound the error by extending this mis-design
> to new areas, but I won't insist.
The question is, how many more Emacs Lisp programmers consider this
mis-design natural, like I do?
But I'm not objecting if you or someone else wants to add such an
attribute, I just warn that it probably won't be trivial.
^ permalink raw reply [flat|nested] 64+ messages in thread
* bug#51821: 29.0.50; Suggest add variable or frame parameter: line-height
2021-11-18 18:26 ` Eli Zaretskii
` (2 preceding siblings ...)
2021-11-19 7:12 ` Lars Ingebrigtsen
@ 2021-11-19 14:48 ` Feng Shu
3 siblings, 0 replies; 64+ messages in thread
From: Feng Shu @ 2021-11-19 14:48 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: luangruo, larsi, 51821
Eli Zaretskii <eliz@gnu.org> writes:
>> Date: Tue, 16 Nov 2021 18:45:16 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: tumashu@163.com, 51821@debbugs.gnu.org
>>
>> > From: Lars Ingebrigtsen <larsi@gnus.org>
>> > Cc: tumashu@163.com, 51821@debbugs.gnu.org
>> > Date: Tue, 16 Nov 2021 16:20:52 +0100
>> >
>> > > That shouldn't happen, normally. Emacs selects fonts of the same size
>> > > for a face.
>> >
>> > No, not really. Here's a trivial example:
>> >
>> >
>> > Note how the lines with jidanni are taller than the rest.
>>
>> OK, let me see what can be done about that.
>
> Please try the patch below. I have a very limited number of cases
> with which I can test it, so I think users of CJK scripts should say
> what they think. Basically, set the new variable line-height-factor
> to something like 1.2 (more if you enlarge the non-ASCII font to make
> double-width characters take close to 2 columns), and see if you get
> the desired effect.
>
> If this is what people want, I will install it and document it.
I have tested with a hacked version of below patch, I think we need two
factors: ascent-factor and descent-factor.
ascent = ascent * ascent-factor
descent = descent * descent-factor
When I set ascent = 1.1 and descent = 1.3,
minibuffer's height do not change when I M-x and paste Chinese.
maybe line-height-factor should be a cons, for example: (1.1 . 1.3)
>
> diff --git a/src/window.c b/src/window.c
> index e801ff8..a368b6d 100644
> --- a/src/window.c
> +++ b/src/window.c
> @@ -5246,7 +5246,10 @@ grow_mini_window (struct window *w, int delta)
> {
> struct frame *f = XFRAME (w->frame);
> int old_height = window_body_height (w, true);
> - int min_height = FRAME_LINE_HEIGHT (f);
> + int min_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
>
> eassert (MINI_WINDOW_P (w));
>
> @@ -5279,7 +5282,11 @@ grow_mini_window (struct window *w, int delta)
> shrink_mini_window (struct window *w)
> {
> struct frame *f = XFRAME (w->frame);
> - int delta = window_body_height (w, true) - FRAME_LINE_HEIGHT (f);
> + int min_line_height =
> + FLOATP (Vline_height_factor)
> + ? FRAME_LINE_HEIGHT (f) * XFLOAT_DATA (Vline_height_factor)
> + : FRAME_LINE_HEIGHT (f);
> + int delta = window_body_height (w, true) - min_line_height;
>
> eassert (MINI_WINDOW_P (w));
>
> diff --git a/src/xdisp.c b/src/xdisp.c
> index ef49297..d1f4043 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -22073,6 +22073,18 @@ append_space_for_newline (struct it *it, bool default_face_p)
> it->descent = it->override_descent;
> boff = it->override_boff;
> }
> + else
> + {
> + /* Enlarge the ascent to make the row higher by
> + line-height-factor if needed. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> + }
> if (EQ (height, Qt))
> extra_line_spacing = 0;
> else
> @@ -30587,9 +30599,9 @@ gui_produce_glyphs (struct it *it)
> /* When no suitable font is found, display this character by
> the method specified in the first extra slot of
> Vglyphless_char_display. */
> - Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
> + Lisp_Object acronym = lookup_glyphless_char_display (-1, it);
>
> - eassert (it->what == IT_GLYPHLESS);
> + eassert (it->what == IT_GLYPHLESS);
> produce_glyphless_glyph (it, true,
> STRINGP (acronym) ? acronym : Qnil);
> goto done;
> @@ -30613,6 +30625,15 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + /* If this glyph uses the ASCII face, enlarge the ascent
> + to make the row higher by line-height-factor. */
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
>
> if (get_char_glyph_code (it->char_to_display, font, &char2b))
> @@ -30763,6 +30784,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> }
>
> @@ -30886,6 +30914,13 @@ gui_produce_glyphs (struct it *it)
> {
> it->ascent = FONT_BASE (font) + boff;
> it->descent = FONT_DESCENT (font) - boff;
> + if (face == face->ascii_face && it->voffset == 0
> + && FLOATP (Vline_height_factor))
> + {
> + int enlarged_height = ((it->ascent + it->descent)
> + * XFLOAT_DATA (Vline_height_factor));
> + it->ascent = enlarged_height - it->descent;
> + }
> }
> it->phys_ascent = it->ascent;
> it->phys_descent = it->descent;
> @@ -31243,6 +31278,24 @@ gui_produce_glyphs (struct it *it)
> if (it->glyph_row
> && (metrics.lbearing < 0 || metrics.rbearing > metrics.width))
> it->glyph_row->contains_overlapping_glyphs_p = true;
> + if (it->voffset == 0 && FLOATP (Vline_height_factor))
> + {
> + Lisp_Object font_object = LGSTRING_FONT (gstring);
> + struct font *gstring_font = XFONT_OBJECT (font_object);
> +
> + /* This is for when the grapheme cluster displays some
> + ligature using ASCII font: if the height is smaller
> + than line-height-factor says, enlarge the ascent. */
> + if (face == face->ascii_face
> + && face->ascii_face->font == gstring_font)
> + {
> + int enlarged_height =
> + ((FONT_BASE (gstring_font) + FONT_DESCENT (gstring_font))
> + * XFLOAT_DATA (Vline_height_factor));
> + if (metrics.ascent + metrics.descent < enlarged_height)
> + metrics.ascent = enlarged_height - metrics.descent;
> + }
> + }
> it->ascent = it->phys_ascent = metrics.ascent;
> it->descent = it->phys_descent = metrics.descent;
> }
> @@ -35581,6 +35634,16 @@ syms_of_xdisp (void)
> mini-window frame's default font's height. */);
> Vmax_mini_window_height = make_float (0.25);
>
> + DEFVAR_LISP ("line-height-factor", Vline_height_factor,
> + doc: /* Factor for enlarging the height of lines that use the default font.
> +The value should be a float number greater than 1. It determines how
> +much will Emacs enlarge the height of a screen line that shows only
> +characters displayed with the default face's font for ASCII characters.
> +This is to avoid differences in height between lines that use the
> +ASCII font and those which use non-ASCII (for example, Chinese)
> +font, which is typically higher than the ASCII one. */);
> + Vline_height_factor = Qnil;
> +
> DEFVAR_LISP ("resize-mini-windows", Vresize_mini_windows,
> doc: /* How to resize mini-windows (the minibuffer and the echo area).
> A value of nil means don't automatically resize mini-windows.
--
^ permalink raw reply [flat|nested] 64+ messages in thread
![[-- Attachment #2: test-2021-11-19_09.50.06.gif --] [-- Type: image/gif, Size: 13643 bytes --]](../../3a9b37c5.5272.17d375f7729.Coremail.tumashu@163.com/2-test-2021-11-19_09.50.06.gif){kind=link}
![[-- Attachment #2: test-2021-11-19_09.50.06.gif --] [-- Type: image/gif, Size: 13643 bytes --]](../../45645462.4262.17d3716d0ef.Coremail.tumashu@163.com/2-test-2021-11-19_09.50.06.gif){kind=link}
![[-- Attachment #2: baseline.png --] [-- Type: image/png, Size: 65077 bytes --]](../../766eabab.5406.17d37676a89.Coremail.tumashu@163.com/2-baseline.png){kind=link}