bug#16051: 24.3.50; Emacs hang - resize frame manually

[Eli Zaretskii]

> Date: Mon, 23 Dec 2013 19:44:33 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: jarekczek@poczta.onet.pl, 16051@debbugs.gnu.org
> 
> -  it.last_visible_x = FRAME_TOTAL_COLS (f) * FRAME_COLUMN_WIDTH (f);
> +  it.last_visible_x = WINDOW_PIXEL_WIDTH (w);
> 
> Wonderful.  The effect of this was visible in the Lucid/Motif builds but
> since I hadn't seen it in the Windows build I didn't expect to find the
> cause in the toolkit independent part of the code.  But is this related
> to the current bug?

Yes, definitely.  redisplay_tool_bar was using WINDOW_PIXEL_WIDTH,
while tool_bar_height was using FRAME_TOTAL_COLS (f) * FRAME_COLUMN_WIDTH (f)
so these two were inconsistent with each other, and when the former
claimed (correctly) that there's not enough space to draw the tool bar
in one row, tool_bar_height was (incorrectly) returning 1 as the
required number of rows.  And then you set the fonts_changed flag,
which starts the infloop...

> As an aside, I have never been able to understand the purpose of the
> tool-bar-lines parameter.  IIUC we are only supposed to read its value
> from Lisp but never to set it to anything but zero or one.  Am I right?

If you are right, then I'm confused: the call to
Fmodify_frame_parameters that changes the tool-bar-lines parameter
leads to a call to x_set_tool_bar_lines, which in turn resizes the
tool-bar window.  And the value is certainly not always 1, I've seen
it as large as 7.

Maybe you meant the n_tool_bar_rows member of 'struct frame' instead?
But that, too, is not always 1, it can be greater.  Confused...

>  > If there is some reasoning behind this "always do that" logic, please
>  > describe it.  Otherwise, I propose the patch below, which cures the
>  > problem altogether for me; if you agree, I will install it.
> 
> I certainly agree :-)

OK, installed as trunk revision 115720.

>  > As an aside, I stared for a long time at this part of
>  > w32fns.c:x_set_tool_bar_lines (which is part of the issue, because it
>  > is called by modify-frame-parameters, when the tool-bar-lines
>  > parameter is changed), and I still don't get why it does what it does:
>  >       root_height = WINDOW_PIXEL_HEIGHT (XWINDOW (FRAME_ROOT_WINDOW (f)));
>  >       if (root_height - delta < unit)
> 
> How would you know - it probably doesn't do anything reasonable.  When
> we enter this part we see only the upper part of the first row of the
> toolbar, the rest of the toolbar, root and minibuffer window being
> clipped by the window manager.

Actually, we enter there any time a change is requested for the
frame's tool-bar-lines parameter, and the change is so large it will
leave no space for the rest of the root window.

>  >    {
>  >      delta = root_height - unit;
>  >      nlines = (root_height / unit) + min (1, (root_height % unit));
>  >    }
>  >
>  > First, delta is recomputed, but the result is never used.  More
>  > importantly, you assign to nlines a value that is the size of the root
>  > window _plus_ one line?  Did you mean minus instead?
> 
> Probably.  If you have any good idea what to do here, please do it.

Well, the old code simply left at least one screen line to the window,
and if the tool bar asked for more than that, its request was not
honored:

  delta = nlines - FRAME_TOOL_BAR_LINES (f);

  /* Don't resize the tool-bar to more than we have room for.  */
  root_window = FRAME_ROOT_WINDOW (f);
  root_height = WINDOW_TOTAL_LINES (XWINDOW (root_window));
  if (root_height - delta < 1)
    {
      delta = root_height - 1;
      nlines = FRAME_TOOL_BAR_LINES (f) + delta;
    }

  FRAME_TOOL_BAR_LINES (f) = nlines;

Translation of this to pixels is straightforward, but it looks like
you wanted to do something different here?

> IIUC we want to pretend that the frame has the full toolbar (probably as
> many rows as it has items), a one line root window and, if it's present,
> a one line minibuffer window.  This should be robust enough to avoid a
> crash.

There's no crash anymore, at least not on Windows.  But doing what you
suggest above means we will need to resize the entire frame, something
we never did.

> But the underlying problem, namely that shrinking the width of the frame
> may mean that we'd have to enlarge its height, remains.  Currently, our
> internal toolbar gets nominally larger than the containing frame.

It gets larger and is displayed only partially.  I think this is
reasonable under these circumstances: if the user does dumb things,
which shouldn't we let her suffer?

>  > Finally, the corresponding xfns.c implementation still works in lines,
>  > not in pixels, as w32fns.c did before your pixelwise changes.  Is this
>  > disparity on purpose or an oversight?
> 
> When I installed my changes I tested this only with the Windows and the
> Gtk builds.  It's only now that I encountered problems with the Lucid
> and Motif builds.  I'm working on this currently.

OK, thanks.