* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize @ 2015-08-23 22:06 Pip Cet 2015-08-24 8:18 ` martin rudalics 2015-08-24 14:35 ` Eli Zaretskii 0 siblings, 2 replies; 57+ messages in thread From: Pip Cet @ 2015-08-23 22:06 UTC (permalink / raw) To: 21333 [-- Attachment #1: Type: text/plain, Size: 4418 bytes --] This is possibly only a documentation issue. Recipe: eval (progn (push (lambda (&rest args) (message "window size changed")) window-size-change-functions) (message (make-string 3000 ?*))) Expected result: a "window size changed" message. Actual result: no such message. The symptom is that the window size change function is not run after a mini-window size change. So far, I can produce this behavior only when the minibuffer or echo area grows to several lines; when it shrinks afterwards, my window size change function is called. I cannot reproduce the behavior with other windows. Is this a bug? The documentation says: [...] to be called if the size of any window changes for any reason. Please correct me if I'm wrong, but when the minibuffer/echo area gets resized (and the windows on top of it, too), that counts as a change of size, I would say. If this is merely a documentation issue, the exception should be noted in the manual. Analysis: First, some warnings: - `window_resize_apply' and `Fwindow_resize_apply' (aka `window-resize-apply') are two different functions - `resize-mini-window' and `resize-mini-window-internal' are called only when the mini-window is explicitly resized by a Lisp call of `resize-mini-window'. Implicit resizes as a consequence of having too much text in the echo area do not appear to call it. The problem is that FRAME_WINDOW_SIZES_CHANGED (f) is not set to true after a mini-window resize. Fwindow_resize_apply would set this flag, but window_resize_apply does not. If this behavior is deliberate, I believe it is inconsistent to set FRAME_WINDOW_SIZES_CHANGED (f) in `resize-mini-window-internal'. Suggested solution: Trivial. Add FRAME_WINDOW_SIZES_CHANGED (f) = true to all callers of window_resize_apply. In GNU Emacs 25.0.50.51 (x86_64-unknown-linux-gnu, GTK+ Version 3.16.6) of 2015-08-23 on ... Repository revision: bfb06826feac9151877069590d5dc91b60337b6b Windowing system distributor `The X.Org Foundation', version 11.0.11702000 System Description: Debian GNU/Linux unstable (sid) Configured using: `configure 'CFLAGS=-O0 -g3'' Configured features: XPM JPEG TIFF GIF PNG RSVG SOUND DBUS GCONF GSETTINGS NOTIFY LIBSELINUX GNUTLS LIBXML2 FREETYPE XFT ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 Important settings: value of $LANG: en_US.UTF-8 locale-coding-system: utf-8-unix Major mode: Lisp Interaction Minor modes in effect: tooltip-mode: t global-eldoc-mode: t electric-indent-mode: t mouse-wheel-mode: t tool-bar-mode: t menu-bar-mode: t file-name-shadow-mode: t global-font-lock-mode: t font-lock-mode: t blink-cursor-mode: t auto-composition-mode: t auto-encryption-mode: t auto-compression-mode: t line-number-mode: t transient-mark-mode: t Recent messages: For information about GNU Emacs and the GNU system, type C-h C-a. Load-path shadows: None found. Features: (shadow sort gnus-util mail-extr emacsbug message dired format-spec rfc822 mml mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util help-fns help-mode easymenu cl-loaddefs pcase cl-lib mail-prsvr mail-utils misearch multi-isearch time-date mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment elisp-mode lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cl-generic cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese charscript case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer cl-preloaded nadvice loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5 base64 format env code-pages mule custom widget hashtable-print-readable backquote dbusbind inotify dynamic-setting system-font-setting font-render-setting move-toolbar gtk x-toolkit x multi-tty make-network-process emacs) Memory information: ((conses 16 81668 9633) (symbols 48 19040 0) (miscs 40 42 136) (strings 32 13288 4444) (string-bytes 1 380166) (vectors 16 11279) (vector-slots 8 414205 3702) (floats 8 131 186) (intervals 56 190 0) (buffers 976 11) (heap 1024 18666 1015)) [-- Attachment #2: 0001-Call-window-size-change-functions-after-mini-window-.patch --] [-- Type: text/x-patch, Size: 1376 bytes --] From 243be700591979554e61bbdff0f00f30cc386f7b Mon Sep 17 00:00:00 2001 From: Philip <pipcet@gmail.com> Date: Sun, 23 Aug 2015 21:46:42 +0000 Subject: [PATCH] Call `window-size-change-functions' after mini-window size changes. * window.c (resize_frame_windows): Set FRAME_WINDOW_SIZES_CHANGED flag. (grow_mini_window): Likewise. (shrink_mini_window): Likewise. --- src/window.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/window.c b/src/window.c index 863a792..68bc9e5 100644 --- a/src/window.c +++ b/src/window.c @@ -4094,6 +4094,7 @@ resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise) } fset_redisplay (f); + FRAME_WINDOW_SIZES_CHANGED (f) = true; } @@ -4531,6 +4532,7 @@ grow_mini_window (struct window *w, int delta, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); + FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } @@ -4570,6 +4572,7 @@ shrink_mini_window (struct window *w, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); + FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } -- 2.5.0 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-23 22:06 bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize Pip Cet @ 2015-08-24 8:18 ` martin rudalics 2015-08-24 11:08 ` Pip Cet 2015-08-24 14:35 ` Eli Zaretskii 1 sibling, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-24 8:18 UTC (permalink / raw) To: Pip Cet, 21333 > This is possibly only a documentation issue. > > Recipe: eval > (progn (push (lambda (&rest args) (message "window size changed")) > window-size-change-functions) > (message (make-string 3000 ?*))) > > Expected result: a "window size changed" message. > > Actual result: no such message. > > > The symptom is that the window size change function is not run after a > mini-window size change. > > So far, I can produce this behavior only when the minibuffer or echo > area grows to several lines; when it shrinks afterwards, my window size > change function is called. I cannot reproduce the behavior with other > windows. > > Is this a bug? The documentation says: > > [...] to be called if the size of any window changes for any reason. > > Please correct me if I'm wrong, but when the minibuffer/echo area gets > resized (and the windows on top of it, too), that counts as a change of > size, I would say. > > If this is merely a documentation issue, the exception should be noted > in the manual. Looks like bug #830 23.0.60; window-size-change-functions sometimes not called in action. > Analysis: > First, some warnings: > - `window_resize_apply' and `Fwindow_resize_apply' (aka > `window-resize-apply') are two different functions > - `resize-mini-window' and `resize-mini-window-internal' are called > only when the mini-window is explicitly resized by a Lisp call of > `resize-mini-window'. Implicit resizes as a consequence of having > too much text in the echo area do not appear to call it. > > The problem is that FRAME_WINDOW_SIZES_CHANGED (f) is not set to true > after a mini-window resize. Fwindow_resize_apply would set this flag, > but window_resize_apply does not. > > If this behavior is deliberate, I believe it is inconsistent to set > FRAME_WINDOW_SIZES_CHANGED (f) in `resize-mini-window-internal'. > > > Suggested solution: > > Trivial. Add FRAME_WINDOW_SIZES_CHANGED (f) = true to all callers of > window_resize_apply. Your patch looks fine to me. I'd suggest to postpone installing it until your paperwork is complete. OK? Thanks, martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 8:18 ` martin rudalics @ 2015-08-24 11:08 ` Pip Cet 2015-08-24 12:41 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-24 11:08 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 On Mon, Aug 24, 2015 at 8:18 AM, martin rudalics <rudalics@gmx.at> wrote: > Looks like bug #830 > > 23.0.60; window-size-change-functions sometimes not called > > in action. Agreed, that appears to be the same bug. (I appear to have searched for window-size-changed-functions when reporting this, not window-size-change-functions). I don't know what the Emacs policy is on merging bug reports, but we probably should. > Your patch looks fine to me. I'd suggest to postpone installing it until > your paperwork is complete. OK? Absolutely. I must admit I was expecting a discussion first, so I decided to send this right away to get it over with :-) Thank you, Pip ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 11:08 ` Pip Cet @ 2015-08-24 12:41 ` martin rudalics 0 siblings, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-24 12:41 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 >> Looks like bug #830 >> >> 23.0.60; window-size-change-functions sometimes not called >> >> in action. > > Agreed, that appears to be the same bug. (I appear to have searched > for window-size-changed-functions when reporting this, not > window-size-change-functions). I don't know what the Emacs policy is > on merging bug reports, but we probably should. I just merged them. `window-size-change-functions' is a Cinderella of our hooks. Nobody really knows when it should be used, me included. Markus considered it necessary for ‘linum-mode’ but apparently changed his mind later. The only "real" user is probably ‘follow-mode’. >> Your patch looks fine to me. I'd suggest to postpone installing it until >> your paperwork is complete. OK? > > Absolutely. I must admit I was expecting a discussion first, so I > decided to send this right away to get it over with :-) It might be worth pursuing the simpler solution to set FRAME_WINDOW_SIZES_CHANGED "only" in window_resize_apply because conceptually _all_ window size changes "must" pass through that function. I never checked the truth of that "must" but am quite confident that anything else would result in a bug. There is one prominent exception from this rule - `delete-other-windows' when the sole remaining window is a leaf window. That was my last remedy when I tested the resize code and `window-resize-apply' failed for whatever reason. C-x 1 reliably got me out of that mess. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-23 22:06 bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize Pip Cet 2015-08-24 8:18 ` martin rudalics @ 2015-08-24 14:35 ` Eli Zaretskii 2015-08-24 18:06 ` martin rudalics ` (2 more replies) 1 sibling, 3 replies; 57+ messages in thread From: Eli Zaretskii @ 2015-08-24 14:35 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Sun, 23 Aug 2015 22:06:45 +0000 > From: Pip Cet <pipcet@gmail.com> > > This is possibly only a documentation issue. Possibly. > Recipe: eval > (progn (push (lambda (&rest args) (message "window size changed")) > window-size-change-functions) > (message (make-string 3000 ?*))) > > Expected result: a "window size changed" message. > > Actual result: no such message. > > The symptom is that the window size change function is not run after a > mini-window size change. Note that resizing the mini-window involves resizing of at least one other window on the same frame. So this is not exactly about the mini-window. > So far, I can produce this behavior only when the minibuffer or echo > area grows to several lines; when it shrinks afterwards, my window size > change function is called. I don't see the message even when the mini-window shrinks back. Are you sure the message you see is not triggered by some other event? It's way too easy to trigger it, see below. > I cannot reproduce the behavior with other windows. See above: at least one other window is resized in this recipe, so the exemption is not about the mini-window itself, it's about any window involved in resizing in this particular scenario. > Is this a bug? The documentation says: > > [...] to be called if the size of any window changes for any reason. > > Please correct me if I'm wrong, but when the minibuffer/echo area gets > resized (and the windows on top of it, too), that counts as a change of > size, I would say. I believe the original reason for this is largely historical: window-size-change-functions exists since Emacs 19.29, whereas automatic resizing of mini-window was introduced in Emacs 21. Since Emacs before 21 didn't have the mini-window resizing functionality, Emacs 21 was careful not to gratuitously trigger these functions by something that is purely a new redisplay feature. That said, I wonder whether changing the code now to call these functions due to automatic resizing would make sense. What would be the real-life use cases for using that? I believe window-size-change-functions is meant for taking notice of resizes done by the user or some Lisp code, not for automated resizes whose sole purpose is to allow some message be read in its entirety. If you agree, then the current behavior will make sense to you. If anything, IMO we should _reduce_ the number of unrelated events that trigger a call to these functions. For example, currently any command that reads from the minibuffer will trigger it, because when read-from-minibuffer exits, it restores the window configuration by calling set-window-configuration, which is documented to trigger these functions. That just doesn't make any sense to me, since most reads from the minibuffer don't resize any windows! > If this is merely a documentation issue, the exception should be noted > in the manual. That's easy. Deciding what's TRT in this case is harder. > If this behavior is deliberate, I believe it is inconsistent to set > FRAME_WINDOW_SIZES_CHANGED (f) in `resize-mini-window-internal'. It's consistent if we adopt the POV that this feature only catches resizes from Lisp code. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 14:35 ` Eli Zaretskii @ 2015-08-24 18:06 ` martin rudalics 2015-08-24 18:30 ` Eli Zaretskii 2015-08-24 18:13 ` Pip Cet 2016-02-22 12:59 ` Fix `window-configuration-change-hook' and `window-size-change-functions' martin rudalics 2 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-24 18:06 UTC (permalink / raw) To: Eli Zaretskii, Pip Cet; +Cc: 21333 > That said, I wonder whether changing the code now to call these > functions due to automatic resizing would make sense. What would be > the real-life use cases for using that? 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. I don't know how these packages currently work around the problem that ‘window-size-change-functions’ is not called for automatic minibuffer resizing. Maybe they use the ‘post-command-hook’ function instead. > If anything, IMO we should _reduce_ the number of unrelated events > that trigger a call to these functions. For example, currently any > command that reads from the minibuffer will trigger it, because when > read-from-minibuffer exits, it restores the window configuration by > calling set-window-configuration, which is documented to trigger these > functions. That just doesn't make any sense to me, since most reads > from the minibuffer don't resize any windows! This is, in fact, an abuse of ‘set-window-configuration’. But how fix it? We'd need a hook, say ‘window-size-change-functions’, that tracks, among other things, whether a window was resized due to a change of the minibuffer height and, if that happens, set a flag to indicate that the window configuration must be restored. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 18:06 ` martin rudalics @ 2015-08-24 18:30 ` Eli Zaretskii 2015-08-25 7:25 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-24 18:30 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Mon, 24 Aug 2015 20:06:54 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: 21333@debbugs.gnu.org > > > That said, I wonder whether changing the code now to call these > > functions due to automatic resizing would make sense. What would be > > the real-life use cases for using that? > > 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. > Maybe they use the ‘post-command-hook’ function instead. Of course, they do! The flag of bad design. > > If anything, IMO we should _reduce_ the number of unrelated events > > that trigger a call to these functions. For example, currently any > > command that reads from the minibuffer will trigger it, because when > > read-from-minibuffer exits, it restores the window configuration by > > calling set-window-configuration, which is documented to trigger these > > functions. That just doesn't make any sense to me, since most reads > > from the minibuffer don't resize any windows! > > This is, in fact, an abuse of ‘set-window-configuration’. But how fix > it? We'd need a hook, say ‘window-size-change-functions’, that tracks, > among other things, whether a window was resized due to a change of the > minibuffer height and, if that happens, set a flag to indicate that the > window configuration must be restored. I'd say, don't set the "size changed" flag unless the size really changed. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 18:30 ` Eli Zaretskii @ 2015-08-25 7:25 ` martin rudalics 2015-08-25 10:34 ` Pip Cet 2015-08-25 15:11 ` Eli Zaretskii 0 siblings, 2 replies; 57+ messages in thread From: martin rudalics @ 2015-08-25 7:25 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 >> 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’? 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. >> Maybe they use the ‘post-command-hook’ function instead. > > Of course, they do! The flag of bad design. IIUC they didn't have another choice before ‘pre-redisplay-function’ was added. >> This is, in fact, an abuse of ‘set-window-configuration’. But how fix >> it? We'd need a hook, say ‘window-size-change-functions’, that tracks, >> among other things, whether a window was resized due to a change of the >> minibuffer height and, if that happens, set a flag to indicate that the >> window configuration must be restored. > > I'd say, don't set the "size changed" flag unless the size really > changed. Sure. Nevertheless we would have to also track changes due to automatic minibuffer resizing. 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. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 7:25 ` martin rudalics @ 2015-08-25 10:34 ` Pip Cet 2015-08-25 15:19 ` Eli Zaretskii 2015-08-26 7:08 ` martin rudalics 2015-08-25 15:11 ` Eli Zaretskii 1 sibling, 2 replies; 57+ messages in thread From: Pip Cet @ 2015-08-25 10:34 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- 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 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 10:34 ` Pip Cet @ 2015-08-25 15:19 ` Eli Zaretskii 2015-08-26 7:08 ` martin rudalics 1 sibling, 0 replies; 57+ messages in thread From: Eli Zaretskii @ 2015-08-25 15:19 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Tue, 25 Aug 2015 10:34:23 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, 21333@debbugs.gnu.org > > > Via ‘pre-redisplay-function’? > > Well, we have `pre-redisplay-functions', with an s, defined in > simple.el That's a Lisp-level trick, but the variable used by the display engine is pre-redisplay-function, without an s. > 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. Not sure I understand why it wouldn't work. Can you elaborate? The way I see it, the windows passed to pre-redisplay-function are those that needed redisplay, so if the list is different on the second call, the rest of the windows were already redisplayed, and your hook shouldn't care about them, because you already processed them on the previous call. > I will study the code in xdisp.c further and see whether I can > understand what the purpose of must_finish is. We have "goto retry" in redisplay_internal in more than one place, and only one of them is conditioned by must_finish. In general must_finish is true when we must complete the redisplay cycle by calling update_frame. But I'm not sure this answers your question in full. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 10:34 ` Pip Cet 2015-08-25 15:19 ` Eli Zaretskii @ 2015-08-26 7:08 ` martin rudalics 1 sibling, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-26 7:08 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > I do think it would be best not to use set-window-configuration in > restoring state after exiting the minibuffer at all. I profoundly dislike window configurations but in this particular case they are needed. If the minibuffer gets enlarged, a window above it shrinks and as a consequence that window's start position changes to keep point on screen, it's important to restore the previous start position of that window. Everything else would be disconcerting. >> 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. You mean to compare the properties one can get by walking all windows on the frame with those of the configuration that shall be restored? Note that looking into a window configuration is not entirely trivial. > + if (!FRAME_WINDOW_SIZES_CHANGED (f)) { For consistency, please don't use hanging braces. > + if (w->pixel_left != XFASTINT (p->pixel_left) || > + w->pixel_top != XFASTINT (p->pixel_top) || Why do you think we need to check these? martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 7:25 ` martin rudalics 2015-08-25 10:34 ` Pip Cet @ 2015-08-25 15:11 ` Eli Zaretskii 2015-08-26 7:09 ` martin rudalics 1 sibling, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-25 15:11 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Tue, 25 Aug 2015 09:25:10 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org Before I respond to your comments, let me just clarify that what I say about this should not be interpreted as a veto of some kind. You are our window-management czar: if you think my objections should be overruled, by all means go ahead and make the changes. > >> 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’? No, by arranging the buffer contents and letting redisplay do its job. Problems of this kind happen only when a mode changes buffer contents and related data structures (such as properties and overlays) in response to redisplay, something that is bad idea to begin with, because at the very least it immediately triggers another redisplay cycle, and kills many redisplay optimizations. > >> Maybe they use the ‘post-command-hook’ function instead. > > > > Of course, they do! The flag of bad design. > > IIUC they didn't have another choice before ‘pre-redisplay-function’ was > added. IMO, lack of infrastructure is not an excuse for bad design. Either the missing infrastructure should be added, or the design changed (if possible) to some better-behaving alternative. In extreme cases, the whole idea should be dropped as unworkable. > >> This is, in fact, an abuse of ‘set-window-configuration’. But how fix > >> it? We'd need a hook, say ‘window-size-change-functions’, that tracks, > >> among other things, whether a window was resized due to a change of the > >> minibuffer height and, if that happens, set a flag to indicate that the > >> window configuration must be restored. > > > > I'd say, don't set the "size changed" flag unless the size really > > changed. > > Sure. Nevertheless we would have to also track changes due to automatic > minibuffer resizing. As an option, perhaps. And it should be opt-in IMO, because most use cases shouldn't care about automatic resizing such as this one. > 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. As I said too many times in this thread, performance is the last thing I care about in this respect. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 15:11 ` Eli Zaretskii @ 2015-08-26 7:09 ` martin rudalics 2015-08-26 15:29 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-26 7:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > You are > our window-management czar: A carpenter, at best. > No, by arranging the buffer contents and letting redisplay do its > job. Problems of this kind happen only when a mode changes buffer > contents and related data structures (such as properties and overlays) > in response to redisplay, something that is bad idea to begin with, > because at the very least it immediately triggers another redisplay > cycle, and kills many redisplay optimizations. ‘follow-mode’, to stick to one of the examples I cited earlier, uses neither properties nor overlays. It must, however, know the exact ‘window-end’ position of any window that could be followed by another. This also means that a correct implementation of ‘follow-mode’ should be allowed to specify the order in which windows are redisplayed. So ideally, redisplay would be able to process each window separately, tell us its new start and end positions and allow modes to react properly. > IMO, lack of infrastructure is not an excuse for bad design. Either > the missing infrastructure should be added, or the design changed (if > possible) to some better-behaving alternative. In extreme cases, the > whole idea should be dropped as unworkable. I'm convinced that the current version of ‘linum-mode’ doesn't behave well and line numbers should be calculated and written by the display engine. But I'm too lazy to do that (probably because I don't use line numbers myself). > As an option, perhaps. And it should be opt-in IMO, because most use > cases shouldn't care about automatic resizing such as this one. So let's provide an option for this. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 7:09 ` martin rudalics @ 2015-08-26 15:29 ` Eli Zaretskii 2015-08-27 7:57 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-26 15:29 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Wed, 26 Aug 2015 09:09:30 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > > You are our window-management czar: > > A carpenter, at best. I see no contradiction here. > > No, by arranging the buffer contents and letting redisplay do its > > job. Problems of this kind happen only when a mode changes buffer > > contents and related data structures (such as properties and overlays) > > in response to redisplay, something that is bad idea to begin with, > > because at the very least it immediately triggers another redisplay > > cycle, and kills many redisplay optimizations. > > ‘follow-mode’, to stick to one of the examples I cited earlier, uses > neither properties nor overlays. It must, however, know the exact > ‘window-end’ position of any window that could be followed by another. > This also means that a correct implementation of ‘follow-mode’ should be > allowed to specify the order in which windows are redisplayed. So > ideally, redisplay would be able to process each window separately, tell > us its new start and end positions and allow modes to react properly. OK, but if you want to stick to this example, please explain how is mini-window resizing relevant to follow-mode, because I think it isn't. I think follow-mode should not care at all about this automatic resizing. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 15:29 ` Eli Zaretskii @ 2015-08-27 7:57 ` martin rudalics 2015-08-27 15:29 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-27 7:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > OK, but if you want to stick to this example, please explain how is > mini-window resizing relevant to follow-mode, because I think it > isn't. I think follow-mode should not care at all about this > automatic resizing. `follow-mode' has to synchronize the ‘window-end’ of one window with the ‘window-start’ of another. Since mini-window resizing can persistently change the end position of the first of these windows, ‘follow-mode’ eventually has to react even if might dismiss the temporary effect that the enlarged mini-window obscures the bottom of the first window. OT1H we do care about point being visible when its window is partially obscured by the mini-window and deliberately scroll the window in that case. OTOH we'd say that `follow-mode' should not care about keeping its text coherent in that case. Is that fair? martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 7:57 ` martin rudalics @ 2015-08-27 15:29 ` Eli Zaretskii 2015-08-27 17:05 ` Pip Cet 2015-08-27 17:58 ` martin rudalics 0 siblings, 2 replies; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 15:29 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Thu, 27 Aug 2015 09:57:15 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > > OK, but if you want to stick to this example, please explain how is > > mini-window resizing relevant to follow-mode, because I think it > > isn't. I think follow-mode should not care at all about this > > automatic resizing. > > `follow-mode' has to synchronize the ‘window-end’ of one window with the > ‘window-start’ of another. Not when one of them is temporarily obscured by a resized mini-window, IMO. > Since mini-window resizing can persistently > change the end position of the first of these windows Any command that clears the echo area will resize it back, AFAIK. So it's not persistent. > OT1H we do care about point being visible when its window is partially > obscured by the mini-window and deliberately scroll the window in that > case. OTOH we'd say that `follow-mode' should not care about keeping > its text coherent in that case. Is that fair? Yes. In that situation, the user most probably reads the mini-window text anyway, and if not, then she reads the text at point. The probability that she is reading the text that will be scrolled out of view as result of the mini-window resize is IMO minuscule. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 15:29 ` Eli Zaretskii @ 2015-08-27 17:05 ` Pip Cet 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:35 ` Eli Zaretskii 2015-08-27 17:58 ` martin rudalics 1 sibling, 2 replies; 57+ messages in thread From: Pip Cet @ 2015-08-27 17:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 2441 bytes --] On Thu, Aug 27, 2015 at 3:29 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > `follow-mode' has to synchronize the ‘window-end’ of one window with the > > ‘window-start’ of another. > > Not when one of them is temporarily obscured by a resized mini-window, > IMO. > I think that is indeed a matter of opinion (and should thus be subject to customization), but I don't think we should assume that recursive editing invocations or long message scenarios are always short-lived. > Since mini-window resizing can persistently > > change the end position of the first of these windows > > Any command that clears the echo area will resize it back, AFAIK. So > it's not persistent. > That's not what I'm seeing here. When I run: (progn (run-with-timer 2 nil (lambda () (message "hi"))) (run-with-timer 3 nil (lambda () (message ""))) (read-from-minibuffer (make-string 3000 ?*))) The minibuffer resizes once, when the read-from-minibuffer prompt makes it grow to its maximal size; it then stays at that size as the short message is being displayed, then restores the long minibuffer prompt without changing size again when the echo area is cleared. > > OT1H we do care about point being visible when its window is partially > > obscured by the mini-window and deliberately scroll the window in that > > case. OTOH we'd say that `follow-mode' should not care about keeping > > its text coherent in that case. Is that fair? > > Yes. In that situation, the user most probably reads the mini-window > text anyway, and if not, then she reads the text at point. The > probability that she is reading the text that will be scrolled out of > view as result of the mini-window resize is IMO minuscule. > So if, for whatever reason, I have a timer function that displays a two-line message once a second (so the echo area never goes back to its single-line state), my follow-mode buffer will be and remain in an inconsistent state until I manually resize a window, when it will go back to a consistent state, but then if I cancel the timer (and the mini-window shrinks) it'll be in an inconsistent state again, and the only way out of that one is another manual window resize? I think that last case is something we haven't considered yet: if I resize windows manually while the mini-window is enlarged, they will be in an inconsistent state when it goes back to normal, right? [-- Attachment #2: Type: text/html, Size: 3325 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 17:05 ` Pip Cet @ 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:04 ` Pip Cet 2015-08-27 18:35 ` Eli Zaretskii 1 sibling, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-27 17:59 UTC (permalink / raw) To: Pip Cet, Eli Zaretskii; +Cc: 21333 > I think that last case is something we haven't considered yet: if I resize > windows manually while the mini-window is enlarged, they will be in an > inconsistent state when it goes back to normal, right? FWIW here resizing a window "manually" makes the mini-window "go normal" first. Resizing the frame leaves the mini-window alone. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 17:59 ` martin rudalics @ 2015-08-27 18:04 ` Pip Cet 2015-08-28 8:03 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-27 18:04 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 669 bytes --] On Thu, Aug 27, 2015 at 5:59 PM, martin rudalics <rudalics@gmx.at> wrote: > > I think that last case is something we haven't considered yet: if I > resize > > windows manually while the mini-window is enlarged, they will be in an > > inconsistent state when it goes back to normal, right? > > FWIW here resizing a window "manually" makes the mini-window "go normal" > first. Strange, that doesn't work here. Are you using the echo area or the minibuffer to test? I'm using the minibuffer. > Resizing the frame leaves the mini-window alone. > ...which means they'll be in an inconsistent state afterwards, having been resized while the mini-window was enlarged. [-- Attachment #2: Type: text/html, Size: 1278 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 18:04 ` Pip Cet @ 2015-08-28 8:03 ` martin rudalics 2015-08-28 8:19 ` Pip Cet 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-28 8:03 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 >> FWIW here resizing a window "manually" makes the mini-window "go normal" >> first. > > Strange, that doesn't work here. Are you using the echo area or the > minibuffer to test? I'm using the minibuffer. It goes back to normal in the echo area but leaves the minibuffer window alone as you say. What precisely did you use? >> Resizing the frame leaves the mini-window alone. >> > ...which means they'll be in an inconsistent state afterwards, having been > resized while the mini-window was enlarged. Can you tell me more about this inconsistency? martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 8:03 ` martin rudalics @ 2015-08-28 8:19 ` Pip Cet 2015-08-28 8:45 ` Pip Cet 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-28 8:19 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 1427 bytes --] On Fri, Aug 28, 2015 at 8:03 AM, martin rudalics <rudalics@gmx.at> wrote: > >> FWIW here resizing a window "manually" makes the mini-window "go normal" > >> first. > > > > Strange, that doesn't work here. Are you using the echo area or the > > minibuffer to test? I'm using the minibuffer. > > It goes back to normal in the echo area but leaves the minibuffer window > alone as you say. I had assumed "go normal" meant "shrink back to normal size", which is true for the echo area but not the minibuffer. > >> Resizing the frame leaves the mini-window alone. > >> > > ...which means they'll be in an inconsistent state afterwards, having > been > > resized while the mini-window was enlarged. > > Can you tell me more about this inconsistency? > Well, first, to clarify, I was talking about unpatched Emacs, and using window-size-change-functions in an attempt to catch window size changes. 1. window at 80x24, mini-window at 1 line, window thinks it's 80x24. Everything okay. 2. mini-window grows to 2 lines 3. window at 80x23, mini-window at 2 lines, window thinks it's 80x24. Inconsistent, but temporary. 4. user resizes window to 79x23. size-change-functions called. 5. window at 79x23, mini-window at 2 lines, window thinks it's 79x23. Everything okay again. 6. mini-window shrinks. 7. window at 79x24, mini-window at 1 line, window thinks it's 79x23. Permanent inconsistency. Am I missing something here? Pip [-- Attachment #2: Type: text/html, Size: 2298 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 8:19 ` Pip Cet @ 2015-08-28 8:45 ` Pip Cet 0 siblings, 0 replies; 57+ messages in thread From: Pip Cet @ 2015-08-28 8:45 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 839 bytes --] On Fri, Aug 28, 2015 at 8:19 AM, Pip Cet <pipcet@gmail.com> wrote: > > Am I missing something here? > Turns out I was missing (at least!) one thing: step 6 needs to be triggered by a recursive minibuffer invocation, because exiting the minibuffer will restore the old window configuration. The inconsistency can still happen, but it's much more difficult to trigger than I thought. (I must say I don't think restoring the window configuration upon minibuffer exit is the right thing to do: if the user created, split, or resized windows manually during the recursive editing session, at least those manual changes should not be undone when they end that session. As you've pointed out, looking into window configurations is quite tricky (and impossible from Lisp code, IIUC), and I can currently only think of a kludge to make it work.) [-- Attachment #2: Type: text/html, Size: 1311 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 17:05 ` Pip Cet 2015-08-27 17:59 ` martin rudalics @ 2015-08-27 18:35 ` Eli Zaretskii 1 sibling, 0 replies; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 18:35 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Thu, 27 Aug 2015 17:05:35 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: martin rudalics <rudalics@gmx.at>, 21333@debbugs.gnu.org > > > Since mini-window resizing can persistently > > change the end position of the first of these windows > > Any command that clears the echo area will resize it back, AFAIK. So > it's not persistent. > > > That's not what I'm seeing here. When I run: > > (progn > (run-with-timer 2 nil (lambda () (message "hi"))) > (run-with-timer 3 nil (lambda () (message ""))) > (read-from-minibuffer (make-string 3000 ?*))) > > The minibuffer resizes once, when the read-from-minibuffer prompt makes it grow > to its maximal size; it then stays at that size as the short message is being > displayed, then restores the long minibuffer prompt without changing size again > when the echo area is cleared. That's a different situation, quite bizarre btw. Just run normal code, like only the message above, then move the cursor, and you will see the echo area shrink back. That's the normal use case. > > OT1H we do care about point being visible when its window is partially > > obscured by the mini-window and deliberately scroll the window in that > > case. OTOH we'd say that `follow-mode' should not care about keeping > > its text coherent in that case. Is that fair? > > Yes. In that situation, the user most probably reads the mini-window > text anyway, and if not, then she reads the text at point. The > probability that she is reading the text that will be scrolled out of > view as result of the mini-window resize is IMO minuscule. > > So if, for whatever reason, I have a timer function that displays a two-line > message once a second (so the echo area never goes back to its single-line > state), my follow-mode buffer will be and remain in an inconsistent state until > I manually resize a window, when it will go back to a consistent state, but > then if I cancel the timer (and the mini-window shrinks) it'll be in an > inconsistent state again, and the only way out of that one is another manual > window resize? I'd say just don't do that, it's terribly annoying to see such display even without the other issues. > I think that last case is something we haven't considered yet: if I resize > windows manually while the mini-window is enlarged, they will be in an > inconsistent state when it goes back to normal, right? What do you mean by "inconsistent state"? ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 15:29 ` Eli Zaretskii 2015-08-27 17:05 ` Pip Cet @ 2015-08-27 17:58 ` martin rudalics 1 sibling, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-27 17:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > Any command that clears the echo area will resize it back, AFAIK. So > it's not persistent. I was talking about the changed window-start position. That one _is_ persistent, that is, not restored. Maybe that case can be handled via ‘window-scroll-functions’ but that's not immediately clear to me. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 14:35 ` Eli Zaretskii 2015-08-24 18:06 ` martin rudalics @ 2015-08-24 18:13 ` Pip Cet 2015-08-24 19:03 ` Eli Zaretskii 2016-02-22 12:59 ` Fix `window-configuration-change-hook' and `window-size-change-functions' martin rudalics 2 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-24 18:13 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 12522 bytes --] Thank you for your response! Sorry this is a long email, but I really, really want the full-featured window-size-change-functions. I understand if you don't have time to read it all, so let me summarize: - Redisplay is so slow that it really doesn't matter that we call an extra bit of Lisp code before running it. - Doing it "my way" is cheap in terms of performance (I benchmarked), easy to implement, enables cool but not necessarily useful hacks, but requires very slow hook functions to rate-limit themselves in order to keep the minibuffer usable. - Doing it your way is even cheaper in terms of performance, not very hard to implement, requires those hacks to hook into pre-redisplay-functions instead, and has the questionable benefit of allowing badly-written non-rate-limiting hook functions to keep the minibuffer usable, but still break drag-resizing. On Mon, Aug 24, 2015 at 2:35 PM, Eli Zaretskii <eliz@gnu.org> wrote: >> Recipe: eval >> (progn (push (lambda (&rest args) (message "window size changed")) >> window-size-change-functions) >> (message (make-string 3000 ?*))) >> >> Expected result: a "window size changed" message. >> >> Actual result: no such message. >> >> The symptom is that the window size change function is not run after a >> mini-window size change. > > Note that resizing the mini-window involves resizing of at least one > other window on the same frame. So this is not exactly about the > mini-window. Noted. >> So far, I can produce this behavior only when the minibuffer or echo >> area grows to several lines; when it shrinks afterwards, my window size >> change function is called. > > I don't see the message even when the mini-window shrinks back. Are > you sure the message you see is not triggered by some other event? No, I'm not sure, and that turns out to be correct. Thanks for helping to clarify that point. > It's way too easy to trigger it, see below. I'm going to go into this in more detail, but I think that point can be restated as "since it's called frequently, in most cases, the hook function should return immediately after checking that the event isn't interesting", and I agree with that. >> I cannot reproduce the behavior with other windows. > > See above: at least one other window is resized in this recipe, so the > exemption is not about the mini-window itself, it's about any window > involved in resizing in this particular scenario. I understand what's happening, but wanted to make clear that the mini-window involvement is the decisive factor. >> Is this a bug? The documentation says: >> >> [...] to be called if the size of any window changes for any reason. >> >> Please correct me if I'm wrong, but when the minibuffer/echo area gets >> resized (and the windows on top of it, too), that counts as a change of >> size, I would say. > > I believe the original reason for this is largely historical: > window-size-change-functions exists since Emacs 19.29, whereas > automatic resizing of mini-window was introduced in Emacs 21. Since > Emacs before 21 didn't have the mini-window resizing functionality, > Emacs 21 was careful not to gratuitously trigger these functions by > something that is purely a new redisplay feature. > > That said, I wonder whether changing the code now to call these > functions due to automatic resizing would make sense. What would be > the real-life use cases for using that? I am beginning to suspect I'm missing something here. Is there another way for windows to find out they're resized? There's pre-redisplay function, but I believe that is called far, far more often. The real-life use case I'm thinking of (which I'm not advocating as a good idea, but am advocating strongly as something that it should be possible to do) is EXWM, an X window manager written in Elisp that creates dummy buffers and dynamically maps X windows to cover their Emacs windows. But it seems to me every attempt to make an Emacs window's graphical representation fit in with its environment requires a way of knowing when that window is resized; and, if it wants to warp the mouse cursor (again, almost certainly not a good idea, but something that should be possible) it needs to know when its position within the frame changed, as well, since mouse coordinates are frame-relative. For example, I don't see another way to implement dynamic fit-image-to-window applications. What I would consider a valid use case is to call doc-view-fit-page-to-window if the window has been resized (of course, a practical implementation would ensure that such automatic resize operations would not cause a full re-rendering to happen upon every size change, but that's an implementation detail). I suspect you will object that in that case, there's no harm in the minibuffer temporarily hiding some of the document, but imagine reading the last line on a PDF page and, for some reason, deciding to type in some of that text into the minibuffer: the minibuffer would resize, obstructing the last line, and you could no longer see the text you're trying to type in. (Yes, it's somewhat far-fetched, but you're the one suggesting to change the documented behaviour of a feature to remove it once and for all, and I'm the conservative one arguing we should keep it.) For something that's less far-fetched, I'm thinking of accessibility and automation applications: if you want to point a screen reader to the right text to read, you're going to need its X coordinates, which might change due to a mini-window resize (either because you want the screen reader to read the mini-window contents, or because a window above the minibuffer is close to its minimum height and needs to move vertically to accomodate the growing mini-window). And if your accessibility utility is stupid (I understand most are) and only allows you to enter text by warping the mouse pointer and generating key press events, well, that's easy if you know how to update its idea of where to type. I can think of many X hacks that would probably be possible with or without window-size-change-functions, but much easier to implement with that hook: for example, while I doubt this could be made really secure (you'd have to synchronize with x11vnc), you could clip x11vnc to a given Emacs window without revealing the contents of the minibuffer to the remote viewer. Use cases: 1. X hacks. I want to do something with the Emacs X windows and need to know their coordinates to do so. EXWM, accessibility, compositing WM hacks (transparency, duplicating windows, rendering them upside-down, whatever), VNC clipping. Most of those are possible without a reliable w-s-c-f hook, but are much easier to implement with one. 2. automatic-resize applications. These, strictly speaking, need to know only the window size, not its frame-relative position. 3. things I'm too stupid to think of. I think this is the big one: let's leave the feature in and see what smart people come up with. > I believe window-size-change-functions is meant for taking notice of > resizes done by the user or some Lisp code, not for automated resizes > whose sole purpose is to allow some message be read in its entirety. I disagree with the notion that the possible applications of a software feature are limited to what I can think of beforehand :-) You appear to be arguing for considering the mini-window a temporary overlay that obstructs "real" windows while it exists and goes away when it's no longer needed. That's fine, you can have that: create a minibuffer-only frame and raise or lower it to your heart's content. I'm arguing that the mini-window should be treated like any other window. That's what Emacs has historically implemented, it's still clearly reflected in the code, and as you yourself point out, auto-resizing affects all windows, not just mini-windows. Let's also consider the potential harm done by both options: not calling the hook when I think it should be will result in something being done to the wrong buffer. That's potentially disastrous, either because information is revealed that shouldn't be (my minibuffer extended into the VNC-shared window) or not accessible when it should be (while I specifically want the last line of my PDF to be visible at all times, the "temporary" minibuffer has covered it). In the case of EXWM, the mini-window will resize so the top of it is hidden underneath another X window, rendering echo area messages unreadable. Calling the hook "too often" will result, well, for most users, in twenty lost CPU cycles as a variable is tested and found to be nil. That's negligible. Those users that do have hooks, and specifically only want to do work if the normal size of the window changed, need one "equal" call to detect that window-normal-size hasn't changed and no work needs to be done. That's not quite as negligible, but on today's machines I think it's acceptable overhead to add to what's most likely a user-triggered action. That leaves a third group, that of users who have hooks and want them to do work only after explicit resizes, but wrote buggy code and accidentally called window-size instead of window-normal-size. While I sympathize with everyone who writes buggy code, I do not think that third group should be accommodated. > If you agree, then the current behavior will make sense to you. I hope I've made it clear so far that I disagree. > If anything, IMO we should _reduce_ the number of unrelated events > that trigger a call to these functions. For example, currently any > command that reads from the minibuffer will trigger it, because when > read-from-minibuffer exits, it restores the window configuration by > calling set-window-configuration, which is documented to trigger these > functions. That just doesn't make any sense to me, since most reads > from the minibuffer don't resize any windows! I believe that's a separate issue. From my point of view, it makes sense to call our hook, since one window (the echo area) has been replaced by another (the minibuffer). I do not understand why you seem so loath to calling a rarely-used hook somewhat more often; I conjecture you might be overestimating the cost of checking whether a window's size has actually changed and returning if it hasn't. I've attached a test file that uses benchmark-run to time a million invocations of the test-and-return code, and it takes less than two seconds on this very old machine; IOW, to take up 1% of one CPU core, you'd need 5000 resize events per second. I can run `redisplay' 20,000 times per second, and that's only because those calls return without doing anything. It's cheap. >> If this is merely a documentation issue, the exception should be noted >> in the manual. > > That's easy. Deciding what's TRT in this case is harder. Let's go for doing what the documentation says. I'm thinking of your implied proposal as two changes: 1. rip out the current feature 2. implement a new, crippled feature that satisfies some users of the old feature but not all of them. That's not "extensible". It's not conservative either, IMHO. Maybe it would be useful to add to the documentation of the hook an example of how to test whether a frame's current mini-window is at what the manual calls its "normal size", and to suggest that expensive hook functions not be run if it isn't. The exact code that does that is left as an exercise to the reader (i.e. I haven't figured it out yet). >> If this behavior is deliberate, I believe it is inconsistent to set >> FRAME_WINDOW_SIZES_CHANGED (f) in `resize-mini-window-internal'. > > It's consistent if we adopt the POV that this feature only catches > resizes from Lisp code. And those from explicit user interaction, yes. That's a valid POV but not, I think, the one we should adopt. Let me repeat that what I understand your POV to be is already fully accommodated by minibuffer-only frames: they're temporary and obstruct what's underneath them, but leave the window sizes in the real frame alone. The converse does not hold: if we remove the feature/fail to fix it, there's no good way to implement my bad ideas. Let's go for "extensible" and "customizable" over "negligibly faster". Sorry, again, that this email has gotten so long. (This is a separate issue, but on first reading it, the pre-redisplay-function(s) code seems like it would be a much better candidate for optimization and elimination, though that might require rewriting some of the simple.el code in C). Thank you again for responding, Pip [-- Attachment #2: emacs-benchmark-013.el --] [-- Type: text/x-emacs-lisp, Size: 498 bytes --] (defvar my-frame (selected-frame)) (defvar my-window (selected-window)) (defvar my-size nil) (defun w-s-c-f (frame) (and (eq frame my-frame) (not (equal (cons (window-size my-window) (window-size my-window t)) my-size)) (setq my-size (cons (window-size my-window) (window-size my-window t))))) (setq window-size-change-functions nil) (push #'w-s-c-f window-size-change-functions) (benchmark-run 1000000 (dolist (f window-size-change-functions) (funcall f (selected-frame)))) ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 18:13 ` Pip Cet @ 2015-08-24 19:03 ` Eli Zaretskii 2015-08-25 7:25 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-24 19:03 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Mon, 24 Aug 2015 18:13:29 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: 21333@debbugs.gnu.org > > Sorry this is a long email There's long, and there's long. I mean, geeez.... > - Redisplay is so slow that it really doesn't matter that we call an > extra bit of Lisp code before running it. I'm not worried by performance aspects of this. And this hook doesn't slow down redisplay anyway, because redisplay just sets a flag. > - Doing it "my way" is cheap in terms of performance (I benchmarked), > easy to implement, enables cool but not necessarily useful hacks, but > requires very slow hook functions to rate-limit themselves in order to > keep the minibuffer usable. This is not about performance. It's about utility and complexity. Having to program for a system where your hook is called in umpteen unrelated situations makes your program much more complex and fragile than it could be if the hook was more accurate. > - Doing it your way is even cheaper in terms of performance, not very > hard to implement, requires those hacks to hook into > pre-redisplay-functions instead, and has the questionable benefit of > allowing badly-written non-rate-limiting hook functions to keep the > minibuffer usable, but still break drag-resizing. Once again, this is not about performance. > >> I cannot reproduce the behavior with other windows. > > > > See above: at least one other window is resized in this recipe, so the > > exemption is not about the mini-window itself, it's about any window > > involved in resizing in this particular scenario. > > I understand what's happening, but wanted to make clear that the > mini-window involvement is the decisive factor. No, it's not. The decisive factor is that the way this is implemented simply doesn't set the flag in this situation. IOW, the code to set it wasn't written. > > That said, I wonder whether changing the code now to call these > > functions due to automatic resizing would make sense. What would be > > the real-life use cases for using that? > > I am beginning to suspect I'm missing something here. Is there another > way for windows to find out they're resized? They are not resized, from my POV. > For example, I don't see another way to implement dynamic > fit-image-to-window applications. > > What I would consider a valid use case is to call > doc-view-fit-page-to-window if the window has been resized (of course, > a practical implementation would ensure that such automatic resize > operations would not cause a full re-rendering to happen upon every > size change, but that's an implementation detail). How is this different from something else, like a tooltip or a drop-down menu, temporarily obscuring a portion of the display? If you want to make the hook scroll the display when the minibuffer is resized, you will cause annoying jumpy display when some Lisp program repeatedly displays echo-area messages and clears the echo area -- for example, consider a Lisp compilation of a large buffer that produces many warnings. Moreover, you are worried by the last line of a PDF text, but what about the first one? When the window becomes smaller, some part will have to disappear from display, there's no way around that. Why should we care about the last portion more than about the first? > For something that's less far-fetched, I'm thinking of accessibility > and automation applications: if you want to point a screen reader to > the right text to read, you're going to need its X coordinates, which > might change due to a mini-window resize But the coordinates of the text that stays on screen don't change in such a resize. Some text is obscured, but what's left doesn't move. So I see no problem here. > Use cases: > 1. X hacks. I want to do something with the Emacs X windows and need > to know their coordinates to do so. EXWM, accessibility, compositing > WM hacks (transparency, duplicating windows, rendering them > upside-down, whatever), VNC clipping. Most of those are possible > without a reliable w-s-c-f hook, but are much easier to implement with > one. I'm not sure I understand the use case, but if I do, it's impossible: you need to have redisplay finish before you can ask Emacs about coordinates of some point. So this cannot be done while the minibuffer is being resized. > 2. automatic-resize applications. These, strictly speaking, need to > know only the window size, not its frame-relative position. Already covered above. I see no problems here that are specific to this particular resize. > 3. things I'm too stupid to think of. I think this is the big one: > let's leave the feature in and see what smart people come up with. That's not a use case. > You appear to be arguing for considering the mini-window a temporary > overlay that obstructs "real" windows while it exists and goes away > when it's no longer needed. That's fine, you can have that: create a > minibuffer-only frame and raise or lower it to your heart's content. > > I'm arguing that the mini-window should be treated like any other > window. I'm not special-casing the mini-window here. I'm special-casing this particular case of resizing the windows. > Let's also consider the potential harm done by both options: not > calling the hook when I think it should be will result in something > being done to the wrong buffer. I don't see why it should, not without a more detailed description. > Calling the hook "too often" will result, well, for most users, in > twenty lost CPU cycles as a variable is tested and found to be nil. Once again, this isn't about performance. > > If you agree, then the current behavior will make sense to you. > > I hope I've made it clear so far that I disagree. Then I guess we will have to agree to disagree. > > If anything, IMO we should _reduce_ the number of unrelated events > > that trigger a call to these functions. For example, currently any > > command that reads from the minibuffer will trigger it, because when > > read-from-minibuffer exits, it restores the window configuration by > > calling set-window-configuration, which is documented to trigger these > > functions. That just doesn't make any sense to me, since most reads > > from the minibuffer don't resize any windows! > > I believe that's a separate issue. No, it's not. It's the same issue: this hook is already called in situations where it shouldn't have been, and thus imposes on the programmers who use it complex ways of deciding whether there was or wasn't a change they should care about. You suggest to add one more situation in that class, something that most application that define this hook shouldn't and don't care. It's the complexity that worries me. > I do not understand why you seem so loath to calling a rarely-used > hook somewhat more often; I conjecture you might be overestimating the > cost of checking whether a window's size has actually changed and > returning if it hasn't. I've attached a test file that uses > benchmark-run to time a million invocations of the test-and-return > code, and it takes less than two seconds on this very old machine; > IOW, to take up 1% of one CPU core, you'd need 5000 resize events per > second. I can run `redisplay' 20,000 times per second, and that's only > because those calls return without doing anything. It's cheap. Once again, this is not about performance. Never was. It's about complexity, IMO gratuitous in this case. Emacs is already too complex for this kinds of job due to several hooks that are called indiscriminately, and leave it to the programmer to figure out whether some change really happened. Anyway, it's clear we disagree. I just wanted to voice my protest against such creeping featurism. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-24 19:03 ` Eli Zaretskii @ 2015-08-25 7:25 ` martin rudalics 2015-08-25 15:12 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-25 7:25 UTC (permalink / raw) To: Eli Zaretskii, Pip Cet; +Cc: 21333 > But the coordinates of the text that stays on screen don't change in > such a resize. Some text is obscured, but what's left doesn't move. > So I see no problem here. I'm not sure what you mean here: When the minibuffer resizes and point is near the bottom of the window above, the window above will scroll and stick to the new window start position even after the minibuffer gets sized back. When the window above the minibuffer is a one line window or fixed-size, the window above that window will be subject to those changes. > No, it's not. It's the same issue: this hook is already called in > situations where it shouldn't have been, and thus imposes on the > programmers who use it complex ways of deciding whether there was or > wasn't a change they should care about. You suggest to add one more > situation in that class, something that most application that define > this hook shouldn't and don't care. It's the complexity that worries > me. You mean when ‘set-window-configuration’ doesn't change the size of a single window the hook shouldn't be called? Ideally it shouldn't but this is a problem similar to that of indenting a paragraph changing the buffer modified state although in reality nothing changed. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 7:25 ` martin rudalics @ 2015-08-25 15:12 ` Eli Zaretskii 2015-08-26 7:09 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-25 15:12 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Tue, 25 Aug 2015 09:25:14 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: 21333@debbugs.gnu.org > > > But the coordinates of the text that stays on screen don't change in > > such a resize. Some text is obscured, but what's left doesn't move. > > So I see no problem here. > > I'm not sure what you mean here: When the minibuffer resizes and point > is near the bottom of the window above, the window above will scroll and > stick to the new window start position even after the minibuffer gets > sized back. When the window above the minibuffer is a one line window > or fixed-size, the window above that window will be subject to those > changes. In these two cases, yes. In all the others (which are vast majority), no. And I'm still not sure I understand the relevance. How exactly knowing about the automatic resize will help with coordinates in this case? If the Lisp program recomputes coordinates inside the hook, it will get the same results in most cases (when point is not in the obscured lines). So an alternative that doesn't need any hook is simply to recompute the coordinates every time they are needed. It's not like this calculation is expensive, is it? > > No, it's not. It's the same issue: this hook is already called in > > situations where it shouldn't have been, and thus imposes on the > > programmers who use it complex ways of deciding whether there was or > > wasn't a change they should care about. You suggest to add one more > > situation in that class, something that most application that define > > this hook shouldn't and don't care. It's the complexity that worries > > me. > > You mean when ‘set-window-configuration’ doesn't change the size of a > single window the hook shouldn't be called? Yes, of course. > Ideally it shouldn't but this is a problem similar to that of > indenting a paragraph changing the buffer modified state although in > reality nothing changed. And we get regular complaints about that as well. Moreover, the setting of the modified status by fill-paragraph is just an annoyance that doesn't cost anyone any extra complexity, whereas the situation with this and similar hooks costs us quite a lot in that aspect. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-25 15:12 ` Eli Zaretskii @ 2015-08-26 7:09 ` martin rudalics 2015-08-26 10:07 ` Pip Cet 2015-08-26 15:32 ` Eli Zaretskii 0 siblings, 2 replies; 57+ messages in thread From: martin rudalics @ 2015-08-26 7:09 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > And I'm still not sure I understand the relevance. How exactly > knowing about the automatic resize will help with coordinates in this > case? If the Lisp program recomputes coordinates inside the hook, it > will get the same results in most cases (when point is not in the > obscured lines). So an alternative that doesn't need any hook is > simply to recompute the coordinates every time they are needed. It's > not like this calculation is expensive, is it? Correct. So ‘window-size-change-functions’ is dispensable. > And we get regular complaints about that as well. Moreover, the > setting of the modified status by fill-paragraph is just an annoyance > that doesn't cost anyone any extra complexity, whereas the situation > with this and similar hooks costs us quite a lot in that aspect. But you don't worry about performance anyway. OTOH for the person who writes the function on the hook it might hardly matter whether some window size really changed. The more important case might be to not miss one single case where the size really changes. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 7:09 ` martin rudalics @ 2015-08-26 10:07 ` Pip Cet 2015-08-26 13:01 ` martin rudalics 2015-08-26 15:36 ` Eli Zaretskii 2015-08-26 15:32 ` Eli Zaretskii 1 sibling, 2 replies; 57+ messages in thread From: Pip Cet @ 2015-08-26 10:07 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 1779 bytes --] On Wed, Aug 26, 2015 at 7:09 AM, martin rudalics <rudalics@gmx.at> wrote: >> So an alternative that doesn't need any hook is >> simply to recompute the coordinates every time they are needed. It's >> not like this calculation is expensive, is it? > > Correct. I disagree, if I understand correctly. The coordinates might be passed to another program, for example, and then we simply don't know when that program decides to use them, so we still need a way to update our (and its) idea of what the window size and position is, in my opinion. > So ‘window-size-change-functions’ is dispensable. Iff we keep/fix pre-redisplay-function, I agree. I had, wrongly, assumed that pre-redisplay-function is run many times per redisplay call, but that appears to be incorrect. The huge majority of redisplays call it just once, and that makes the overhead negligible. So what's the way forward, assuming my paperwork ever gets here? - document window-size-change-functions aren't called for mini-window resizes - document window-configuration-change-hook isn't called for mini-window resizes - document set-window-configuration doesn't call window-size-change-functions if nothing changed - fix set_window_configuration not to set the window size change flag unless there was an actual size change - wishlist item: call pre-redisplay-function between grow/shrink_mini_window and the actual X repaint, rather than before. - mark window-size-change-functions as obsolete and see whether anyone complains? > OTOH for the person who > writes the function on the hook it might hardly matter whether some > window size really changed. The more important case might be to not > miss one single case where the size really changes. Precisely. [-- Attachment #2: 0001-Document-and-fix-behavior-of-window-pre-redisplay-ho.patch --] [-- Type: text/x-patch, Size: 7522 bytes --] From bf0c0615392b7f796c23690e10620e21a5926ad4 Mon Sep 17 00:00:00 2001 From: Philip <pipcet@gmail.com> Date: Wed, 26 Aug 2015 09:34:12 +0000 Subject: [PATCH] Document and fix behavior of window/pre-redisplay hooks. * xdisp.c (redisplay_internal): Call `pre-redisplay-function' after resizing mini-windows rather than before. * window.c (Fset_window_configuration): Only set the window size change flag if a window actually changed size. * windows.texi (Window Configurations): Document `window-size-change-functions' isn't called for mini-window resizes. (Window Hooks): Likewise, likewise for `window-configuration-change-hook'. --- doc/lispref/windows.texi | 34 ++++++++++++++++------------------ src/window.c | 8 +++++++- src/xdisp.c | 46 +++++++++++++++++++++++++--------------------- 3 files changed, 48 insertions(+), 40 deletions(-) diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi index 4656938..bb9a5d2 100644 --- a/doc/lispref/windows.texi +++ b/doc/lispref/windows.texi @@ -3971,11 +3971,7 @@ was created for. The argument @var{configuration} must be a value that was previously returned by @code{current-window-configuration}. The configuration is restored in the frame from which @var{configuration} was made, whether -that frame is selected or not. This always counts as a window size -change and triggers execution of the @code{window-size-change-functions} -(@pxref{Window Hooks}), because @code{set-window-configuration} doesn't -know how to tell whether the new configuration actually differs from the -old one. +that frame is selected or not. If the frame from which @var{configuration} was saved is dead, all this function does is restore the three variables @code{window-min-height}, @@ -4010,8 +4006,8 @@ windows might be opened in other frames (@pxref{Choosing Window}), and configuration on the current frame. Do not use this macro in @code{window-size-change-functions}; exiting -the macro triggers execution of @code{window-size-change-functions}, -leading to an endless loop. +the macro can trigger execution of +@code{window-size-change-functions}, leading to an endless loop. @end defmac @defun window-configuration-p object @@ -4262,10 +4258,9 @@ work. @end defvar @defvar window-size-change-functions -This variable holds a list of functions to be called if the size of any -window changes for any reason. The functions are called just once per -redisplay, and just once for each frame on which size changes have -occurred. +This variable holds a list of functions to be called if the size of +any window changes. The functions are called just once per redisplay, +and just once for each frame on which size changes have occurred. Each function receives the frame as its sole argument. There is no direct way to find out which windows on that frame have changed size, or @@ -4275,20 +4270,23 @@ present sizes and the previous sizes. Creating or deleting windows counts as a size change, and therefore causes these functions to be called. Changing the frame size also -counts, because it changes the sizes of the existing windows. +counts, because it changes the sizes of the existing windows. When +the minibuffer or echo area grows or shrinks, these functions are not +called. You may use @code{save-selected-window} in these functions (@pxref{Selecting Windows}). However, do not use @code{save-window-excursion} (@pxref{Window Configurations}); exiting -that macro counts as a size change, which would cause these functions -to be called over and over. +that macro can count as a size change, which would cause these +functions to be called over and over. @end defvar @defvar window-configuration-change-hook -A normal hook that is run every time you change the window configuration -of an existing frame. This includes splitting or deleting windows, -changing the sizes of windows, or displaying a different buffer in a -window. +A normal hook that is run every time you change the window +configuration of an existing frame. This includes splitting or +deleting windows, changing the sizes of windows, or displaying a +different buffer in a window. However, this hook is not run for a size +change that is solely due to a resized minibuffer or echo area. The buffer-local part of this hook is run once for each window on the affected frame, with the relevant window selected and its buffer diff --git a/src/window.c b/src/window.c index 863a792..be1693c 100644 --- a/src/window.c +++ b/src/window.c @@ -6072,7 +6072,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 @@ -6124,6 +6123,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); diff --git a/src/xdisp.c b/src/xdisp.c index 8be7497..97fc0d8 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -11566,27 +11566,6 @@ prepare_menu_bars (void) tooltip_frame = Qnil; #endif - if (FUNCTIONP (Vpre_redisplay_function)) - { - Lisp_Object windows = all_windows ? Qt : Qnil; - if (all_windows && some_windows) - { - Lisp_Object ws = window_list (); - for (windows = Qnil; CONSP (ws); ws = XCDR (ws)) - { - Lisp_Object this = XCAR (ws); - struct window *w = XWINDOW (this); - if (w->redisplay - || XFRAME (w->frame)->redisplay - || XBUFFER (w->contents)->text->redisplay) - { - windows = Fcons (this, windows); - } - } - } - safe__call1 (true, Vpre_redisplay_function, windows); - } - /* Update all frame titles based on their buffer names, etc. We do this before the menu bars so that the buffer-menu will show the up-to-date frame titles. */ @@ -13514,6 +13493,31 @@ redisplay_internal (void) clear_garbaged_frames (); } + if (FUNCTIONP (Vpre_redisplay_function)) + { + bool all_windows = windows_or_buffers_changed || update_mode_lines; + bool some_windows = REDISPLAY_SOME_P (); + Lisp_Object windows = all_windows ? Qt : Qnil; + + if (all_windows && some_windows) + { + Lisp_Object ws = window_list (); + for (windows = Qnil; CONSP (ws); ws = XCDR (ws)) + { + Lisp_Object this = XCAR (ws); + struct window *w = XWINDOW (this); + if (w->redisplay + || XFRAME (w->frame)->redisplay + || XBUFFER (w->contents)->text->redisplay) + { + windows = Fcons (this, windows); + } + } + } + safe__call1 (true, Vpre_redisplay_function, windows); + } + + if (windows_or_buffers_changed && !update_mode_lines) /* Code that sets windows_or_buffers_changed doesn't distinguish whether only the windows's contents needs to be refreshed, or whether the -- 2.5.0 ^ permalink raw reply related [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 10:07 ` Pip Cet @ 2015-08-26 13:01 ` martin rudalics 2015-08-26 16:00 ` Pip Cet 2015-08-26 15:36 ` Eli Zaretskii 1 sibling, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-26 13:01 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 >>> So an alternative that doesn't need any hook is >>> simply to recompute the coordinates every time they are needed. It's >>> not like this calculation is expensive, is it? >> >> Correct. > > I disagree, if I understand correctly. My "correct" referred to Eli's "It's not like this calculation is expensive, is it?". I suppose we all agree on that. > The coordinates might be passed > to another program, for example, and then we simply don't know when > that program decides to use them, so we still need a way to update our > (and its) idea of what the window size and position is, in my opinion. I don't understand you here. IIUC Eli means that any application can store the old coordinates somewhere and easily compare them to the current ones in order to detect whether window sizes have changed. Even simpler: We could store the sizes in the window structure. This way prepare_menu_bars would simply walk the window tree of each frame to detect whether the size of any window has changed, call the functions on ‘window-size-change-functions’ and afterwards (better when redisplay completed) store the current size values into the old ones. The functions on the hook, OTOH, could check for each window individually whether and how its size changed. The advantage of this approach is that we never accumulate any fake size changes. >> So ‘window-size-change-functions’ is dispensable. > > Iff we keep/fix pre-redisplay-function, I agree. I had, wrongly, > assumed that pre-redisplay-function is run many times per redisplay > call, but that appears to be incorrect. The huge majority of > redisplays call it just once, and that makes the overhead negligible. We can declare ‘window-size-change-functions’ obsolete and suggest to use ‘pre-redisplay-function’ instead. Still we'd have to support it for quite some time. > So what's the way forward, assuming my paperwork ever gets here? > - document window-size-change-functions aren't called for mini-window resizes ... and maybe neither when the menu or tool bar wrap. Yes (if we decide to not change anything else). > - document window-configuration-change-hook isn't called for > mini-window resizes No (because we nowhere say that `window-configuration-change-hook' is called for resize operations). > - document set-window-configuration doesn't call > window-size-change-functions if nothing changed No (because IIRC we never document fixed bugs). > - fix set_window_configuration not to set the window size change flag > unless there was an actual size change Unless we decide to move that check to prepare_menu_bars as I sketched above. > - wishlist item: call pre-redisplay-function between > grow/shrink_mini_window and the actual X repaint, rather than before. Are you sure? If prepare_menu_bars is called before auto-resizing the minibuffer window, then ‘window-size-change-functions’ wouldn't catch those auto-resizes either. > - mark window-size-change-functions as obsolete and see whether > anyone complains? This will happen if we don't provide a good subsitute. > + 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)) You still didn't explain why you find it necessary to compare the pixel_left and pixel_top values. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 13:01 ` martin rudalics @ 2015-08-26 16:00 ` Pip Cet 2015-08-27 7:59 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-26 16:00 UTC (permalink / raw) To: martin rudalics; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 6481 bytes --] On Wed, Aug 26, 2015 at 1:01 PM, martin rudalics <rudalics@gmx.at> wrote: >>>> So an alternative that doesn't need any hook is >>>> simply to recompute the coordinates every time they are needed. It's >>>> not like this calculation is expensive, is it? >>> >>> Correct. >> >> I disagree, if I understand correctly. > > My "correct" referred to Eli's "It's not like this calculation is > expensive, is it?". I suppose we all agree on that. We do. >> The coordinates might be passed >> to another program, for example, and then we simply don't know when >> that program decides to use them, so we still need a way to update our >> (and its) idea of what the window size and position is, in my opinion. > > I don't understand you here. IIUC Eli means that any application can > store the old coordinates somewhere and easily compare them to the > current ones in order to detect whether window sizes have changed. I understood Eli differently. If I'm not misquoting him, his suggestion was to "simply recompute the coordinates every time they are needed". My objection to that is that in many applications, you don't know when the coordinates will be needed without changing other programs. > Even simpler: We could store the sizes in the window structure. This > way prepare_menu_bars would simply walk the window tree of each frame to > detect whether the size of any window has changed, call the functions on > ‘window-size-change-functions’ and afterwards (better when redisplay > completed) store the current size values into the old ones. Would (window-size) return the old or the new size values? > The > functions on the hook, OTOH, could check for each window individually > whether and how its size changed. The advantage of this approach is > that we never accumulate any fake size changes. Agreed. >>> So ‘window-size-change-functions’ is dispensable. >> >> Iff we keep/fix pre-redisplay-function, I agree. I had, wrongly, >> assumed that pre-redisplay-function is run many times per redisplay >> call, but that appears to be incorrect. The huge majority of >> redisplays call it just once, and that makes the overhead negligible. > > We can declare ‘window-size-change-functions’ obsolete and suggest to > use ‘pre-redisplay-function’ instead. Still we'd have to support it for > quite some time. Is there consensus for keeping pre-redisplay-function? >> So what's the way forward, assuming my paperwork ever gets here? >> - document window-size-change-functions aren't called for mini-window >> resizes > > ... and maybe neither when the menu or tool bar wrap. > > Yes (if we decide to not change anything else). > >> - document window-configuration-change-hook isn't called for >> mini-window resizes > > No (because we nowhere say that `window-configuration-change-hook' is > called for resize operations). windows.texi: @defvar window-configuration-change-hook A normal hook that is run every time you change the window configuration of an existing frame. This includes splitting or deleting windows, *changing the sizes of windows*, or displaying a different buffer in a window. I think it's at least possible to read that as applying to window resize operations. >> - document set-window-configuration doesn't call >> window-size-change-functions if nothing changed > > No (because IIRC we never document fixed bugs). Are you sure? It occurs to me I should have said "remove the bit in the documentation that says window-size-change-functions is always called by set-window-configuration". It's not about documenting the bugfix, but undocumenting the old bug. >> - fix set_window_configuration not to set the window size change flag >> unless there was an actual size change > > Unless we decide to move that check to prepare_menu_bars as I sketched > above. And get rid of the window size change flag entirely. That's a good idea. >> - wishlist item: call pre-redisplay-function between >> grow/shrink_mini_window and the actual X repaint, rather than before. > > Are you sure? If prepare_menu_bars is called before auto-resizing the > minibuffer window, then ‘window-size-change-functions’ wouldn't catch > those auto-resizes either. That's why I consider it "wishlist". My Christmas wish is to be told whenever the X rectangle corresponding to my Emacs window has changed, just before the X server is told about the change. Should that be impossible? As far as I can tell, the documentation reads like it should be possible. At least some people make use of it, if in ways you probably think useless. I consider the documentation to be more authoritative than current behavior: if a feature should work according to the documentation, but doesn't, I think the right thing to do is almost always to fix the feature and then, afterwards, discuss whether it should be removed. >> - mark window-size-change-functions as obsolete and see whether >> anyone complains? > > This will happen if we don't provide a good subsitute. Wait, don't you mean that will happen if we do provide a good substitute? >> + 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)) > > You still didn't explain why you find it necessary to compare the > pixel_left and pixel_top values. (Sorry, I missed that email initially! And thanks for pointing out my coding style typo, I'll try to be more careful in future.) I no longer do so. I think pre-redisplay-function, but not window-size-change-functions, should be told about a change in those values, so let's remove those checks. Reasons for calling pre-redisplay-function in these cases: Mouseover, if you need more than text properties give you. Cursor warping. X hacks. It's what the previous code did, so I didn't dare change it. One thing it occurs to me might be useful is an option to warp the mouse cursor when the echo area is resized, so you don't end up clicking the wrong link when the resize happens precisely at the wrong moment. An option I could live with, though it's not my preference, is to add a hook especially for mini-window resizes. Thank you again for spending so much time on this, Pip [-- Attachment #2: Type: text/html, Size: 7695 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 16:00 ` Pip Cet @ 2015-08-27 7:59 ` martin rudalics 2015-08-27 15:25 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-27 7:59 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > I understood Eli differently. If I'm not misquoting him, his suggestion was > to "simply recompute the coordinates every time they are needed". My > objection to that is that in many applications, you don't know when the > coordinates will be needed without changing other programs. There's no need to ever recompute coordinates. They are here all the time. What's needed is to record previous coordinates and to compare them with the actual ones. >> Even simpler: We could store the sizes in the window structure. This >> way prepare_menu_bars would simply walk the window tree of each frame to >> detect whether the size of any window has changed, call the functions on >> ‘window-size-change-functions’ and afterwards (better when redisplay >> completed) store the current size values into the old ones. > > Would (window-size) return the old or the new size values? All existing functions (like ‘window-size’) would obviously remain unchanged. We would provide a new function, say ‘window-old-size’, to return the width or height of a window at the time the last redisplay finished. > Is there consensus for keeping pre-redisplay-function? Sure. It's been only added in 2013. > windows.texi: > @defvar window-configuration-change-hook > A normal hook that is run every time you change the window configuration > of an existing frame. This includes splitting or deleting windows, > *changing the sizes of windows*, or displaying a different buffer in a > window. Damn. > I think it's at least possible to read that as applying to window resize > operations. At least. >>> - document set-window-configuration doesn't call >>> window-size-change-functions if nothing changed >> >> No (because IIRC we never document fixed bugs). > > Are you sure? It occurs to me I should have said "remove the bit in the > documentation that says window-size-change-functions is always called by > set-window-configuration". It's not about documenting the bugfix, but > undocumenting the old bug. You're right again. I suppose that what we wanted to say there is that we always call ‘window-configuration-change-hook’. >>> - wishlist item: call pre-redisplay-function between >>> grow/shrink_mini_window and the actual X repaint, rather than before. >> >> Are you sure? If prepare_menu_bars is called before auto-resizing the >> minibuffer window, then ‘window-size-change-functions’ wouldn't catch >> those auto-resizes either. > > That's why I consider it "wishlist". My Christmas wish is to be told > whenever the X rectangle corresponding to my Emacs window has changed, just > before the X server is told about the change. I still don't understand: Do you mean that ‘pre-redisplay-function’ is called in the wrong place? Where else would you call it? > Should that be impossible? As far as I can tell, the documentation reads > like it should be possible. At least some people make use of it, if in ways > you probably think useless. > > I consider the documentation to be more authoritative than current > behavior: if a feature should work according to the documentation, but > doesn't, I think the right thing to do is almost always to fix the feature > and then, afterwards, discuss whether it should be removed. If we can fix the feature, yes. >>> - mark window-size-change-functions as obsolete and see whether >>> anyone complains? >> >> This will happen if we don't provide a good subsitute. > > Wait, don't you mean that will happen if we do provide a good substitute? If ‘pre-redisplay-function’ can do the job it's a good substitute. >>> + 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)) >> >> You still didn't explain why you find it necessary to compare the >> pixel_left and pixel_top values. ... > Reasons for calling pre-redisplay-function in these cases: Mouseover, if > you need more than text properties give you. Cursor warping. X hacks. It's > what the previous code did, so I didn't dare change it. But this would imply that we also had to care about the case where a window's body width or height changes. Discriminating that is pretty difficult since we don't store that in a window configuration. And even the body width remains unchanged when I move a window's scroll bar from the left to the right. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 7:59 ` martin rudalics @ 2015-08-27 15:25 ` Eli Zaretskii 2015-08-27 16:35 ` Pip Cet 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 15:25 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Thu, 27 Aug 2015 09:59:04 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: Eli Zaretskii <eliz@gnu.org>, 21333@debbugs.gnu.org > > >>> - wishlist item: call pre-redisplay-function between > >>> grow/shrink_mini_window and the actual X repaint, rather than before. > >> > >> Are you sure? If prepare_menu_bars is called before auto-resizing the > >> minibuffer window, then ‘window-size-change-functions’ wouldn't catch > >> those auto-resizes either. > > > > That's why I consider it "wishlist". My Christmas wish is to be told > > whenever the X rectangle corresponding to my Emacs window has changed, just > > before the X server is told about the change. > > I still don't understand: Do you mean that ‘pre-redisplay-function’ is > called in the wrong place? Where else would you call it? I don't understand much more, it seems: the original wishlist feature. The call to grow/shrink_mini_window only recomputes data about the windows for the next redisplay cycle. When that next cycle comes, it will first call pre-redisplay-function and window-size-change-functions, from prepare_menu_bars, and then, after the rest of redisplay finishes, actually perform the X repaint, by calling update_frame. So it sounds like the wish has been granted already, no? Moreover, the scenario where "prepare_menu_bars is called before auto-resizing the minibuffer window", and as result "‘window-size-change-functions’ wouldn't catch those auto-resizes", seems impossible. If that's not what is being wished, please tell more about the issue. Likewise if I'm missing something here. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 15:25 ` Eli Zaretskii @ 2015-08-27 16:35 ` Pip Cet 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:57 ` Eli Zaretskii 0 siblings, 2 replies; 57+ messages in thread From: Pip Cet @ 2015-08-27 16:35 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 4461 bytes --] On Thu, Aug 27, 2015 at 3:25 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > Date: Thu, 27 Aug 2015 09:59:04 +0200 > > From: martin rudalics <rudalics@gmx.at> > > CC: Eli Zaretskii <eliz@gnu.org>, 21333@debbugs.gnu.org > > > > >>> - wishlist item: call pre-redisplay-function between > > >>> grow/shrink_mini_window and the actual X repaint, rather than before. > > >> > > >> Are you sure? If prepare_menu_bars is called before auto-resizing the > > >> minibuffer window, then ‘window-size-change-functions’ wouldn't catch > > >> those auto-resizes either. > > > > > > That's why I consider it "wishlist". My Christmas wish is to be told > > > whenever the X rectangle corresponding to my Emacs window has changed, > just > > > before the X server is told about the change. > > > > I still don't understand: Do you mean that ‘pre-redisplay-function’ is > > called in the wrong place? Where else would you call it? > > I don't understand much more, it seems: the original wishlist > feature. > The call to grow/shrink_mini_window only recomputes data > about the windows for the next redisplay cycle. No. It computes data about the windows for the cycle that's currently happening, that has already called prepare_menu_bars and will most likely not do so again. Note that grow_mini_window is called by redisplay_internal, via resize_mini_window, not just by display_echo_area. If you set breakpoints on prepare_menu_bars, grow_mini_window, and redisplay_internal, the log is: Breakpoint 12, redisplay_internal () at xdisp.c:13310 Breakpoint 10, prepare_menu_bars () at xdisp.c:11558 Breakpoint 11, grow_mini_window (w=0x12676a0, delta=15, pixelwise=true) at window.c:4491 Breakpoint 12, redisplay_internal () at xdisp.c:13310 The call order is that redisplay_internal calls prepare_menu_bars, then calls grow_mini_window, then performs the frame update. It doesn't go back to calling prepare_menu_bars, but it does call update_frame, and that actually does its job. If I stop just before the second redisplay_internal and x_sync(), the screen will be in a mildly inconsistent state. When that next cycle comes, it will first call pre-redisplay-function Yes. With a nil argument. I don't fully understand why. > and window-size-change-functions No. Miniwindow resizes do not set the WINDOW_SIZES_CHANGED flag even if they resize other windows. window-size-change-functions won't be called on the next redisplay, and it might not be called again for a very long time. > , from prepare_menu_bars, and then, after the rest of redisplay finishes, > actually perform the X repaint, by > calling update_frame. > No. The sequence is redisplay_internal, then prepare_menu_bars, then grow_mini_window, then update_frame. Breakpoint 14, redisplay_internal () at xdisp.c:13310 Breakpoint 10, prepare_menu_bars () at xdisp.c:11558 Breakpoint 11, grow_mini_window (w=0x12676a0, delta=15, pixelwise=true) at window.c:4491 Breakpoint 15, update_frame (f=0x319fa40, force_p=false, inhibit_hairy_id_p=false) at dispnew.c:3055 Breakpoint 15, update_frame (f=0x30a6070, force_p=false, inhibit_hairy_id_p=false) at dispnew.c:3055 Breakpoint 15, update_frame (f=0x313cc58, force_p=false, inhibit_hairy_id_p=false) at dispnew.c:3055 Breakpoint 15, update_frame (f=0x12ac950, force_p=false, inhibit_hairy_id_p=false) at dispnew.c:3055 Breakpoint 14, redisplay_internal () at xdisp.c:13310 > So it sounds like the wish has been granted already, no? If 1. I use pre-redisplay-function, not pre-redisplay-functions or window-size-change-functions 2. I ignore its arguments and check all windows 3. I don't mind that one X update has already happened with the new sizes immediately beforehand I get the behavior I want (modulo point 3). That's empirical, with the code that I posted that makes a window display its current size. > Moreover, the scenario where "prepare_menu_bars is > called before auto-resizing the minibuffer window", and as result > "‘window-size-change-functions’ wouldn't catch those auto-resizes", > seems impossible. > I don't think it's impossible, I think it's clearly happening to produce the breakpoint order that I'm seeing. (This is speculation, but I think my call order only applies to minibuffer window resizes, as stated above, not echo area resizes triggered by message3. That might be wrong, though). [-- Attachment #2: Type: text/html, Size: 6241 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 16:35 ` Pip Cet @ 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:57 ` Eli Zaretskii 1 sibling, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-27 17:59 UTC (permalink / raw) To: Pip Cet, Eli Zaretskii; +Cc: 21333 > The call order is that redisplay_internal calls prepare_menu_bars, then > calls grow_mini_window, then performs the frame update. It doesn't go back > to calling prepare_menu_bars, ... But this also means that your initial patch wouldn't help anyway. Even if we set WINDOW_SIZES_CHANGED in grow_/shrink_mini_window, ‘window-size-change-functions’ won't be called any more in the present cycle. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 16:35 ` Pip Cet 2015-08-27 17:59 ` martin rudalics @ 2015-08-27 18:57 ` Eli Zaretskii 2015-08-27 20:49 ` Pip Cet 1 sibling, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 18:57 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Thu, 27 Aug 2015 16:35:44 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: martin rudalics <rudalics@gmx.at>, 21333@debbugs.gnu.org > > The call to grow/shrink_mini_window only recomputes data > about the windows for the next redisplay cycle. > > No. It computes data about the windows for the cycle that's currently > happening, that has already called prepare_menu_bars and will most likely not > do so again. That's exactly what I said, just in other words. > Note that grow_mini_window is called by redisplay_internal, via > resize_mini_window, not just by display_echo_area. They are both called _after_ prepare_menu_bars. 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. > If you set breakpoints on prepare_menu_bars, grow_mini_window, and > redisplay_internal, the log is: > > Breakpoint 12, redisplay_internal () at xdisp.c:13310 > Breakpoint 10, prepare_menu_bars () at xdisp.c:11558 > Breakpoint 11, grow_mini_window (w=0x12676a0, delta=15, pixelwise=true) at > window.c:4491 > Breakpoint 12, redisplay_internal () at xdisp.c:13310 I already did that, before writing my message. > The call order is that redisplay_internal calls prepare_menu_bars, then calls > grow_mini_window, then performs the frame update. It doesn't go back to calling > prepare_menu_bars, but it does call update_frame, and that actually does its > job. Yes, and that is not what you want because?... > When that next cycle comes, it will first call pre-redisplay-function > > Yes. With a nil argument. I don't fully understand why. > > and window-size-change-functions > > No. Miniwindow resizes do not set the WINDOW_SIZES_CHANGED flag even if they > resize other windows. I was talking about the situation after you proposed changes, which will cause the flag to be set (AFAIU). > , from prepare_menu_bars, and then, after the rest of redisplay finishes, > actually perform the X repaint, by > calling update_frame. > > > 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 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. > Moreover, the scenario where "prepare_menu_bars is > called before auto-resizing the minibuffer window", and as result > "‘window-size-change-functions’ wouldn't catch those auto-resizes", > seems impossible. > > > I don't think it's impossible, I think it's clearly happening to produce the > breakpoint order that I'm seeing. (This is speculation, but I think my call > order only applies to minibuffer window resizes, as stated above, not echo area > resizes triggered by message3. That might be wrong, though). Careful with drawing conclusions from the call order alone. The fact that redisplay_internal was called doesn't mean it actually decided to redisplay a specific window, or any window. The fact that update_frame was called doesn't necessarily mean that anything at all was written to the glass. These functions have a lot of optimizations in them, and try to avoid doing stuff if they think it isn't necessary. You need to trace into the functions' guts to see if they actually update anything. Especially update_frame, which tries very hard to avoid writing to the glass, if it thinks the desired and the current contents are the same. That function is the last line of defense against redisplaying the same stuff over and over again. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 18:57 ` Eli Zaretskii @ 2015-08-27 20:49 ` Pip Cet 2015-08-28 10:02 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-27 20:49 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 7654 bytes --] On Thu, Aug 27, 2015 at 6:57 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > Date: Thu, 27 Aug 2015 16:35:44 +0000 > > From: Pip Cet <pipcet@gmail.com> > > Cc: martin rudalics <rudalics@gmx.at>, 21333@debbugs.gnu.org > > > > The call to grow/shrink_mini_window only recomputes data > > about the windows for the next redisplay cycle. > > > > No. It computes data about the windows for the cycle that's currently > > happening, that has already called prepare_menu_bars and will most > likely not > > do so again. > > That's exactly what I said, just in other words. > I'm sorry, I totally misread that, then. 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. I don't insist on my terminology, but I don't understand what yours is. I understood "next redisplay cycle" to refer to the redisplay cycle for which redisplay() has not yet been called, not the one for which redisplay() has already been called. > > Note that grow_mini_window is called by redisplay_internal, via > > resize_mini_window, not just by display_echo_area. > > They are both called _after_ prepare_menu_bars. Except when they're not, I've just seen this backtrace: #0 grow_mini_window (w=0x1265ef0, delta=210, pixelwise=true) at window.c:4488 #1 0x0000000000452c01 in resize_mini_window (w=0x1265ef0, exact_p=true) at xdisp.c:10833 #2 0x0000000000426fb9 in do_switch_frame (frame=54632789, track=1, for_deletion=0, norecord=43920) at frame.c:1163 #3 0x000000000042718d in Fselect_frame (frame=54632789, norecord=43920) at frame.c:1225 #4 0x0000000000496566 in select_window (window=54633277, norecord=43920, inhibit_point_swap=false) at window.c:499 #5 0x00000000004967de in Fselect_window (window=54633277, norecord=43920) at window.c:575 #6 0x0000000000453fe7 in x_consider_frame_title (frame=54632789) at xdisp.c:11487 #7 0x0000000000454437 in prepare_menu_bars () at xdisp.c:11595 That's calling resize_mini_window *between* pre-redisplay-function and window-size-change-functions. I had not thought of that possibility. This was triggered by C-x C-e in an elisp buffer after "(read-from-minibuffer longstring)". 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. I had assumed your message referred to the state of Emacs as of HEAD, not with my proposed changes included. > The call order is that redisplay_internal calls prepare_menu_bars, then > calls > > grow_mini_window, then performs the frame update. It doesn't go back to > calling > > prepare_menu_bars, but it does call update_frame, and that actually does > its > > job. > > Yes, and that is not what you want because?... > Because the next call to redisplay() might not be until five seconds later. > When that next cycle comes, it will first call pre-redisplay-function > > > > Yes. With a nil argument. I don't fully understand why. > > > > and window-size-change-functions > > > > No. Miniwindow resizes do not set the WINDOW_SIZES_CHANGED flag even if > they > > resize other windows. > > I was talking about the situation after you proposed changes, which > will cause the flag to be set (AFAIU). > That wasn't clear to me. If you're asking "will you shut up already if the changes you've already proposed go in", the answer is yes. So, in the hopes of finally clarifying matters: I want two things. 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. Wish #2: a way of knowing, in advance, that a window is going to be resized or moved, even temporarily, before any of that hits the X server. That would eliminate the (small) window left by #1 when the X server has been updated but redisplay hasn't been called again. 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. Wish #2 eliminates the race condition. > , from prepare_menu_bars, and then, after the rest of redisplay > finishes, > > actually perform the X repaint, by > > calling update_frame. > > > > > > 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"? 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'm still grateful for the warning, though. > > Moreover, the scenario where "prepare_menu_bars is > > called before auto-resizing the minibuffer window", and as result > > "‘window-size-change-functions’ wouldn't catch those auto-resizes", > > seems impossible. > > > > > > I don't think it's impossible, I think it's clearly happening to produce > the > > breakpoint order that I'm seeing. (This is speculation, but I think my > call > > order only applies to minibuffer window resizes, as stated above, not > echo area > > resizes triggered by message3. That might be wrong, though). > > Careful with drawing conclusions from the call order alone. > The fact > that redisplay_internal was called doesn't mean it actually decided to > redisplay a specific window, or any window. The fact that > update_frame was called doesn't necessarily mean that anything at all > was written to the glass. These functions have a lot of optimizations > in them, and try to avoid doing stuff if they think it isn't > necessary. You need to trace into the functions' guts to see if they > actually update anything. Especially update_frame, which tries very > hard to avoid writing to the glass, if it thinks the desired and the > current contents are the same. That function is the last line of > defense against redisplaying the same stuff over and over again. > Thanks for the warning. I verified with x_sync that the changes had been written to the glass. I think we agree on what's happening: 1. user input 2. redisplay called 3. prepare_menu_bars called, calls hooks. Hooks think nothing has changed. 4. grow_mini_window called, calculates new window size 5. update_frame is called, writes the new, enlarged mini-window to the glass 6. control returns to command loop 7. redisplay is called again 8. prepare_menu_bars called, calls hooks. Hooks notice mini-window has resized. Right? 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. Thanks again, particularly for the warning about update_frame not doing anything, Pip [-- Attachment #2: Type: text/html, Size: 10684 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 20:49 ` Pip Cet @ 2015-08-28 10:02 ` Eli Zaretskii 2015-08-28 12:34 ` Pip Cet 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-28 10:02 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Thu, 27 Aug 2015 20:49:17 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: rudalics@gmx.at, 21333@debbugs.gnu.org > > > The call to grow/shrink_mini_window only recomputes data > > about the windows for the next redisplay cycle. > > > > No. It computes data about the windows for the cycle that's currently > > happening, that has already called prepare_menu_bars and will most likely > not > > do so again. > > That's exactly what I said, just in other words. > > I'm sorry, I totally misread that, then. I don't think you've misread, no. See below. > 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. There's some misunderstanding here, but I'm not sure what it is. 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. 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; . both window-size-change-functions and pre-redisplay-function is being discussed, although it should be clear their purpose is different, 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; and . you introduce issues into the discussion that are not directly related to it, which just muddies the waters, for example: > > Note that grow_mini_window is called by redisplay_internal, via > > resize_mini_window, not just by display_echo_area. > > They are both called _after_ prepare_menu_bars. > > Except when they're not, I've just seen this backtrace: > > #0 grow_mini_window (w=0x1265ef0, delta=210, pixelwise=true) at window.c:4488 > #1 0x0000000000452c01 in resize_mini_window (w=0x1265ef0, exact_p=true) at > xdisp.c:10833 > #2 0x0000000000426fb9 in do_switch_frame (frame=54632789, track=1, > for_deletion=0, norecord=43920) at frame.c:1163 > #3 0x000000000042718d in Fselect_frame (frame=54632789, norecord=43920) at > frame.c:1225 > #4 0x0000000000496566 in select_window (window=54633277, norecord=43920, > inhibit_point_swap=false) at window.c:499 > #5 0x00000000004967de in Fselect_window (window=54633277, norecord=43920) at > window.c:575 > #6 0x0000000000453fe7 in x_consider_frame_title (frame=54632789) at > xdisp.c:11487 > #7 0x0000000000454437 in prepare_menu_bars () at xdisp.c:11595 This code path is realized when Emacs needs to perform a thorough redisplay, which happens relatively rarely. How does adding this complication to the already troubled discussion help to reach an agreement and move on? IMO, we should first agree on what happens in 99% of cases and on solutions to that, and then see if the remaining 1% needs any minor corrections. By contrast, if we keep adding more and more marginal cases, we are just expanding the tree of issues that need to be discussed without control and without any hope to advance any time soon. > 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. Because the frame's "windows changed" flag is tested before the flag is set (if it will be set by resize_mini_window). > I had assumed your message referred to the state of Emacs as of HEAD, not with > my proposed changes included. It was. > > The call order is that redisplay_internal calls prepare_menu_bars, then > calls > > grow_mini_window, then performs the frame update. It doesn't go back to > calling > > prepare_menu_bars, but it does call update_frame, and that actually does > its > > job. > > Yes, and that is not what you want because?... > > Because the next call to redisplay() might not be until five seconds later. > > > When that next cycle comes, it will first call pre-redisplay-function > > > > Yes. With a nil argument. I don't fully understand why. > > > > and window-size-change-functions > > > > No. Miniwindow resizes do not set the WINDOW_SIZES_CHANGED flag even if > they > > resize other windows. > > I was talking about the situation after you proposed changes, which > will cause the flag to be set (AFAIU). > > > That wasn't clear to me. If you're asking "will you shut up already if the > changes you've already proposed go in", the answer is yes. > > So, in the hopes of finally clarifying matters: > > I want two things. > 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 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. And please keep in mind that the first 2 steps mentioned above are not in a single place in the code. For example, redisplay_window attempts several optimizations, each one of which might eventually succeed; or all of them might fail, indicating to redisplay_internal that a thorough redisplay of the entire frame is needed. > Wish #2: a way of knowing, in advance, that a window is going to be resized or > moved, even temporarily, before any of that hits the X server. That would > eliminate the (small) window left by #1 when the X server has been updated but > redisplay hasn't been called again. See above: this seems to indicate that your wishlist hook needs to be called somewhere inside update_window. But where exactly, and whether indeed update_window is the right place, depends on what would you like to do with the resizing information. E.g., if you want to do something that will require another redisplay, then update_window is not the right place. In addition, it is not clear whether your hook will care if the window actually didn't change at all. That is something update_window decides on a screen line by screen line basis, as it goes about its job. The result can only be known at the end, and it's quite possible that we will have to add code to collect the results of individual screen lines (I didn't check if we already do something like that). Also, you never actually explained, AFAIR, why it is so important for you to be called "just before the change hits the X server". 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. > 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? 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. > Wish #2 eliminates the race condition. Maybe, but it also limits what you can usefully do from such a hook. > > 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. It all depends on how redisplay was entered and which windows/buffers have their 'redisplay' flags set. > 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, but you will see in update_frame and its subroutines code that compares the current and the desired contents of the displayed windows. That code only redraws the portions that actually changed. When redisplay decides nothing needs to be recomputed, the current and the desired contents will be identical. > 1. user input > 2. redisplay called That is inaccurate. Redisplay is called after Emacs returns to its main loop and finds that no input is available. 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. > 3. prepare_menu_bars called, calls hooks. Hooks think nothing has changed. > 4. grow_mini_window called, calculates new window size You omitted the main part of redisplay here, entered via redisplay_windows (to potentially redisplay all windows on a frame) or redisplay_window_1 (to redisplay only the selected window). That is where data required by update_frame is computed. > 5. update_frame is called, writes the new, enlarged mini-window to the glass > 6. control returns to command loop > 7. redisplay is called again > 8. prepare_menu_bars called, calls hooks. Hooks notice mini-window has resized. > > Right? Given what I wrote above, I think you will understand when I answer "yes and no". For starters, when you say "hooks" which one(s) do you have in mind? > 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. For which hook? You cannot do that for pre-redisplay-function, because it _must_ run before redisplay. (And "swap" is inaccurate, see my explanations above.) ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 10:02 ` Eli Zaretskii @ 2015-08-28 12:34 ` Pip Cet 2015-08-28 13:13 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: Pip Cet @ 2015-08-28 12:34 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- 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 --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 12:34 ` Pip Cet @ 2015-08-28 13:13 ` Eli Zaretskii 2015-08-28 13:26 ` Pip Cet 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-28 13:13 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Fri, 28 Aug 2015 12:34:53 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: rudalics@gmx.at, 21333@debbugs.gnu.org > > . 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" There are at least 2 differences. (1) pre-redisplay-function _must_ be run before the bulk of the redisplay code, because it is meant for functions that want to affect the current redisplay cycle. (2) The place where window-size-change-functions is currently called is too early, if it is supposed to pick up all of the resizes during this redisplay cycle, because later code that's part of redisplay could decide it wants to resize mini-window. More generally, if window-size-change-functions is supposed to be able to access the changed sizes, it must be called closer to the end of redisplay cycle. > > 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. But then the results of resizing will be incomplete for the first two alternatives. That might not be important for your wish, but could be important in other use cases. > 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. I do: redisplay knows that, so it could tell the hook, if we called the hook at the right place. > > 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. Of course you are allowed. I'm just saying it's not the good place for what I think you want to do. > > 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). Yes, I know. But how does that let you know what update_frame actually did? It could do very little or nothing at all, and you need to put a breakpoint at the right place (like the call to the draw_glyphs method of the display interface) to see which parts are being actually redrawn. > 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 I think step 1 should be omitted. It doesn't add any information, and is inaccurate (because input that triggers step 2 could come from other sources). > 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. Agreed. Which is why I said that your proposed change will cause window-size-change-functions to be called on the next redisplay cycle, not the one which actually resized the windows. > Are you suggesting a hook between steps 8 and 9? If we want window-size-change-functions to be able to say whether any windows were resized by the current redisplay cycle, I don't see how any other place would do. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 13:13 ` Eli Zaretskii @ 2015-08-28 13:26 ` Pip Cet 0 siblings, 0 replies; 57+ messages in thread From: Pip Cet @ 2015-08-28 13:26 UTC (permalink / raw) To: Eli Zaretskii; +Cc: 21333 [-- Attachment #1: Type: text/plain, Size: 1192 bytes --] As I said, that would be the perfect solution! On Fri, Aug 28, 2015 at 1:13 PM, Eli Zaretskii <eliz@gnu.org> wrote: > > 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). > > Yes, I know. But how does that let you know what update_frame > actually did? By actually looking at the screen to see what's on it. 1. set breakpoint in update_frame 2. "p x_sync($f)" (if necessary) 3. look at screen to see before-state 4. "finish" 5. "p x_sync($f)" 6. look at screen It could do very little or nothing at all, and you need > to put a breakpoint at the right place (like the call to the > draw_glyphs method of the display interface) to see which parts are > being actually redrawn. > I can conclude that all parts that changed on the screen have been redrawn. You're right that I cannot conclude that the other parts have not been redrawn, so my method is of limited utility, but in this case, it works. I agree with everything else you said in the last email. Pip [-- Attachment #2: Type: text/html, Size: 1903 bytes --] ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 10:07 ` Pip Cet 2015-08-26 13:01 ` martin rudalics @ 2015-08-26 15:36 ` Eli Zaretskii 2015-08-27 7:58 ` martin rudalics 1 sibling, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-26 15:36 UTC (permalink / raw) To: Pip Cet; +Cc: 21333 > Date: Wed, 26 Aug 2015 10:07:28 +0000 > From: Pip Cet <pipcet@gmail.com> > Cc: Eli Zaretskii <eliz@gnu.org>, 21333@debbugs.gnu.org > > >> So an alternative that doesn't need any hook is > >> simply to recompute the coordinates every time they are needed. It's > >> not like this calculation is expensive, is it? > > > > Correct. > > I disagree, if I understand correctly. The coordinates might be passed > to another program, for example, and then we simply don't know when > that program decides to use them, so we still need a way to update our > (and its) idea of what the window size and position is, in my opinion. If we don't know enough about that imaginary program, how do we know it is interested in ephemeral resizes such as the one being discussed? More generally, it is impossible to reason about a theoretical use case whose details are not revealed, while you alone keep the right to introduce additional traits that help you make your point, without describing the whole picture. A problem should be solved top-down, by starting with an overall description. It cannot be solved bottom up, one detail at a time. > > So ‘window-size-change-functions’ is dispensable. > > Iff we keep/fix pre-redisplay-function, I agree. There are no plans to retire it any time soon. Quite the contrary, I'd say. > > OTOH for the person who > > writes the function on the hook it might hardly matter whether some > > window size really changed. The more important case might be to not > > miss one single case where the size really changes. > > Precisely. There are simpler ways not to miss those events, using much simpler hooks. Like post-command-hook, for example. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 15:36 ` Eli Zaretskii @ 2015-08-27 7:58 ` martin rudalics 2015-08-27 15:24 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-27 7:58 UTC (permalink / raw) To: Eli Zaretskii, Pip Cet; +Cc: 21333 > There are simpler ways not to miss those events, using much simpler > hooks. Like post-command-hook, for example. A window's size might also change when input arrives. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 7:58 ` martin rudalics @ 2015-08-27 15:24 ` Eli Zaretskii 2015-08-27 17:58 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 15:24 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Thu, 27 Aug 2015 09:58:08 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: 21333@debbugs.gnu.org > > > There are simpler ways not to miss those events, using much simpler > > hooks. Like post-command-hook, for example. > > A window's size might also change when input arrives. In what way? I thought it can only change as part of redisplay, which always happens after some command finishes. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 15:24 ` Eli Zaretskii @ 2015-08-27 17:58 ` martin rudalics 2015-08-27 18:39 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-27 17:58 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 >> A window's size might also change when input arrives. > > In what way? I thought it can only change as part of redisplay, which > always happens after some command finishes. Always but not only. IIUC redisplay can also happen when running a timer or upon receiving input. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 17:58 ` martin rudalics @ 2015-08-27 18:39 ` Eli Zaretskii 2015-08-27 19:00 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 18:39 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Thu, 27 Aug 2015 19:58:37 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > >> A window's size might also change when input arrives. > > > > In what way? I thought it can only change as part of redisplay, which > > always happens after some command finishes. > > Always but not only. IIUC redisplay can also happen when running a > timer or upon receiving input. Only if the last command finished. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 18:39 ` Eli Zaretskii @ 2015-08-27 19:00 ` Eli Zaretskii 2015-08-28 8:04 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-27 19:00 UTC (permalink / raw) To: rudalics; +Cc: pipcet, 21333 > Date: Thu, 27 Aug 2015 21:39:15 +0300 > From: Eli Zaretskii <eliz@gnu.org> > Cc: pipcet@gmail.com, 21333@debbugs.gnu.org > > > Date: Thu, 27 Aug 2015 19:58:37 +0200 > > From: martin rudalics <rudalics@gmx.at> > > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > > > >> A window's size might also change when input arrives. > > > > > > In what way? I thought it can only change as part of redisplay, which > > > always happens after some command finishes. > > > > Always but not only. IIUC redisplay can also happen when running a > > timer or upon receiving input. > > Only if the last command finished. That was confusing. Let me rephrase: Emacs only reads subprocess input or runs timers when it's idle, so these events are only possible after some command finishes, and Emacs gets back to its command loop. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-27 19:00 ` Eli Zaretskii @ 2015-08-28 8:04 ` martin rudalics 2015-08-28 8:47 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-28 8:04 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > That was confusing. Let me rephrase: Emacs only reads subprocess > input or runs timers when it's idle, so these events are only possible > after some command finishes, and Emacs gets back to its command loop. Obviously. My point was that ‘post-command-hook’ won't catch these. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 8:04 ` martin rudalics @ 2015-08-28 8:47 ` Eli Zaretskii 2015-08-28 10:51 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-28 8:47 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Fri, 28 Aug 2015 10:04:25 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > > That was confusing. Let me rephrase: Emacs only reads subprocess > > input or runs timers when it's idle, so these events are only possible > > after some command finishes, and Emacs gets back to its command loop. > > Obviously. My point was that ‘post-command-hook’ won't catch these. Are you sure you aren't making some incorrect assumptions about the exact place where the command loop calls post-command-hook? What if it is called immediately after processing the above events? ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 8:47 ` Eli Zaretskii @ 2015-08-28 10:51 ` martin rudalics 2015-08-28 12:46 ` Eli Zaretskii 0 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2015-08-28 10:51 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 >> > That was confusing. Let me rephrase: Emacs only reads subprocess >> > input or runs timers when it's idle, so these events are only possible >> > after some command finishes, and Emacs gets back to its command loop. >> >> Obviously. My point was that ‘post-command-hook’ won't catch these. > > Are you sure you aren't making some incorrect assumptions about the > exact place where the command loop calls post-command-hook? What if > it is called immediately after processing the above events? I don't understand. We don't call ‘post-command-hook’ when resizing a window gets triggered by a timer. The only way an application can react to such a resize is currently either via ‘pre-redisplay-function’ or via ‘window-size-change-functions’. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 10:51 ` martin rudalics @ 2015-08-28 12:46 ` Eli Zaretskii 2015-08-28 13:05 ` martin rudalics 0 siblings, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-28 12:46 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Fri, 28 Aug 2015 12:51:28 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > >> > That was confusing. Let me rephrase: Emacs only reads subprocess > >> > input or runs timers when it's idle, so these events are only possible > >> > after some command finishes, and Emacs gets back to its command loop. > >> > >> Obviously. My point was that ‘post-command-hook’ won't catch these. > > > > Are you sure you aren't making some incorrect assumptions about the > > exact place where the command loop calls post-command-hook? What if > > it is called immediately after processing the above events? > > I don't understand. We don't call ‘post-command-hook’ when resizing a > window gets triggered by a timer. Can you show me some Lisp which causes this? I'd like to see what am I missing here. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-28 12:46 ` Eli Zaretskii @ 2015-08-28 13:05 ` martin rudalics 0 siblings, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-28 13:05 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 >> I don't understand. We don't call ‘post-command-hook’ when resizing a >> window gets triggered by a timer. > > Can you show me some Lisp which causes this? I'd like to see what am > I missing here. I'm not sure what you mean but put the following snippet into your *scratch* with emacs -Q and ‘eval-buffer’. (defvar timer nil) (defvar foo 0) (defvar bar 0) (defun foo () "..." (setq foo (1+ foo)) (if (>= (length (window-list)) 2) (delete-other-windows) (split-window)) (when (timerp timer) (cancel-timer timer)) (setq timer (run-with-idle-timer (time-add (current-idle-time) 2) t 'foo)) (message "foo: %s ... %s" foo bar)) (defun bar () "..." (setq bar (1+ bar)) (message "bar: %s ... %s" foo bar)) (setq timer (run-with-idle-timer 2 t 'foo)) (add-hook 'post-command-hook 'bar) You will see that eventually only foo increases while bar remains fixed. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 7:09 ` martin rudalics 2015-08-26 10:07 ` Pip Cet @ 2015-08-26 15:32 ` Eli Zaretskii 2015-08-27 7:57 ` martin rudalics 1 sibling, 1 reply; 57+ messages in thread From: Eli Zaretskii @ 2015-08-26 15:32 UTC (permalink / raw) To: martin rudalics; +Cc: pipcet, 21333 > Date: Wed, 26 Aug 2015 09:09:48 +0200 > From: martin rudalics <rudalics@gmx.at> > CC: pipcet@gmail.com, 21333@debbugs.gnu.org > > > And we get regular complaints about that as well. Moreover, the > > setting of the modified status by fill-paragraph is just an annoyance > > that doesn't cost anyone any extra complexity, whereas the situation > > with this and similar hooks costs us quite a lot in that aspect. > > But you don't worry about performance anyway. I said "complexity". A hook function for a hook that calls you indiscriminately can be very cheap performance-wise, but its complexity could be overwhelming, and the result could easily be fragile. > OTOH for the person who writes the function on the hook it might > hardly matter whether some window size really changed. The more > important case might be to not miss one single case where the size > really changes. Inaccurate hooks make this task very hard and prone to breakage with each new release. ^ permalink raw reply [flat|nested] 57+ messages in thread
* bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize 2015-08-26 15:32 ` Eli Zaretskii @ 2015-08-27 7:57 ` martin rudalics 0 siblings, 0 replies; 57+ messages in thread From: martin rudalics @ 2015-08-27 7:57 UTC (permalink / raw) To: Eli Zaretskii; +Cc: pipcet, 21333 > I said "complexity". A hook function for a hook that calls you > indiscriminately can be very cheap performance-wise, but its > complexity could be overwhelming, and the result could easily be > fragile. ... > Inaccurate hooks make this task very hard and prone to breakage with > each new release. We agree. And we are currently trying to become more discriminative. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
* Fix `window-configuration-change-hook' and `window-size-change-functions' 2015-08-24 14:35 ` Eli Zaretskii 2015-08-24 18:06 ` martin rudalics 2015-08-24 18:13 ` Pip Cet @ 2016-02-22 12:59 ` martin rudalics 2016-02-23 11:31 ` martin rudalics 2 siblings, 1 reply; 57+ messages in thread From: martin rudalics @ 2016-02-22 12:59 UTC (permalink / raw) Cc: emacs-devel [-- Attachment #1: Type: text/plain, Size: 3224 bytes --] From archived bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize: > If anything, IMO we should _reduce_ the number of unrelated events > that trigger a call to these functions. For example, currently any > command that reads from the minibuffer will trigger it, because when > read-from-minibuffer exits, it restores the window configuration by > calling set-window-configuration, which is documented to trigger these > functions. That just doesn't make any sense to me, since most reads > from the minibuffer don't resize any windows! I plan to install in the next days the attached patch which tries to fix this and also some other issues discussed in this thread. Please have a look. martin Fix `window-configuration-change-hook' and `window-size-change-functions' (1) Run `window-configuration-change-hook' if and only if a window was deleted/created or shows another buffer. (2) Run `window-size-change-functions' if and only if at least one window changed its size (in a few cases `window-size-change-functions' will also run when no window changed its size). (3) Provide two functions `window-old-pixel-height' and `window-old-pixel-width' that allow to easily detect which window changed size. * src/frame.h (struct frame): New boolean member window_configuration_changed. (FRAME_WINDOW_SIZES_CHANGED): Remove macro. (FRAME_WINDOW_CONFIGURATION_CHANGED): New macro. * src/frame.c (adjust_frame_size): Don't run `window-configuration-change-hook'. * src/window.h (struct window): New fields old_pixel_width and old_pixel_height. (WINDOW_INTERNAL_P): New macro. * src/window.c (Fwindow_old_pixel_width) (Fwindow_old_pixel_height): New functions. (Fdelete_other_windows_internal, Fwindow_resize_apply) (resize_frame_windows, Fsplit_window_internal) (Fdelete_window_internal, grow_mini_window) (shrink_mini_window, Fresize_mini_window_internal): Don't call FRAME_WINDOW_SIZES_CHANGED. (window_size_changed, window_set_old_pixel_sizes) (run_window_size_change_functions): New functions. (make_window): Initialize old_pixel_width and old_pixel_height fields. (Fdelete_window_internal): Don't call run_window_configuration_change_hook. (struct saved_window): Add old_pixel_height and old_pixel_width fields. (Fset_window_configuration): Try to identify window configuration changes correctly so run_window_configuration_change_hook and run_window_size_change_functions run only if configuration and size really changed. (save_window_save): Set the old_pixel_height and old_pixel_width fields. (Vwindow_size_change_functions): Move definiton from xdisp.c. * src/xdisp.c (prepare_menu_bars, redisplay_internal): Call run_window_size_change_functions. (Vwindow_size_change_functions): Move definition to window.c. * src/xfns.c (x_set_menu_bar_lines): Don't call run_window_configuration_change_hook. * doc/lispref/windows.texi (Window Sizes): Document new functions `window-old-pixel-height' and `window-old-pixel-width'. (Window Configurations): Mention that this may trigger execution of `window-size-change-functions' although no window changed size. (Window Hooks): Update descriptions of `window-size-change-functions' and `window-configuration-change-hook'. [-- Attachment #2: window-hooks.diff --] [-- Type: text/plain, Size: 58722 bytes --] diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi index f61f08a..eac3bd6 100644 --- a/doc/lispref/windows.texi +++ b/doc/lispref/windows.texi @@ -545,6 +545,12 @@ Window Sizes children. @end defun +@defun window-old-pixel-height &optional Lisp_Object &optional window +This function returns the height of window @var{window} in pixels at the +time @code{window-size-change-functions} was run for the last time on +@var{window}'s frame (@pxref{Window Hooks}). +@end defun + @cindex window pixel width @cindex pixel width of a window @cindex total pixel width of a window @@ -559,6 +565,12 @@ Window Sizes the screen areas spanned by its children. @end defun +@defun window-old-pixel-width &optional Lisp_Object &optional window +This function returns the width of window @var{window} in pixels at the +time @code{window-size-change-functions} was run for the last time on +@var{window}'s frame (@pxref{Window Hooks}). +@end defun + @cindex full-width window @cindex full-height window The following functions can be used to determine whether a given @@ -4087,11 +4099,11 @@ Window Configurations The argument @var{configuration} must be a value that was previously returned by @code{current-window-configuration}. The configuration is restored in the frame from which @var{configuration} was made, whether -that frame is selected or not. This always counts as a window size -change and triggers execution of the @code{window-size-change-functions} -(@pxref{Window Hooks}), because @code{set-window-configuration} doesn't -know how to tell whether the new configuration actually differs from the -old one. +that frame is selected or not. In some rare cases this may trigger +execution of the @code{window-size-change-functions} (@pxref{Window +Hooks}) even if the size of windows did not change at all. The +@code{window-configuration-change-hook} functions will be called if and +only if at least one window was added to or deleted from the frame. If the frame from which @var{configuration} was saved is dead, all this function does is restore the three variables @code{window-min-height}, @@ -4378,33 +4390,37 @@ Window Hooks @end defvar @defvar window-size-change-functions -This variable holds a list of functions to be called if the size of -any window changes for any reason. The functions are called at the -beginning of a redisplay cycle, and just once for each frame on which -size changes have occurred. - -Each function receives the frame as its sole argument. There is no -direct way to find out which windows on that frame have changed size, or -precisely how. However, if a size-change function records, at each -call, the existing windows and their sizes, it can also compare the -present sizes and the previous sizes. - -Creating or deleting windows counts as a size change, and therefore -causes these functions to be called. Changing the frame size also -counts, because it changes the sizes of the existing windows. +This variable holds a list of functions to be called if the size of any +window changes for any reason. The functions are called once per +redisplay, and once for each frame on which size changes have occurred. + +Each function receives the frame as its sole argument. To find out +whether a specific window has changed size, compare the return values of +@code{window-old-pixel-width} and @code{window-pixel-width} respectively +@code{window-old-pixel-height} and @code{window-pixel-height} for that +window (@pxref{Window Sizes}). + +These function are usually only called when at least one window was +added or has changed size since the last time this hook was run for the +associated frame. In some rare cases this hook also runs when a window +that was added intermittently has been deleted afterwards. In these +cases none of the windows on the frame will appear to have changed its +size. You may use @code{save-selected-window} in these functions (@pxref{Selecting Windows}). However, do not use @code{save-window-excursion} (@pxref{Window Configurations}); exiting -that macro counts as a size change, which would cause these functions -to be called over and over. +that macro counts as a size change, which would cause these functions to +be called again. @end defvar @defvar window-configuration-change-hook -A normal hook that is run every time you change the window configuration -of an existing frame. This includes splitting or deleting windows, -changing the sizes of windows, or displaying a different buffer in a -window. +A normal hook that is run every time the window configuration of a frame +changes. Window configuration changes include splitting and deleting +windows and the display of a different buffer in a window. Resizing the +frame or individual windows do not count as configuration changes. Use +@code{window-size-change-functions}, see above, when you want to track +size changes that are not caused by the deletion or creation of windows. The buffer-local part of this hook is run once for each window on the affected frame, with the relevant window selected and its buffer diff --git a/lisp/window.el b/lisp/window.el index e4669c1..1c679ae 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -3136,10 +3136,10 @@ window--sanitize-window-sizes (walk-window-tree (lambda (window) (let ((delta (- (window-min-size window horizontal nil t) - (window-size window horizontal t)))) - (when (> delta 0) - (if (window-resizable-p window delta horizontal nil t) - (window-resize window delta horizontal nil t) + (window-size window horizontal t)))) + (when (> delta 0) + (if (window-resizable-p window delta horizontal nil t) + (window-resize window delta horizontal nil t) (setq value nil)))))) value)) diff --git a/src/frame.c b/src/frame.c index 8c86afe..df473ae 100644 --- a/src/frame.c +++ b/src/frame.c @@ -591,8 +591,6 @@ adjust_frame_size (struct frame *f, int new_width, int new_height, int inhibit, || new_pixel_height != old_pixel_height); unblock_input (); - - run_window_configuration_change_hook (f); } /* Allocate basically initialized frame. */ diff --git a/src/frame.h b/src/frame.h index 71dab4b..d9424ab 100644 --- a/src/frame.h +++ b/src/frame.h @@ -288,8 +288,9 @@ struct frame cleared. */ bool_bf explicit_name : 1; - /* True if size of some window on this frame has changed. */ - bool_bf window_sizes_changed : 1; + /* True if configuration of windows on this frame has changed since + last call of run_window_size_change_functions. */ + bool_bf window_configuration_changed : 1; /* True if the mouse has moved on this display device since the last time we checked. */ @@ -828,10 +829,10 @@ default_pixels_per_inch_y (void) are frozen on frame F. */ #define FRAME_WINDOWS_FROZEN(f) (f)->frozen_window_starts -/* True if a size change has been requested for frame F - but not yet really put into effect. This can be true temporarily - when an X event comes in at a bad time. */ -#define FRAME_WINDOW_SIZES_CHANGED(f) (f)->window_sizes_changed +/* True if the frame's window configuration has changed since last call + of run_window_size_change_functions. */ +#define FRAME_WINDOW_CONFIGURATION_CHANGED(f) \ + ((f)->window_configuration_changed) /* The minibuffer window of frame F, if it has one; otherwise nil. */ #define FRAME_MINIBUF_WINDOW(f) f->minibuffer_window diff --git a/src/window.c b/src/window.c index e1a30ee..7267a25 100644 --- a/src/window.c +++ b/src/window.c @@ -720,6 +720,30 @@ the height of the screen areas spanned by its children. */) return make_number (decode_valid_window (window)->pixel_height); } +DEFUN ("window-old-pixel-width", Fwindow_old_pixel_width, Swindow_old_pixel_width, 0, 1, 0, + doc: /* Return the old width of window WINDOW in pixels. +WINDOW must be a valid window and defaults to the selected one. + +The return value is the pixel width of WINDOW at the last time +`window-size-change-functions' was run. It's zero if WINDOW was made +after that. */) + (Lisp_Object window) +{ + return make_number (decode_valid_window (window)->old_pixel_width); +} + +DEFUN ("window-old-pixel-height", Fwindow_old_pixel_height, Swindow_old_pixel_height, 0, 1, 0, + doc: /* Return the old height of window WINDOW in pixels. +WINDOW must be a valid window and defaults to the selected one. + +The return value is the pixel height of WINDOW at the last time +`window-size-change-functions' was run. It's zero if WINDOW was made +after that. */) + (Lisp_Object window) +{ + return make_number (decode_valid_window (window)->old_pixel_height); +} + DEFUN ("window-total-height", Fwindow_total_height, Swindow_total_height, 0, 2, 0, doc: /* Return the height of window WINDOW in lines. WINDOW must be a valid window and defaults to the selected one. @@ -2879,6 +2903,7 @@ window-start value is reasonable when this function is called. */) Lisp_Object sibling, pwindow, swindow IF_LINT (= Qnil), delta; ptrdiff_t startpos IF_LINT (= 0), startbyte IF_LINT (= 0); int top IF_LINT (= 0), new_top; + bool resize_failed = false; w = decode_valid_window (window); XSETWINDOW (window, w); @@ -2978,8 +3003,6 @@ window-start value is reasonable when this function is called. */) fset_redisplay (f); Vwindow_list = Qnil; - FRAME_WINDOW_SIZES_CHANGED (f) = true; - bool resize_failed = false; if (!WINDOW_LEAF_P (w)) { @@ -3229,6 +3252,76 @@ If WINDOW is omitted or nil, it defaults to the selected window. */) return Qnil; } + +/* Compare old and present pixel sizes of windows in tree rooted at W. + Return true iff any of these windows differs in size. */ + +static bool +window_size_changed (struct window *w) +{ + if (w->pixel_width != w->old_pixel_width + || w->pixel_height != w->old_pixel_height) + return true; + + if (WINDOW_INTERNAL_P (w)) + { + w = XWINDOW (w->contents); + while (w) + { + if (window_size_changed (w)) + return true; + + w = NILP (w->next) ? 0 : XWINDOW (w->next); + } + } + + return false; +} + +/* Set old pixel sizes of windows in tree rooted at W to their present + pixel sizes. */ + +static void +window_set_old_pixel_sizes (struct window *w) +{ + w->old_pixel_width = w->pixel_width; + w->old_pixel_height = w->pixel_height; + + if (WINDOW_INTERNAL_P (w)) + { + w = XWINDOW (w->contents); + while (w) + { + window_set_old_pixel_sizes (w); + w = NILP (w->next) ? 0 : XWINDOW (w->next); + } + } +} + + +void +run_window_size_change_functions (Lisp_Object frame) +{ + struct frame *f = XFRAME (frame); + struct window *r = XWINDOW (FRAME_ROOT_WINDOW (f)); + Lisp_Object functions = Vwindow_size_change_functions; + + if (FRAME_WINDOW_CONFIGURATION_CHANGED (f) || + window_size_changed (r)) + { + while (CONSP (functions)) + { + if (!EQ (XCAR (functions), Qt)) + call1 (XCAR (functions), frame); + functions = XCDR (functions); + } + + window_set_old_pixel_sizes (r); + FRAME_WINDOW_CONFIGURATION_CHANGED (f) = false; + } +} + + /* Make WINDOW display BUFFER. RUN_HOOKS_P means it's allowed to run hooks. See make_frame for a case where it's not allowed. KEEP_MARGINS_P means that the current margins, fringes, and @@ -3263,15 +3356,9 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, if (!(keep_margins_p && samebuf)) { /* If we're not actually changing the buffer, don't reset hscroll - and vscroll. This case happens for example when called from - change_frame_size_1, where we use a dummy call to - Fset_window_buffer on the frame's selected window (and no - other) just in order to run window-configuration-change-hook - (no longer true since change_frame_size_1 directly calls - run_window_configuration_change_hook). Resetting hscroll and - vscroll here is problematic for things like image-mode and - doc-view-mode since it resets the image's position whenever we - resize the frame. */ + and vscroll. Resetting hscroll and vscroll here is problematic + for things like image-mode and doc-view-mode since it resets + the image's position whenever we resize the frame. */ w->hscroll = w->min_hscroll = w->hscroll_whole = 0; w->suspend_auto_hscroll = false; w->vscroll = 0; @@ -3283,10 +3370,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, w->start_at_line_beg = false; w->force_start = false; } - /* Maybe we could move this into the `if' but it's not obviously safe and - I doubt it's worth the trouble. */ - wset_redisplay (w); + wset_redisplay (w); wset_update_mode_line (w); /* We must select BUFFER to run the window-scroll-functions and to look up @@ -3314,7 +3399,7 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, if (run_hooks_p) { - if (! NILP (Vwindow_scroll_functions)) + if (!NILP (Vwindow_scroll_functions)) run_hook_with_args_2 (Qwindow_scroll_functions, window, Fmarker_position (w->start)); if (!samebuf) @@ -3559,6 +3644,8 @@ make_window (void) w->phys_cursor_width = -1; #endif w->sequence_number = ++sequence_number; + w->old_pixel_width = 0; + w->old_pixel_height = 0; w->scroll_bar_width = -1; w->scroll_bar_height = -1; w->column_number_displayed = -1; @@ -3922,7 +4009,6 @@ be applied on the Elisp level. */) window_resize_apply (r, horflag); fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); @@ -4087,7 +4173,6 @@ resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise) } } - FRAME_WINDOW_SIZES_CHANGED (f) = true; fset_redisplay (f); } @@ -4214,7 +4299,6 @@ set correctly. See the code of `split-window' for how this is done. */) p = XWINDOW (o->parent); fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; new = make_window (); n = XWINDOW (new); wset_frame (n, frame); @@ -4383,7 +4467,6 @@ Signal an error when WINDOW is the only window on its frame. */) fset_redisplay (f); Vwindow_list = Qnil; - FRAME_WINDOW_SIZES_CHANGED (f) = true; wset_next (w, Qnil); /* Don't delete w->next too. */ free_window_matrices (w); @@ -4451,9 +4534,6 @@ Signal an error when WINDOW is the only window on its frame. */) } else unblock_input (); - - /* Must be run by the caller: - run_window_configuration_change_hook (f); */ } else /* We failed: Relink WINDOW into window tree. */ @@ -4527,7 +4607,6 @@ grow_mini_window (struct window *w, int delta, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } @@ -4567,7 +4646,6 @@ shrink_mini_window (struct window *w, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } @@ -4610,7 +4688,6 @@ DEFUN ("resize-mini-window-internal", Fresize_mini_window_internal, Sresize_mini w->top_line = r->top_line + r->total_lines; fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); return Qt; @@ -5948,6 +6025,7 @@ struct saved_window Lisp_Object window, buffer, start, pointm, old_pointm; Lisp_Object pixel_left, pixel_top, pixel_height, pixel_width; + Lisp_Object old_pixel_height, old_pixel_width; Lisp_Object left_col, top_line, total_cols, total_lines; Lisp_Object normal_cols, normal_lines; Lisp_Object hscroll, min_hscroll, hscroll_whole, suspend_auto_hscroll; @@ -6063,6 +6141,12 @@ the return value is nil. Otherwise the value is t. */) struct window *root_window; struct window **leaf_windows; ptrdiff_t i, k, n_leaf_windows; + /* Records whether a window has been added or removed wrt the + original configuration. */ + bool window_changed = false; + /* Records whether a window has changed its buffer wrt the + original configuration. */ + bool buffer_changed = false; /* Don't do this within the main loop below: This may call Lisp code and is thus potentially unsafe while input is blocked. */ @@ -6071,12 +6155,18 @@ the return value is nil. Otherwise the value is t. */) p = SAVED_WINDOW_N (saved_windows, k); window = p->window; w = XWINDOW (window); + + if (NILP (w->contents)) + /* A dead window that will be resurrected, the window + configuration will change. */ + window_changed = true; + if (BUFFERP (w->contents) && !EQ (w->contents, p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If a window we restore gets another buffer, record the window's old buffer. */ - call1 (Qrecord_window_buffer, window); + call1 (Qrecord_window_buffer, window); } /* Disallow x_set_window_size, temporarily. */ @@ -6100,7 +6190,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 @@ -6156,6 +6245,8 @@ the return value is nil. Otherwise the value is t. */) w->pixel_top = XFASTINT (p->pixel_top); w->pixel_width = XFASTINT (p->pixel_width); w->pixel_height = XFASTINT (p->pixel_height); + w->old_pixel_width = XFASTINT (p->old_pixel_width); + w->old_pixel_height = XFASTINT (p->old_pixel_height); w->left_col = XFASTINT (p->left_col); w->top_line = XFASTINT (p->top_line); w->total_cols = XFASTINT (p->total_cols); @@ -6203,6 +6294,9 @@ the return value is nil. Otherwise the value is t. */) if (BUFFERP (p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If saved buffer is alive, install it. */ { + if (!EQ (w->contents, p->buffer)) + /* Record buffer configuration change. */ + buffer_changed = true; wset_buffer (w, p->buffer); w->start_at_line_beg = !NILP (p->start_at_line_beg); set_marker_restricted (w->start, p->start, w->contents); @@ -6236,6 +6330,8 @@ the return value is nil. Otherwise the value is t. */) else if (!NILP (w->start)) /* Leaf window has no live buffer, get one. */ { + /* Record buffer configuration change. */ + buffer_changed = true; /* Get the buffer via other_buffer_safely in order to avoid showing an unimportant buffer and, if necessary, to recreate *scratch* in the course (part of Juanma's bs-show @@ -6283,7 +6379,10 @@ the return value is nil. Otherwise the value is t. */) /* Now, free glyph matrices in windows that were not reused. */ for (i = 0; i < n_leaf_windows; i++) if (NILP (leaf_windows[i]->contents)) - free_window_matrices (leaf_windows[i]); + { + free_window_matrices (leaf_windows[i]); + window_changed = true; + } /* Allow x_set_window_size again and apply frame size changes if needed. */ @@ -6303,7 +6402,8 @@ the return value is nil. Otherwise the value is t. */) /* Record the selected window's buffer here. The window should already be the selected one from the call above. */ - select_window (data->current_window, Qnil, false); + if (WINDOW_LIVE_P (data->current_window)) + select_window (data->current_window, Qnil, false); /* Fselect_window will have made f the selected frame, so we reselect the proper frame here. Fhandle_switch_frame will change the @@ -6313,7 +6413,32 @@ the return value is nil. Otherwise the value is t. */) if (FRAME_LIVE_P (XFRAME (data->selected_frame))) do_switch_frame (data->selected_frame, 0, 0, Qnil); - run_window_configuration_change_hook (f); + if (window_changed) + /* At least one window has been added or removed. Run + `window-configuration-change-hook' and make sure + `window-size-change-functions' get run later. + + We have to do this in order to capture the following + scenario: Suppose our frame contains two live windows W1 and + W2 and ‘set-window-configuration’ replaces them by two + windows W3 and W4 that were dead the last time + run_window_size_change_functions was run. If W3 and W4 have + the same values for their old and new pixel sizes but these + values differ from those of W1 and W2, the sizes of our + frame's two live windows changed but window_size_changed has + no means to detect that fact. + + Obviously, this will get us false positives, for example, + when we restore the original configuration with W1 and W2 + before run_window_size_change_functions gets called. */ + { + run_window_configuration_change_hook (f); + FRAME_WINDOW_CONFIGURATION_CHANGED (f) = true; + } + else if (buffer_changed) + /* At least one window has changed its buffer. Run + `window-configuration-change-hook' only. */ + run_window_configuration_change_hook (f); } if (!NILP (new_current_buffer)) @@ -6464,6 +6589,8 @@ save_window_save (Lisp_Object window, struct Lisp_Vector *vector, ptrdiff_t i) p->pixel_top = make_number (w->pixel_top); p->pixel_width = make_number (w->pixel_width); p->pixel_height = make_number (w->pixel_height); + p->old_pixel_width = make_number (w->old_pixel_width); + p->old_pixel_height = make_number (w->old_pixel_height); p->left_col = make_number (w->left_col); p->top_line = make_number (w->top_line); p->total_cols = make_number (w->total_cols); @@ -7246,6 +7373,16 @@ selected; while the global part is run only once for the modified frame, with the relevant frame selected. */); Vwindow_configuration_change_hook = Qnil; + DEFVAR_LISP ("window-size-change-functions", Vwindow_size_change_functions, + doc: /* Functions called during redisplay, if window sizes have changed. +The value should be a list of functions that take one argument. +During the first part of redisplay, for each frame, if any of its windows +have changed size since the last redisplay, or have been split or deleted, +all the functions in the list are called, with the frame as argument. +If redisplay decides to resize the minibuffer window, it calls these +functions on behalf of that as well. */); + Vwindow_size_change_functions = Qnil; + DEFVAR_LISP ("recenter-redisplay", Vrecenter_redisplay, doc: /* Non-nil means `recenter' redraws entire frame. If this option is non-nil, then the `recenter' command with a nil @@ -7374,6 +7511,8 @@ displayed after a scrolling operation to be somewhat inaccurate. */); defsubr (&Swindow_use_time); defsubr (&Swindow_pixel_width); defsubr (&Swindow_pixel_height); + defsubr (&Swindow_old_pixel_width); + defsubr (&Swindow_old_pixel_height); defsubr (&Swindow_total_width); defsubr (&Swindow_total_height); defsubr (&Swindow_normal_size); diff --git a/src/window.h b/src/window.h index c29207d..dea1d19 100644 --- a/src/window.h +++ b/src/window.h @@ -214,6 +214,11 @@ struct window int pixel_width; int pixel_height; + /* The pixel sizes of the window at the last time + `window-size-change-functions' was run. */ + int old_pixel_width; + int old_pixel_height; + /* The size of the window. */ int total_cols; int total_lines; @@ -499,15 +504,17 @@ wset_next_buffers (struct window *w, Lisp_Object val) #define WINDOW_LEAF_P(W) \ (BUFFERP ((W)->contents)) -/* True if W is a member of horizontal combination. */ +/* Non-nil if W is internal. */ +#define WINDOW_INTERNAL_P(W) \ + (WINDOWP ((W)->contents)) +/* True if W is a member of horizontal combination. */ #define WINDOW_HORIZONTAL_COMBINATION_P(W) \ - (WINDOWP ((W)->contents) && (W)->horizontal) + (WINDOW_INTERNAL_P (W) && (W)->horizontal) /* True if W is a member of vertical combination. */ - #define WINDOW_VERTICAL_COMBINATION_P(W) \ - (WINDOWP ((W)->contents) && !(W)->horizontal) + (WINDOW_INTERNAL_P (W) && !(W)->horizontal) /* WINDOW's XFRAME. */ #define WINDOW_XFRAME(W) (XFRAME (WINDOW_FRAME ((W)))) @@ -1014,6 +1021,7 @@ extern void shrink_mini_window (struct window *, bool); extern int window_relative_x_coord (struct window *, enum window_part, int); void run_window_configuration_change_hook (struct frame *f); +void run_window_size_change_functions (Lisp_Object); /* Make WINDOW display BUFFER. RUN_HOOKS_P means it's allowed to run hooks. See make_frame for a case where it's not allowed. */ diff --git a/src/xdisp.c b/src/xdisp.c index fed5879..4330f10 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -11786,24 +11786,7 @@ prepare_menu_bars (void) && !XBUFFER (w->contents)->text->redisplay) continue; - /* If a window on this frame changed size, report that to - the user and clear the size-change flag. */ - if (FRAME_WINDOW_SIZES_CHANGED (f)) - { - Lisp_Object functions; - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (f) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), frame); - functions = XCDR (functions); - } - } - + run_window_size_change_functions (frame); menu_bar_hooks_run = update_menu_bar (f, false, menu_bar_hooks_run); #ifdef HAVE_WINDOW_SYSTEM update_tool_bar (f, false); @@ -13599,24 +13582,12 @@ redisplay_internal (void) it's too late for the hooks in window-size-change-functions, which have been examined already in prepare_menu_bars. So in that case we call the hooks here only for the selected frame. */ - if (sf->redisplay && FRAME_WINDOW_SIZES_CHANGED (sf)) + if (sf->redisplay) { - Lisp_Object functions; ptrdiff_t count1 = SPECPDL_INDEX (); record_unwind_save_match_data (); - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (sf) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), selected_frame); - functions = XCDR (functions); - } - + run_window_size_change_functions (selected_frame); unbind_to (count1, Qnil); } @@ -13638,22 +13609,10 @@ redisplay_internal (void) { if (sf->redisplay) { - Lisp_Object functions; ptrdiff_t count1 = SPECPDL_INDEX (); record_unwind_save_match_data (); - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (sf) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), selected_frame); - functions = XCDR (functions); - } - + run_window_size_change_functions (selected_frame); unbind_to (count1, Qnil); } @@ -31447,16 +31406,6 @@ If nil, disable message logging. If t, log messages but don't truncate the buffer when it becomes large. */); Vmessage_log_max = make_number (1000); - DEFVAR_LISP ("window-size-change-functions", Vwindow_size_change_functions, - doc: /* Functions called during redisplay, if window sizes have changed. -The value should be a list of functions that take one argument. -During the first part of redisplay, for each frame, if any of its windows -have changed size since the last redisplay, or have been split or deleted, -all the functions in the list are called, with the frame as argument. -If redisplay decides to resize the minibuffer window, it calls these -functions on behalf of that as well. */); - Vwindow_size_change_functions = Qnil; - DEFVAR_LISP ("window-scroll-functions", Vwindow_scroll_functions, doc: /* List of functions to call before redisplaying a window with scrolling. Each function is called with two arguments, the window and its new diff --git a/src/xfns.c b/src/xfns.c index 20ac627..2a50a5a 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -1313,7 +1313,6 @@ x_set_menu_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval) } #endif /* not USE_X_TOOLKIT && not USE_GTK */ adjust_frame_glyphs (f); - run_window_configuration_change_hook (f); } c:\emacs-git\quick>c:/Programme/MinGW/msys/1.0/bin/bash.exe --login -i -c "cd /c/emacs-git/quick/ ; git status" c:/Programme/MinGW/msys/1.0/bin/bash.exe --login -i -c "cd /c/emacs-git/quick/ ; git status" On branch master Your branch is up-to-date with 'origin/master'. Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory) modified: doc/lispref/windows.texi modified: lisp/window.el modified: src/frame.c modified: src/frame.h modified: src/window.c modified: src/window.h modified: src/xdisp.c modified: src/xfns.c no changes added to commit (use "git add" and/or "git commit -a") c:\emacs-git\quick>c:/Programme/MinGW/msys/1.0/bin/bash.exe --login -i -c "cd /c/emacs-git/quick/ ; git diff" c:/Programme/MinGW/msys/1.0/bin/bash.exe --login -i -c "cd /c/emacs-git/quick/ ; git diff" diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi index f61f08a..eac3bd6 100644 --- a/doc/lispref/windows.texi +++ b/doc/lispref/windows.texi @@ -545,6 +545,12 @@ Window Sizes children. @end defun +@defun window-old-pixel-height &optional Lisp_Object &optional window +This function returns the height of window @var{window} in pixels at the +time @code{window-size-change-functions} was run for the last time on +@var{window}'s frame (@pxref{Window Hooks}). +@end defun + @cindex window pixel width @cindex pixel width of a window @cindex total pixel width of a window @@ -559,6 +565,12 @@ Window Sizes the screen areas spanned by its children. @end defun +@defun window-old-pixel-width &optional Lisp_Object &optional window +This function returns the width of window @var{window} in pixels at the +time @code{window-size-change-functions} was run for the last time on +@var{window}'s frame (@pxref{Window Hooks}). +@end defun + @cindex full-width window @cindex full-height window The following functions can be used to determine whether a given @@ -4087,11 +4099,11 @@ Window Configurations The argument @var{configuration} must be a value that was previously returned by @code{current-window-configuration}. The configuration is restored in the frame from which @var{configuration} was made, whether -that frame is selected or not. This always counts as a window size -change and triggers execution of the @code{window-size-change-functions} -(@pxref{Window Hooks}), because @code{set-window-configuration} doesn't -know how to tell whether the new configuration actually differs from the -old one. +that frame is selected or not. In some rare cases this may trigger +execution of the @code{window-size-change-functions} (@pxref{Window +Hooks}) even if the size of windows did not change at all. The +@code{window-configuration-change-hook} functions will be called if and +only if at least one window was added to or deleted from the frame. If the frame from which @var{configuration} was saved is dead, all this function does is restore the three variables @code{window-min-height}, @@ -4378,33 +4390,37 @@ Window Hooks @end defvar @defvar window-size-change-functions -This variable holds a list of functions to be called if the size of -any window changes for any reason. The functions are called at the -beginning of a redisplay cycle, and just once for each frame on which -size changes have occurred. - -Each function receives the frame as its sole argument. There is no -direct way to find out which windows on that frame have changed size, or -precisely how. However, if a size-change function records, at each -call, the existing windows and their sizes, it can also compare the -present sizes and the previous sizes. - -Creating or deleting windows counts as a size change, and therefore -causes these functions to be called. Changing the frame size also -counts, because it changes the sizes of the existing windows. +This variable holds a list of functions to be called if the size of any +window changes for any reason. The functions are called once per +redisplay, and once for each frame on which size changes have occurred. + +Each function receives the frame as its sole argument. To find out +whether a specific window has changed size, compare the return values of +@code{window-old-pixel-width} and @code{window-pixel-width} respectively +@code{window-old-pixel-height} and @code{window-pixel-height} for that +window (@pxref{Window Sizes}). + +These function are usually only called when at least one window was +added or has changed size since the last time this hook was run for the +associated frame. In some rare cases this hook also runs when a window +that was added intermittently has been deleted afterwards. In these +cases none of the windows on the frame will appear to have changed its +size. You may use @code{save-selected-window} in these functions (@pxref{Selecting Windows}). However, do not use @code{save-window-excursion} (@pxref{Window Configurations}); exiting -that macro counts as a size change, which would cause these functions -to be called over and over. +that macro counts as a size change, which would cause these functions to +be called again. @end defvar @defvar window-configuration-change-hook -A normal hook that is run every time you change the window configuration -of an existing frame. This includes splitting or deleting windows, -changing the sizes of windows, or displaying a different buffer in a -window. +A normal hook that is run every time the window configuration of a frame +changes. Window configuration changes include splitting and deleting +windows and the display of a different buffer in a window. Resizing the +frame or individual windows do not count as configuration changes. Use +@code{window-size-change-functions}, see above, when you want to track +size changes that are not caused by the deletion or creation of windows. The buffer-local part of this hook is run once for each window on the affected frame, with the relevant window selected and its buffer diff --git a/src/frame.c b/src/frame.c index 8c86afe..df473ae 100644 --- a/src/frame.c +++ b/src/frame.c @@ -591,8 +591,6 @@ adjust_frame_size (struct frame *f, int new_width, int new_height, int inhibit, || new_pixel_height != old_pixel_height); unblock_input (); - - run_window_configuration_change_hook (f); } /* Allocate basically initialized frame. */ diff --git a/src/frame.h b/src/frame.h index 71dab4b..d9424ab 100644 --- a/src/frame.h +++ b/src/frame.h @@ -288,8 +288,9 @@ struct frame cleared. */ bool_bf explicit_name : 1; - /* True if size of some window on this frame has changed. */ - bool_bf window_sizes_changed : 1; + /* True if configuration of windows on this frame has changed since + last call of run_window_size_change_functions. */ + bool_bf window_configuration_changed : 1; /* True if the mouse has moved on this display device since the last time we checked. */ @@ -828,10 +829,10 @@ default_pixels_per_inch_y (void) are frozen on frame F. */ #define FRAME_WINDOWS_FROZEN(f) (f)->frozen_window_starts -/* True if a size change has been requested for frame F - but not yet really put into effect. This can be true temporarily - when an X event comes in at a bad time. */ -#define FRAME_WINDOW_SIZES_CHANGED(f) (f)->window_sizes_changed +/* True if the frame's window configuration has changed since last call + of run_window_size_change_functions. */ +#define FRAME_WINDOW_CONFIGURATION_CHANGED(f) \ + ((f)->window_configuration_changed) /* The minibuffer window of frame F, if it has one; otherwise nil. */ #define FRAME_MINIBUF_WINDOW(f) f->minibuffer_window diff --git a/src/window.c b/src/window.c index e1a30ee..5b03a24 100644 --- a/src/window.c +++ b/src/window.c @@ -720,6 +720,30 @@ the height of the screen areas spanned by its children. */) return make_number (decode_valid_window (window)->pixel_height); } +DEFUN ("window-old-pixel-width", Fwindow_old_pixel_width, Swindow_old_pixel_width, 0, 1, 0, + doc: /* Return the old width of window WINDOW in pixels. +WINDOW must be a valid window and defaults to the selected one. + +The return value is the pixel width of WINDOW at the last time +`window-size-change-functions' was run. It's zero if WINDOW was made +after that. */) + (Lisp_Object window) +{ + return make_number (decode_valid_window (window)->old_pixel_width); +} + +DEFUN ("window-old-pixel-height", Fwindow_old_pixel_height, Swindow_old_pixel_height, 0, 1, 0, + doc: /* Return the old height of window WINDOW in pixels. +WINDOW must be a valid window and defaults to the selected one. + +The return value is the pixel height of WINDOW at the last time +`window-size-change-functions' was run. It's zero if WINDOW was made +after that. */) + (Lisp_Object window) +{ + return make_number (decode_valid_window (window)->old_pixel_height); +} + DEFUN ("window-total-height", Fwindow_total_height, Swindow_total_height, 0, 2, 0, doc: /* Return the height of window WINDOW in lines. WINDOW must be a valid window and defaults to the selected one. @@ -2879,6 +2903,7 @@ window-start value is reasonable when this function is called. */) Lisp_Object sibling, pwindow, swindow IF_LINT (= Qnil), delta; ptrdiff_t startpos IF_LINT (= 0), startbyte IF_LINT (= 0); int top IF_LINT (= 0), new_top; + bool resize_failed = false; w = decode_valid_window (window); XSETWINDOW (window, w); @@ -2978,8 +3003,6 @@ window-start value is reasonable when this function is called. */) fset_redisplay (f); Vwindow_list = Qnil; - FRAME_WINDOW_SIZES_CHANGED (f) = true; - bool resize_failed = false; if (!WINDOW_LEAF_P (w)) { @@ -3229,6 +3252,76 @@ If WINDOW is omitted or nil, it defaults to the selected window. */) return Qnil; } + +/* Compare old and present pixel sizes of windows in tree rooted at W. + Return true iff any of these windows differs in size. */ + +static bool +window_size_changed (struct window *w) +{ + if (w->pixel_width != w->old_pixel_width + || w->pixel_height != w->old_pixel_height) + return true; + + if (WINDOW_INTERNAL_P (w)) + { + w = XWINDOW (w->contents); + while (w) + { + if (window_size_changed (w)) + return true; + + w = NILP (w->next) ? 0 : XWINDOW (w->next); + } + } + + return false; +} + +/* Set old pixel sizes of windows in tree rooted at W to their present + pixel sizes. */ + +static void +window_set_old_pixel_sizes (struct window *w) +{ + w->old_pixel_width = w->pixel_width; + w->old_pixel_height = w->pixel_height; + + if (WINDOW_INTERNAL_P (w)) + { + w = XWINDOW (w->contents); + while (w) + { + window_set_old_pixel_sizes (w); + w = NILP (w->next) ? 0 : XWINDOW (w->next); + } + } +} + + +void +run_window_size_change_functions (Lisp_Object frame) +{ + struct frame *f = XFRAME (frame); + struct window *r = XWINDOW (FRAME_ROOT_WINDOW (f)); + Lisp_Object functions = Vwindow_size_change_functions; + + if (FRAME_WINDOW_CONFIGURATION_CHANGED (f) || + window_size_changed (r)) + { + while (CONSP (functions)) + { + if (!EQ (XCAR (functions), Qt)) + call1 (XCAR (functions), frame); + functions = XCDR (functions); + } + + window_set_old_pixel_sizes (r); + FRAME_WINDOW_CONFIGURATION_CHANGED (f) = false; + } +} + + /* Make WINDOW display BUFFER. RUN_HOOKS_P means it's allowed to run hooks. See make_frame for a case where it's not allowed. KEEP_MARGINS_P means that the current margins, fringes, and @@ -3263,15 +3356,9 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, if (!(keep_margins_p && samebuf)) { /* If we're not actually changing the buffer, don't reset hscroll - and vscroll. This case happens for example when called from - change_frame_size_1, where we use a dummy call to - Fset_window_buffer on the frame's selected window (and no - other) just in order to run window-configuration-change-hook - (no longer true since change_frame_size_1 directly calls - run_window_configuration_change_hook). Resetting hscroll and - vscroll here is problematic for things like image-mode and - doc-view-mode since it resets the image's position whenever we - resize the frame. */ + and vscroll. Resetting hscroll and vscroll here is problematic + for things like image-mode and doc-view-mode since it resets + the image's position whenever we resize the frame. */ w->hscroll = w->min_hscroll = w->hscroll_whole = 0; w->suspend_auto_hscroll = false; w->vscroll = 0; @@ -3283,10 +3370,8 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, w->start_at_line_beg = false; w->force_start = false; } - /* Maybe we could move this into the `if' but it's not obviously safe and - I doubt it's worth the trouble. */ - wset_redisplay (w); + wset_redisplay (w); wset_update_mode_line (w); /* We must select BUFFER to run the window-scroll-functions and to look up @@ -3314,7 +3399,7 @@ set_window_buffer (Lisp_Object window, Lisp_Object buffer, if (run_hooks_p) { - if (! NILP (Vwindow_scroll_functions)) + if (!NILP (Vwindow_scroll_functions)) run_hook_with_args_2 (Qwindow_scroll_functions, window, Fmarker_position (w->start)); if (!samebuf) @@ -3559,6 +3644,8 @@ make_window (void) w->phys_cursor_width = -1; #endif w->sequence_number = ++sequence_number; + w->old_pixel_width = 0; + w->old_pixel_height = 0; w->scroll_bar_width = -1; w->scroll_bar_height = -1; w->column_number_displayed = -1; @@ -3922,7 +4009,6 @@ be applied on the Elisp level. */) window_resize_apply (r, horflag); fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); @@ -4087,7 +4173,6 @@ resize_frame_windows (struct frame *f, int size, bool horflag, bool pixelwise) } } - FRAME_WINDOW_SIZES_CHANGED (f) = true; fset_redisplay (f); } @@ -4214,7 +4299,6 @@ set correctly. See the code of `split-window' for how this is done. */) p = XWINDOW (o->parent); fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; new = make_window (); n = XWINDOW (new); wset_frame (n, frame); @@ -4383,7 +4467,6 @@ Signal an error when WINDOW is the only window on its frame. */) fset_redisplay (f); Vwindow_list = Qnil; - FRAME_WINDOW_SIZES_CHANGED (f) = true; wset_next (w, Qnil); /* Don't delete w->next too. */ free_window_matrices (w); @@ -4451,9 +4534,6 @@ Signal an error when WINDOW is the only window on its frame. */) } else unblock_input (); - - /* Must be run by the caller: - run_window_configuration_change_hook (f); */ } else /* We failed: Relink WINDOW into window tree. */ @@ -4527,7 +4607,6 @@ grow_mini_window (struct window *w, int delta, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } @@ -4567,7 +4646,6 @@ shrink_mini_window (struct window *w, bool pixelwise) /* Enforce full redisplay of the frame. */ /* FIXME: Shouldn't window--resize-root-window-vertically do it? */ fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); } @@ -4610,7 +4688,6 @@ DEFUN ("resize-mini-window-internal", Fresize_mini_window_internal, Sresize_mini w->top_line = r->top_line + r->total_lines; fset_redisplay (f); - FRAME_WINDOW_SIZES_CHANGED (f) = true; adjust_frame_glyphs (f); unblock_input (); return Qt; @@ -5948,6 +6025,7 @@ struct saved_window Lisp_Object window, buffer, start, pointm, old_pointm; Lisp_Object pixel_left, pixel_top, pixel_height, pixel_width; + Lisp_Object old_pixel_height, old_pixel_width; Lisp_Object left_col, top_line, total_cols, total_lines; Lisp_Object normal_cols, normal_lines; Lisp_Object hscroll, min_hscroll, hscroll_whole, suspend_auto_hscroll; @@ -6063,6 +6141,12 @@ the return value is nil. Otherwise the value is t. */) struct window *root_window; struct window **leaf_windows; ptrdiff_t i, k, n_leaf_windows; + /* Records whether a window has been added or removed wrt the + original configuration. */ + bool window_changed = false; + /* Records whether a window has changed its buffer wrt the + original configuration. */ + bool buffer_changed = false; /* Don't do this within the main loop below: This may call Lisp code and is thus potentially unsafe while input is blocked. */ @@ -6071,6 +6155,12 @@ the return value is nil. Otherwise the value is t. */) p = SAVED_WINDOW_N (saved_windows, k); window = p->window; w = XWINDOW (window); + + if (NILP (w->contents)) + /* A dead window that will be resurrected, the window + configuration will change. */ + window_changed = true; + if (BUFFERP (w->contents) && !EQ (w->contents, p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) @@ -6100,7 +6190,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 @@ -6156,6 +6245,8 @@ the return value is nil. Otherwise the value is t. */) w->pixel_top = XFASTINT (p->pixel_top); w->pixel_width = XFASTINT (p->pixel_width); w->pixel_height = XFASTINT (p->pixel_height); + w->old_pixel_width = XFASTINT (p->old_pixel_width); + w->old_pixel_height = XFASTINT (p->old_pixel_height); w->left_col = XFASTINT (p->left_col); w->top_line = XFASTINT (p->top_line); w->total_cols = XFASTINT (p->total_cols); @@ -6203,6 +6294,9 @@ the return value is nil. Otherwise the value is t. */) if (BUFFERP (p->buffer) && BUFFER_LIVE_P (XBUFFER (p->buffer))) /* If saved buffer is alive, install it. */ { + if (!EQ (w->contents, p->buffer)) + /* Record buffer configuration change. */ + buffer_changed = true; wset_buffer (w, p->buffer); w->start_at_line_beg = !NILP (p->start_at_line_beg); set_marker_restricted (w->start, p->start, w->contents); @@ -6236,6 +6330,8 @@ the return value is nil. Otherwise the value is t. */) else if (!NILP (w->start)) /* Leaf window has no live buffer, get one. */ { + /* Record buffer configuration change. */ + buffer_changed = true; /* Get the buffer via other_buffer_safely in order to avoid showing an unimportant buffer and, if necessary, to recreate *scratch* in the course (part of Juanma's bs-show @@ -6283,7 +6379,10 @@ the return value is nil. Otherwise the value is t. */) /* Now, free glyph matrices in windows that were not reused. */ for (i = 0; i < n_leaf_windows; i++) if (NILP (leaf_windows[i]->contents)) - free_window_matrices (leaf_windows[i]); + { + free_window_matrices (leaf_windows[i]); + window_changed = true; + } /* Allow x_set_window_size again and apply frame size changes if needed. */ @@ -6303,7 +6402,8 @@ the return value is nil. Otherwise the value is t. */) /* Record the selected window's buffer here. The window should already be the selected one from the call above. */ - select_window (data->current_window, Qnil, false); + if (WINDOW_LIVE_P (data->current_window)) + select_window (data->current_window, Qnil, false); /* Fselect_window will have made f the selected frame, so we reselect the proper frame here. Fhandle_switch_frame will change the @@ -6313,7 +6413,32 @@ the return value is nil. Otherwise the value is t. */) if (FRAME_LIVE_P (XFRAME (data->selected_frame))) do_switch_frame (data->selected_frame, 0, 0, Qnil); - run_window_configuration_change_hook (f); + if (window_changed) + /* At least one window has been added or removed. Run + `window-configuration-change-hook' and make sure + `window-size-change-functions' get run later. + + We have to do this in order to capture the following + scenario: Suppose our frame contains two live windows W1 and + W2 and ‘set-window-configuration’ replaces them by two + windows W3 and W4 that were dead the last time + run_window_size_change_functions was run. If W3 and W4 have + the same values for their old and new pixel sizes but these + values differ from those of W1 and W2, the sizes of our + frame's two live windows changed but window_size_changed has + no means to detect that fact. + + Obviously, this will get us false positives, for example, + when we restore the original configuration with W1 and W2 + before run_window_size_change_functions gets called. */ + { + run_window_configuration_change_hook (f); + FRAME_WINDOW_CONFIGURATION_CHANGED (f) = true; + } + else if (buffer_changed) + /* At least one window has changed its buffer. Run + `window-configuration-change-hook' only. */ + run_window_configuration_change_hook (f); } if (!NILP (new_current_buffer)) @@ -6464,6 +6589,8 @@ save_window_save (Lisp_Object window, struct Lisp_Vector *vector, ptrdiff_t i) p->pixel_top = make_number (w->pixel_top); p->pixel_width = make_number (w->pixel_width); p->pixel_height = make_number (w->pixel_height); + p->old_pixel_width = make_number (w->old_pixel_width); + p->old_pixel_height = make_number (w->old_pixel_height); p->left_col = make_number (w->left_col); p->top_line = make_number (w->top_line); p->total_cols = make_number (w->total_cols); @@ -7246,6 +7373,16 @@ selected; while the global part is run only once for the modified frame, with the relevant frame selected. */); Vwindow_configuration_change_hook = Qnil; + DEFVAR_LISP ("window-size-change-functions", Vwindow_size_change_functions, + doc: /* Functions called during redisplay, if window sizes have changed. +The value should be a list of functions that take one argument. +During the first part of redisplay, for each frame, if any of its windows +have changed size since the last redisplay, or have been split or deleted, +all the functions in the list are called, with the frame as argument. +If redisplay decides to resize the minibuffer window, it calls these +functions on behalf of that as well. */); + Vwindow_size_change_functions = Qnil; + DEFVAR_LISP ("recenter-redisplay", Vrecenter_redisplay, doc: /* Non-nil means `recenter' redraws entire frame. If this option is non-nil, then the `recenter' command with a nil @@ -7374,6 +7511,8 @@ displayed after a scrolling operation to be somewhat inaccurate. */); defsubr (&Swindow_use_time); defsubr (&Swindow_pixel_width); defsubr (&Swindow_pixel_height); + defsubr (&Swindow_old_pixel_width); + defsubr (&Swindow_old_pixel_height); defsubr (&Swindow_total_width); defsubr (&Swindow_total_height); defsubr (&Swindow_normal_size); diff --git a/src/window.h b/src/window.h index c29207d..dea1d19 100644 --- a/src/window.h +++ b/src/window.h @@ -214,6 +214,11 @@ struct window int pixel_width; int pixel_height; + /* The pixel sizes of the window at the last time + `window-size-change-functions' was run. */ + int old_pixel_width; + int old_pixel_height; + /* The size of the window. */ int total_cols; int total_lines; @@ -499,15 +504,17 @@ wset_next_buffers (struct window *w, Lisp_Object val) #define WINDOW_LEAF_P(W) \ (BUFFERP ((W)->contents)) -/* True if W is a member of horizontal combination. */ +/* Non-nil if W is internal. */ +#define WINDOW_INTERNAL_P(W) \ + (WINDOWP ((W)->contents)) +/* True if W is a member of horizontal combination. */ #define WINDOW_HORIZONTAL_COMBINATION_P(W) \ - (WINDOWP ((W)->contents) && (W)->horizontal) + (WINDOW_INTERNAL_P (W) && (W)->horizontal) /* True if W is a member of vertical combination. */ - #define WINDOW_VERTICAL_COMBINATION_P(W) \ - (WINDOWP ((W)->contents) && !(W)->horizontal) + (WINDOW_INTERNAL_P (W) && !(W)->horizontal) /* WINDOW's XFRAME. */ #define WINDOW_XFRAME(W) (XFRAME (WINDOW_FRAME ((W)))) @@ -1014,6 +1021,7 @@ extern void shrink_mini_window (struct window *, bool); extern int window_relative_x_coord (struct window *, enum window_part, int); void run_window_configuration_change_hook (struct frame *f); +void run_window_size_change_functions (Lisp_Object); /* Make WINDOW display BUFFER. RUN_HOOKS_P means it's allowed to run hooks. See make_frame for a case where it's not allowed. */ diff --git a/src/xdisp.c b/src/xdisp.c index fed5879..4330f10 100644 --- a/src/xdisp.c +++ b/src/xdisp.c @@ -11786,24 +11786,7 @@ prepare_menu_bars (void) && !XBUFFER (w->contents)->text->redisplay) continue; - /* If a window on this frame changed size, report that to - the user and clear the size-change flag. */ - if (FRAME_WINDOW_SIZES_CHANGED (f)) - { - Lisp_Object functions; - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (f) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), frame); - functions = XCDR (functions); - } - } - + run_window_size_change_functions (frame); menu_bar_hooks_run = update_menu_bar (f, false, menu_bar_hooks_run); #ifdef HAVE_WINDOW_SYSTEM update_tool_bar (f, false); @@ -13599,24 +13582,12 @@ redisplay_internal (void) it's too late for the hooks in window-size-change-functions, which have been examined already in prepare_menu_bars. So in that case we call the hooks here only for the selected frame. */ - if (sf->redisplay && FRAME_WINDOW_SIZES_CHANGED (sf)) + if (sf->redisplay) { - Lisp_Object functions; ptrdiff_t count1 = SPECPDL_INDEX (); record_unwind_save_match_data (); - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (sf) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), selected_frame); - functions = XCDR (functions); - } - + run_window_size_change_functions (selected_frame); unbind_to (count1, Qnil); } @@ -13638,22 +13609,10 @@ redisplay_internal (void) { if (sf->redisplay) { - Lisp_Object functions; ptrdiff_t count1 = SPECPDL_INDEX (); record_unwind_save_match_data (); - - /* Clear flag first in case we get an error below. */ - FRAME_WINDOW_SIZES_CHANGED (sf) = false; - functions = Vwindow_size_change_functions; - - while (CONSP (functions)) - { - if (!EQ (XCAR (functions), Qt)) - call1 (XCAR (functions), selected_frame); - functions = XCDR (functions); - } - + run_window_size_change_functions (selected_frame); unbind_to (count1, Qnil); } @@ -31447,16 +31406,6 @@ If nil, disable message logging. If t, log messages but don't truncate the buffer when it becomes large. */); Vmessage_log_max = make_number (1000); - DEFVAR_LISP ("window-size-change-functions", Vwindow_size_change_functions, - doc: /* Functions called during redisplay, if window sizes have changed. -The value should be a list of functions that take one argument. -During the first part of redisplay, for each frame, if any of its windows -have changed size since the last redisplay, or have been split or deleted, -all the functions in the list are called, with the frame as argument. -If redisplay decides to resize the minibuffer window, it calls these -functions on behalf of that as well. */); - Vwindow_size_change_functions = Qnil; - DEFVAR_LISP ("window-scroll-functions", Vwindow_scroll_functions, doc: /* List of functions to call before redisplaying a window with scrolling. Each function is called with two arguments, the window and its new diff --git a/src/xfns.c b/src/xfns.c index 20ac627..2a50a5a 100644 --- a/src/xfns.c +++ b/src/xfns.c @@ -1313,7 +1313,6 @@ x_set_menu_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval) } #endif /* not USE_X_TOOLKIT && not USE_GTK */ adjust_frame_glyphs (f); - run_window_configuration_change_hook (f); } ^ permalink raw reply related [flat|nested] 57+ messages in thread
* Re: Fix `window-configuration-change-hook' and `window-size-change-functions' 2016-02-22 12:59 ` Fix `window-configuration-change-hook' and `window-size-change-functions' martin rudalics @ 2016-02-23 11:31 ` martin rudalics 0 siblings, 0 replies; 57+ messages in thread From: martin rudalics @ 2016-02-23 11:31 UTC (permalink / raw) To: eliz@gnu.org; +Cc: emacs-devel >> (3) Provide two functions `window-old-pixel-height' and >> `window-old-pixel-width' that allow to easily detect which window >> changed size. > > May I suggest to look for names that don't include "old" in them? > Perhaps window-pixel-width-before-size-change? Installed with the names you suggested. Please have another look. martin ^ permalink raw reply [flat|nested] 57+ messages in thread
end of thread, other threads:[~2016-02-23 11:31 UTC | newest] Thread overview: 57+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-08-23 22:06 bug#21333: 25.0.50; window-size-change-functions not called after mini-window resize Pip Cet 2015-08-24 8:18 ` martin rudalics 2015-08-24 11:08 ` Pip Cet 2015-08-24 12:41 ` martin rudalics 2015-08-24 14:35 ` Eli Zaretskii 2015-08-24 18:06 ` martin rudalics 2015-08-24 18:30 ` Eli Zaretskii 2015-08-25 7:25 ` martin rudalics 2015-08-25 10:34 ` Pip Cet 2015-08-25 15:19 ` Eli Zaretskii 2015-08-26 7:08 ` martin rudalics 2015-08-25 15:11 ` Eli Zaretskii 2015-08-26 7:09 ` martin rudalics 2015-08-26 15:29 ` Eli Zaretskii 2015-08-27 7:57 ` martin rudalics 2015-08-27 15:29 ` Eli Zaretskii 2015-08-27 17:05 ` Pip Cet 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:04 ` Pip Cet 2015-08-28 8:03 ` martin rudalics 2015-08-28 8:19 ` Pip Cet 2015-08-28 8:45 ` Pip Cet 2015-08-27 18:35 ` Eli Zaretskii 2015-08-27 17:58 ` martin rudalics 2015-08-24 18:13 ` Pip Cet 2015-08-24 19:03 ` Eli Zaretskii 2015-08-25 7:25 ` martin rudalics 2015-08-25 15:12 ` Eli Zaretskii 2015-08-26 7:09 ` martin rudalics 2015-08-26 10:07 ` Pip Cet 2015-08-26 13:01 ` martin rudalics 2015-08-26 16:00 ` Pip Cet 2015-08-27 7:59 ` martin rudalics 2015-08-27 15:25 ` Eli Zaretskii 2015-08-27 16:35 ` Pip Cet 2015-08-27 17:59 ` martin rudalics 2015-08-27 18:57 ` Eli Zaretskii 2015-08-27 20:49 ` Pip Cet 2015-08-28 10:02 ` Eli Zaretskii 2015-08-28 12:34 ` Pip Cet 2015-08-28 13:13 ` Eli Zaretskii 2015-08-28 13:26 ` Pip Cet 2015-08-26 15:36 ` Eli Zaretskii 2015-08-27 7:58 ` martin rudalics 2015-08-27 15:24 ` Eli Zaretskii 2015-08-27 17:58 ` martin rudalics 2015-08-27 18:39 ` Eli Zaretskii 2015-08-27 19:00 ` Eli Zaretskii 2015-08-28 8:04 ` martin rudalics 2015-08-28 8:47 ` Eli Zaretskii 2015-08-28 10:51 ` martin rudalics 2015-08-28 12:46 ` Eli Zaretskii 2015-08-28 13:05 ` martin rudalics 2015-08-26 15:32 ` Eli Zaretskii 2015-08-27 7:57 ` martin rudalics 2016-02-22 12:59 ` Fix `window-configuration-change-hook' and `window-size-change-functions' martin rudalics 2016-02-23 11:31 ` martin rudalics
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.