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

[Pip Cet]

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.