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: 2134 bytes --]

On Tue, Aug 25, 2015 at 7:25 AM, martin rudalics <rudalics@gmx.at> wrote:
>>> Naively spoken it's obvious that when you shrink the minibuffer you show
>>> more lines in the window above and ‘linum-mode’ has to add numbers for
>>> those lines.  And when you enlarge the minibuffer, ‘follow-mode’ will
>>> lose some lines at the bottom of the left window and has to show them at
>>> the top of the right window.
>>
>> In well-behaved modes this happens automatically, as part of
>> redisplay.
>
> Via ‘pre-redisplay-function’?

Well, we have `pre-redisplay-functions', with an s, defined in
simple.el, and I've attached some (trivial) code that takes things a
little further and calls a normal hook when a window's size changed.

> How does a well-behaved mode detect
> whether it has to make "this happen automatically"?  By walking all
> windows and checking whether their sizes, start and end positions
> changed, I suppose.

That's what my code does. I thought I could get away with using the
arguments passed to pre-redisplay-function to limit which windows to
check, but that doesn't work when we "goto retry" and re-run
pre-redisplay-function. I will study the code in xdisp.c further and
see whether I can understand what the purpose of must_finish is.

>> I'd say, don't set the "size changed" flag unless the size really
>> changed.
>
> Sure.

Patch attached (no news on the paperwork so far). I'm not sure
precisely which properties should be checked for change, though.

I do think it would be best not to use set-window-configuration in
restoring state after exiting the minibuffer at all.

> Alternatively, Fset_window_configuration could run a modified version of
> ‘compare-window-configurations’ to compare the current configuration
> with the one to be restored and restore the old configuration iff these
> differ.  I'm not sure whether this would be any cheaper, especially when
> the configuration does change frequently.

I think it would be better to do this explicitly, even if we have to
compare all properties.

Thanks again to both of you,
Pip

[-- Attachment #2: emacs-via-redisplay-014.el --]
[-- Type: text/x-emacs-lisp, Size: 1548 bytes --]

;; generic code:
(defvar predisplay-hook nil
  "Normal hook run before predisplay.")
(defvar window-changed-size-hook nil
  "Normal hook run when a window displaying this buffer changed size.")
(defvar predisplay-sizes (make-hash-table :weakness 'key :test 'eq)
  "Hash table of old window sizes to detect size changes.")

(defun predisplay-check-window ()
  "Run in hook `predisplay-hook' to determine which windows changed size."
  (let* ((w (selected-window))
	 (old-size (gethash w predisplay-sizes))
	 (new-size (cons (window-pixel-width w)
			 (window-pixel-height w))))
    (unless (equal old-size new-size)
      (run-hooks 'window-changed-size-hook)
      (puthash w new-size predisplay-sizes))))

(add-hook 'predisplay-hook #'predisplay-check-window)

(defun predisplay-function (&rest args)
  (dolist (w (window-list-1 nil t t))
    (with-selected-window w
      (run-hooks 'predisplay-hook))))

(add-function :before pre-redisplay-function
              #'predisplay-function)
;; demonstration code:
(defun my-window-changed-size ()
  (insert (format "%dx%d\n" (window-pixel-width (selected-window))
                  (window-pixel-height (selected-window)))))

(defun display-dynamic-size ()
  (add-hook 'window-changed-size-hook #'my-window-changed-size nil t))

;; dead code:

;; I thought this would work, but it doesn't, for reasons I believe
;; Eli described.

;; (defun predisplay-run-hook (window)
;;   (with-selected-window window
;;     (run-hooks 'predisplay-hook)))

;; (push #'predisplay-run-hook pre-redisplay-functions)


[-- Attachment #3: 0001-Only-set-FRAME_WINDOW_SIZES_CHANGED-if-necessary-213.patch --]
[-- Type: text/x-patch, Size: 1441 bytes --]

From 94ba0a8ed127dc147eb68bf019a1e8370622c740 Mon Sep 17 00:00:00 2001
From: Philip <pipcet@gmail.com>
Date: Tue, 25 Aug 2015 10:21:18 +0000
Subject: [PATCH] Only set FRAME_WINDOW_SIZES_CHANGED if necessary (#21333)

---
 src/window.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/src/window.c b/src/window.c
index 68bc9e5..6bc0982 100644
--- a/src/window.c
+++ b/src/window.c
@@ -6075,7 +6075,6 @@ the return value is nil.  Otherwise the value is t.  */)
 	}
 
       fset_redisplay (f);
-      FRAME_WINDOW_SIZES_CHANGED (f) = true;
 
       /* Problem: Freeing all matrices and later allocating them again
 	 is a serious redisplay flickering problem.  What we would
@@ -6127,6 +6126,13 @@ the return value is nil.  Otherwise the value is t.  */)
 	  /* If we squirreled away the buffer, restore it now.  */
 	  if (BUFFERP (w->combination_limit))
 	    wset_buffer (w, w->combination_limit);
+          if (!FRAME_WINDOW_SIZES_CHANGED (f)) {
+            if (w->pixel_left != XFASTINT (p->pixel_left) ||
+                w->pixel_top != XFASTINT (p->pixel_top) ||
+                w->pixel_width != XFASTINT (p->pixel_width) ||
+                w->pixel_height != XFASTINT (p->pixel_height))
+              FRAME_WINDOW_SIZES_CHANGED (f) = true;
+          }
 	  w->pixel_left = XFASTINT (p->pixel_left);
 	  w->pixel_top = XFASTINT (p->pixel_top);
 	  w->pixel_width = XFASTINT (p->pixel_width);
-- 
2.5.0