bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

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

This bug can be reproduced here with Emacs 26 and current master.  The
mode line customization was provided by Jonas Bernoulli in Bug#38181.

With emacs -Q load the attached test-mode-line.el and hit F1.  This
should show the *scratch* window with an expanded mode line and a
normal scroll bar and a two line echo area showing "test-mode-line".

Now hit any other key.  This sould shrink the echo area and redisplay
the *scratch* window as before but "extends" the vertical scroll bar
into the mode line.

The reason for this behavior is that the vertical scroll bar height in
the first case is calculated from a window_box_height call where
ml_row->mode_line_p is set and thus ml_row->height is used.

In the second case ml_row->mode_line_p is not set and the value is
taken via estimate_mode_line_height which, given the specification
from 'test-mode-line', doesn't estimate the height well.

martin

[-- Attachment #2: test-mode-line.el --]
[-- Type: text/plain, Size: 914 bytes --]

(defun test-mode-line ()
  (interactive)
  (setq
   mode-line-format
   (cons (propertize " " 'display
                     (create-image "/* XPM */ static char * image[] = {
\"3 60 1 1\",
\"0 c #00aaff\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\"
};" 'xpm t :ascent 'center))
	 mode-line-format))
  (message "\ntest-mode-line"))

(global-set-key [f1] 'test-mode-line)

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

For the record: Rewriting 'window_box_height' as

int
window_box_height (struct window *w)
{
   return WINDOW_BOX_TEXT_HEIGHT (w);
}

fixes the problem here.

martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Tue, 31 Dec 2019 10:57:55 +0100
> 
> Now hit any other key.  This sould shrink the echo area and redisplay
> the *scratch* window as before but "extends" the vertical scroll bar
> into the mode line.
> 
> The reason for this behavior is that the vertical scroll bar height in
> the first case is calculated from a window_box_height call where
> ml_row->mode_line_p is set and thus ml_row->height is used.
> 
> In the second case ml_row->mode_line_p is not set and the value is
> taken via estimate_mode_line_height which, given the specification
> from 'test-mode-line', doesn't estimate the height well.

AFAICT, when the ml_row->mode_line_p flag is reset, ml_row->height is
also zero, so disregarding the flag would not have helped us in this
case.

> For the record: Rewriting 'window_box_height' as
> 
> int
> window_box_height (struct window *w)
> {
>    return WINDOW_BOX_TEXT_HEIGHT (w);
> }
> 
> fixes the problem here.

Thanks, but that's not an idea I would like entertaining.  As much as
window_box_text_height and WINDOW_BOX_TEXT_HEIGHT look similar,
there's a subtle difference between them.  The latter is supposed not
to be called during redisplay, because window's mode_line_height field
is not guaranteed to be accurate then, and the fallback is the same
estimate_mode_line_height which doesn't support non-character display
elements.  It is generally okay to use WINDOW_BOX_TEXT_HEIGHT in
window.c, because that code runs outside of redisplay, and the window
object should be up-to-date.  But window_box_text_height is called
_a_lot_ from within redisplay itself.  And here we get to another
subtlety: window_box_text_height uses the window's current_matrix,
which might not be up-to-date yet, depending on where it is called,
because we only update it in update_frame, and set_vertical_scroll_bar
is called long before that.

When a window is resized, as happens in this scenario when the
echo-area message is cleared, the mode-line row changes to a different
one (because it is always the last row of the window), and the new row
didn't yet get updated.  We have the correct data in the
desired_matrix, but not in the current_matrix.  I considered using the
desired_matrix in this case, but that is also somewhat scary: the
desired matrix could be very sparse under some redisplay
optimizations, so we'd need some logic to verify the data is valid
before using it.  Eventually, a simpler solution is just to fall back
to the window's mode_line_height field, before falling back to
estimate_mode_line_height, because when the mode-line height changes,
we schedule an immediate thorough redisplay of the window, and
invalidate that window's mode_line_height field, to be recomputed by
the rescheduled redisplay.  See the proposed patch below.

diff --git a/src/xdisp.c b/src/xdisp.c
index 6b677b63ae..fab95076fe 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1105,6 +1105,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (ml_row && ml_row->mode_line_p)
 	height -= ml_row->height;
+      else if (w->mode_line_height >= 0)
+	height -= w->mode_line_height;
       else
 	height -= estimate_mode_line_height (f, CURRENT_MODE_LINE_FACE_ID (w));
     }
@@ -1117,6 +1119,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (tl_row && tl_row->mode_line_p)
 	height -= tl_row->height;
+      else if (w->tab_line_height >= 0)
+	height -= w->tab_line_height;
       else
 	height -= estimate_mode_line_height (f, TAB_LINE_FACE_ID);
     }
@@ -1129,6 +1133,8 @@ window_box_height (struct window *w)
 	   : 0);
       if (hl_row && hl_row->mode_line_p)
 	height -= hl_row->height;
+      else if (w->header_line_height >= 0)
+	height -= w->header_line_height;
       else
 	height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
     }

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

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

 > AFAICT, when the ml_row->mode_line_p flag is reset, ml_row->height is
 > also zero, so disregarding the flag would not have helped us in this
 > case.

Right, I've been there initially.

 >> For the record: Rewriting 'window_box_height' as
 >>
 >> int
 >> window_box_height (struct window *w)
 >> {
 >>     return WINDOW_BOX_TEXT_HEIGHT (w);
 >> }
 >>
 >> fixes the problem here.
 >
 > Thanks, but that's not an idea I would like entertaining.

Neither do I.

 > As much as
 > window_box_text_height

"window_box_height" presumably, also everywhere below.

 > and WINDOW_BOX_TEXT_HEIGHT look similar,
 > there's a subtle difference between them.  The latter is supposed not
 > to be called during redisplay, because window's mode_line_height field
 > is not guaranteed to be accurate then,

Your patch does use it though.

 > and the fallback is the same
 > estimate_mode_line_height which doesn't support non-character display
 > elements.  It is generally okay to use WINDOW_BOX_TEXT_HEIGHT in
 > window.c,

window.c doesn't use it.  It's used in xdisp.c only.  And I'm sure it
can return negative values, sometimes.  So I presume we can do away
with it then?

 > because that code runs outside of redisplay, and the window
 > object should be up-to-date.  But window_box_text_height is called
 > _a_lot_ from within redisplay itself.  And here we get to another
 > subtlety: window_box_text_height uses the window's current_matrix,
 > which might not be up-to-date yet, depending on where it is called,
 > because we only update it in update_frame, and set_vertical_scroll_bar
 > is called long before that.
 >
 > When a window is resized, as happens in this scenario when the
 > echo-area message is cleared, the mode-line row changes to a different
 > one (because it is always the last row of the window), and the new row
 > didn't yet get updated.  We have the correct data in the
 > desired_matrix, but not in the current_matrix.  I considered using the
 > desired_matrix in this case, but that is also somewhat scary: the
 > desired matrix could be very sparse under some redisplay
 > optimizations, so we'd need some logic to verify the data is valid
 > before using it.

I've tried via

	  w->mode_line_height
	    = MATRIX_MODE_LINE_ROW (w->current_matrix)->height
	    = DESIRED_MODE_LINE_HEIGHT (w);

after displaying the mode lines without great success (likely because
window_box_height didn't go for it then).

 > Eventually, a simpler solution is just to fall back
 > to the window's mode_line_height field, before falling back to
 > estimate_mode_line_height, because when the mode-line height changes,
 > we schedule an immediate thorough redisplay of the window, and
 > invalidate that window's mode_line_height field, to be recomputed by
 > the rescheduled redisplay.  See the proposed patch below.

This fails here when loading the attached test-popup-2.el and typing,
for example, F2 <up> F3 <up> F2 <up>.

martin

[-- Attachment #2: test-popup-2.el --]
[-- Type: text/plain, Size: 1490 bytes --]

(defvar old-mode-line-format mode-line-format)

(defun foo-message (&optional newline)
  (message
   (concat (if newline "\n" "")
	   "total: %s, body: %s, mode: %s")
   (window-pixel-height) (window-body-height nil t)
   (window-mode-line-height)))

(defun foo ()
  (interactive)
  (foo-message))

(global-set-key [f1] 'foo)

(defun test-popup ()
  (interactive)
;;   (set-face-attribute 'mode-line nil :height 200)
;;   (set-face-attribute 'mode-line-inactive nil :height 200)
  (setq
   mode-line-format
   (cons (propertize " " 'display
                     (create-image "/* XPM */ static char * image[] = {
\"3 60 1 1\",
\"0 c #00aaff\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\",
\"000\",\n\"000\",\n\"000\",\n\"000\",\n\"000\"
};" 'xpm t :ascent 'center))
	 mode-line-format))
  (foo-message t))

(global-set-key [f2] 'test-popup)

(defun bar ()
  (interactive)
  (setq mode-line-format old-mode-line-format))

(global-set-key [f3] 'bar)

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Wed, 1 Jan 2020 18:49:30 +0100
> 
>  > and the fallback is the same
>  > estimate_mode_line_height which doesn't support non-character display
>  > elements.  It is generally okay to use WINDOW_BOX_TEXT_HEIGHT in
>  > window.c,
> 
> window.c doesn't use it.  It's used in xdisp.c only.  And I'm sure it
> can return negative values, sometimes.  So I presume we can do away
> with it then?

I think we could.  But I'd rather not do that on the emacs-27 branch,
not unless we find situations where it actually causes bugs.

>  > Eventually, a simpler solution is just to fall back
>  > to the window's mode_line_height field, before falling back to
>  > estimate_mode_line_height, because when the mode-line height changes,
>  > we schedule an immediate thorough redisplay of the window, and
>  > invalidate that window's mode_line_height field, to be recomputed by
>  > the rescheduled redisplay.  See the proposed patch below.
> 
> This fails here when loading the attached test-popup-2.el and typing,
> for example, F2 <up> F3 <up> F2 <up>.

There's more than one problem here (didn't you wonder why pressing F3
doesn't immediately redraw the mode line?).  A more thorough patch is
below.  The dispnew.c part is semi-cleanup: it isn't strictly needed
after the changes in window_box_height, although originally that flag
not being reset upon window resizing was exactly the root cause for
the problem after "F2 up".  But I don't like the idea of this flag
remaining set, even if no one should be looking at it.  Hmm... maybe I
should reset the flag even if we did reallocate current_matrix...

diff --git a/lisp/frame.el b/lisp/frame.el
index c533e5a23f..16ee7580f8 100644
--- a/lisp/frame.el
+++ b/lisp/frame.el
@@ -2725,6 +2725,9 @@ 'automatic-hscrolling
         line-prefix
         wrap-prefix
         truncate-lines
+        mode-line-format
+        header-line-format
+        tab-line-format
         display-line-numbers
         display-line-numbers-width
         display-line-numbers-current-absolute
diff --git a/src/dispnew.c b/src/dispnew.c
index b2a257090c..8fe72df7ed 100644
--- a/src/dispnew.c
+++ b/src/dispnew.c
@@ -534,6 +534,13 @@ adjust_glyph_matrix (struct window *w, struct glyph_matrix *matrix, int x, int y
       eassert (left >= 0 && right >= 0);
       matrix->left_margin_glyphs = left;
       matrix->right_margin_glyphs = right;
+
+      /* If we are resizing a window without allocating new rows, make
+	 sure the previous mode-line row of the window's current
+	 matrix is no longer marked as such.  */
+      if (w && matrix == w->current_matrix
+	  && dim.height != matrix->nrows && !new_rows)
+	MATRIX_MODE_LINE_ROW (matrix)->mode_line_p = false;
     }
 
   /* Number of rows to be used by MATRIX.  */
diff --git a/src/xdisp.c b/src/xdisp.c
index 6b677b63ae..4856a7b13b 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -1093,44 +1093,59 @@ window_box_height (struct window *w)
 
   /* Note: the code below that determines the mode-line/header-line/tab-line
      height is essentially the same as that contained in the macro
-     CURRENT_{MODE,HEADER}_LINE_HEIGHT, except that it checks whether
-     the appropriate glyph row has its `mode_line_p' flag set,
-     and if it doesn't, uses estimate_mode_line_height instead.  */
+     CURRENT_{MODE,HEADER,TAB}_LINE_HEIGHT, except that it checks whether
+     the appropriate glyph row has its `mode_line_p' flag set, and if
+     it doesn't, uses estimate_mode_line_height instead.  */
 
   if (window_wants_mode_line (w))
     {
-      struct glyph_row *ml_row
-	= (w->current_matrix && w->current_matrix->rows
-	   ? MATRIX_MODE_LINE_ROW (w->current_matrix)
-	   : 0);
-      if (ml_row && ml_row->mode_line_p)
-	height -= ml_row->height;
+      if (w->mode_line_height >= 0)
+	height -= w->mode_line_height;
       else
-	height -= estimate_mode_line_height (f, CURRENT_MODE_LINE_FACE_ID (w));
+	{
+	  struct glyph_row *ml_row
+	    = (w->current_matrix && w->current_matrix->rows
+	       ? MATRIX_MODE_LINE_ROW (w->current_matrix)
+	       : 0);
+	  if (ml_row && ml_row->mode_line_p)
+	    height -= ml_row->height;
+	  else
+	    height -= estimate_mode_line_height (f,
+						 CURRENT_MODE_LINE_FACE_ID (w));
+	}
     }
 
   if (window_wants_tab_line (w))
     {
-      struct glyph_row *tl_row
-	= (w->current_matrix && w->current_matrix->rows
-	   ? MATRIX_TAB_LINE_ROW (w->current_matrix)
-	   : 0);
-      if (tl_row && tl_row->mode_line_p)
-	height -= tl_row->height;
+      if (w->tab_line_height >= 0)
+	height -= w->tab_line_height;
       else
-	height -= estimate_mode_line_height (f, TAB_LINE_FACE_ID);
+	{
+	  struct glyph_row *tl_row
+	    = (w->current_matrix && w->current_matrix->rows
+	       ? MATRIX_TAB_LINE_ROW (w->current_matrix)
+	       : 0);
+	  if (tl_row && tl_row->mode_line_p)
+	    height -= tl_row->height;
+	  else
+	    height -= estimate_mode_line_height (f, TAB_LINE_FACE_ID);
+	}
     }
 
   if (window_wants_header_line (w))
     {
-      struct glyph_row *hl_row
-	= (w->current_matrix && w->current_matrix->rows
-	   ? MATRIX_HEADER_LINE_ROW (w->current_matrix)
-	   : 0);
-      if (hl_row && hl_row->mode_line_p)
-	height -= hl_row->height;
-      else
-	height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
+      if (w->header_line_height >= 0)
+	height -= w->header_line_height;
+      {
+	struct glyph_row *hl_row
+	  = (w->current_matrix && w->current_matrix->rows
+	     ? MATRIX_HEADER_LINE_ROW (w->current_matrix)
+	     : 0);
+	if (hl_row && hl_row->mode_line_p)
+	  height -= hl_row->height;
+	else
+	  height -= estimate_mode_line_height (f, HEADER_LINE_FACE_ID);
+      }
     }
 
   /* With a very small font and a mode-line that's taller than

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 >> window.c doesn't use it.  It's used in xdisp.c only.  And I'm sure it
 >> can return negative values, sometimes.  So I presume we can do away
 >> with it then?
 >
 > I think we could.  But I'd rather not do that on the emacs-27 branch,
 > not unless we find situations where it actually causes bugs.

Agreed.  I'd be more interested anyway in (1) getting the

ml_row && ml_row->mode_line_p

check into CURRENT_MODE_LINE_HEIGHT (if that makes sense) so we could
use CURRENT_MODE_LINE_HEIGHT everywhere (maybe without its embedded
assignment) and (2) improve and (remove from there) the

&& WINDOW_PIXEL_HEIGHT (w) > WINDOW_FRAME_LINE_HEIGHT (w));

check from window_wants_mode_line to use the latest calculated mode
line height instead.  I'd like to reserve one bit for each window's
scroll bars, margins, fringes, mode lines whether they would fit into
the window and suppress their display if they don't.  This would mean
that one can easily shrink a window to some bare minimum, maybe even
to zero height/width.

 > There's more than one problem here (didn't you wonder why pressing F3
 > doesn't immediately redraw the mode line?).

It never did that unless I changed the size of some window too, so why
should I have wondered?

 > A more thorough patch is
 > below.

It's a pleasant experience now.  How expensive is watching a variable?
So far I was convinced this would be used for debugging purposes only.

 > The dispnew.c part is semi-cleanup: it isn't strictly needed
 > after the changes in window_box_height, although originally that flag
 > not being reset upon window resizing was exactly the root cause for
 > the problem after "F2 up".  But I don't like the idea of this flag
 > remaining set, even if no one should be looking at it.  Hmm... maybe I
 > should reset the flag even if we did reallocate current_matrix...

What does "resizing a window without allocating new rows" encompass?
Is this only shrinking the window height ('shrink-/split-window') or
also making a special line like the mode line taller (eating away some
of the remaining matrix lines, naively spoken)?

martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Thu, 2 Jan 2020 20:19:05 +0100
> 
>  > There's more than one problem here (didn't you wonder why pressing F3
>  > doesn't immediately redraw the mode line?).
> 
> It never did that unless I changed the size of some window too, so why
> should I have wondered?

Well, because if you don't do anything after F3, the mode line stays
without update forever.

> How expensive is watching a variable?

Just an extra 'if' in a few places where we set the variable, to check
if it's being watched.  Grep the sources for SYMBOL_TRAPPED_WRITE.

> So far I was convinced this would be used for debugging purposes only.

You can see at the end of frame.el we have a list of variables we
watch that have nothing to do with debugging.

> What does "resizing a window without allocating new rows" encompass?
> Is this only shrinking the window height ('shrink-/split-window') or
> also making a special line like the mode line taller (eating away some
> of the remaining matrix lines, naively spoken)?

Glyph matrices don't care about the pixel size of the window, they
only care about the maximum number of glyph rows they can
accommodate.  So increasing the size of a mode line has the effect of
making the window's glyph matrices use fewer rows (assuming the
window's pixel size doesn't change).  In addition, a GUI window
usually has a matrix allocated for more rows than it actually uses,
which is why each matrix has both 'nrows' and 'rows_allocated'
members, and the former is likely to be smaller than the latter.  When
a window's resize needs less rows that 'rows_allocated', there's no
reallocation, just a change in the 'nrows' field.

But I now think that we need to reset the mode_line_p flag even if we
reallocate, because the rows which existed before reallocation are
copied to the enlarged matrix, so that flag is kept.  So I think I
will install the patch with that small change.

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Date: Thu, 02 Jan 2020 21:58:49 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 38828@debbugs.gnu.org
> 
> But I now think that we need to reset the mode_line_p flag even if we
> reallocate, because the rows which existed before reallocation are
> copied to the enlarged matrix, so that flag is kept.  So I think I
> will install the patch with that small change.

Done, and closing the bug.

Thanks for the test cases and the feedback.

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 > You can see at the end of frame.el we have a list of variables we
 > watch that have nothing to do with debugging.

I noticed that in the meantime.  So we could easily add, for example,
'scroll-bar-width' to that list and simplify text like the following

      If you do not specify a window’s scroll bar settings via
   ‘set-window-scroll-bars’, the buffer-local variables
   ‘vertical-scroll-bar’, ‘horizontal-scroll-bar’, ‘scroll-bar-width’ and
   ‘scroll-bar-height’ in the buffer being displayed control the window’s
   scroll bars.  The function ‘set-window-buffer’ examines these variables.
   If you change them in a buffer that is already visible in a window, you
   can make the window take note of the new values by calling
   ‘set-window-buffer’ specifying the same buffer that is already
   displayed.

in the Elisp manual?  I wonder why Fset_buffer_redisplay apparently
doesn't have to call bset_redisplay to redisplay_other_windows.  What
am I missing here?

 > Glyph matrices don't care about the pixel size of the window,

... but when the pixel height grows we may have to allocate new glyph
rows too, I suppose ...

 > they
 > only care about the maximum number of glyph rows they can
 > accommodate.  So increasing the size of a mode line has the effect of
 > making the window's glyph matrices use fewer rows (assuming the
 > window's pixel size doesn't change).  In addition, a GUI window
 > usually has a matrix allocated for more rows than it actually uses,
 > which is why each matrix has both 'nrows' and 'rows_allocated'
 > members, and the former is likely to be smaller than the latter.  When
 > a window's resize

... where a "resize" includes changes of its buffer's default face
font, line spacing, the mode line height ...

 > needs less rows that 'rows_allocated', there's no
 > reallocation, just a change in the 'nrows' field.
 >
 > But I now think that we need to reset the mode_line_p flag even if we
 > reallocate, because the rows which existed before reallocation are
 > copied to the enlarged matrix, so that flag is kept.  So I think I
 > will install the patch with that small change.

Works fine now.

Thanks, martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 3 Jan 2020 10:38:46 +0100
> 
>  > You can see at the end of frame.el we have a list of variables we
>  > watch that have nothing to do with debugging.
> 
> I noticed that in the meantime.  So we could easily add, for example,
> 'scroll-bar-width' to that list and simplify text like the following
> 
>       If you do not specify a window’s scroll bar settings via
>    ‘set-window-scroll-bars’, the buffer-local variables
>    ‘vertical-scroll-bar’, ‘horizontal-scroll-bar’, ‘scroll-bar-width’ and
>    ‘scroll-bar-height’ in the buffer being displayed control the window’s
>    scroll bars.  The function ‘set-window-buffer’ examines these variables.
>    If you change them in a buffer that is already visible in a window, you
>    can make the window take note of the new values by calling
>    ‘set-window-buffer’ specifying the same buffer that is already
>    displayed.
> 
> in the Elisp manual?

Yes, I think so.  You can easily try that yourself, though.

> I wonder why Fset_buffer_redisplay apparently doesn't have to call
> bset_redisplay to redisplay_other_windows.  What am I missing here?

Probably the fact that prevent_redisplay_optimizations_p is stronger
than windows_or_buffers_changed?

>  > Glyph matrices don't care about the pixel size of the window,
> 
> ... but when the pixel height grows we may have to allocate new glyph
> rows too, I suppose ...

Yes.

>  > they
>  > only care about the maximum number of glyph rows they can
>  > accommodate.  So increasing the size of a mode line has the effect of
>  > making the window's glyph matrices use fewer rows (assuming the
>  > window's pixel size doesn't change).  In addition, a GUI window
>  > usually has a matrix allocated for more rows than it actually uses,
>  > which is why each matrix has both 'nrows' and 'rows_allocated'
>  > members, and the former is likely to be smaller than the latter.  When
>  > a window's resize
> 
> ... where a "resize" includes changes of its buffer's default face
> font, line spacing, the mode line height ...

Right.

>  > But I now think that we need to reset the mode_line_p flag even if we
>  > reallocate, because the rows which existed before reallocation are
>  > copied to the enlarged matrix, so that flag is kept.  So I think I
>  > will install the patch with that small change.
> 
> Works fine now.

Thanks.

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 > Yes, I think so.  You can easily try that yourself, though.

Will do.  On master though and as soon as I can build again (Paul's
last commit broke that on Windows here).

martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 3 Jan 2020 11:43:15 +0100
> 
>  > Yes, I think so.  You can easily try that yourself, though.
> 
> Paul's last commit broke that on Windows here

It did? what are the signs of the breakage?  (It does build for me.)

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 > It did? what are the signs of the breakage?  (It does build for me.)

It crashes thusly:

   CC       lastfile.o
   CCLD     temacs.exe
/usr/bin/mkdir -p ../etc
make -C ../lisp update-subdirs
make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird betreten
make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird verlassen
cp -f temacs.exe bootstrap-emacs.exe
rm -f bootstrap-emacs.pdmp
./temacs --batch  -l loadup --temacs=pbootstrap
make[1]: *** [Makefile:817: bootstrap-emacs.pdmp] Fehler 255
make[1]: Verzeichnis „/c/emacs/trunk/non-32/src“ wird verlassen
make: *** [Makefile:424: src] Fehler 2

Building HEAD detached at d36adb544d proceeds normally.  I'll wait a
couple of days and do a bootstrap then.  Maybe someone sees the same
problem before that.

martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 3 Jan 2020 16:57:12 +0100
> 
> It crashes thusly:
> 
>    CC       lastfile.o
>    CCLD     temacs.exe
> /usr/bin/mkdir -p ../etc
> make -C ../lisp update-subdirs
> make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird betreten
> make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird verlassen
> cp -f temacs.exe bootstrap-emacs.exe
> rm -f bootstrap-emacs.pdmp
> ./temacs --batch  -l loadup --temacs=pbootstrap
> make[1]: *** [Makefile:817: bootstrap-emacs.pdmp] Fehler 255

If you run the crashing command under GDB, what does it show at the
point of the crash?

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 3 Jan 2020 16:57:12 +0100
> 
>    CC       lastfile.o
>    CCLD     temacs.exe
> /usr/bin/mkdir -p ../etc
> make -C ../lisp update-subdirs
> make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird betreten
> make[2]: Verzeichnis „/c/emacs/trunk/non-32/lisp“ wird verlassen
> cp -f temacs.exe bootstrap-emacs.exe
> rm -f bootstrap-emacs.pdmp
> ./temacs --batch  -l loadup --temacs=pbootstrap
> make[1]: *** [Makefile:817: bootstrap-emacs.pdmp] Fehler 255

I see the problem: Paul changed Emacs to use calloc, and we didn't
have its implementation consistent with our malloc in w32heap.c.  Now
I wonder why it didn't crash for me.

Please try the latest master.

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 > If you run the crashing command under GDB, what does it show at the
 > point of the crash?

If you told me how to do that (note, I'm building out of the tree in
spearate directories) I'll give it a try.  So far I was only able to
crash gdb.

martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> Cc: 38828@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 3 Jan 2020 17:34:34 +0100
> 
>  > If you run the crashing command under GDB, what does it show at the
>  > point of the crash?
> 
> If you told me how to do that (note, I'm building out of the tree in
> spearate directories) I'll give it a try.  So far I was only able to
> crash gdb.

Shouldn't be necessary anymore for this problem (I hope), but for the
future:

  $ cd ../src
  $ gdb ../build/src/temacs.exe
  (gdb) cd ../build/src
  (gdb) r --batch -l loadup --temacs=pbootstrap

(This assumes you build in build/, which is a sub-directory of the
top-level Emacs directory, a sibling of src/, lisp/, etc.)

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[martin rudalics]

 > I see the problem: Paul changed Emacs to use calloc, and we didn't
 > have its implementation consistent with our malloc in w32heap.c.  Now
 > I wonder why it didn't crash for me.
 >
 > Please try the latest master.

Builds without any problems now.

Many thanks for the fix, martin

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Amin Bandali]

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

Hi Eli,

[ Sorry if this isn’t the best place to report this ]

With 37f9182b68c62ee1912cf28d4ea0c30b4f8d59e0 (which cites this bug
report), I’ve noticed some weird display issues in a few different
modes:

- In `erc-mode' (the main ERC mode, used for for IRC channel buffers),
  if I press M-v to scroll to an earlier point in buffer and then scroll
  down line by line[1], the line previously at the bottom of the screen
  starts getting repeated, rather than the actual buffer content.  One
  way to get rid of this is to jump to the end of the buffer with M->.
  Also, I noticed that parts of the buffer that react to mouse hover
  (e.g. buttonized links) get “fixed” (the correct text appears) once
  the mouse cursor passes over them.

- In `magit-revision-mode' (displays the details of a commit), parts of
  the diff (IIRC the last line visible) has a different background than
  its surrounding lines.

In both of the above cases, the issue appears to be like some kind of
‘update’ not taking place anymore.  In both cases, the ‘artifacts’ are
gone if the frame is resized or toggled full-screen.  It’s quite likely
that the issue will happen with other modes as well, but the above two
are ones I’ve noticed and reliably reproduced so far.

Sorry if this isn’t too helpful of a report, as I’m not at all familiar
Emacs’s display code.  But I’ll be happy to try and help pin point the
issue if you could tell me how. :-)

Thanks,
amin

Footnotes:
[1]  I have the following scroll-related settings in my init file:

     (setq scroll-step 1
           scroll-conservatively 10
           scroll-preserve-screen-position 1)

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Eli Zaretskii]

> From: Amin Bandali <bandali@gnu.org>
> Cc: martin rudalics <rudalics@gmx.at>,  38828@debbugs.gnu.org
> Date: Sat, 04 Jan 2020 14:36:01 -0500
> 
> With 37f9182b68c62ee1912cf28d4ea0c30b4f8d59e0 (which cites this bug
> report), I’ve noticed some weird display issues in a few different
> modes:

There was a stupid typo in that commit, which was meanwhile fixed by
Martin.  Are still seeing these problems with the latest emacs-27
branch?

bug#38828: 26.3; Customized mode line breaks height of vertical scroll bar

[Amin Bandali]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Amin Bandali <bandali@gnu.org>
>> Cc: martin rudalics <rudalics@gmx.at>,  38828@debbugs.gnu.org
>> Date: Sat, 04 Jan 2020 14:36:01 -0500
>> 
>> With 37f9182b68c62ee1912cf28d4ea0c30b4f8d59e0 (which cites this bug
>> report), I’ve noticed some weird display issues in a few different
>> modes:
>
> There was a stupid typo in that commit, which was meanwhile fixed by
> Martin.  Are still seeing these problems with the latest emacs-27
> branch?
>

Ah, yes!  Confirming that cherry-picking Martin’s commit from emacs-27
to master seems to fix those issues.  Sorry for the noise.  I somehow
missed that commit when glancing at recent logs, and since it isn’t yet
merged into master these issues still occur there.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 857 bytes --]