Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme

From: Eli Zaretskii <[email-masked]>
Date: Fri, 01 Aug 2014 15:55:10 +0300

> Date: Fri, 01 Aug 2014 10:57:06 +0200
> From: martin rudalics <[email-masked]>
> CC: [email-masked], [email-masked]
> 
>  > That will not work well (you can try and see yourself).  But the
>  > opposite, i.e. having a 80-line frame on a 100-line terminal, is quite
>  > usable.  In fact, some people seem to use that to have minibuffer-only
>  > frames on a TTY.
> 
> Weird.  Such setting gets lost immediately when the terminal window is
> resized.

Which I'd expect, since resizing the terminal window is equivalent to
resizing the TTY frames on that terminal.

>  >    /* Add in menu bar lines, if any.  */
>  >    matrix_dim.height += top_window_y;
> 
> Doesn't this add an extra glyph row?

Yes (if menu-bar-mode is turned on; otherwise top_window_y is zero).
But, crucially, it does not update FrameRows for the frame's terminal.

>  >> Can I call adjust_frame_size directly from init_display?
>  >
>  > If all the rest is a no-op, yes, why not?
> 
> You mean it must not call Lisp?

No, I mean if everything else does nothing, in which case you call
adjust_frame_size already.

Maybe I don't understand what bothers you in this scenario that caused
you to ask that question.

>  >> IIUC FrameRows is the height of the terminal window and when I change
>  >> the height of that window I want to change the height of the Emacs frame
>  >> as well via handle_window_change_signal/change_frame_size.  This means I
>  >> can set FrameCols where I do it now because whenever I change the size
>  >> of the outer frame that of the frame's windows changes too.
>  >
>  > Sorry, you lost me here.  First, you use "window" in several
>  > overloaded meanings, or so it seems.  And second, why are we suddenly
>  > talking about SIGWINCH and its handling?  This is not the scenario in
>  > which this bug happens.
> 
> Because adjust_frame_size has to handle SIGWINCH as well.

OK, and...?

Both SIGWINCH and set-frame-size change the frame dimensions.  The
difference is that the former gets the new dimensions from
get_tty_size, which queries the terminal driver, while the latter gets
the dimensions from the caller.

>  >> Still it seems to me contrived to set FrameCols/FrameRows when adjusting
>  >> the sizes of a frame's windows.
>  >
>  > How else will FrameCols/FrameRows be updated if the user calls
>  > set-frame-size and its ilk?
> 
> I thought that FrameCols/FrameRows store the sizes of the terminal
> window.  IIUC this means that `set-frame-size' makes us lie about the
> terminal sizes.

FrameCols/FrameRows communicates the terminal size to cursor-motion
commands in cm.c.  When we want to use a frame smaller than the
terminal screen, we modify these values accordingly.

> Unfortunately, restoring the init_display change as you proposed earlier
> by simply doing
> 
> 
> === modified file 'src/dispnew.c'
> --- src/dispnew.c	2014-07-28 09:39:09 +0000
> +++ src/dispnew.c	2014-08-01 08:23:58 +0000
> @@ -6069,8 +6069,7 @@
>       t->display_info.tty->top_frame = selected_frame;
>       change_frame_size (XFRAME (selected_frame),
>                          FrameCols (t->display_info.tty),
> -                       FrameRows (t->display_info.tty)
> -		       - FRAME_MENU_BAR_LINES (f), 0, 0, 1, 0);
> +                       FrameRows (t->display_info.tty), 0, 0, 1, 0);
> 
>       /* Delete the initial terminal. */
>       if (--initial_terminal->reference_count == 0
> 
> 
> doesn't remove the cmcheckmagic abort here.  IUUC the problem is with
> the deliberate mixture of frame and terminal sizes when using cursor
> coordinates within term.c, like
> 
>            && curY (tty) == FrameRows (tty) - 1
> 
> and
> 
>        && curY (tty) + 1 == FRAME_LINES (f)
> 
> So far this can have worked only by some strange magic assuring that
> FRAME_LINES always returns the same value as FrameRows.

IMO, FRAME_LINES for the TTY frame that is currently displayed on its
terminal should always equal to FrameRows for that terminal (and
similarly for FrameCols).  I thought we previously agreed on this,
since a TTY frame usually behaves as a maximized frame.

>  >> Wouldn't it be principally cleaner if we set FrameCols and FrameRows
>  >> after calling get_tty_size only?
>  >
>  > You can't.  get_tty_size reports the _physical_ dimensions of the
>  > terminal screen, so it cannot support set-frame-size and its ilk,
>  > which leave the physical dimensions unaltered.
> 
> Does that mean `set-frame-size' should not set FrameCols/FrameRows?

No, it means the opposite: any change in dimensions of a TTY frame
should be mirrored in FrameCols/FrameRows.

> I'm still too silly to understand this: Please tell me whether FrameRows
> stands for the height of the terminal window as reported by get_tty_size
> or for the height of the frame as set by `set-frame-size'?

Neither.  FrameRows stands for the cm.c notion of the terminal's
height.  Its value can be modified either (1) by
handle_window_change_signal, which queries the terminal via
get_tty_size, or (2) by some Lisp that calls set-frame-size, which
should eventually call adjust_frame_size.