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

[martin rudalics]

> It turns out this bug has 3 separate parts.  2 of them are very clear
> and uncontroversial, so I simply fixed them; see revision 115718 on
> the trunk.  After that, there are no more assertion violations like
> above, and no hangs, and it is much harder to cause a redisplay loop.

Thanks.

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

The second issue belongs in the category of things I expected to happen
hoping that sooner or later you would detect and fix them.  I would
certainly be able to spend some time studying your fix and still not
grasp it.

>      if (change_height_p)
>        {
>          int new_lines = ((new_height + FRAME_LINE_HEIGHT (f) - 1)
>                           / FRAME_LINE_HEIGHT (f));
>
>          XSETFRAME (frame, f);
>          Fmodify_frame_parameters (frame,
>                                    list1 (Fcons (Qtool_bar_lines,
>                                                  make_number (new_lines))));
>          /* Always do that now.  */
>          clear_glyph_matrix (w->desired_matrix);
>          f->n_tool_bar_rows = nrows;
>          f->fonts_changed = 1;
>          return 1;
>        }
>
> Why does it make sense to "always do that"?  Suppose new_lines is
> exactly equal to the current number of canonical lines used by the
> tool-bar window (it can happen even if pixel sizes are different, due
> to integer truncation).  Or suppose you are asking for N lines, but
> don't get it because the frame is too small.  Why set the
> fonts_changed flag in those cases?  That flag causes redisplay to give
> up, abandon the current glyph matrices, and retry anew.  What happens
> with those endless cycles is precisely that: redisplay_tool_bar sets
> the fonts_changed flag, which causes redisplay_internal to retry,
> which again calls redisplay_tool_bar, which again sets the flag,
> etc., ad nauseam.

"Always" referred to the glyph matrix clearing part.  I was completely
ignorant of the fact that setting the fonts_changed flag would cause
this to loop.  Admittedly, I normally don't use the toolbar and so have
not given it enough testing with more extreme behavior.

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

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

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

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.

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

martin