bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Pankaj Jangid]

This is occurring on MacOS build. Branch: master.
Steps:
1. ./src/emacs -Q
2. M-x set-frame-width RET 160 RET

Result: Frame size is increased but mode-line still has original width.

Probably one of these commits caused the issue:
a65eb23f5cbf1ff408585574e4c95a9eebf2a9dc
6b2d017ead856c244a0b5c5a162254094877bc54


In GNU Emacs 28.0.50 (build 1, x86_64-apple-darwin20.4.0, NS appkit-2022.44 Version 11.3 (Build 20E232))
 of 2021-05-02 built on mb2.local
Repository revision: a65eb23f5cbf1ff408585574e4c95a9eebf2a9dc
Repository branch: HEAD
Windowing system distributor 'Apple', version 10.3.2022
System Description:  macOS 11.3

Configured features:
ACL DBUS GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
KQUEUE NS PDUMPER PNG RSVG THREADS TIFF TOOLKIT_SCROLL_BARS XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Group

Minor modes in effect:
  gnus-undo-mode: t
  savehist-mode: t
  desktop-save-mode: t
  shell-dirtrack-mode: t
  direnv-mode: t
  TeX-PDF-mode: t
  override-global-mode: t
  show-paren-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-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
  buffer-read-only: t
  column-number-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
None found.

Features:
(shadow sort emacsbug gnus-cite mail-extr flyspell ispell gnus-async
gnus-ml hl-line disp-table nndraft nnmh utf-7 nnml nnfolder epa-file
gnutls network-stream nsm gnus-agent gnus-srvr gnus-score score-mode
nnvirtual nntp gnus-cache .gnus ebdb-message sendmail ebdb-gnus gnus-msg
conf-mode vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs eglot
array filenotify jsonrpc ert pp ewoc debug backtrace xref flymake-proc
flymake warnings project elec-pair display-line-numbers vc-git ol-eww
eww xdg url-queue mm-url ol-rmail ol-mhe ol-irc ol-info ol-gnus nnselect
gnus-search gnus-art mm-uu mml2015 mm-view mml-smime smime dig gnus-sum
shr kinsoku svg dom gnus-group gnus-undo gnus-start gnus-dbus dbus xml
gnus-cloud nnimap nnmail mail-source utf7 netrc nnoo gnus-spec gnus-int
gnus-range message rmc puny rfc822 mml mml-sec epa derived epg
epg-config mm-decode mm-bodies mm-encode mail-parse rfc2231 gmm-utils
mailheader gnus-win gnus nnheader gnus-util rmail rmail-loaddefs rfc2047
rfc2045 ietf-drums mail-utils mm-util mail-prsvr wid-edit ol-docview
doc-view jka-compr image-mode exif dired dired-loaddefs ol-bibtex bibtex
ol-bbdb ol-w3m init savehist desktop frameset ob-plantuml ob-sql ob-css
ob-js ob-java ob-C cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine cc-vars cc-defs ob-python python tramp-sh tramp
tramp-loaddefs trampver tramp-integration files-x tramp-compat shell
parse-time iso8601 ls-lisp ob-R ebdb-mua ebdb-com ebdb-format ebdb
mailabbrev eieio-opt speedbar ezimage dframe eieio-base pcase timezone
direnv diff-mode dash po-mode sesman vc vc-dispatcher edmacro kmacro
clojure-mode lisp-mnt align imenu rust-utils thingatpt rust-mode rx
rust-rustfmt rust-playpen rust-compile compile text-property-search
rust-cargo org-mime ox-org ox-odt rng-loc rng-uri rng-parse rng-match
rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok nxml-util
ox-latex ox-icalendar ox-html table ox-ascii ox-publish ox org-element
avl-tree generator org ob ob-tangle ob-ref ob-lob ob-table ob-exp
org-macro org-footnote org-src ob-comint org-pcomplete pcomplete comint
ansi-color ring org-list org-faces org-entities time-date org-version
ob-emacs-lisp ob-core ob-eval org-table ol org-keys org-compat advice
org-macs org-loaddefs format-spec find-func cal-menu calendar
cal-loaddefs noutline outline delight hideshow tex crm texmathp cl-extra
help-mode finder-inf use-package use-package-ensure use-package-delight
use-package-diminish use-package-bind-key bind-key easy-mmode
use-package-core server avoid paren cus-load tex-site info early-init
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/ns-win ns-win
ucs-normalize mule-util term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer 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 composite charscript charprop
case-table epa-hook jka-cmpr-hook help simple abbrev obarray
cl-preloaded nadvice button loaddefs faces cus-face macroexp files
window text-properties overlay sha1 md5 base64 format env code-pages
mule custom widget hashtable-print-readable backquote threads dbusbind
kqueue cocoa ns lcms2 multi-tty make-network-process emacs)

Memory information:
((conses 16 657665 32701)
 (symbols 48 41648 1)
 (strings 32 179715 12671)
 (string-bytes 1 5599709)
 (vectors 16 74780)
 (vector-slots 8 915326 45460)
 (floats 8 335 61)
 (intervals 56 741 0)
 (buffers 992 41))

-- 
Regards,
Pankaj Jangid

GnuPG Fingerprint: 0B62 7424 3B26 A911 052A DDE6 7C95 6E6F F858 7689

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[martin rudalics]

 > This is occurring on MacOS build. Branch: master.
 > Steps:
 > 1. ./src/emacs -Q
 > 2. M-x set-frame-width RET 160 RET
 >
 > Result: Frame size is increased but mode-line still has original width.

Could you please try the patch below.

Thanks, martin


diff --git a/src/nsterm.m b/src/nsterm.m
index 6e7ab1266b..b089146e64 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7310,9 +7310,9 @@ - (void)viewDidResize:(NSNotification *)notification

    /* Don't want to do anything when the view size hasn't changed. */
    if ((oldh == newh && oldw == neww)
-      || (emacsframe->new_size_p
-          && newh == emacsframe->new_height
-          && neww == emacsframe->new_width))
+      && (!emacsframe->new_size_p
+          || (newh == emacsframe->new_height
+	      && neww == emacsframe->new_width)))
      {
        NSTRACE_MSG ("No change");
        return;

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Alan Third]

On Sun, May 02, 2021 at 06:12:45PM +0200, martin rudalics wrote:
> > This is occurring on MacOS build. Branch: master.
> > Steps:
> > 1. ./src/emacs -Q
> > 2. M-x set-frame-width RET 160 RET
> >
> > Result: Frame size is increased but mode-line still has original width.
> 
> Could you please try the patch below.
> 
> Thanks, martin
> 
> 
> diff --git a/src/nsterm.m b/src/nsterm.m
> index 6e7ab1266b..b089146e64 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -7310,9 +7310,9 @@ - (void)viewDidResize:(NSNotification *)notification
> 
>    /* Don't want to do anything when the view size hasn't changed. */
>    if ((oldh == newh && oldw == neww)
> -      || (emacsframe->new_size_p
> -          && newh == emacsframe->new_height
> -          && neww == emacsframe->new_width))
> +      && (!emacsframe->new_size_p
> +          || (newh == emacsframe->new_height
> +	      && neww == emacsframe->new_width)))
>      {
>        NSTRACE_MSG ("No change");
>        return;

This patch reintroduces the infinite loop I was trying to avoid.

viewDidResize is called every time the view's "frame" is touched. Most
of the time this happens when nothing has changed, but calling
change_frame_size, even when nothing has changed, appears to cause the
toolbar to redraw, which causes viewDidResize to be called again, and
so on ad infinitum.

So I put in the check whether the new frame size that's reported is
actually the same as the previous frame size, and if so do nothing.
Then realised that I need to check the new_height and new_width
settings in case the change is already pending.

I'm not sure where new_size_p comes into it, perhaps we don't even
need to check it in viewDidResize, because all we care about is the
final outcome?

Maybe this really isn't the way to do it and I need to keep track of
the frame size separately, which is something I was hoping to avoid.
-- 
Alan Third

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[martin rudalics]

 > This patch reintroduces the infinite loop I was trying to avoid.
 >
 > viewDidResize is called every time the view's "frame" is touched. Most
 > of the time this happens when nothing has changed, but calling
 > change_frame_size, even when nothing has changed, appears to cause the
 > toolbar to redraw, which causes viewDidResize to be called again, and
 > so on ad infinitum.
 >
 > So I put in the check whether the new frame size that's reported is
 > actually the same as the previous frame size, and if so do nothing.
 > Then realised that I need to check the new_height and new_width
 > settings in case the change is already pending.
 >
 > I'm not sure where new_size_p comes into it, perhaps we don't even
 > need to check it in viewDidResize, because all we care about is the
 > final outcome?
 >
 > Maybe this really isn't the way to do it and I need to keep track of
 > the frame size separately, which is something I was hoping to avoid.

new_size_p true means that Emacs has already received a resize event for
this frame but was not yet able to process it.  So the additional

           || (newh == emacsframe->new_height
	      && neww == emacsframe->new_width)))

in that case should "do nothing" when "nothing has changed".  Where does
my reasoning go wrong?

martin

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Alan Third]

On Sun, May 02, 2021 at 06:57:08PM +0200, martin rudalics wrote:
> > This patch reintroduces the infinite loop I was trying to avoid.
> >
> > viewDidResize is called every time the view's "frame" is touched. Most
> > of the time this happens when nothing has changed, but calling
> > change_frame_size, even when nothing has changed, appears to cause the
> > toolbar to redraw, which causes viewDidResize to be called again, and
> > so on ad infinitum.
> >
> > So I put in the check whether the new frame size that's reported is
> > actually the same as the previous frame size, and if so do nothing.
> > Then realised that I need to check the new_height and new_width
> > settings in case the change is already pending.
> >
> > I'm not sure where new_size_p comes into it, perhaps we don't even
> > need to check it in viewDidResize, because all we care about is the
> > final outcome?
> >
> > Maybe this really isn't the way to do it and I need to keep track of
> > the frame size separately, which is something I was hoping to avoid.
> 
> new_size_p true means that Emacs has already received a resize event for
> this frame but was not yet able to process it.  So the additional
> 
>           || (newh == emacsframe->new_height
> 	      && neww == emacsframe->new_width)))
> 
> in that case should "do nothing" when "nothing has changed".  Where does
> my reasoning go wrong?

I'm not sure. I was hoping you were going to tell me where I'm going
wrong! :)

The only thing I can see is that in order to go into the block and
return when new_size_p is false requires newh == oldh == new_height.
And the same with width.

Should it be this?

  if ((oldh == newh && oldw == neww && !emacsframe->new_size_p)
      || (newh == emacsframe->new_height
          && neww == emacsframe->new_width))

But I'm still not sure. If we have a change pending and the WM sends
another change through, should we call change_frame_size with the new
sizes?

Perhaps it just needs to look like this?

  if ((oldh == newh && oldw == neww)
      || (newh == emacsframe->new_height
          && neww == emacsframe->new_width))

This is hard, because I can't replicate either of the reported bugs
and I suspect my reasoning is very off here.
-- 
Alan Third

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[martin rudalics]

 > I'm not sure. I was hoping you were going to tell me where I'm going
 > wrong! :)
 >
 > The only thing I can see is that in order to go into the block and
 > return when new_size_p is false requires newh == oldh == new_height.
 > And the same with width.
 >
 > Should it be this?
 >
 >    if ((oldh == newh && oldw == neww && !emacsframe->new_size_p)
 >        || (newh == emacsframe->new_height
 >            && neww == emacsframe->new_width))
 >
 > But I'm still not sure. If we have a change pending and the WM sends
 > another change through, should we call change_frame_size with the new
 > sizes?

We should.

 > Perhaps it just needs to look like this?
 >
 >    if ((oldh == newh && oldw == neww)
 >        || (newh == emacsframe->new_height
 >            && neww == emacsframe->new_width))
 >
 > This is hard, because I can't replicate either of the reported bugs
 > and I suspect my reasoning is very off here.

I think I understand now: When Emacs is busy it cannot set oldh and oldw
immediately so oldh == newh && oldw == neww will continue to fail and we
cannot return.  How about something like the below?

martin


--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7309,10 +7309,10 @@ - (void)viewDidResize:(NSNotification *)notification
    oldh = FRAME_PIXEL_HEIGHT (emacsframe);

    /* Don't want to do anything when the view size hasn't changed. */
-  if ((oldh == newh && oldw == neww)
-      || (emacsframe->new_size_p
-          && newh == emacsframe->new_height
-          && neww == emacsframe->new_width))
+  if (emacsframe->new_size_p
+      ? (newh == emacsframe->new_height
+	 && neww == emacsframe->new_width)
+      : (oldh == newh && oldw == neww))
      {
        NSTRACE_MSG ("No change");
        return;

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Alan Third]

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

On Sun, May 02, 2021 at 09:19:20PM +0200, martin rudalics wrote:
> > Perhaps it just needs to look like this?
> >
> >    if ((oldh == newh && oldw == neww)
> >        || (newh == emacsframe->new_height
> >            && neww == emacsframe->new_width))
> >
> > This is hard, because I can't replicate either of the reported bugs
> > and I suspect my reasoning is very off here.
> 
> I think I understand now: When Emacs is busy it cannot set oldh and oldw
> immediately so oldh == newh && oldw == neww will continue to fail and we
> cannot return.  How about something like the below?

I think the attached works. I finally managed to reproduce the bug and
realised that the offscreen buffer's size doesn't really have anything
to do with what Emacs thinks the frame size is, so I've separated its
resizing code out.

Your suggested code for dealing with the Emacs frame size appears to
work right without any crashes on GNUstep too, so I've included it.

Now I just need to work out why using the menus causes GNUstep builds
to crash and we might have a semi-usable port again. ;)
-- 
Alan Third

[-- Attachment #2: 0001-Fix-incorrect-resizing-behaviour-on-macOS-bug-48157-.patch --]
[-- Type: text/x-diff, Size: 2365 bytes --]

From c4048b3cd6841a7f2ccdfb3f6c880c9ee176390a Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sun, 2 May 2021 22:38:13 +0100
Subject: [PATCH] Fix incorrect resizing behaviour on macOS (bug#48157,
 bug#48162)

* src/nsterm.m ([EmacsView viewDidResize:]): The drawing buffer can be
resized independently of Emacs's idea of the frame size.

Co-authored-by: martin rudalics <rudalics@gmx.at>
---
 src/nsterm.m | 36 ++++++++++++++++++++++--------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 6e7ab1266b..bb20886ab1 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7303,16 +7303,34 @@ - (void)viewDidResize:(NSNotification *)notification
 
   NSTRACE ("[EmacsView viewDidResize]");
 
+#ifdef NS_DRAW_TO_BUFFER
+  /* If the buffer size doesn't match the view's backing size, destroy
+     the buffer and let it be recreated at the correct size later.  */
+  if ([self wantsUpdateLayer] && surface)
+    {
+      NSRect surfaceRect = {{0, 0}, [surface getSize]};
+      NSRect frameRect = [[self window] convertRectToBacking:frame];
+
+      if (!NSEqualRects (frameRect, surfaceRect))
+        {
+          [surface release];
+          surface = nil;
+
+          [self setNeedsDisplay:YES];
+        }
+    }
+#endif
+
   neww = (int)NSWidth (frame);
   newh = (int)NSHeight (frame);
   oldw = FRAME_PIXEL_WIDTH (emacsframe);
   oldh = FRAME_PIXEL_HEIGHT (emacsframe);
 
   /* Don't want to do anything when the view size hasn't changed. */
-  if ((oldh == newh && oldw == neww)
-      || (emacsframe->new_size_p
-          && newh == emacsframe->new_height
-          && neww == emacsframe->new_width))
+  if (emacsframe->new_size_p
+      ? (newh == emacsframe->new_height
+         && neww == emacsframe->new_width)
+      : (oldh == newh && oldw == neww))
     {
       NSTRACE_MSG ("No change");
       return;
@@ -7321,16 +7339,6 @@ - (void)viewDidResize:(NSNotification *)notification
   NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
   NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
 
-#ifdef NS_DRAW_TO_BUFFER
-  if ([self wantsUpdateLayer])
-    {
-      [surface release];
-      surface = nil;
-
-      [self setNeedsDisplay:YES];
-    }
-#endif
-
   change_frame_size (emacsframe, neww, newh, false, YES, false);
 
   SET_FRAME_GARBAGED (emacsframe);
-- 
2.30.2

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Pankaj Jangid]

Alan Third <alan@idiocy.org> writes:

> I think the attached works. I finally managed to reproduce the bug and
> realised that the offscreen buffer's size doesn't really have anything
> to do with what Emacs thinks the frame size is, so I've separated its
> resizing code out.

This patch worked for me.

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[martin rudalics]

 > Now I just need to work out why using the menus causes GNUstep builds
 > to crash and we might have a semi-usable port again. ;)

It crashes here when trying to close a submenu.  Other than that
GNUstep seems indeed usable.  Good work.

martin

bug#48162: 28.0.50; Resizing using set-frame-width doesn't expand mode-line

[Alan Third]

On Mon, May 03, 2021 at 09:10:22AM +0530, Pankaj Jangid wrote:
> Alan Third <alan@idiocy.org> writes:
> 
> > I think the attached works. I finally managed to reproduce the bug and
> > realised that the offscreen buffer's size doesn't really have anything
> > to do with what Emacs thinks the frame size is, so I've separated its
> > resizing code out.
> 
> This patch worked for me.

Thanks. I think I'll just push this for now.
-- 
Alan Third