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

From: Eli Zaretskii <[email-masked]>
Date: Tue, 07 Sep 2021 13:41:38 +0300

> From: Po Lu <[email-masked]>
> Cc: [email-masked]
> 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)));
	}