bug#50424: 27.2; Tab bar button mouse face not clearing entirely

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Enable tab-bar-mode, move the mouse pointer over the "+" button used to
create new tabs.  Once the button is highlighted by displaying a relief,
move the pointer back outside the tab bar.

While most the relief around part of the button will be cleared, part of
the left side of the relief that surrounded the "+" button when it was
highlighted will be retained.

To the best of my knowledge, this also appears on Emacs 28.

In GNU Emacs 27.2 (build 1, i386-pc-solaris2.11, X toolkit, Xaw scroll bars)
 of 2021-06-25 built on saphire
Windowing system distributor 'The X.Org Foundation', version 11.0.12101001
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
Tab-Bar mode enabled

Configured using:
 'configure --with-gnutls=ifavailable'

Configured features:
XPM JPEG TIFF GIF PNG ACL ZLIB TOOLKIT_SCROLL_BARS LUCID X11 XDBE XIM
MODULES THREADS PDUMPER

Important settings:
  value of $LANG: en_GB.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  tab-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message rmc puny dired dired-loaddefs
format-spec rfc822 mml mml-sec epa derived epg epg-config gnus-util
rmail rmail-loaddefs text-property-search mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils time-date sly-autoloads
package easymenu browse-url url-handlers url-parse auth-source cl-seq
eieio eieio-core cl-macs eieio-loaddefs password-cache json subr-x map
url-vars seq byte-opt gv bytecomp byte-compile cconv cl-loaddefs cl-lib
tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type
mwheel term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
timer select scroll-bar mouse jit-lock font-lock syntax facemenu
font-core term/tty-colors frame minibuffer cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote threads dynamic-setting
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 8 59679 8162)
 (symbols 24 7234 1)
 (strings 16 20406 2463)
 (string-bytes 1 658456)
 (vectors 8 12821)
 (vector-slots 4 211903 12844)
 (floats 8 26 20)
 (intervals 28 221 7)
 (buffers 568 13))

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> Date: Mon, 06 Sep 2021 16:12:56 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> 
> Enable tab-bar-mode, move the mouse pointer over the "+" button used to
> create new tabs.  Once the button is highlighted by displaying a relief,
> move the pointer back outside the tab bar.
> 
> While most the relief around part of the button will be cleared, part of
> the left side of the relief that surrounded the "+" button when it was
> highlighted will be retained.
> 
> To the best of my knowledge, this also appears on Emacs 28.

FWIW, I cannot reproduce this, neither in Emacs 27.2 nor in Emacs 28.

Maybe this is window-system dependent?

Alternatively, if you have some "optimization" features enabled in
your video drivers, disable them and try again.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Stephen Berman]

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

On Mon, 06 Sep 2021 13:50:52 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Mon, 06 Sep 2021 16:12:56 +0800
>> From:  Po Lu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>>
>>
>> Enable tab-bar-mode, move the mouse pointer over the "+" button used to
>> create new tabs.  Once the button is highlighted by displaying a relief,
>> move the pointer back outside the tab bar.
>>
>> While most the relief around part of the button will be cleared, part of
>> the left side of the relief that surrounded the "+" button when it was
>> highlighted will be retained.
>>
>> To the best of my knowledge, this also appears on Emacs 28.
>
> FWIW, I cannot reproduce this, neither in Emacs 27.2 nor in Emacs 28.
>
> Maybe this is window-system dependent?
>
> Alternatively, if you have some "optimization" features enabled in
> your video drivers, disable them and try again.

I see the problem with Emacs 27 and 28 built with Gtk+ and with 28 built
with no toolkit (I don't have a built of 27 with no toolkit).  It looks
like a remnant pixel on the lower left, see attached screenshot.  It
disappears as soon as the window size changes.

Steve Berman


[-- Attachment #2: tab-bar-bug.png --]
[-- Type: image/png, Size: 8314 bytes --]

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Stephen Berman <stephen.berman@gmx.net> writes:

> I see the problem with Emacs 27 and 28 built with Gtk+ and with 28 built
> with no toolkit (I don't have a built of 27 with no toolkit).  It looks
> like a remnant pixel on the lower left, see attached screenshot.  It
> disappears as soon as the window size changes.

Indeed, that is the problem I reported.  Thanks for providing an image
example.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> FWIW, I cannot reproduce this, neither in Emacs 27.2 nor in Emacs 28.
>
> Maybe this is window-system dependent?

Perhaps.  I have only observed it on X-Windows.

> Alternatively, if you have some "optimization" features enabled in
> your video drivers, disable them and try again.

I don't believe that is the problem here, as I have no optimization
features enabled, but it is still reproducible on a build of Emacs 27.2
made with -O0 -g3 running in Xephyr.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


It does seem to be window-system dependent.  In fact, the relief
actually renders differently on MS-Windows as compared to how it renders
on X.

I have attached two images detailing the difference, and additionally,
an image of the bug on X.


[-- Attachment #2: Relief surrounding tab-bar + button on MS-Windows --]
[-- Type: image/png, Size: 832 bytes --]

[-- Attachment #3: The same relief on X-windows --]
[-- Type: image/png, Size: 748 bytes --]

[-- Attachment #4: The bug, exhibited on X --]
[-- Type: image/png, Size: 838 bytes --]

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 50424@debbugs.gnu.org
> Date: Mon, 06 Sep 2021 20:30:47 +0800
> 
> It does seem to be window-system dependent.  In fact, the relief
> actually renders differently on MS-Windows as compared to how it renders
> on X.
> 
> I have attached two images detailing the difference, and additionally,
> an image of the bug on X.

Thanks, but which one is which, and which one(s) show the bug?

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: 50424@debbugs.gnu.org
>> Date: Mon, 06 Sep 2021 20:30:47 +0800
>> 
>> It does seem to be window-system dependent.  In fact, the relief
>> actually renders differently on MS-Windows as compared to how it renders
>> on X.
>> 
>> I have attached two images detailing the difference, and additionally,
>> an image of the bug on X.
>
> Thanks, but which one is which, and which one(s) show the bug?

Are the descriptions not visible for you?  If so, the following should
help:

"Screenshot from 2021-09-06 20-27-24" depicts the relief drawn on
MS-Windows.  (Which does not exhibit the bug)

"Screenshot from 2021-09-06 20-28-53" depicts the relief drawn on
X-windows.

"Screenshot from 2021-09-06 20-30-11" depicts the bug on X-windows.

Thanks.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: 50424@debbugs.gnu.org
> Date: Tue, 07 Sep 2021 08:30:43 +0800
> 
> >> I have attached two images detailing the difference, and additionally,
> >> an image of the bug on X.
> >
> > Thanks, but which one is which, and which one(s) show the bug?
> 
> Are the descriptions not visible for you?

What descriptions? I only see the file names.

> If so, the following should help:
> 
> "Screenshot from 2021-09-06 20-27-24" depicts the relief drawn on
> MS-Windows.  (Which does not exhibit the bug)
> 
> "Screenshot from 2021-09-06 20-28-53" depicts the relief drawn on
> X-windows.
> 
> "Screenshot from 2021-09-06 20-30-11" depicts the bug on X-windows.

Thanks.

I think I understand what is going on there: the definitions of the
button margins and relief are incorrect.  I show below the code which
sets up the tool-bar buttons to display correctly both when the mouse
is over a button and when the mouse moves off the button.  As you see,
there's a subtle dance with the computed values of the image margins
and relief, depending on whether the button is selected or not and
whether auto-raise-tool-bar-buttons is on or off.  The computed values
of the margin and the relief are then injected into the image spec of
the button.

By contrast, the corresponding spec of the tab-bar buttons specifies a
fixed margin, and specifies no relief at all:

    (add-text-properties 0 (length tab-bar-new-button)
                         `(display (image :type xpm
                                          :file "tabs/new.xpm"
                                          :margin (2 . 0)
                                          :ascent center))

Moreover, the value of :margin here is inconsistent with the value of
tab-bar-button-margin.  And there's no code there which attempts to do
the same job for tab-bar buttons' margins and relief as xdisp.c does
for tool-bar buttons (see below).

I've fixed tab-bar.el to request the same margin as specified by
tab-bar-button-margin, and I also fixed the code in w32term.c and
xterm.c which used the corresponding tool-bar values when tab-bar
values should have been used.  This improved the situation to some
extent, but we still leave behind an artifact when the mouse moves off
the tab-bar button: a 1-pixel vertical line.  I think that's because
there's no relief definition in the image, but the value of
tab-bar-button-relief is non-zero (and xterm.c/w32term.c use that
non-zero value when they redraw the buttons).

I hope the information here will allow Juri and people who really
understand the meaning of an image margin and relief (Alan?) fix the
rest of the problem.

Here's the code which sets up margins and relief on tool-bar buttons:

      /* Display the tool-bar button pressed, or depressed.  */
      plist = Fcopy_sequence (XCDR (image));

      /* Compute margin and relief to draw.  */
      relief = (tool_bar_button_relief >= 0
		? min (tool_bar_button_relief,
		       min (INT_MAX, MOST_POSITIVE_FIXNUM))
		: DEFAULT_TOOL_BAR_BUTTON_RELIEF);
      hmargin = vmargin = relief;

      if (RANGED_FIXNUMP (1, Vtool_bar_button_margin,
			   INT_MAX - max (hmargin, vmargin)))
	{
	  hmargin += XFIXNAT (Vtool_bar_button_margin);
	  vmargin += XFIXNAT (Vtool_bar_button_margin);
	}
      else if (CONSP (Vtool_bar_button_margin))
	{
	  if (RANGED_FIXNUMP (1, XCAR (Vtool_bar_button_margin),
			       INT_MAX - hmargin))
	    hmargin += XFIXNAT (XCAR (Vtool_bar_button_margin));

	  if (RANGED_FIXNUMP (1, XCDR (Vtool_bar_button_margin),
			       INT_MAX - vmargin))
	    vmargin += XFIXNAT (XCDR (Vtool_bar_button_margin));
	}

      if (auto_raise_tool_bar_buttons_p)
	{
	  /* Add a `:relief' property to the image spec if the item is
	     selected.  */
	  if (selected_p)
	    {
	      plist = Fplist_put (plist, QCrelief, make_fixnum (-relief));
	      hmargin -= relief;
	      vmargin -= relief;
	    }
	}
      else
	{
	  /* If image is selected, display it pressed, i.e. with a
	     negative relief.  If it's not selected, display it with a
	     raised relief.  */
	  plist = Fplist_put (plist, QCrelief,
			      (selected_p
			       ? make_fixnum (-relief)
			       : make_fixnum (relief)));
	  hmargin -= relief;
	  vmargin -= relief;
	}

      /* Put a margin around the image.  */
      if (hmargin || vmargin)
	{
	  if (hmargin == vmargin)
	    plist = Fplist_put (plist, QCmargin, make_fixnum (hmargin));
	  else
	    plist = Fplist_put (plist, QCmargin,
				Fcons (make_fixnum (hmargin),
				       make_fixnum (vmargin)));
	}

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

> I've fixed tab-bar.el to request the same margin as specified by
> tab-bar-button-margin, and I also fixed the code in w32term.c and
> xterm.c which used the corresponding tool-bar values when tab-bar
> values should have been used.  This improved the situation to some
> extent, but we still leave behind an artifact when the mouse moves off
> the tab-bar button: a 1-pixel vertical line.  I think that's because
> there's no relief definition in the image, but the value of
> tab-bar-button-relief is non-zero (and xterm.c/w32term.c use that
> non-zero value when they redraw the buttons).

Thanks.  Interestingly enough, I don't see a 1-pixel vertical line when
the highlight is cleared, but it could be an issue with my display.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: Juri Linkov <juri@linkov.net>,  Alan Third <alan@idiocy.org>,
>   50424@debbugs.gnu.org
> Date: Tue, 07 Sep 2021 21:02:43 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I've fixed tab-bar.el to request the same margin as specified by
> > tab-bar-button-margin, and I also fixed the code in w32term.c and
> > xterm.c which used the corresponding tool-bar values when tab-bar
> > values should have been used.  This improved the situation to some
> > extent, but we still leave behind an artifact when the mouse moves off
> > the tab-bar button: a 1-pixel vertical line.  I think that's because
> > there's no relief definition in the image, but the value of
> > tab-bar-button-relief is non-zero (and xterm.c/w32term.c use that
> > non-zero value when they redraw the buttons).
> 
> Thanks.  Interestingly enough, I don't see a 1-pixel vertical line when
> the highlight is cleared, but it could be an issue with my display.

Try a larger default font, as in

   emacs -Q -fn "Your Font-20"

where "Your Font" is the font you get by default in Emacs.  I only see
the problem clearly when I use such a large font.

Also try "M-x tab-bar-history-mode RET".

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Alan Third]

On Tue, Sep 07, 2021 at 01:41:38PM +0300, Eli Zaretskii wrote:
> 
> I hope the information here will allow Juri and people who really
> understand the meaning of an image margin and relief (Alan?) fix the
> rest of the problem.

AFAIK the margin and relief are entirely handled by the term code and
don't really have anything to do with the image itself. I expect
that's why this is an OS specific problem.

This actually looks slightly like a bug we have with normal images on
the NS port, where if an image is replaced with a slightly smaller one
part of the original image *may* not be cleared. I've never managed to
work out why it happens.

-- 
Alan Third

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> Date: Wed, 8 Sep 2021 00:13:15 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: Po Lu <luangruo@yahoo.com>, Juri Linkov <juri@linkov.net>,
> 	50424@debbugs.gnu.org
> 
> On Tue, Sep 07, 2021 at 01:41:38PM +0300, Eli Zaretskii wrote:
> > 
> > I hope the information here will allow Juri and people who really
> > understand the meaning of an image margin and relief (Alan?) fix the
> > rest of the problem.
> 
> AFAIK the margin and relief are entirely handled by the term code and
> don't really have anything to do with the image itself. I expect
> that's why this is an OS specific problem.

The code to deal with that is identical in xterm.c and w32term.c.

Can you perhaps help us understand the purpose and semantics of the
delicate dance in the xdisp.c code I posted regarding images on the
tool-bar buttons?  AFAIU, the xterm/w32term code was written to
reflect that, and the tab-bar code is simply a copy of the tool-bar
code, except that the image spec is defined entirely in Lisp, instead
of being dynamically redefined on the fly by the C code in the display
engine.  So we need to understand the meaning of the margins and the
relief settings and their relation to the button being "selected" as
well as to the value of auto-raise-tool-bar-buttons, in order to do in
Lisp the same thing.  Because xterm/w32term rely on this logic to
clear the area when the mouse pointer moves off the button.

> This actually looks slightly like a bug we have with normal images on
> the NS port, where if an image is replaced with a slightly smaller one
> part of the original image *may* not be cleared. I've never managed to
> work out why it happens.

It's similar, yes.  But in the case in point the code which clears the
area is working well for tool-bar buttons, and we don't change the
image dimensions for the tab bar buttons.

Thanks.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Alan Third]

On Wed, Sep 08, 2021 at 09:05:50AM +0300, Eli Zaretskii wrote:
> > Date: Wed, 8 Sep 2021 00:13:15 +0100
> > From: Alan Third <alan@idiocy.org>
> > Cc: Po Lu <luangruo@yahoo.com>, Juri Linkov <juri@linkov.net>,
> > 	50424@debbugs.gnu.org
> > 
> > On Tue, Sep 07, 2021 at 01:41:38PM +0300, Eli Zaretskii wrote:
> > > 
> > > I hope the information here will allow Juri and people who really
> > > understand the meaning of an image margin and relief (Alan?) fix the
> > > rest of the problem.
> > 
> > AFAIK the margin and relief are entirely handled by the term code and
> > don't really have anything to do with the image itself. I expect
> > that's why this is an OS specific problem.
> 
> The code to deal with that is identical in xterm.c and w32term.c.
> 
> Can you perhaps help us understand the purpose and semantics of the
> delicate dance in the xdisp.c code I posted regarding images on the
> tool-bar buttons?  AFAIU, the xterm/w32term code was written to
> reflect that, and the tab-bar code is simply a copy of the tool-bar
> code, except that the image spec is defined entirely in Lisp, instead
> of being dynamically redefined on the fly by the C code in the display
> engine.  So we need to understand the meaning of the margins and the
> relief settings and their relation to the button being "selected" as
> well as to the value of auto-raise-tool-bar-buttons, in order to do in
> Lisp the same thing.  Because xterm/w32term rely on this logic to
> clear the area when the mouse pointer moves off the button.

I think all it's doing is ensuring the total area the image takes up
is the same whether there's a relief or not.

The total width = image + relief width + margin width, so if you
reduce the size of the relief you have to increase the size of the
margin by the same amount to make sure it takes up the same amount of
space and doesn't move.

As far as I can see, what's happening here is that the margin is
pushing the relief one pixel into the separator to the left, so I
suspect that's why it's not being cleared.

I'm somewhat surprised that it's not pushing the right hand side one
pixel too far in the other direction and causing it to not be cleared
either, but maybe that's just chance.

The change below fixes it here, but I've not made sure it does the
right thing with different sized margins and so on.

modified   src/w32term.c
@@ -2057,11 +2057,11 @@ w32_draw_image_relief (struct glyph_string *s)
 	  && FIXNUMP (XCAR (Vtab_bar_button_margin))
 	  && FIXNUMP (XCDR (Vtab_bar_button_margin)))
 	{
-	  extra_x = XFIXNUM (XCAR (Vtab_bar_button_margin));
-	  extra_y = XFIXNUM (XCDR (Vtab_bar_button_margin));
+	  extra_x = XFIXNUM (XCAR (Vtab_bar_button_margin)) - thick;
+	  extra_y = XFIXNUM (XCDR (Vtab_bar_button_margin)) - thick;
 	}
       else if (FIXNUMP (Vtab_bar_button_margin))
-	extra_x = extra_y = XFIXNUM (Vtab_bar_button_margin);
+	extra_x = extra_y = XFIXNUM (Vtab_bar_button_margin) - thick;
     }
 
   if (s->face->id == TOOL_BAR_FACE_ID)
modified   src/xterm.c
@@ -3235,11 +3235,11 @@ x_draw_image_relief (struct glyph_string *s)
 	  && FIXNUMP (XCAR (Vtab_bar_button_margin))
 	  && FIXNUMP (XCDR (Vtab_bar_button_margin)))
 	{
-	  extra_x = XFIXNUM (XCAR (Vtab_bar_button_margin));
-	  extra_y = XFIXNUM (XCDR (Vtab_bar_button_margin));
+	  extra_x = XFIXNUM (XCAR (Vtab_bar_button_margin)) - thick;
+	  extra_y = XFIXNUM (XCDR (Vtab_bar_button_margin)) - thick;
 	}
       else if (FIXNUMP (Vtab_bar_button_margin))
-	extra_x = extra_y = XFIXNUM (Vtab_bar_button_margin);
+	extra_x = extra_y = XFIXNUM (Vtab_bar_button_margin) - thick;
     }
 
   if (s->face->id == TOOL_BAR_FACE_ID)

-- 
Alan Third

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> Date: Wed, 8 Sep 2021 17:57:50 +0100
> From: Alan Third <alan@idiocy.org>
> Cc: luangruo@yahoo.com, juri@linkov.net, 50424@debbugs.gnu.org
> 
> I think all it's doing is ensuring the total area the image takes up
> is the same whether there's a relief or not.
> 
> The total width = image + relief width + margin width, so if you
> reduce the size of the relief you have to increase the size of the
> margin by the same amount to make sure it takes up the same amount of
> space and doesn't move.
> 
> As far as I can see, what's happening here is that the margin is
> pushing the relief one pixel into the separator to the left, so I
> suspect that's why it's not being cleared.
> 
> I'm somewhat surprised that it's not pushing the right hand side one
> pixel too far in the other direction and causing it to not be cleared
> either, but maybe that's just chance.
> 
> The change below fixes it here, but I've not made sure it does the
> right thing with different sized margins and so on.

Thanks, I installed this, since it fixes at least the default case.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

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

> I think all it's doing is ensuring the total area the image takes up
> is the same whether there's a relief or not.
>
> The total width = image + relief width + margin width, so if you
> reduce the size of the relief you have to increase the size of the
> margin by the same amount to make sure it takes up the same amount of
> space and doesn't move.
>
> As far as I can see, what's happening here is that the margin is
> pushing the relief one pixel into the separator to the left, so I
> suspect that's why it's not being cleared.
>
> I'm somewhat surprised that it's not pushing the right hand side one
> pixel too far in the other direction and causing it to not be cleared
> either, but maybe that's just chance.
>
> The change below fixes it here, but I've not made sure it does the
> right thing with different sized margins and so on.

Thanks, I really appreciate your help.  After seeing the result
of your patch, one question that came up is whether it's possible
not to change the previously used dimensions?  With enlarged height
and width, now the buttons are not vertically aligned:


[-- Attachment #2: tab-1.png --]
[-- Type: image/png, Size: 7993 bytes --]

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  luangruo@yahoo.com,  50424@debbugs.gnu.org
> Date: Sat, 11 Sep 2021 21:49:32 +0300
> 
> With enlarged height and width, now the buttons are not vertically
> aligned:

Is this a screenshot of a tab bar or of a tab line?

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

>> The change below fixes it here, but I've not made sure it does the
>> right thing with different sized margins and so on.
>
> Thanks, I really appreciate your help.  After seeing the result
> of your patch, one question that came up is whether it's possible
> not to change the previously used dimensions?  With enlarged height
> and width, now the buttons are not vertically aligned:

Sorry, actually this question was addressed to Eli
for the commit with the same subject but different hash:
db74a93659 that contains such changes:

- :margin (2 . 0)
+ :margin ,tab-bar-button-margin

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

>> With enlarged height and width, now the buttons are not vertically
>> aligned:
>
> Is this a screenshot of a tab bar or of a tab line?

Tab bar.  And the change was caused by your commit db74a93659.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: alan@idiocy.org,  luangruo@yahoo.com,  50424@debbugs.gnu.org
> Date: Sat, 11 Sep 2021 22:45:24 +0300
> 
> >> With enlarged height and width, now the buttons are not vertically
> >> aligned:
> >
> > Is this a screenshot of a tab bar or of a tab line?
> 
> Tab bar.  And the change was caused by your commit db74a93659.

Then I don't understand the two horizontally-adjacent images.  What
are they? two separate frame?  If they are two frames, then where are
the frame decorations?  In short, I don't understand what am I seeing
there, and how did you get this display.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sat, 11 Sep 2021 22:43:44 +0300
> Cc: luangruo@yahoo.com, 50424@debbugs.gnu.org
> 
> > Thanks, I really appreciate your help.  After seeing the result
> > of your patch, one question that came up is whether it's possible
> > not to change the previously used dimensions?  With enlarged height
> > and width, now the buttons are not vertically aligned:
> 
> Sorry, actually this question was addressed to Eli
> for the commit with the same subject but different hash:
> db74a93659 that contains such changes:
> 
> - :margin (2 . 0)
> + :margin ,tab-bar-button-margin

This is to make the code do what the variable's documentation says.
Without that change, the images were sized using :margin '(2 . 0)',
but the C code used the value of tab-bar-button-margin, a simple
scalar, to place the image and clear its background.  The way to
affect the image dimensions is by changing the value of
tab-bar-button-margin, not by hard-coding the margin in the image
properties.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> Date: Sun, 12 Sep 2021 06:46:55 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: luangruo@yahoo.com, 50424@debbugs.gnu.org, alan@idiocy.org
> 
> > Tab bar.  And the change was caused by your commit db74a93659.
> 
> Then I don't understand the two horizontally-adjacent images.  What
> are they? two separate frame?  If they are two frames, then where are
> the frame decorations?  In short, I don't understand what am I seeing
> there, and how did you get this display.

And also, when you say "buttons are not vertically aligned", what
exactly do you mean?  Not aligned with what?

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

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

>> >> With enlarged height and width, now the buttons are not vertically
>> >> aligned:
>> >
>> > Is this a screenshot of a tab bar or of a tab line?
>>
>> Tab bar.  And the change was caused by your commit db74a93659.
>
> Then I don't understand the two horizontally-adjacent images.  What
> are they? two separate frame?  If they are two frames, then where are
> the frame decorations?  In short, I don't understand what am I seeing
> there, and how did you get this display.

These are two separate images that show the changed tab dimensions
before changes when :margin was (2 . 0), and after changes
when now :margin is 4.

> And also, when you say "buttons are not vertically aligned", what
> exactly do you mean?  Not aligned with what?

Now buttons are not aligned to the baseline, and thus are
not centered vertically anymore.

>> db74a93659 that contains such changes:
>>
>> - :margin (2 . 0)
>> + :margin ,tab-bar-button-margin
>
> This is to make the code do what the variable's documentation says.
> Without that change, the images were sized using :margin '(2 . 0)',
> but the C code used the value of tab-bar-button-margin, a simple
> scalar, to place the image and clear its background.  The way to
> affect the image dimensions is by changing the value of
> tab-bar-button-margin, not by hard-coding the margin in the image
> properties.

This patch restores the original tab dimensions, while also keeps fixed
the reported problem of mouse face not clearing entirely:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: tab-bar-button-margin.patch --]
[-- Type: text/x-diff, Size: 2688 bytes --]

diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
index 8be08d4b8b..edbadec09d 100644
--- a/lisp/tab-bar.el
+++ b/lisp/tab-bar.el
@@ -161,7 +161,7 @@ tab-bar--load-buttons
     (add-text-properties 0 (length tab-bar-new-button)
                          `(display (image :type xpm
                                           :file "tabs/new.xpm"
-                                          :margin ,tab-bar-button-margin
+                                          :margin ,(cons tab-bar-button-margin 0)
                                           :ascent center))
                          tab-bar-new-button))
 
@@ -171,7 +171,7 @@ tab-bar--load-buttons
     (add-text-properties 0 (length tab-bar-close-button)
                          `(display (image :type xpm
                                           :file "tabs/close.xpm"
-                                          :margin ,tab-bar-button-margin
+                                          :margin ,(cons tab-bar-button-margin 0)
                                           :ascent center))
                          tab-bar-close-button)))
 
@@ -1659,7 +1659,7 @@ tab-bar-history-mode
           (add-text-properties 0 (length tab-bar-back-button)
                                `(display (image :type xpm
                                                 :file "tabs/left-arrow.xpm"
-                                                :margin ,tab-bar-button-margin
+                                                :margin ,(cons tab-bar-button-margin 0)
                                                 :ascent center))
                                tab-bar-back-button))
         (when (and tab-bar-mode (not (get-text-property 0 'display tab-bar-forward-button)))
@@ -1667,7 +1667,7 @@ tab-bar-history-mode
           (add-text-properties 0 (length tab-bar-forward-button)
                                `(display (image :type xpm
                                                 :file "tabs/right-arrow.xpm"
-                                                :margin ,tab-bar-button-margin
+                                                :margin ,(cons tab-bar-button-margin 0)
                                                 :ascent center))
                                tab-bar-forward-button))
 
diff --git a/src/dispextern.h b/src/dispextern.h
index f4c7575b35..0ede76ebf5 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3202,7 +3202,7 @@ #define IMAGE_CACHE_BUCKETS_SIZE 1001
 
 /* Default values of the above variables.  */
 
-#define DEFAULT_TAB_BAR_BUTTON_MARGIN 4
+#define DEFAULT_TAB_BAR_BUTTON_MARGIN 2
 #define DEFAULT_TAB_BAR_BUTTON_RELIEF 1
 
 /* The height in pixels of the default tab-bar images.  */

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: alan@idiocy.org,  luangruo@yahoo.com,  50424@debbugs.gnu.org
> Date: Sun, 12 Sep 2021 10:06:09 +0300
> 
> > Then I don't understand the two horizontally-adjacent images.  What
> > are they? two separate frame?  If they are two frames, then where are
> > the frame decorations?  In short, I don't understand what am I seeing
> > there, and how did you get this display.
> 
> These are two separate images that show the changed tab dimensions
> before changes when :margin was (2 . 0), and after changes
> when now :margin is 4.
> 
> > And also, when you say "buttons are not vertically aligned", what
> > exactly do you mean?  Not aligned with what?
> 
> Now buttons are not aligned to the baseline, and thus are
> not centered vertically anymore.

Isn't that evidence that something is wrong in how the labels and
buttons are displayed?  The margin of 4 is symmetrical, so why aren't
the buttons centered?  And why do we need to use zero vertical margin
to get them centered?  This seems to mean we have some hidden problem
here.

> This patch restores the original tab dimensions, while also keeps fixed
> the reported problem of mouse face not clearing entirely:
> 
> diff --git a/lisp/tab-bar.el b/lisp/tab-bar.el
> index 8be08d4b8b..edbadec09d 100644
> --- a/lisp/tab-bar.el
> +++ b/lisp/tab-bar.el
> @@ -161,7 +161,7 @@ tab-bar--load-buttons
>      (add-text-properties 0 (length tab-bar-new-button)
>                           `(display (image :type xpm
>                                            :file "tabs/new.xpm"
> -                                          :margin ,tab-bar-button-margin
> +                                          :margin ,(cons tab-bar-button-margin 0)
>                                            :ascent center))
>                           tab-bar-new-button))

Sorry, this cannot be right.  tab-bar-button-margin is documented and
used as determining the button margins completely:

      doc: /* Margin around tab-bar buttons in pixels.
  If an integer, use that for both horizontal and vertical margins.
  Otherwise, value should be a pair of integers `(HORZ . VERT)' with
  HORZ specifying the horizontal margin, and VERT specifying the
  vertical margin.  */);

We cannot arbitrarily decide that the value matters only for the
horizontal margin, but not for the vertical one.  When the user or a
Lisp program change the value of that variable, they should get the
effect they expect based on the documentation.  Also, the code in
xterm.c/w32term.c clearly behaves according to the doc string of
tab-bar-button-margin, which is yet another reason not to do what you
propose.  And what will happen with your code if tab-bar-button-margin
is set to a cons cell, as allowed by the documentation above?

If we want the default value of tab-bar-button-margin to be a cons
cell, let's change the default value to be a cons cell, it's not more
complicated than setting it to a scalar, even in C.  But tab-bar.el
should use in its image specs the exact value of
tab-bar-button-margin, it cannot decide to change it behind the back
of the caller.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

> If we want the default value of tab-bar-button-margin to be a cons
> cell, let's change the default value to be a cons cell, it's not more
> complicated than setting it to a scalar, even in C.  But tab-bar.el
> should use in its image specs the exact value of
> tab-bar-button-margin, it cannot decide to change it behind the back
> of the caller.

I tried a cons cell, but it looks ugly for buttons other than the close button.
OTOH, margin 1 looks nice for all buttons:

diff --git a/src/dispextern.h b/src/dispextern.h
index f4c7575b35..6aefe43e19 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3202,7 +3202,7 @@ #define IMAGE_CACHE_BUCKETS_SIZE 1001
 
 /* Default values of the above variables.  */
 
-#define DEFAULT_TAB_BAR_BUTTON_MARGIN 4
+#define DEFAULT_TAB_BAR_BUTTON_MARGIN 1
 #define DEFAULT_TAB_BAR_BUTTON_RELIEF 1

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: alan@idiocy.org,  luangruo@yahoo.com,  50424@debbugs.gnu.org
> Date: Sun, 12 Sep 2021 19:06:57 +0300
> 
> > If we want the default value of tab-bar-button-margin to be a cons
> > cell, let's change the default value to be a cons cell, it's not more
> > complicated than setting it to a scalar, even in C.  But tab-bar.el
> > should use in its image specs the exact value of
> > tab-bar-button-margin, it cannot decide to change it behind the back
> > of the caller.
> 
> I tried a cons cell, but it looks ugly for buttons other than the close button.
> OTOH, margin 1 looks nice for all buttons:

Fine by me.

> diff --git a/src/dispextern.h b/src/dispextern.h
> index f4c7575b35..6aefe43e19 100644
> --- a/src/dispextern.h
> +++ b/src/dispextern.h
> @@ -3202,7 +3202,7 @@ #define IMAGE_CACHE_BUCKETS_SIZE 1001
>  
>  /* Default values of the above variables.  */
>  
> -#define DEFAULT_TAB_BAR_BUTTON_MARGIN 4
> +#define DEFAULT_TAB_BAR_BUTTON_MARGIN 1
>  #define DEFAULT_TAB_BAR_BUTTON_RELIEF 1

Btw, I don't see much sense in keeping the
DEFAULT_TAB_BAR_BUTTON_MARGIN macro: it's used in one place only,
AFAICT, so we could simply have it there literally.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Juri Linkov]

tags 50424 fixed
close 50424 28.0.50
quit

>> > If we want the default value of tab-bar-button-margin to be a cons
>> > cell, let's change the default value to be a cons cell, it's not more
>> > complicated than setting it to a scalar, even in C.  But tab-bar.el
>> > should use in its image specs the exact value of
>> > tab-bar-button-margin, it cannot decide to change it behind the back
>> > of the caller.
>>
>> I tried a cons cell, but it looks ugly for buttons other than the close button.
>> OTOH, margin 1 looks nice for all buttons:
>
> Fine by me.

So pushed to master.

>> -#define DEFAULT_TAB_BAR_BUTTON_MARGIN 4
>> +#define DEFAULT_TAB_BAR_BUTTON_MARGIN 1
>>  #define DEFAULT_TAB_BAR_BUTTON_RELIEF 1
>
> Btw, I don't see much sense in keeping the
> DEFAULT_TAB_BAR_BUTTON_MARGIN macro: it's used in one place only,
> AFAICT, so we could simply have it there literally.

I thought about this too, but it will break the symmetry with
DEFAULT_TOOL_BAR_BUTTON_MARGIN and DEFAULT_TOOL_BAR_BUTTON_RELIEF when
DEFAULT_TAB_BAR_BUTTON_MARGIN will be used in more places.

bug#50424: 27.2; Tab bar button mouse face not clearing entirely

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: alan@idiocy.org,  luangruo@yahoo.com,  50424@debbugs.gnu.org
> Date: Mon, 13 Sep 2021 10:59:12 +0300
> 
> >> -#define DEFAULT_TAB_BAR_BUTTON_MARGIN 4
> >> +#define DEFAULT_TAB_BAR_BUTTON_MARGIN 1
> >>  #define DEFAULT_TAB_BAR_BUTTON_RELIEF 1
> >
> > Btw, I don't see much sense in keeping the
> > DEFAULT_TAB_BAR_BUTTON_MARGIN macro: it's used in one place only,
> > AFAICT, so we could simply have it there literally.
> 
> I thought about this too, but it will break the symmetry with
> DEFAULT_TOOL_BAR_BUTTON_MARGIN and DEFAULT_TOOL_BAR_BUTTON_RELIEF when
> DEFAULT_TAB_BAR_BUTTON_MARGIN will be used in more places.

The *_RELIEF variables are used in more than one place, so it makes
sense to have them.

As for the "symmetry" with the tool bar: it doesn't really exist,
since you do almost everything related to the tab-bar button display
in Lisp, not in C.  That "symmetry" is very misleading, so much so
that the other day it cost me 2 hours of frustrating debugging before
I realized why the image parameters I see in w32term.c are so
different from what Vtab_bar_button_margin says it should be.