GNU bug report logs - #18136
24.4.50; crash in redisplay when calling load-theme

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 18136 in the body.
You can then email your comments to 18136 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mark Oteiza <mvoteiza <at> udel.edu>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.4.50; crash in redisplay when calling load-theme
Date: Mon, 28 Jul 2014 20:36:06 -0400

gdb emacs
run -nw -Q

M-x load-theme RET some-default-theme RET

(gdb) xbacktrace al (C function)" (0xbbeea0)
"redisplay_internal (C function)" (0xbbeea0)
(gdb) bt
#0  terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=40) at emacs.c:359
#1  0x00000000004fe2e3 in emacs_abort () at sysdep.c:2198
#2  0x00000000004a81c1 in cmcheckmagic (tty=0x13d82a0) at cm.c:120
#3  0x0000000000418225 in update_frame_line (f=f <at> entry=0xc0ab08, vpos=50) at dispnew.c:4839
#4  0x000000000041bb64 in update_frame_1 (f=f <at> entry=0xc0ab08, force_p=<optimized out>, force_p <at> entry=true, inhibit_id_p=inhibit_id_p <at> entry=false, 
    set_cursor_p=set_cursor_p <at> entry=true) at dispnew.c:4509
#5  0x000000000041d11e in update_frame (f=0xc0ab08, force_p=<optimized out>, inhibit_hairy_id_p=<optimized out>) at dispnew.c:3110
#6  0x000000000044fccf in redisplay_internal () at xdisp.c:13869
#7  0x00000000004510a5 in redisplay () at xdisp.c:13115
#8  0x00000000004ef679 in read_char (commandflag=1, map=map <at> entry=19890598, prev_event=12545906, used_mouse_menu=used_mouse_menu <at> entry=0x7fffffffdb0b, 
    end_time=end_time <at> entry=0x0) at keyboard.c:2560
#9  0x00000000004f0e05 in read_key_sequence (keybuf=keybuf <at> entry=0x7fffffffdbe0, prompt=12545906, dont_downcase_last=dont_downcase_last <at> entry=false, 
    can_return_switch_frame=can_return_switch_frame <at> entry=true, fix_current_buffer=fix_current_buffer <at> entry=true, prevent_redisplay=prevent_redisplay <at> entry=false, bufsize=30)
    at keyboard.c:9120
#10 0x00000000004f2bb0 in command_loop_1 () at keyboard.c:1438
#11 0x00000000005570a7 in internal_condition_case (bfun=bfun <at> entry=0x4f29b0 <command_loop_1>, handlers=<optimized out>, hfun=hfun <at> entry=0x4e9960 <cmd_error>) at eval.c:1347
#12 0x00000000004e4eae in command_loop_2 (ignore=ignore <at> entry=12545906) at keyboard.c:1169
#13 0x0000000000556f8b in internal_catch (tag=12593330, func=func <at> entry=0x4e4e90 <command_loop_2>, arg=12545906) at eval.c:1111
#14 0x00000000004e9577 in command_loop () at keyboard.c:1148
#15 recursive_edit_1 () at keyboard.c:769
#16 0x00000000004e9890 in Frecursive_edit () at keyboard.c:840
#17 0x00000000004140f1 in main (argc=0, argv=0x7fffffffdf48) at emacs.c:1650

Lisp Backtrace:
"redisplay_internal (C function)" (0xbbeea0)
(gdb)





In GNU Emacs 24.4.50.1 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
 of 2014-07-28 on holos
Configured using:
 `configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=lucid 'CFLAGS=-march=x86-64
 -mtune=generic -O2 -pipe -fstack-protector-strong
 --param=ssp-buffer-size=4' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  flycheck-mode: t
  company-mode: t
  winner-mode: t
  show-paren-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t

Recent input:
ESC [ > 8 4 ; 0 ; 0 c ESC x r e p o r t TAB RET

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
/usr/share/emacs/24.4.50/lisp/loaddefs hides /home/mvo/.emacs.d/site-lisp/loaddefs
/usr/share/emacs/24.4.50/lisp/env hides /home/mvo/.emacs.d/site-lisp/expand-region/features/support/env

Features:
(shadow sort gnus-util mail-extr emacsbug message idna dired format-spec
rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util help-fns mail-prsvr mail-utils xterm flycheck find-func
help-mode easymenu rx f dash s company-files company-oddmuse
company-keywords company-etags etags company-gtags company-dabbrev-code
company-dabbrev company-capf company-cmake company-ropemacs
company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb company pcase
easy-mmode cl-macs gv package windmove edmacro kmacro cl-loaddefs cl-lib
saveplace winner ring time-date paren tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer 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 make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting x-toolkit x multi-tty emacs)

Memory information:
((conses 16 126325 4289)
 (symbols 48 21390 0)
 (miscs 40 52 121)
 (strings 32 22506 4247)
 (string-bytes 1 610204)
 (vectors 16 10940)
 (vector-slots 8 372009 5422)
 (floats 8 89 369)
 (intervals 56 212 0)
 (buffers 976 11)
 (heap 1024 29754 891))

Message #8 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mark Oteiza <mvoteiza <at> udel.edu>, Martin Rudalics <rudalics <at> gmx.at>
Cc: 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 12:05:11 +0300

> From: Mark Oteiza <mvoteiza <at> udel.edu>
> Date: Mon, 28 Jul 2014 20:36:06 -0400
> 
> 
> gdb emacs
> run -nw -Q
> 
> M-x load-theme RET some-default-theme RET
> 
> (gdb) xbacktrace al (C function)" (0xbbeea0)
> "redisplay_internal (C function)" (0xbbeea0)
> (gdb) bt
> #0  terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=40) at emacs.c:359
> #1  0x00000000004fe2e3 in emacs_abort () at sysdep.c:2198
> #2  0x00000000004a81c1 in cmcheckmagic (tty=0x13d82a0) at cm.c:120
> #3  0x0000000000418225 in update_frame_line (f=f <at> entry=0xc0ab08, vpos=50) at dispnew.c:4839

Martin, this is because of this change in the call to
change_frame_size by init_display:

  @@ -6171,7 +6069,8 @@ init_display (void)
       t->display_info.tty->top_frame = selected_frame;
       change_frame_size (XFRAME (selected_frame),
			  FrameCols (t->display_info.tty),
  -                       FrameRows (t->display_info.tty), 0, 0, 1, 0);
  +                       FrameRows (t->display_info.tty)
  +                      - FRAME_MENU_BAR_LINES (f), 0, 0, 1, 0);

change_frame_size_1 then calls adjust_frame_size with the value one
less than the terminal height, and adjust_frame_size plugs that value
into FrameRows:

      if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
	FrameRows (FRAME_TTY (f)) = new_lines;

This lies to Emacs about the terminal height, and cmcheckmagic catches
that.

I don't understand why you subtract FRAME_MENU_BAR_LINES, that sounds
wrong at least for TTY frames.  (We could add that back inside
adjust_frame_size, but it's IMO a bad idea to spread this non-trivial
logic between 2 functions.)

Message #11 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>, Mark Oteiza <mvoteiza <at> udel.edu>
Cc: 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 12:44:22 +0200

>> (gdb) xbacktrace al (C function)" (0xbbeea0)
>> "redisplay_internal (C function)" (0xbbeea0)
>> (gdb) bt
>> #0  terminate_due_to_signal (sig=sig <at> entry=6, backtrace_limit=backtrace_limit <at> entry=40) at emacs.c:359
>> #1  0x00000000004fe2e3 in emacs_abort () at sysdep.c:2198
>> #2  0x00000000004a81c1 in cmcheckmagic (tty=0x13d82a0) at cm.c:120
>> #3  0x0000000000418225 in update_frame_line (f=f <at> entry=0xc0ab08, vpos=50) at dispnew.c:4839
>
> Martin, this is because of this change in the call to
> change_frame_size by init_display:
>
>    @@ -6171,7 +6069,8 @@ init_display (void)
>         t->display_info.tty->top_frame = selected_frame;
>         change_frame_size (XFRAME (selected_frame),
> 			  FrameCols (t->display_info.tty),
>    -                       FrameRows (t->display_info.tty), 0, 0, 1, 0);
>    +                       FrameRows (t->display_info.tty)
>    +                      - FRAME_MENU_BAR_LINES (f), 0, 0, 1, 0);
>
> change_frame_size_1 then calls adjust_frame_size with the value one
> less than the terminal height, and adjust_frame_size plugs that value
> into FrameRows:
>
>        if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
> 	FrameRows (FRAME_TTY (f)) = new_lines;
>
> This lies to Emacs about the terminal height, and cmcheckmagic catches
> that.
>
> I don't understand why you subtract FRAME_MENU_BAR_LINES, that sounds
> wrong at least for TTY frames.  (We could add that back inside
> adjust_frame_size, but it's IMO a bad idea to spread this non-trivial
> logic between 2 functions.)

All functions like change_frame_size or adjust_frame_size now expect the
frame's text height as argument and menu and tool bars should not be
part of it.  I was expecting some breakage of this approach for TTY
frames because I didn't fully understand the logic of how frames get
assigned sizes.  Probably this assignment

      if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
	FrameCols (FRAME_TTY (f)) = new_cols;

is completely misplaced and should be either removed or inhibited when
called from change_frame_size_1, that is when INHIBIT equals 5.  Can you
tell me what this assignment is for?

martin

Message #14 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 15:12:50 +0300

> Date: Tue, 29 Jul 2014 12:44:22 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: 18136 <at> debbugs.gnu.org
> 
>  > I don't understand why you subtract FRAME_MENU_BAR_LINES, that sounds
>  > wrong at least for TTY frames.  (We could add that back inside
>  > adjust_frame_size, but it's IMO a bad idea to spread this non-trivial
>  > logic between 2 functions.)
> 
> All functions like change_frame_size or adjust_frame_size now expect the
> frame's text height as argument and menu and tool bars should not be
> part of it.

Why does it make sense to do that for TTY frames?  The terminal screen
cannot be resized from within Emacs, so the arguments for treating the
menu bar as an add-on are not really valid in this case.

> I was expecting some breakage of this approach for TTY frames
> because I didn't fully understand the logic of how frames get
> assigned sizes.

Please ask questions about what you don't understand.  Having just
completed a debugging session for bug #18112, which was all about
assignment of TTY frame sizes, I think I can explain at least some of
that.

> Probably this assignment
> 
>        if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
> 	FrameCols (FRAME_TTY (f)) = new_cols;
> 
> is completely misplaced and should be either removed or inhibited when
> called from change_frame_size_1, that is when INHIBIT equals 5.  Can you
> tell me what this assignment is for?

It cannot be removed or inhibited.  It was introduced to fix a bug
(#17875).  The problem is that different TTY frames on the same
terminal can potentially have different dimensions, and OTOH FrameCols
and FrameRows are "normally" set only at terminal initiation and in
response to a SIGWINCH signal.  These assignments take care of keeping
FrameCols and FrameRows in sync with frame dimensions in all other
cases, because they all go through change_frame_size.

Message #17 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 16:02:13 +0200

> Why does it make sense to do that for TTY frames?  The terminal screen
> cannot be resized from within Emacs, so the arguments for treating the
> menu bar as an add-on are not really valid in this case.

The idea is that `frame-height' should have the same semantics on all
platforms.  If you think we can ignore this difference for TTY frames
I'm obviously OK with it.

> Please ask questions about what you don't understand.  Having just
> completed a debugging session for bug #18112, which was all about
> assignment of TTY frame sizes, I think I can explain at least some of
> that.
>
>> Probably this assignment
>>
>>         if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
>> 	FrameCols (FRAME_TTY (f)) = new_cols;
>>
>> is completely misplaced and should be either removed or inhibited when
>> called from change_frame_size_1, that is when INHIBIT equals 5.  Can you
>> tell me what this assignment is for?
>
> It cannot be removed or inhibited.

Inhibited exclusively for the case that this function is called from
change_frame_size (that is when INHIBIT equals 5).

> It was introduced to fix a bug
> (#17875).  The problem is that different TTY frames on the same
> terminal can potentially have different dimensions, and OTOH FrameCols
> and FrameRows are "normally" set only at terminal initiation and in
> response to a SIGWINCH signal.  These assignments take care of keeping
> FrameCols and FrameRows in sync with frame dimensions in all other
> cases, because they all go through change_frame_size.

Which means FrameCols and FrameRows always have the correct values when
entering adjust_frame_size and we shouldn't change them there.  Or am I
missing something?

martin

Message #20 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 17:47:47 +0300

> Date: Tue, 29 Jul 2014 16:02:13 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  > Why does it make sense to do that for TTY frames?  The terminal screen
>  > cannot be resized from within Emacs, so the arguments for treating the
>  > menu bar as an add-on are not really valid in this case.
> 
> The idea is that `frame-height' should have the same semantics on all
> platforms.  If you think we can ignore this difference for TTY frames
> I'm obviously OK with it.

I don't have strong feelings about it, and will probably adapt if we
stay with this semantics.  But it feels strange, as on a TTY the menu
bar is a line just like any other line, and when the menu bar is not
displayed, I'd expect that line to be used for text.

E.g., with your suggested semantics, what would you expect from this:

  emacs -Q
  M-: (frame-height) RET
  M-x menu-bar-mode RET
  M-: (frame-height) RET

Would you expect to see the 2 results of frame-height identical or
different?

The current trunk displays 2 identical results, and actually resizes
the root window to be consistent with that, so that there's an unused
empty line below the echo area.  Is that intended?  It looks like a
misfeature to me.

>  >> Probably this assignment
>  >>
>  >>         if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
>  >> 	FrameCols (FRAME_TTY (f)) = new_cols;
>  >>
>  >> is completely misplaced and should be either removed or inhibited when
>  >> called from change_frame_size_1, that is when INHIBIT equals 5.  Can you
>  >> tell me what this assignment is for?
>  >
>  > It cannot be removed or inhibited.
> 
> Inhibited exclusively for the case that this function is called from
> change_frame_size (that is when INHIBIT equals 5).

You can't: this case is _precisely_ the case where we do need to
update FrameCols and FrameRows.

>  > It was introduced to fix a bug
>  > (#17875).  The problem is that different TTY frames on the same
>  > terminal can potentially have different dimensions, and OTOH FrameCols
>  > and FrameRows are "normally" set only at terminal initiation and in
>  > response to a SIGWINCH signal.  These assignments take care of keeping
>  > FrameCols and FrameRows in sync with frame dimensions in all other
>  > cases, because they all go through change_frame_size.
> 
> Which means FrameCols and FrameRows always have the correct values when
> entering adjust_frame_size

No, they don't, at least not necessarily.  If they did, what would be
the point of adding that to fix the bug?

Again, FrameRows and FrameCols updates are triggered in 3 possible
ways:

  . when the terminal is created

  . when we get SIGWINCH

  . when we call change_frame_size

The last one was missing, which caused bug #17875, whereby switching
to a different frame on the same terminal failed to update FrameRows
and FrameCols, because neither of the first 2 triggers happened.

Message #23 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 17:41:56 +0200

> I don't have strong feelings about it, and will probably adapt if we
> stay with this semantics.  But it feels strange, as on a TTY the menu
> bar is a line just like any other line, and when the menu bar is not
> displayed, I'd expect that line to be used for text.

The one use case I can think of is the following: Someone tries to do
something special if a specific frame is not as high as needed for
displaying some sort of text.  In this case it would be nice to have a
uniform behavior.  But the point is rather moot since the object of
reference in this regard is the window and the frame height also counts
in the minibuffer and a modeline.  So I have no strong feelings about
this either.

Note that this was an attempt to make the various toolkits behave more
similar.  But so far I failed in a number of aspects.  For example, I
was not able to keep the frame height constant when adding/removing the
menubar in fullheight mode on a number of GUI builds.

> E.g., with your suggested semantics, what would you expect from this:
>
>    emacs -Q
>    M-: (frame-height) RET
>    M-x menu-bar-mode RET
>    M-: (frame-height) RET
>
> Would you expect to see the 2 results of frame-height identical or
> different?

Ideally different in fullscreen/maximized/fullheight mode or with
`frame-inhibit-implied-resize' non-nil, identical otherwise.

> Again, FrameRows and FrameCols updates are triggered in 3 possible
> ways:
>
>    . when the terminal is created
>
>    . when we get SIGWINCH
>
>    . when we call change_frame_size
>
> The last one was missing, which caused bug #17875, whereby switching
> to a different frame on the same terminal failed to update FrameRows
> and FrameCols, because neither of the first 2 triggers happened.

My bad.  For some reason I thought these were set in change_frame_size.
Is calling change_frame_size necessary when switching frames?  What a
strange thing to do.

martin

Message #26 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 19:31:21 +0300

> Date: Tue, 29 Jul 2014 17:41:56 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  > E.g., with your suggested semantics, what would you expect from this:
>  >
>  >    emacs -Q
>  >    M-: (frame-height) RET
>  >    M-x menu-bar-mode RET
>  >    M-: (frame-height) RET
>  >
>  > Would you expect to see the 2 results of frame-height identical or
>  > different?
> 
> Ideally different in fullscreen/maximized/fullheight mode or with
> `frame-inhibit-implied-resize' non-nil, identical otherwise.

Shouldn't TTY frames behave as if they were fullscreen?  That's what
they (normally) are, right?

>  > Again, FrameRows and FrameCols updates are triggered in 3 possible
>  > ways:
>  >
>  >    . when the terminal is created
>  >
>  >    . when we get SIGWINCH
>  >
>  >    . when we call change_frame_size
>  >
>  > The last one was missing, which caused bug #17875, whereby switching
>  > to a different frame on the same terminal failed to update FrameRows
>  > and FrameCols, because neither of the first 2 triggers happened.
> 
> My bad.  For some reason I thought these were set in change_frame_size.
> Is calling change_frame_size necessary when switching frames?  What a
> strange thing to do.

No, my bad, sorry.  I confused this code with a similar one on
do_switch_frame, which was added due to bug #17875.  Obviously,
do_switch_frame _is_ called when we switch frames.

The code in change_frame_size_1 we are talking about was there since a
very long time (I see it in Emacs 21), and its purpose is to update
FrameRows and FrameCols when the user changes dimensions of a TTY
frame (e.g., by calling set-frame-height).  If you remove it, how can
we update those attributes otherwise?

Message #29 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 20:23:50 +0200

> Shouldn't TTY frames behave as if they were fullscreen?  That's what
> they (normally) are, right?

I think so, especially because that's how they behaved before.

> The code in change_frame_size_1 we are talking about was there since a
> very long time (I see it in Emacs 21), and its purpose is to update
> FrameRows and FrameCols when the user changes dimensions of a TTY
> frame (e.g., by calling set-frame-height).  If you remove it, how can
> we update those attributes otherwise?

I wonder why this is done in this part of the code.  Here we resize the
windows of a frame but the frame's sizes remain unchanged.

martin

Message #32 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Tue, 29 Jul 2014 21:29:13 +0300

> Date: Tue, 29 Jul 2014 20:23:50 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  > Shouldn't TTY frames behave as if they were fullscreen?  That's what
>  > they (normally) are, right?
> 
> I think so, especially because that's how they behaved before.

If so, we agree, and this means the menu bar should be part of the TTY
frame's dimensions.

>  > The code in change_frame_size_1 we are talking about was there since a
>  > very long time (I see it in Emacs 21), and its purpose is to update
>  > FrameRows and FrameCols when the user changes dimensions of a TTY
>  > frame (e.g., by calling set-frame-height).  If you remove it, how can
>  > we update those attributes otherwise?
> 
> I wonder why this is done in this part of the code.  Here we resize the
> windows of a frame but the frame's sizes remain unchanged.

No, we resize the frame and then redistribute the frame dimensions
between its windows.  When change_frame_size_1 is called with the same
dimensions as the current frame's dimensions, it simply does nothing
and returns.

Message #35 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Wed, 30 Jul 2014 18:45:02 +0200

>> I think so, especially because that's how they behaved before.
>
> If so, we agree, and this means the menu bar should be part of the TTY
> frame's dimensions.

Just that for a GUI frame the menubar is never counted in the text
height.

> No, we resize the frame and then redistribute the frame dimensions
> between its windows.  When change_frame_size_1 is called with the same
> dimensions as the current frame's dimensions, it simply does nothing
> and returns.

change_frame_size_1 _always_ calls adjust_frame_size now.  And the later
does (almost) nothing only if the following condition holds:

  if (new_text_width == old_text_width
      && new_text_height == old_text_height
      && new_windows_width == old_windows_width
      && new_windows_height == old_windows_height
      && new_pixel_width == old_pixel_width
      && new_pixel_height == old_pixel_height)
    /* No change.  Sanitize window sizes and return.  */

martin

Message #38 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Wed, 30 Jul 2014 20:18:16 +0300

> Date: Wed, 30 Jul 2014 18:45:02 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  >> I think so, especially because that's how they behaved before.
>  >
>  > If so, we agree, and this means the menu bar should be part of the TTY
>  > frame's dimensions.
> 
> Just that for a GUI frame the menubar is never counted in the text
> height.

Yes, I agree.

>  > No, we resize the frame and then redistribute the frame dimensions
>  > between its windows.  When change_frame_size_1 is called with the same
>  > dimensions as the current frame's dimensions, it simply does nothing
>  > and returns.
> 
> change_frame_size_1 _always_ calls adjust_frame_size now.  And the later
> does (almost) nothing only if the following condition holds:
> 
>    if (new_text_width == old_text_width
>        && new_text_height == old_text_height
>        && new_windows_width == old_windows_width
>        && new_windows_height == old_windows_height
>        && new_pixel_width == old_pixel_width
>        && new_pixel_height == old_pixel_height)
>      /* No change.  Sanitize window sizes and return.  */

OK, but that's the moral equivalent of what I described (based on what
the code did previously).  Right?

Message #41 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Wed, 30 Jul 2014 19:36:00 +0200

>>   > No, we resize the frame and then redistribute the frame dimensions
>>   > between its windows.  When change_frame_size_1 is called with the same
>>   > dimensions as the current frame's dimensions, it simply does nothing
>>   > and returns.
>>
>> change_frame_size_1 _always_ calls adjust_frame_size now.  And the later
>> does (almost) nothing only if the following condition holds:
>>
>>     if (new_text_width == old_text_width
>>         && new_text_height == old_text_height
>>         && new_windows_width == old_windows_width
>>         && new_windows_height == old_windows_height
>>         && new_pixel_width == old_pixel_width
>>         && new_pixel_height == old_pixel_height)
>>       /* No change.  Sanitize window sizes and return.  */
>
> OK, but that's the moral equivalent of what I described (based on what
> the code did previously).  Right?

I'm not good in morals but if I remove the menubar and the "frame
dimensions" remain the same, the above conjunct does not hold because the
new text height is larger than the old one and the new windows height too.

martin

Message #44 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Wed, 30 Jul 2014 20:52:47 +0300

> Date: Wed, 30 Jul 2014 19:36:00 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  >>   > No, we resize the frame and then redistribute the frame dimensions
>  >>   > between its windows.  When change_frame_size_1 is called with the same
>  >>   > dimensions as the current frame's dimensions, it simply does nothing
>  >>   > and returns.
>  >>
>  >> change_frame_size_1 _always_ calls adjust_frame_size now.  And the later
>  >> does (almost) nothing only if the following condition holds:
>  >>
>  >>     if (new_text_width == old_text_width
>  >>         && new_text_height == old_text_height
>  >>         && new_windows_width == old_windows_width
>  >>         && new_windows_height == old_windows_height
>  >>         && new_pixel_width == old_pixel_width
>  >>         && new_pixel_height == old_pixel_height)
>  >>       /* No change.  Sanitize window sizes and return.  */
>  >
>  > OK, but that's the moral equivalent of what I described (based on what
>  > the code did previously).  Right?
> 
> I'm not good in morals but if I remove the menubar and the "frame
> dimensions" remain the same, the above conjunct does not hold because the
> new text height is larger than the old one and the new windows height too.

I thought we agreed that TTY frames should include the menu bar in
their height, and therefore change_frame_size should not have its 3rd
argument decreased by FRAME_MENU_BAR_LINES for TTY frames.

Message #47 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Thu, 31 Jul 2014 10:49:11 +0200

> I thought we agreed that TTY frames should include the menu bar in
> their height, and therefore change_frame_size should not have its 3rd
> argument decreased by FRAME_MENU_BAR_LINES for TTY frames.

I'm afraid that doing so might run into problems in adjust_frame_size.
Basically, the flow of things for turning on/off menubars should be as
follows where "outer frame" stands for what Windows calls the client
area and what on a TTY would stand for the screen space within a
terminal window allotted to Emacs.

(1) The user decides to enable/disable the menubar.  The request is
    passed to adjust_frame_size.

(2) adjust_frame_size (or better, frame_inhibit_resize) decides that
    we're on TTY and therefore the outer frame should _not_ be resized.
    Thus inhibit_horizontal and inhibit_vertical should be both true
    after frame_inhibit_resize has been called.  This means that
    removing the menu bar must be handled internally by calling
    resize_frame_windows to either enlarge or shrink the root window by
    the menubar line.

But I don't really understand about `set-frame-height' and friends on a
TTY and what they are supposed to do.  Should these be allowed to change
the frame size?  If not we should assure that inhibit_horizontal and
inhibit_vertical are always true (regardless of the INHIBIT argument) on
TTYs.

Now about the

    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);

call in init_display.  What precisely is this supposed to accomplish?
Note one thing: change_frame_size is just a relay to detect whether
applying (Emacs-)window changes and running Lisp is safe in the current
state.  If it is not, it simply waits.  Otherwise, it passes all
information to adjust_frame_size with INHIBIT set to 5 which means that
the size of the outer frame has been set already and adjust_frame_size
should only care about how to handle these sizes internally by resizing
(Emacs-)windows appropriately.

Now what does init_display?  IIUC it just gets the height and width
values from t->display_info.tty and passes them on to change_frame_size.
That is, the outer frame sizes are set and adjust_frame_size has only to
deal with resizing the (Emacs-)windows corectly.  This is done by
setting the new height of these windows from the height of the outer
frame via

  new_windows_height = (new_pixel_height
			- FRAME_TOP_MARGIN_HEIGHT (f)
			- 2 * FRAME_INTERNAL_BORDER_WIDTH (f));

where new_pixel_height was earlier calculated from new_text_height as

  new_pixel_height = ((inhibit_vertical && (inhibit < 5))
		      ? old_pixel_height
		      : max (FRAME_TEXT_TO_PIXEL_HEIGHT (f, new_text_height),
			     min_windows_height
			     + FRAME_TOP_MARGIN_HEIGHT (f)
			     + 2 * FRAME_INTERNAL_BORDER_WIDTH (f)));

where new_text_height is the normalized value passed to
adjust_frame_size via the HEIGHT argument.  This sounds contrived (why
don't I call adjust_frame_size directly with the size of the outer
frame?) but note that adjust_frame_size is more often called from inside
Emacs and there we don't care about the outer frame (on GTK we certainly
don't).

Back to init_display and the question I asked earlier: Why should the
subsequent adjust_frame_size call do

      if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
	FrameRows (FRAME_TTY (f)) = new_lines;

here?  IIUC this _has_ already been set correctly when calling
change_frame_size and so should not be set here any more.  Agreed?

Now please tell me one thing: Which calls of change_frame_size or
adjust_frame_size _should_ set FrameRows or FrameCols and how can
I distinguish them?

martin

Message #50 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Thu, 31 Jul 2014 13:52:40 +0300

> Date: Thu, 31 Jul 2014 10:49:11 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
> But I don't really understand about `set-frame-height' and friends on a
> TTY and what they are supposed to do.  Should these be allowed to change
> the frame size?

Yes.

> Now about the
> 
>      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);
> 
> call in init_display.  What precisely is this supposed to accomplish?

Allocate the glyph matrices, at the very least, I guess.

> Back to init_display and the question I asked earlier: Why should the
> subsequent adjust_frame_size call do
> 
>        if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
> 	FrameRows (FRAME_TTY (f)) = new_lines;
> 
> here?

Maybe it shouldn't, when called from init_display.  But we should at
least leave an eassert there, in case we overlook something.

In any case, this begs the question: why do you at all call
adjust_frame_size in this case, if the frame already has the required
size?  I think the answer is that adjust_frame_size does something
else: it calls adjust_frame_glyphs.  That call is required at
init_display time for obvious reasons, but it is inside
adjust_frame_size which is only called when the frame size changes,
which sounds like a contradiction in the design.

> Now please tell me one thing: Which calls of change_frame_size or
> adjust_frame_size _should_ set FrameRows or FrameCols

Any calls that actually change the frame size.

> and how can I distinguish them?

You already do, if I understand your description.  You already avoid
calling adjust_frame_size unless the size really changes.

Message #53 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Thu, 31 Jul 2014 18:53:33 +0200

>> But I don't really understand about `set-frame-height' and friends on a
>> TTY and what they are supposed to do.  Should these be allowed to change
>> the frame size?
>
> Yes.

In the sense that the frame can get larger or smaller than the terminal
window?  So we internally work with say 100 frame lines although the
terminal can display only 80?  Or am I missing something?

>> Now about the
>>
>>       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);
>>
>> call in init_display.  What precisely is this supposed to accomplish?
>
> Allocate the glyph matrices, at the very least, I guess.

OK.  BTW doesn't adjust_frame_glyphs_for_frame_redisplay count the
menubar specially?  What is

  top_window_y = FRAME_TOP_MARGIN (f);

for?

>> Back to init_display and the question I asked earlier: Why should the
>> subsequent adjust_frame_size call do
>>
>>         if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
>> 	FrameRows (FRAME_TTY (f)) = new_lines;
>>
>> here?
>
> Maybe it shouldn't, when called from init_display.  But we should at
> least leave an eassert there, in case we overlook something.

Can I call adjust_frame_size directly from init_display?

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.

Still it seems to me contrived to set FrameCols/FrameRows when adjusting
the sizes of a frame's windows.  And setting FrameCols when called from
init_display is certainly not TRT IMHO.

> In any case, this begs the question: why do you at all call
> adjust_frame_size in this case, if the frame already has the required
> size?  I think the answer is that adjust_frame_size does something
> else: it calls adjust_frame_glyphs.  That call is required at
> init_display time for obvious reasons, but it is inside
> adjust_frame_size which is only called when the frame size changes,
> which sounds like a contradiction in the design.

Think of turning off/on the menubar of a maximized frame.  In this case
I do not change the size of the frame either.  Yet I have to call
adjust_frame_glyphs.  BTW in an earlier mail you said that

> E.g., with your suggested semantics, what would you expect from this:
>
>   emacs -Q
>   M-: (frame-height) RET
>   M-x menu-bar-mode RET
>   M-: (frame-height) RET
>
> Would you expect to see the 2 results of frame-height identical or
> different?
>
> The current trunk displays 2 identical results, and actually resizes
> the root window to be consistent with that, so that there's an unused
> empty line below the echo area.  Is that intended?  It looks like a
> misfeature to me.

Where and how did you get that?  I can't reproduce it here.

>> Now please tell me one thing: Which calls of change_frame_size or
>> adjust_frame_size _should_ set FrameRows or FrameCols
>
> Any calls that actually change the frame size.

Is turning off/on the TTY menubar one of them?  I think not.  Yet we set
FrameCols.  Wouldn't it be principally cleaner if we set FrameCols and
FrameRows after calling get_tty_size only?

martin

Message #56 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Thu, 31 Jul 2014 20:55:11 +0300

> Date: Thu, 31 Jul 2014 18:53:33 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  >> But I don't really understand about `set-frame-height' and friends on a
>  >> TTY and what they are supposed to do.  Should these be allowed to change
>  >> the frame size?
>  >
>  > Yes.
> 
> In the sense that the frame can get larger or smaller than the terminal
> window?

Yes.

> So we internally work with say 100 frame lines although the
> terminal can display only 80?

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.

>  >> Now about the
>  >>
>  >>       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);
>  >>
>  >> call in init_display.  What precisely is this supposed to accomplish?
>  >
>  > Allocate the glyph matrices, at the very least, I guess.
> 
> OK.  BTW doesn't adjust_frame_glyphs_for_frame_redisplay count the
> menubar specially?

It knows about it, yes:

  /* Add in menu bar lines, if any.  */
  matrix_dim.height += top_window_y;

>  >> Back to init_display and the question I asked earlier: Why should the
>  >> subsequent adjust_frame_size call do
>  >>
>  >>         if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
>  >> 	FrameRows (FRAME_TTY (f)) = new_lines;
>  >>
>  >> here?
>  >
>  > Maybe it shouldn't, when called from init_display.  But we should at
>  > least leave an eassert there, in case we overlook something.
> 
> Can I call adjust_frame_size directly from init_display?

If all the rest is a no-op, yes, why not?

> 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.

> 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?

> And setting FrameCols when called from init_display is certainly not
> TRT IMHO.

If you are sure they are already set by then, OK.  Evidently, in this
case, the call to change_frame_size tried to decrease the frame size
by one line, so something is still out of sync somewhere.

>  > In any case, this begs the question: why do you at all call
>  > adjust_frame_size in this case, if the frame already has the required
>  > size?  I think the answer is that adjust_frame_size does something
>  > else: it calls adjust_frame_glyphs.  That call is required at
>  > init_display time for obvious reasons, but it is inside
>  > adjust_frame_size which is only called when the frame size changes,
>  > which sounds like a contradiction in the design.
> 
> Think of turning off/on the menubar of a maximized frame.  In this case
> I do not change the size of the frame either.  Yet I have to call
> adjust_frame_glyphs.

Is that supposed to be the answer to my question, or just say what I
said in other words?

> BTW in an earlier mail you said that
> 
>  > E.g., with your suggested semantics, what would you expect from this:
>  >
>  >   emacs -Q
>  >   M-: (frame-height) RET
>  >   M-x menu-bar-mode RET
>  >   M-: (frame-height) RET
>  >
>  > Would you expect to see the 2 results of frame-height identical or
>  > different?
>  >
>  > The current trunk displays 2 identical results, and actually resizes
>  > the root window to be consistent with that, so that there's an unused
>  > empty line below the echo area.  Is that intended?  It looks like a
>  > misfeature to me.
> 
> Where and how did you get that?  I can't reproduce it here.

I can reproduce it with the current trunk on GNU/Linux where I invoke
"emacs -Q -nw" via PuTTY.  The resize is _after_ I invoke frame-height
the second time, which is already the sign of a problem.

>  >> Now please tell me one thing: Which calls of change_frame_size or
>  >> adjust_frame_size _should_ set FrameRows or FrameCols
>  >
>  > Any calls that actually change the frame size.
> 
> Is turning off/on the TTY menubar one of them?

No.

> 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.

Message #59 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Fri, 01 Aug 2014 10:57:06 +0200

> 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.

>    /* Add in menu bar lines, if any.  */
>    matrix_dim.height += top_window_y;

Doesn't this add an extra glyph row?

>> 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?

>> 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.

>> 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.

>> And setting FrameCols when called from init_display is certainly not
>> TRT IMHO.
>
> If you are sure they are already set by then, OK.  Evidently, in this
> case, the call to change_frame_size tried to decrease the frame size
> by one line, so something is still out of sync somewhere.

Obviously.

>>   > In any case, this begs the question: why do you at all call
>>   > adjust_frame_size in this case, if the frame already has the required
>>   > size?  I think the answer is that adjust_frame_size does something
>>   > else: it calls adjust_frame_glyphs.  That call is required at
>>   > init_display time for obvious reasons, but it is inside
>>   > adjust_frame_size which is only called when the frame size changes,
>>   > which sounds like a contradiction in the design.
>>
>> Think of turning off/on the menubar of a maximized frame.  In this case
>> I do not change the size of the frame either.  Yet I have to call
>> adjust_frame_glyphs.
>
> Is that supposed to be the answer to my question, or just say what I
> said in other words?

A complement to what you said.

> I can reproduce it with the current trunk on GNU/Linux where I invoke
> "emacs -Q -nw" via PuTTY.  The resize is _after_ I invoke frame-height
> the second time, which is already the sign of a problem.

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.

>> 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?

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'?

martin

Message #62 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: martin rudalics <rudalics <at> gmx.at>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Fri, 01 Aug 2014 15:55:10 +0300

> Date: Fri, 01 Aug 2014 10:57:06 +0200
> From: martin rudalics <rudalics <at> gmx.at>
> CC: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
> 
>  > 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.

Message #65 received at 18136 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mvoteiza <at> udel.edu, 18136 <at> debbugs.gnu.org
Subject: Re: bug#18136: 24.4.50; crash in redisplay when calling load-theme
Date: Mon, 04 Aug 2014 19:23:05 +0200

> 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.

We agreed that the total (pixel) height of the frame should not change
and the frame's windows should get resized when the menubar is turned
off and on.  But the problem is that you want that the frame's text
height should not change either and this is much more difficult to
achieve.

In principle, I have to lie about the text height, for example, in this
part of frame.c

  new_text_height = FRAME_PIXEL_TO_TEXT_HEIGHT (f, new_pixel_height);

and I'm not yet sure how to do that :-(

IIUC it amounts to changing FRAME_PIXEL_TO_TEXT_HEIGHT and
FRAME_TEXT_TO_PIXEL_HEIGHT for TTYs but I'm afraid that that is not
sufficient.  In any case, the change will be substantial.

martin

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.