[PATCH] Fix placement of toggle and radio button in Lucid menu.

[PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

[-- Attachment #1: Type: text/plain, Size: 170 bytes --]

Hi,

It seems to me that toggle and radio buttons in the Lucid backend are
placed too high in relation to their label in menus. The attach patch
fixes it.

Before patch:

[-- Attachment #2: small-font-orig.png --]
[-- Type: image/png, Size: 6822 bytes --]

[-- Attachment #3: big-font-orig.png --]
[-- Type: image/png, Size: 66627 bytes --]

[-- Attachment #4: Type: text/plain, Size: 14 bytes --]


After patch:

[-- Attachment #5: small-font-patch.png --]
[-- Type: image/png, Size: 6784 bytes --]

[-- Attachment #6: big-font-patch.png --]
[-- Type: image/png, Size: 66863 bytes --]

[-- Attachment #7: Type: text/plain, Size: 15 bytes --]


Best regards,

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #8: 0001-better-placement-of-toggle-radio-button-in-Lucid-wid.patch --]
[-- Type: text/x-patch, Size: 1173 bytes --]

From 83d3948fd12c2a8398898dc2e9536c2e11e28746 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Thu, 24 Feb 2022 09:51:48 +0100
Subject: [PATCH] better placement of toggle/radio button in Lucid widget.

---
 lwlib/xlwmenu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lwlib/xlwmenu.c b/lwlib/xlwmenu.c
index ace5141cdb..56acaa5a4d 100644
--- a/lwlib/xlwmenu.c
+++ b/lwlib/xlwmenu.c
@@ -793,7 +793,7 @@ draw_toggle (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
   width = toggle_button_width (mw);
   height = width;
   x += mw->menu.horizontal_spacing;
-  y += (MENU_FONT_ASCENT (mw) - height) / 2;
+  y += (MENU_FONT_HEIGHT (mw) - height) / 2;
   draw_shadow_rectangle (mw, window, x, y, width, height, False, selected_p);
 }
 
@@ -810,7 +810,7 @@ draw_radio (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
   width = radio_button_width (mw);
   height = width;
   x += mw->menu.horizontal_spacing;
-  y += (MENU_FONT_ASCENT (mw) - height) / 2;
+  y += (MENU_FONT_HEIGHT (mw) - height) / 2;
   draw_shadow_rhombus (mw, window, x, y, width, height, False, selected_p);
 }
 
-- 
2.35.1


[-- Attachment #9: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> Hi,
>
> It seems to me that toggle and radio buttons in the Lucid backend are
> placed too high in relation to their label in menus. The attach patch
> fixes it.

Those buttons are supposed to be placed relative to the descent line,
not the bottom of the text field.  So that's not a bug.

Thanks.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Po Lu <luangruo@yahoo.com> writes:

> Those buttons are supposed to be placed relative to the descent line,
> not the bottom of the text field.  So that's not a bug.

Sorry, I meant that they are supposed to be relative to the baseline.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel <emacs-devel@gnu.org>
> Date: Thu, 24 Feb 2022 18:19:18 +0800
> 
> Po Lu <luangruo@yahoo.com> writes:
> 
> > Those buttons are supposed to be placed relative to the descent line,
> > not the bottom of the text field.  So that's not a bug.
> 
> Sorry, I meant that they are supposed to be relative to the baseline.

That's true, but I wonder how come this code produces such high
position of the buttons?  It doesn't look like the button is centered
relative to the "middle" of the first character of the menu item's
label.  Can you or Manuel show the values of the font's ascent/descent
vs the actual metrics of the upper-case characters shown on the
screenshots, such as 'H', 'C', and 'S'?

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Po Lu <luangruo@yahoo.com> writes:

> Po Lu <luangruo@yahoo.com> writes:
>
>> Those buttons are supposed to be placed relative to the descent line,
>> not the bottom of the text field.  So that's not a bug.
>
> Sorry, I meant that they are supposed to be relative to the baseline.

Makes sense. Or maybe it is the 2/3 of the font height for the toggle
button size that is not correct.
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

[-- Attachment #1: Type: text/plain, Size: 421 bytes --]

Hi,

Ok so here is a new patch that I think fixes this issue. The first line
of the patch sets the toggle to size to be 2/3 of the ascent (and not
height) of the font: it seems more logical if those buttons resides on
the baseline.

As you can see in the following zoomed screenshots, the base of the
pseudo-3d box is now on the same line of the bottom horizontal bar of a
'i' for instance.

Best regards,

Before patch:

[-- Attachment #2: orig-small-bitmap.png --]
[-- Type: image/png, Size: 2050 bytes --]

[-- Attachment #3: orig-small-vector.png --]
[-- Type: image/png, Size: 9359 bytes --]

[-- Attachment #4: orig-big-bitmap.png --]
[-- Type: image/png, Size: 2448 bytes --]

[-- Attachment #5: orig-big-vector.png --]
[-- Type: image/png, Size: 10295 bytes --]

[-- Attachment #6: Type: text/plain, Size: 13 bytes --]


With patch:

[-- Attachment #7: patch-small-bitmap.png --]
[-- Type: image/png, Size: 2000 bytes --]

[-- Attachment #8: patch-small-vector.png --]
[-- Type: image/png, Size: 9427 bytes --]

[-- Attachment #9: patch-big-bitmap.png --]
[-- Type: image/png, Size: 2519 bytes --]

[-- Attachment #10: patch-big-vector.png --]
[-- Type: image/png, Size: 13519 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #11: 0001-fix-toggle-radio-button-on-lucid.patch --]
[-- Type: text/x-patch, Size: 1351 bytes --]

From 1c56fbc1e294e1ed273d8f0f6a5b71b26047d004 Mon Sep 17 00:00:00 2001
From: Manuel Giraud <manuel@ledu-giraud.fr>
Date: Fri, 25 Feb 2022 12:04:37 +0100
Subject: [PATCH] fix toggle/radio button on lucid.

---
 lwlib/xlwmenu.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lwlib/xlwmenu.c b/lwlib/xlwmenu.c
index ace5141cdb..f49fc3697a 100644
--- a/lwlib/xlwmenu.c
+++ b/lwlib/xlwmenu.c
@@ -393,7 +393,7 @@ arrow_width (XlwMenuWidget mw)
 static int
 toggle_button_width (XlwMenuWidget mw)
 {
-  return (MENU_FONT_HEIGHT (mw) * 2 / 3) | 1;
+  return (MENU_FONT_ASCENT (mw) * 2 / 3) | 1;
 }
 
 
@@ -793,7 +793,7 @@ draw_toggle (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
   width = toggle_button_width (mw);
   height = width;
   x += mw->menu.horizontal_spacing;
-  y += (MENU_FONT_ASCENT (mw) - height) / 2;
+  y += (MENU_FONT_ASCENT (mw) - height);
   draw_shadow_rectangle (mw, window, x, y, width, height, False, selected_p);
 }
 
@@ -810,7 +810,7 @@ draw_radio (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
   width = radio_button_width (mw);
   height = width;
   x += mw->menu.horizontal_spacing;
-  y += (MENU_FONT_ASCENT (mw) - height) / 2;
+  y += (MENU_FONT_ASCENT (mw) - height);
   draw_shadow_rhombus (mw, window, x, y, width, height, False, selected_p);
 }
 
-- 
2.35.1


[-- Attachment #12: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Manuel Giraud <manuel@ledu-giraud.fr> writes:

> diff --git a/lwlib/xlwmenu.c b/lwlib/xlwmenu.c
> index ace5141cdb..f49fc3697a 100644
> --- a/lwlib/xlwmenu.c
> +++ b/lwlib/xlwmenu.c
> @@ -393,7 +393,7 @@ arrow_width (XlwMenuWidget mw)
>  static int
>  toggle_button_width (XlwMenuWidget mw)
>  {
> -  return (MENU_FONT_HEIGHT (mw) * 2 / 3) | 1;
> +  return (MENU_FONT_ASCENT (mw) * 2 / 3) | 1;
>  }
>  
>  
> @@ -793,7 +793,7 @@ draw_toggle (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
>    width = toggle_button_width (mw);
>    height = width;
>    x += mw->menu.horizontal_spacing;
> -  y += (MENU_FONT_ASCENT (mw) - height) / 2;
> +  y += (MENU_FONT_ASCENT (mw) - height);
>    draw_shadow_rectangle (mw, window, x, y, width, height, False, selected_p);
>  }
>  
> @@ -810,7 +810,7 @@ draw_radio (XlwMenuWidget mw, Window window, int x, int y, int selected_p)
>    width = radio_button_width (mw);
>    height = width;
>    x += mw->menu.horizontal_spacing;
> -  y += (MENU_FONT_ASCENT (mw) - height) / 2;
> +  y += (MENU_FONT_ASCENT (mw) - height);
>    draw_shadow_rhombus (mw, window, x, y, width, height, False, selected_p);
>  }
>  
> -- 
> 2.35.1

This isn't right at all, since now the button will not be centered
between the origin and the baseline, and will instead be drawn directly
above the baseline.  It doesn't show up with your font, but that will
lose with a larger font.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Date: Fri, 25 Feb 2022 12:16:32 +0100
> Cc: emacs-devel <emacs-devel@gnu.org>
> 
> Ok so here is a new patch that I think fixes this issue. The first line
> of the patch sets the toggle to size to be 2/3 of the ascent (and not
> height) of the font: it seems more logical if those buttons resides on
> the baseline.

Can we please take a step back and try to understand the issue better?
I asked a question in this thread; can we have an answer to that
question?

Basically, I don't understand why the original code doesn't appear to
do its job as intended.  If you do understand, please post a detailed
explanation of the reason.

Once we understand the reason(s), we can discuss solutions.

Thanks.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Eli Zaretskii <eliz@gnu.org> writes:

> Can we please take a step back and try to understand the issue better?
> I asked a question in this thread; can we have an answer to that
> question?

I can answer for font's height and ascent on an example but I don't know
how to have the "actual metrics of the upper-case characters shown"

> Basically, I don't understand why the original code doesn't appear to
> do its job as intended.  If you do understand, please post a detailed
> explanation of the reason.

Ok so I found the button to be a bit to high regarding to their
labels. My second patch place the base of the button *on* the baseline
(ie. a bit lower than the actual code)

But, in the mean time (with help of Po Lu's last mail), I understood
that the code is intended to have the button in the middle of the
baseline and the top of the font (am I correct?).

Still it seems a bit too high. As I suggested in another message, it
might be the size of the button that is too big here.
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: luangruo@yahoo.com,  emacs-devel@gnu.org
> Date: Fri, 25 Feb 2022 13:52:25 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can we please take a step back and try to understand the issue better?
> > I asked a question in this thread; can we have an answer to that
> > question?
> 
> I can answer for font's height and ascent on an example but I don't know
> how to have the "actual metrics of the upper-case characters shown"

They are available in gui_produce_glyphs (look for "pcm").

> > Basically, I don't understand why the original code doesn't appear to
> > do its job as intended.  If you do understand, please post a detailed
> > explanation of the reason.
> 
> Ok so I found the button to be a bit to high regarding to their
> labels. My second patch place the base of the button *on* the baseline
> (ie. a bit lower than the actual code)
> 
> But, in the mean time (with help of Po Lu's last mail), I understood
> that the code is intended to have the button in the middle of the
> baseline and the top of the font (am I correct?).
> 
> Still it seems a bit too high. As I suggested in another message, it
> might be the size of the button that is too big here.

So is this just an aesthetic issue?  If so, perhaps adding a variable
exposed to Lisp could be enough: then users who, like you, dislike the
default display, can control the offset.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> They are available in gui_produce_glyphs (look for "pcm").

Unfortunately, that's not available inside the Lucid menu widget.  What
is actually needed is the entire `font_extents' field of the menu
widget.  In gdb, that would be

  (gdb) p mw->menu.font_extents

Inside `draw_toggle' or some other similar function.

> So is this just an aesthetic issue?  If so, perhaps adding a variable
> exposed to Lisp could be enough: then users who, like you, dislike the
> default display, can control the offset.

It would be better as an X resource, IMHO.  Lisp doesn't affect at
present the behavior of the Lucid menu widget, and it would be cleaner
to keep it that way.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Manuel Giraud <manuel@ledu-giraud.fr>,  emacs-devel@gnu.org
> Date: Fri, 25 Feb 2022 21:46:58 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > They are available in gui_produce_glyphs (look for "pcm").
> 
> Unfortunately, that's not available inside the Lucid menu widget.

Can't you arrange for the same font to be used as the default face's
font, and then look in gui_produce_glyphs?

> > So is this just an aesthetic issue?  If so, perhaps adding a variable
> > exposed to Lisp could be enough: then users who, like you, dislike the
> > default display, can control the offset.
> 
> It would be better as an X resource, IMHO.

I don't think I agree.  We support X resources where they already
exist; we don't (or at least shouldn't) invent new resources.

I don't see why we couldn't use a Lisp variable to affect how Lucid
menus are displayed.  Am I missing something?

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Eli Zaretskii <eliz@gnu.org> writes:

> So is this just an aesthetic issue?  If so, perhaps adding a variable
> exposed to Lisp could be enough: then users who, like you, dislike the
> default display, can control the offset.

Yes, as usually with widget, it is indeed an aesthetic issue. I think
that you also thought about aesthetic when you said "Basically, I don't
understand why the original code doesn't appear to do its job as
intended."

As for having a variable exposed to Lisp, I'm with Po Lu on this one: if
those widgets cannot move for some reasons I think having it exposed to
Lisp is a bit too much.
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: luangruo@yahoo.com,  emacs-devel@gnu.org
> Date: Fri, 25 Feb 2022 15:46:05 +0100
> 
> As for having a variable exposed to Lisp, I'm with Po Lu on this one: if
> those widgets cannot move for some reasons I think having it exposed to
> Lisp is a bit too much.

Having such variables in these situations is as Emacsy as it gets,
IME.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Eli Zaretskii <eliz@gnu.org> writes:

> Having such variables in these situations is as Emacsy as it gets,
> IME.

Well why not… but we are talking of the position of a toggle button
relatively to its label. I don't think that even the GTK backend has
such configurability via Emacs. Is this kind of think not done via gtkrc
of even GTK CSS nowadays?
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Manuel Giraud <manuel@ledu-giraud.fr>,  luangruo@yahoo.com,
>   emacs-devel@gnu.org
> Date: Fri, 25 Feb 2022 16:38:16 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Having such variables in these situations is as Emacsy as it gets,
> > IME.
> 
> Well why not… but we are talking of the position of a toggle button
> relatively to its label. I don't think that even the GTK backend has
> such configurability via Emacs. Is this kind of think not done via gtkrc
> of even GTK CSS nowadays?

I don't know; perhaps Po Lu does.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Can't you arrange for the same font to be used as the default face's
> font, and then look in gui_produce_glyphs?

Not without fix the current mess that is font display in the Lucid
menus.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

>> Well why not… but we are talking of the position of a toggle button
>> relatively to its label. I don't think that even the GTK backend has
>> such configurability via Emacs. Is this kind of think not done via gtkrc
>> of even GTK CSS nowadays?

> I don't know; perhaps Po Lu does.

It's done through GTK stylesheets on GTK 3 and gtkrc files on GTK+ 2.
These days even simple customization like changing the
`scroll-bar-width' variable is no longer possible.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: manuel@ledu-giraud.fr,  emacs-devel@gnu.org
> Date: Sat, 26 Feb 2022 08:20:02 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can't you arrange for the same font to be used as the default face's
> > font, and then look in gui_produce_glyphs?
> 
> Not without fix the current mess that is font display in the Lucid
> menus.

Sorry, I don't understand how this is relevant.  What I meant is to
invoke Emacs like this:

   emacs -Q -fn SOMETHING

where SOMETHING is the font used in the Lucid menus, and the examine
the metrics of the relevant characters and of the font inside
gui_produce_glyphs.

The only difficulty I could see is if somehow it is not easy to know
which font is used in the Lucid menus.  Is that the problem?

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Manuel Giraud <manuel@ledu-giraud.fr>,  emacs-devel@gnu.org
> Date: Sat, 26 Feb 2022 08:21:55 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> Well why not… but we are talking of the position of a toggle button
> >> relatively to its label. I don't think that even the GTK backend has
> >> such configurability via Emacs. Is this kind of think not done via gtkrc
> >> of even GTK CSS nowadays?
> 
> > I don't know; perhaps Po Lu does.
> 
> It's done through GTK stylesheets on GTK 3 and gtkrc files on GTK+ 2.
> These days even simple customization like changing the
> `scroll-bar-width' variable is no longer possible.

I have no problems offering this new variable only for builds that use
the xlwlib code.  We already have variables that affect only GTK
tooltips, so it's nothing new.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Sorry, I don't understand how this is relevant.

The problem is that the Lucid menu bar widget doesn't find fonts like
the rest of Emacs does, using the individual font backends.  Instead, it
relies on various ad hoc methods for font discovery that change
depending on how it is built.  I didn't look too closely, but font
metrics are probably calculated differently as well.

But I thought you were suggesting that the Lucid menu bar widget used
font objects opened by Emacs.

> What I meant is to invoke Emacs like this:
>
>    emacs -Q -fn SOMETHING
>
> where SOMETHING is the font used in the Lucid menus, and the examine
> the metrics of the relevant characters and of the font inside
> gui_produce_glyphs.
>
> The only difficulty I could see is if somehow it is not easy to know
> which font is used in the Lucid menus.  Is that the problem?

That's also a problem, but you can control the font by starting Emacs
with "-xrm 'Emacs.pane.menubar.font: Courier-12''" or something to that
effect.  But keep in mind that the metrics may still be different, due
to the aformentioned caveats.  For instance, it might elect to use X
server-side fonts even if the same font is displayed using Xft by the
rest of Emacs, very often resulting in different metrics.

Thanks.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I have no problems offering this new variable only for builds that use
> the xlwlib code.  We already have variables that affect only GTK
> tooltips, so it's nothing new.

Fair enough.  I don't understand the desired effect well enough to
implement that variable, however, but Manuel could probably implement
it.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: manuel@ledu-giraud.fr,  emacs-devel@gnu.org
> Date: Sat, 26 Feb 2022 15:33:40 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Sorry, I don't understand how this is relevant.
> 
> The problem is that the Lucid menu bar widget doesn't find fonts like
> the rest of Emacs does, using the individual font backends.  Instead, it
> relies on various ad hoc methods for font discovery that change
> depending on how it is built.  I didn't look too closely, but font
> metrics are probably calculated differently as well.

That's not the impression I get when I look at the definition of
MENU_FONT_HEIGHT in xlwmenu.c: it does use the font attributes used by
the "usual" font backends, at least in the case of XFT and Cairo.  Am
I missing something?

> > The only difficulty I could see is if somehow it is not easy to know
> > which font is used in the Lucid menus.  Is that the problem?
> 
> That's also a problem, but you can control the font by starting Emacs
> with "-xrm 'Emacs.pane.menubar.font: Courier-12''" or something to that
> effect.

Yes, that's another idea for obtaining this information.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> That's not the impression I get when I look at the definition of
> MENU_FONT_HEIGHT in xlwmenu.c: it does use the font attributes used by
> the "usual" font backends, at least in the case of XFT and Cairo.  Am
> I missing something?

They are the same attributes, but I think they're computed differently.

AFAIK the font backends calculate ascent and descent on a per-glyph
basis, while the Lucid menu bar widget uses the maximum ascent and
descent values for the entire font.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: manuel@ledu-giraud.fr,  emacs-devel@gnu.org
> Date: Sat, 26 Feb 2022 16:35:51 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > That's not the impression I get when I look at the definition of
> > MENU_FONT_HEIGHT in xlwmenu.c: it does use the font attributes used by
> > the "usual" font backends, at least in the case of XFT and Cairo.  Am
> > I missing something?
> 
> They are the same attributes, but I think they're computed differently.
> 
> AFAIK the font backends calculate ascent and descent on a per-glyph
> basis, while the Lucid menu bar widget uses the maximum ascent and
> descent values for the entire font.

Are you sure?  We have the font's "global" ascent, descent, and height
values in the font backends as well.  See, for example, ftfont.c
around line 1390.

We do also use per-character glyph metrics as well, in
gui_produce_glyphs, which is why I suggested to compare those for the
font used in the Lucid menus.  But we use the font's global values as
fallback.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

Po Lu <luangruo@yahoo.com> writes:

> This isn't right at all, since now the button will not be centered
> between the origin and the baseline, and will instead be drawn directly
> above the baseline.  It doesn't show up with your font, but that will
> lose with a larger font.

Hi,

Just for the record, here is what I get on my machine with a font of
"Sans-40".

Current:

[-- Attachment #2: orig-BIG.png --]
[-- Type: image/png, Size: 60926 bytes --]

[-- Attachment #3: Type: text/plain, Size: 11 bytes --]


My patch:

[-- Attachment #4: patch-BIG.png --]
[-- Type: image/png, Size: 60773 bytes --]

[-- Attachment #5: Type: text/plain, Size: 18 bytes --]

-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

[-- Attachment #1: Type: text/plain, Size: 116 bytes --]

Hi,

I think I've finally understood what you were trying to explain to me 😅
First, yet another screen shot:

[-- Attachment #2: shot.png --]
[-- Type: image/png, Size: 3148 bytes --]

[-- Attachment #3: Type: text/plain, Size: 1561 bytes --]


Here for each menu entry, I have prepend two unicode characters ('FULL
BLOCK' and 'MEDIUM SHADE') that, I think, is showing the full extend of
a given font.

With those "full block", we can see that the toggle button is correctly
placed: at the middle between the baseline and the top of the font.

But I still find it to be high regarding the text label. I have tested
with other fonts with the same result because the capital letters were
always shorter than the full ascent of a font.

So here are the options I think we have now:

   1- Compute the ascent of each label (ie. max ascent of each
      character) and use this value to place the button;

   2- Compute a global ascent for all labels in all menus and use this
      value;

   3- Place the toggle button on the baseline (not at middle of baseline
      and top);

   4- Leave it as is.

Regarding option 1, I understood that having the metrics of a given
character is not yet available in the backend. Plus we probably end up
with non uniformly placed buttons.

Option 2: Fix the "uniformly placed buttons" but is that even possible
given how dynamic Emacs menus are?

As you guess, option 3 has my favour. It fixes my aesthetic issue
without more computation. In fact, I don't even undertand why those
buttons need to be placed in at the middle of the baseline and the top
in the first place (maybe I need an explanation here).

If we stay with option 4, I still think that the toggle button should be
2/3 of the ascent and not 2/3 of the height of the font.

Best regards,
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Mon, 28 Feb 2022 09:45:49 +0100
> 
> Here for each menu entry, I have prepend two unicode characters ('FULL
> BLOCK' and 'MEDIUM SHADE') that, I think, is showing the full extend of
> a given font.
> 
> With those "full block", we can see that the toggle button is correctly
> placed: at the middle between the baseline and the top of the font.

Is it?  I still see the toggle button too high, even if we consider
the higher block characters.

>    1- Compute the ascent of each label (ie. max ascent of each
>       character) and use this value to place the button;
> 
>    2- Compute a global ascent for all labels in all menus and use this
>       value;
> 
>    3- Place the toggle button on the baseline (not at middle of baseline
>       and top);
> 
>    4- Leave it as is.
> 
> Regarding option 1, I understood that having the metrics of a given
> character is not yet available in the backend. Plus we probably end up
> with non uniformly placed buttons.
> 
> Option 2: Fix the "uniformly placed buttons" but is that even possible
> given how dynamic Emacs menus are?
> 
> As you guess, option 3 has my favour. It fixes my aesthetic issue
> without more computation. In fact, I don't even undertand why those
> buttons need to be placed in at the middle of the baseline and the top
> in the first place (maybe I need an explanation here).
> 
> If we stay with option 4, I still think that the toggle button should be
> 2/3 of the ascent and not 2/3 of the height of the font.

I proposed to have a variable users could tweak precisely because it
is a very simple solution that can satisfy everyone.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[chad]

[-- Attachment #1: Type: text/plain, Size: 1048 bytes --]

On Mon, Feb 28, 2022 at 3:47 AM Manuel Giraud <manuel@ledu-giraud.fr> wrote:

> But I still find it to be high regarding the text label. I have tested
> with other fonts with the same result because the capital letters were
> always shorter than the full ascent of a font.
>

FWIW, I put this in front of a friend who used to do UI design, and they
said, roughly "you can try to get fancy with centering checkboxes on font
metrics, but most people actually like "a few pixels above the baseline",
as long as you don't go above the top line. Part of why this works is due
to the fact that very few fonts have capital letters with descenders, so
visually, you almost always get white space below the adjacent letter. If
you want to get fancy, you can compute "a few" as something like '(floor (/
font-size) 4)', but most designers can find a fixed value that works the
vast majority of the time". Tying this in with Eli's suggestion, this could
be a configurable value, defaulting to, say, 3 or some divisor of the font
size.

Hope that helps,
~Chad

[-- Attachment #2: Type: text/html, Size: 1476 bytes --]

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Eli Zaretskii <eliz@gnu.org> writes:

>> With those "full block", we can see that the toggle button is correctly
>> placed: at the middle between the baseline and the top of the font.
>
> Is it?  I still see the toggle button too high, even if we consider
> the higher block characters.

No I think it is correct by this placement definition. That placement is
defined as "centered between baseline and top" or "centered on the full
ascent of the font": the descent is not taken into account.

>> If we stay with option 4, I still think that the toggle button should be
>> 2/3 of the ascent and not 2/3 of the height of the font.
>
> I proposed to have a variable users could tweak precisely because it
> is a very simple solution that can satisfy everyone.

Then I'd prefer to give up and leave it as it is. I think that having a
variable for this kind of tweak would be a bit overkill.
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Eli Zaretskii]

> From: Manuel Giraud <manuel@ledu-giraud.fr>
> Cc: luangruo@yahoo.com,  emacs-devel@gnu.org
> Date: Mon, 28 Feb 2022 22:19:15 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> With those "full block", we can see that the toggle button is correctly
> >> placed: at the middle between the baseline and the top of the font.
> >
> > Is it?  I still see the toggle button too high, even if we consider
> > the higher block characters.
> 
> No I think it is correct by this placement definition. That placement is
> defined as "centered between baseline and top" or "centered on the full
> ascent of the font": the descent is not taken into account.

Yes, I understand.  But I didn't see it centered like that, I saw it
higher.

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

[-- Attachment #1: Type: text/plain, Size: 175 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

> Yes, I understand.  But I didn't see it centered like that, I saw it
> higher.

Hi,

Here is a really zoomed version with some labels:

[-- Attachment #2: drawing.png --]
[-- Type: image/png, Size: 91762 bytes --]

[-- Attachment #3: Type: text/plain, Size: 230 bytes --]


As you can see, the toggle button has exactly 1 pixel above and 1 pixel
below so it is correctly centered on the ascent. Maybe you are seeing it
higher because of the pseudo-3D box of the button.

Best regards,
-- 
Manuel Giraud

Re: [PATCH] Fix placement of toggle and radio button in Lucid menu.

[Manuel Giraud]

Hi,

Thanks for your input! Maybe I'll also get used to "few pixels above the
baseline". Best regards,
-- 
Manuel Giraud