bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize

[Pip Cet <pipcet@gmail.com>]

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

On Fri, Aug 28, 2015 at 10:02 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > Date: Thu, 27 Aug 2015 20:49:17 +0000
> > From: Pip Cet <pipcet@gmail.com>
> > Cc: rudalics@gmx.at, 21333@debbugs.gnu.org
> > I think of redisplay cycles as beginning when redisplay() is called
> > and ending when it returns, so in my terminology the "next"
> > redisplay cycle is the one that will happen after control briefly
> > (or not so briefly, see below) returns to the command loop and
> > redisplay() is called again.
>
> That's my interpretation as well.


See below. The "On further thought" addendum brings us into agreement. It
describes the case I'm actually seeing and testing with here (not just some
contrived case that I can trigger with bizarre elisp code), and it makes
some of the other statements you made incorrect

 We
> keep saying the same things about the higher levels, and for me it's
> clear that our argument is about lower-level details, not about the
> high-level overall picture.

But you for some reason think we disagree
> on that higher level.
>

Again, I think your "On further thought" addendum settles that disagreement.


> Upon re-reading the thread, it's possible that the misunderstanding is
> due to the following factors:
>
>  . it's not always clear whether we are talking about what would
>    happen _after_ resize_mini_window is changed to raise the frame's
>    "windows changed" flag, or about the current code where it doesn't;
>

I agree that is often unclear.


>  . both window-size-change-functions and pre-redisplay-function is
>    being discussed, although it should be clear their purpose is
>    different,


What's the huge difference? As far as I can tell:
pre-redisplay-function: "I'm about to redisplay these windows, and they
might have changed size or content"
window-size-change-functions: "I'm about to redisplay windows on this
frame, and some of them might have changed size"

For many purposes, that's interchangeable. So while the intended uses might
be different, many applications will be happen to use either one, or a
different hook.

and in particular pre-redisplay-function cannot be moved
>    from where it currently lives, at least not significantly: it must
>    run _before_ the bulk of the redisplay code runs, or else it will
>    fail to fulfill its contract
>

See below.


>  . you introduce issues into the discussion that are not directly
>    related to it, which just muddies the waters, for example:
>

I agree we should drop this case. (I mentioned it because it actually
happened to me while I was testing, in more than 1% of cases, so it's not a
totally separate issue).

(As for the general problem, I accept that point of criticism and will try
to stay more focused.)

>
> >     So if
> >     resize_mini_window, however it is called, sets the flag that windows
> >     has been resized, only the next redisplay cycle will notice that and
> >     call the window-size-change-functions.
> >
> > But here you're using "next redisplay cycle" to mean the one that has
> not been
> > started (redisplay() hasn't been called yet), not the one we're
> currently in.
>
> No, I mean the next one.


I'm confused. Do you mean the cycle for which redisplay() has been called
but hasn't returned, the cycle corresponding to what will be the next call
to redisplay(), or some other cycle?

Did you mean to say "Yes, I mean the next one"?


I think our problems are partially due to exchanges like this one:


> > I had assumed your message referred to the state of Emacs as of HEAD,
> not with
> > my proposed changes included.
>
> It was.
>

But before that, you said (referring to the same message):
"I was talking about the situation after you proposed changes, which
will cause the flag to be set (AFAIU)."

I could not determine from context which statement is valid for which parts
of which message. I'm not saying you necessarily contradicted yourself,
just that my interpretations of those statements are contradictory. In
other words, I'm confused.


> Wish #1: a way of knowing "when" windows are resized or moved, even
> > temporarily. Whether that "when" is "just before" or "just after" doesn't
> > really matter, but it should be one of those.
>
> This wish needs to be described in more detail.  Resizing of a window
> is done in several distinct steps:
>
>  . some function (resize_mini_window, if we are talking about the
>    mini-window) decides whether a window should be resized, and if so,
>    computes its start position;
>
>  . redisplay_window, called by redisplay_internal, computes the
>    "desired" contents of the window on screen;
>
>  . update_window, called by update_frame, actually delivers the needed
>    changes to the glass
>

Wish #1 is for the hook to be called at any of the following points:
 - between steps 1 and 2
 - between steps 2 and 3
 - shortly after step 3.

Also, the redisplay code is free to call the hook at other times even
though nothing has changed. That's part of the wish #1 contract.


If I had free choice, my preference would be the second option, but the
other options are also okay.


> I hope it is clear that, depending on what exactly the hook function
> wants to do with the info about resizing, it could or should be called
> in different parts of the code, which are involved in these steps.
>

It is, but I think it would be a valid design decision to leave that
unspecified and just say it's one of the three options.


> > Wish #2: ....


My suggestion is to drop wish #2 for now.

In addition, it is not clear whether your hook will care if the window
> actually didn't change at all.


The hook function must be able to deal with that case, for wish #1 and wish
#2. I see no way around that complication.

Also, you never actually explained, AFAIR, why it is so important for
> you to be called "just before the change hits the X server".
>

Let's drop this for now. (I was thinking about the rather contrived case in
which VNC shares one Emacs window but not the rest of the Emacs frame.
Before we put anything else into the shared rectangle, I want to clear the
previous contents and update the VNC server's shared rectangle coordinates,
then synchronize with VNC, and only then permit the redisplay to continue.)

And finally, I hope you will agree now that pre-redisplay-function is
> not the right vehicle for these 2 wishes, because your wishes cannot
> be granted before redisplay did most of its job regarding the window
> in question.
>

I'm perfectly happy using another option instead of pre-redisplay-function.
I'm also happy to accept your verdict that pre-redisplay-function is not
the right thing to use.

> Wish #1, assuming something like my proposed changes go in and this bug is
> > closed, has been granted; there's a race condition, but I might just
> have to
> > live with that.
>
> Do you still think it has been granted?


If I'm not allowed to use pre-redisplay-function, no.

At the very best, you will be
> able to obtain part of the information about the window being resized;
> the rest of it, which redisplay still needs to figure out, will not
> yet be available.  For example, if the window being resized is the
> mini-window, the buffer position where the window above the
> mini-window ends will not yet be known.
>

That's a very convincing argument. (We can kludge our way around that, of
course).


> >     > No. The sequence is redisplay_internal, then prepare_menu_bars,
> then
> >     > grow_mini_window, then update_frame.
> >
> >     But grow_mini_window only recomputes the start of the window, it does
> >     not redisplay it. The next cycle will.
> >
> > The one I call "the current cycle"?
>
> I meant the one we both call "the next".


On further thought, it's
> possible that the current cycle will do that, at least in some
> situations.


Yes, precisely, that's what I'm seeing! I believe that to be a very common
situation, when the minibuffer grows because the user entered more
characters than fit in a single line.


> >     The function update_frame only reflects on the glass what its
> >     redisplay cycle computed to be the desired display. If redisplay
> >     didn't recompute the window contents, update_frame will change
> >     nothing.
> >
> > That's not what I seeing running x_sync manually.
>
> I don't understand what that means


Let me explain (I think you know all this, of course, but I might have it
wrong):
When update_frame returns, it's possible that it has made Xlib calls to
change the screen content but that Xlib has not yet sent out messages to
the X server to install those changes. In this case, the screen will still
show the pre-redisplay state even though redisplay() is about to exit.
Calling x_sync manually ensures that that doesn't happen.

IOW, there are several ideas of what the screen looks like:
 1. the glyph matrix etc.
 2. what Xlib has been told the screen should look like
 3. what the screen actually looks like.

update_frame synchronizes (2) with (1). x_sync synchronizes (3) with (2).

In all the cases that I did look at in gdb, update_frame actually did make
changes to (2), but did not synchronize them with (3), so it was important
for me to call x_sync in order to avoid falsely concluding that
update_frame hadn't performed the update when, in fact, it had.

When redisplay decides nothing needs to be recomputed, the current and
> the desired contents will be identical.
>

Do you think this is actually happening in our test situation? I think it's
another marginal 1% case that we can drop.

> 1. user input
> > 2. redisplay called
>
> That is inaccurate.


I did not mean to imply that list was complete, just that it was a
description of the typical 99% case when the minibuffer is resized because
the user put in a character.


> Emacs returns to the
> main loop after processing some command, which could be triggered by
> user input, or by input from a subprocess, or by some timer that
> became ripe, or by input from D-bus, or from any number of additional
> input sources.
>

But we just agreed to drop such marginal cases.

I omitted some steps. How about (again, this is a description of the
typical case that I'm actually seeing here):
1. user input
2. command loop handles user input
3. command loop calls redisplay
4. prepare_menu_bars called
5. pre-redisplay-function called
6. window-size-change-functions NOT called.
7. grow_mini_window called, calculates new window size
8. redisplay_windows/redisplay_window_1 called, compute data for
update_frame based on the new window sizes
9. update_frame is called, writes the new, enlarged mini-window to the glass
10. control returns to command loop
11. redisplay is called again
12. prepare_menu_bars called
13. prepare-redisplay-function called
14. window-size-change-functions not called on unmodified Emacs, called
with my initial suggested patch.

> But there's no guarantee that there won't be intermediate commands
> executed
> > between steps 6 and 7. In fact (progn (message "long message") (sleep
> 10.0))
> > will make step 8 happen ten seconds after the changed sizes have been
> written
> > to the glass.
> >
> > Wish #2 would mean swapping steps 3 and 4.
>

(With the new numbering, it would mean moving step 5 to happen between step
7 and step 8, or installing a new hook there).

Are you suggesting a hook between steps 8 and 9? I think that's not a
single position in the C code, but it would be the perfect solution.

[-- Attachment #2: Type: text/html, Size: 17385 bytes --]