GNU bug report logs - #40200
28.0.50; NS: text drawing glitches in maximized frame with frame-inhibit-implied-resize

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 40200 in the body.
You can then email your comments to 40200 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 28.0.50; NS: text drawing glitches in maximized frame with
 frame-inhibit-implied-resize
Date: Mon, 23 Mar 2020 20:13:19 +0200

1. emacs -Q
2. (setq frame-inhibit-implied-resize t)
3. M-x toggle-frame-maximized
4. C-x b RET (to open *Messages* buffer)
5. M-x tool-bar-mode

After some up-down mouse scroll, buffer text is drawn incorrectly.

Not sure if image attaches are allowed here, so screenshot is on imgur:
https://imgur.com/a/QSSNePE

In GNU Emacs 28.0.50 (build 2, x86_64-apple-darwin19.3.0, NS appkit-1894.30 Version 10.15.3 (Build 19D76))
 of 2020-03-18 built on muffinmac.local
Windowing system distributor 'Apple', version 10.3.1894
System Description:  Mac OS X 10.15.3

Message #8 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org
Subject: Re: bug#40200: 28.0.50;
 NS: text drawing glitches in maximized frame with
 frame-inhibit-implied-resize
Date: Mon, 23 Mar 2020 21:22:23 +0200

> From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
> Date: Mon, 23 Mar 2020 20:13:19 +0200
> 
> 1. emacs -Q
> 2. (setq frame-inhibit-implied-resize t)
> 3. M-x toggle-frame-maximized
> 4. C-x b RET (to open *Messages* buffer)
> 5. M-x tool-bar-mode
> 
> After some up-down mouse scroll, buffer text is drawn incorrectly.

I cannot reproduce this, so it's probably Darwin-specific.

> Not sure if image attaches are allowed here

Of course they are.

Thanks.

Message #11 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 40200 <at> debbugs.gnu.org
Subject: Re: bug#40200: 28.0.50; NS: text drawing glitches in maximized frame
 with frame-inhibit-implied-resize
Date: Mon, 23 Mar 2020 21:47:13 +0200

On 23 Mar 2020, at 21:22, Eli Zaretskii <eliz <at> gnu.org> wrote:
> 
>> From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
>> Date: Mon, 23 Mar 2020 20:13:19 +0200
>> 
>> 1. emacs -Q
>> 2. (setq frame-inhibit-implied-resize t)
>> 3. M-x toggle-frame-maximized
>> 4. C-x b RET (to open *Messages* buffer)
>> 5. M-x tool-bar-mode
>> 
>> After some up-down mouse scroll, buffer text is drawn incorrectly.
> 
> I cannot reproduce this, so it's probably Darwin-specific.

It is Darwin-specific. I marked the subject with "NS". Or should the subject contain "macos" for Darwin-specific bugs?

>> Not sure if image attaches are allowed here
> 
> Of course they are.

Got it, thanks.

Message #14 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org
Subject: Re: bug#40200: 28.0.50; NS: text drawing glitches in maximized frame
 with frame-inhibit-implied-resize
Date: Tue, 24 Mar 2020 00:54:35 +0100 (CET)

On Mon, Mar 23, 2020 at 08:13:19PM +0200, Andrii Kolomoiets wrote:
> 1. emacs -Q
> 2. (setq frame-inhibit-implied-resize t)
> 3. M-x toggle-frame-maximized
> 4. C-x b RET (to open *Messages* buffer)
> 5. M-x tool-bar-mode
> 
> After some up-down mouse scroll, buffer text is drawn incorrectly.
> 
> Not sure if image attaches are allowed here, so screenshot is on imgur:
> https://imgur.com/a/QSSNePE

As far as I’m aware images are allowed.

This is surprisingly complicated! It appears that when the tool‐bar is
removed the window doesn’t change size so there is no resize event
sent. I’ve added an NSView resize notification on my local copy and it
now draws OK, but it doesn’t inform Emacs that the frame size has
changed so it leaves a blank space at the bottom of the frame. I think
I’ve seen this happen before on occasion.

It looks to me like the right thing to do here is to add this resize
notification and use it to replace the window resize based system we
use at the moment.

The current resizing code is a little spaghetti‐like as it has built
up over time to do various things, so refactoring it is potentially
troublesome. Perhaps it will simplify things, though.
-- 
Alan Third

Message #17 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: [PATCH] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Wed, 25 Mar 2020 23:28:44 +0100 (CET)

* src/nsmenu.m (update_frame_tool_bar): Remove reference to
updateFrameSize.
* src/nsterm.h: Remove definition of updateFrameSize.
* src/nsterm.m (ns_set_window_size):
(ns_set_undecorated):
([EmacsView windowDidResize:]):
([EmacsView windowDidExitFullScreen]):
(ns_judge_scroll_bars): Remove references to updateFrameSize.
([EmacsView dealloc]): Unset resize notification.
([EmacsView updateFrameSize:]): Remove function.
([EmacsView viewWillStartLiveResize]):
([EmacsView viewDidResize]): New functions.
([EmacsView initFrameFromEmacs:]): Set up resize notification.
([EmacsView toggleFullScreen:]): Set frame to the size of the
contentview, not the whole window, and remove reference to
updateFrameSize.
---
 src/nsmenu.m |   2 -
 src/nsterm.h |   1 -
 src/nsterm.m | 191 ++++++++++++++++++++++-----------------------------
 3 files changed, 81 insertions(+), 113 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 67f9a45a40..b7e4cbd565 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -1141,8 +1141,6 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct frame *)f
     }
 #endif
 
-  if (oldh != FRAME_TOOLBAR_HEIGHT (f))
-    [view updateFrameSize:YES];
   if (view->wait_for_tool_bar && FRAME_TOOLBAR_HEIGHT (f) > 0)
     {
       view->wait_for_tool_bar = NO;
diff --git a/src/nsterm.h b/src/nsterm.h
index 8396a542f7..c454c1ff36 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -462,7 +462,6 @@ #define NS_DRAW_TO_BUFFER 1
 - (void) setWindowClosing: (BOOL)closing;
 - (EmacsToolbar *) toolbar;
 - (void) deleteWorkingText;
-- (void) updateFrameSize: (BOOL) delay;
 - (void) handleFS;
 - (void) setFSValue: (int)value;
 - (void) toggleFullScreen: (id) sender;
diff --git a/src/nsterm.m b/src/nsterm.m
index 04fc051223..41780c9a42 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1864,7 +1864,6 @@ Hide the window (X11 semantics)
 
   [window setFrame: wr display: YES];
 
-  [view updateFrameSize: NO];
   unblock_input ();
 }
 
@@ -1913,7 +1912,6 @@ Hide the window (X11 semantics)
          so some key presses (TAB) are swallowed by the system.  */
       [window makeFirstResponder: view];
 
-      [view updateFrameSize: NO];
       unblock_input ();
     }
 }
@@ -5024,9 +5022,6 @@ in certain situations (rapid incoming events).
       if ([view judge])
         removed = YES;
     }
-
-  if (removed)
-    [eview updateFrameSize: NO];
 }
 
 /* ==========================================================================
@@ -6198,6 +6193,13 @@ - (void) setWindowClosing: (BOOL)closing
 - (void)dealloc
 {
   NSTRACE ("[EmacsView dealloc]");
+
+  /* Clear the view resize notification.  */
+  [[NSNotificationCenter defaultCenter]
+    removeObserver:self
+              name:NSViewFrameDidChangeNotification
+            object:nil];
+
   [toolbar release];
   if (fs_state == FULLSCREEN_BOTH)
     [nonfs_window release];
@@ -7039,103 +7041,6 @@ - (BOOL)windowShouldClose: (id)sender
   return NO;
 }
 
-- (void) updateFrameSize: (BOOL) delay
-{
-  NSWindow *window = [self window];
-  NSRect wr = [window frame];
-  int extra = 0;
-  int oldc = cols, oldr = rows;
-  int oldw = FRAME_PIXEL_WIDTH (emacsframe);
-  int oldh = FRAME_PIXEL_HEIGHT (emacsframe);
-  int neww, newh;
-
-  NSTRACE ("[EmacsView updateFrameSize:]");
-  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
-  NSTRACE_RECT ("Original frame", wr);
-  NSTRACE_MSG  ("Original columns: %d", cols);
-  NSTRACE_MSG  ("Original rows: %d", rows);
-
-  if (! [self isFullscreen])
-    {
-      int toolbar_height;
-#ifdef NS_IMPL_GNUSTEP
-      // GNUstep does not always update the tool bar height.  Force it.
-      if (toolbar && [toolbar isVisible])
-          update_frame_tool_bar (emacsframe);
-#endif
-
-      toolbar_height = FRAME_TOOLBAR_HEIGHT (emacsframe);
-      if (toolbar_height < 0)
-        toolbar_height = 35;
-
-      extra = FRAME_NS_TITLEBAR_HEIGHT (emacsframe)
-        + toolbar_height;
-    }
-
-  if (wait_for_tool_bar)
-    {
-      /* The toolbar height is always 0 in fullscreen and undecorated
-         frames, so don't wait for it to become available.  */
-      if (FRAME_TOOLBAR_HEIGHT (emacsframe) == 0
-          && FRAME_UNDECORATED (emacsframe) == false
-          && ! [self isFullscreen])
-        {
-          NSTRACE_MSG ("Waiting for toolbar");
-          return;
-        }
-      wait_for_tool_bar = NO;
-    }
-
-  neww = (int)wr.size.width - emacsframe->border_width;
-  newh = (int)wr.size.height - extra;
-
-  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
-  NSTRACE_MSG ("FRAME_TOOLBAR_HEIGHT: %d", FRAME_TOOLBAR_HEIGHT (emacsframe));
-  NSTRACE_MSG ("FRAME_NS_TITLEBAR_HEIGHT: %d", FRAME_NS_TITLEBAR_HEIGHT (emacsframe));
-
-  cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
-  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
-
-  if (cols < MINWIDTH)
-    cols = MINWIDTH;
-
-  if (rows < MINHEIGHT)
-    rows = MINHEIGHT;
-
-  NSTRACE_MSG ("New columns: %d", cols);
-  NSTRACE_MSG ("New rows: %d", rows);
-
-  if (oldr != rows || oldc != cols || neww != oldw || newh != oldh)
-    {
-      NSView *view = FRAME_NS_VIEW (emacsframe);
-
-      change_frame_size (emacsframe,
-                         FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
-                         FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
-                         0, delay, 0, 1);
-      SET_FRAME_GARBAGED (emacsframe);
-      cancel_mouse_face (emacsframe);
-
-      /* The next two lines set the frame to the same size as we've
-         already set above.  We need to do this when we switch back
-         from non-native fullscreen, in other circumstances it appears
-         to be a noop.  (bug#28872) */
-      wr = NSMakeRect (0, 0, neww, newh);
-      [view setFrame: wr];
-#ifdef NS_DRAW_TO_BUFFER
-      [self createDrawingBuffer];
-#endif
-
-      // To do: consider using [NSNotificationCenter postNotificationName:].
-      [self windowDidMove: // Update top/left.
-	      [NSNotification notificationWithName:NSWindowDidMoveNotification
-					    object:[view window]]];
-    }
-  else
-    {
-      NSTRACE_MSG ("No change");
-    }
-}
 
 - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
 /* Normalize frame to gridded text size.  */
@@ -7277,15 +7182,21 @@ - (void)windowDidResize: (NSNotification *)notification
   sz = [self windowWillResize: theWindow toSize: sz];
 #endif /* NS_IMPL_GNUSTEP */
 
-  if (cols > 0 && rows > 0)
-    {
-      [self updateFrameSize: YES];
-    }
-
   ns_send_appdefined (-1);
 }
 
 #ifdef NS_IMPL_COCOA
+- (void)viewWillStartLiveResize
+{
+  NSTRACE ("[EmacsView viewWillStartLiveResize]");
+
+  /* We just want a blank frame during live resizes.  Redisplaying the
+     contents is too slow, and stretching the contents looks
+     weird.  */
+  ns_clear_frame (emacsframe);
+}
+
+
 - (void)viewDidEndLiveResize
 {
   NSTRACE ("[EmacsView viewDidEndLiveResize]");
@@ -7302,6 +7213,60 @@ - (void)viewDidEndLiveResize
 #endif /* NS_IMPL_COCOA */
 
 
+- (void)viewDidResize:(NSNotification *)notification
+{
+  NSRect frame = [self frame];
+  int oldw, oldh;
+  int neww, newh;
+
+#ifdef NS_DRAW_TO_BUFFER
+  CGFloat scale = [[self window] backingScaleFactor];
+  oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
+  oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
+#else
+  oldw = FRAME_PIXEL_WIDTH (emacsframe);
+  oldh = FRAME_PIXEL_HEIGHT (emacsframe);
+#endif
+  neww = (int)NSWidth (frame);
+  newh = (int)NSHeight (frame);
+
+  NSTRACE ("[EmacsView viewDidResize]");
+  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
+  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
+
+  /* Don't want to do anything when the actual view size hasn't
+     changed. */
+  if ((oldh == newh && oldw == neww))
+    {
+      NSTRACE_MSG ("No change");
+      return;
+    }
+
+  cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
+  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
+
+  if (cols < MINWIDTH)
+    cols = MINWIDTH;
+
+  if (rows < MINHEIGHT)
+    rows = MINHEIGHT;
+
+  NSTRACE_MSG ("New columns: %d", cols);
+  NSTRACE_MSG ("New rows: %d", rows);
+
+  change_frame_size (emacsframe,
+                     FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
+                     FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
+                     0, YES, 0, 1);
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+  ns_clear_frame (emacsframe);
+  SET_FRAME_GARBAGED (emacsframe);
+  cancel_mouse_face (emacsframe);
+}
+
+
 - (void)windowDidBecomeKey: (NSNotification *)notification
 /* cf. x_detect_focus_change(), x_focus_changed(), x_new_focus_frame() */
 {
@@ -7456,6 +7421,13 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   [self initWithFrame: r];
   [self setAutoresizingMask: NSViewWidthSizable | NSViewHeightSizable];
 
+  /* Set up view resize notifications.  */
+  [self setPostsFrameChangedNotifications:YES];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:self
+         selector:@selector (viewDidResize:)
+             name:NSViewFrameDidChangeNotification object:nil];
+
   FRAME_NS_VIEW (f) = self;
   emacsframe = f;
 #ifdef NS_IMPL_COCOA
@@ -7902,7 +7874,6 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     {
       [toolbar setVisible:YES];
       update_frame_tool_bar (emacsframe);
-      [self updateFrameSize:YES];
       [[self window] display];
     }
   else
@@ -8115,11 +8086,11 @@ - (void)toggleFullScreen: (id)sender
       // send notifications.
 
       [self windowWillExitFullScreen];
-      [fw setFrame: [w frame] display:YES animate:ns_use_fullscreen_animation];
+      [fw setFrame:[[w contentView] frame]
+                    display:YES animate:ns_use_fullscreen_animation];
       [fw close];
       [w makeKeyAndOrderFront:NSApp];
       [self windowDidExitFullScreen];
-      [self updateFrameSize:YES];
     }
 }
 
-- 
2.24.0


-- 
Alan Third

Message #20 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Thu, 26 Mar 2020 19:35:20 +0200

[Message part 1 (text/plain, inline)]

On 26 Mar 2020, at 00:28, Alan Third <alan <at> idiocy.org> wrote:
> 
> * src/nsmenu.m (update_frame_tool_bar): Remove reference to
> updateFrameSize.
> * src/nsterm.h: Remove definition of updateFrameSize.
> * src/nsterm.m (ns_set_window_size):
> (ns_set_undecorated):
> ([EmacsView windowDidResize:]):
> ([EmacsView windowDidExitFullScreen]):
> (ns_judge_scroll_bars): Remove references to updateFrameSize.
> ([EmacsView dealloc]): Unset resize notification.
> ([EmacsView updateFrameSize:]): Remove function.
> ([EmacsView viewWillStartLiveResize]):
> ([EmacsView viewDidResize]): New functions.
> ([EmacsView initFrameFromEmacs:]): Set up resize notification.
> ([EmacsView toggleFullScreen:]): Set frame to the size of the
> contentview, not the whole window, and remove reference to
> updateFrameSize.

Thanks, Alan!

I've installed this patch and will watch for any issues.

For now Emacs is crashing on fullscreen frame deletion:
1. emacs -Q
2. C-x 5 2
3. F11
4. C-x 5 0

Backtrace from macos problem reporter is attached.

[emacs-crash-fs-close.txt (text/plain, attachment)]

Message #23 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Thu, 26 Mar 2020 22:21:29 +0100 (CET)

* src/nsmenu.m (update_frame_tool_bar): Remove reference to
updateFrameSize.
* src/nsterm.h: ([EmacsView updateFrameSize]):
([EmacsView setRows:andColumns:]): Remove unused
method definitions.
* src/nsterm.m (ns_set_window_size):
(ns_set_undecorated):
([EmacsView windowDidResize:]):
([EmacsView windowDidExitFullScreen]):
(ns_judge_scroll_bars): Remove references to updateFrameSize.
([EmacsView dealloc]): Unset resize notification and release buffer.
([EmacsView updateFrameSize:]): Remove function.
([EmacsView windowWillResize:toSize:]): Move some code to
viewDidResize.
([EmacsView viewDidResize]): New function.
([EmacsView initFrameFromEmacs:]): Set up resize notification and move
buffer creation until after the prerequisite objects are created.
([EmacsView toggleFullScreen:]): Set frame to the size of the
contentview, not the whole window, and remove reference to
updateFrameSize.
([EmacsView setRows:andColumns:]): Remove unused method.
---
Hopefully this works better.

src/nsmenu.m |   2 -
 src/nsterm.h |   3 -
 src/nsterm.m | 189 ++++++++++++++++++---------------------------------
 3 files changed, 68 insertions(+), 126 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 67f9a45a40..b7e4cbd565 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -1141,8 +1141,6 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct frame *)f
     }
 #endif
 
-  if (oldh != FRAME_TOOLBAR_HEIGHT (f))
-    [view updateFrameSize:YES];
   if (view->wait_for_tool_bar && FRAME_TOOLBAR_HEIGHT (f) > 0)
     {
       view->wait_for_tool_bar = NO;
diff --git a/src/nsterm.h b/src/nsterm.h
index 8396a542f7..3c2e4c2b07 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -439,7 +439,6 @@ #define NS_DRAW_TO_BUFFER 1
 #endif
 @public
    struct frame *emacsframe;
-   int rows, cols;
    int scrollbarsNeedingUpdate;
    EmacsToolbar *toolbar;
    NSRect ns_userRect;
@@ -458,11 +457,9 @@ #define NS_DRAW_TO_BUFFER 1
 /* Emacs-side interface */
 - (instancetype) initFrameFromEmacs: (struct frame *) f;
 - (void) createToolbar: (struct frame *)f;
-- (void) setRows: (int) r andColumns: (int) c;
 - (void) setWindowClosing: (BOOL)closing;
 - (EmacsToolbar *) toolbar;
 - (void) deleteWorkingText;
-- (void) updateFrameSize: (BOOL) delay;
 - (void) handleFS;
 - (void) setFSValue: (int)value;
 - (void) toggleFullScreen: (id) sender;
diff --git a/src/nsterm.m b/src/nsterm.m
index 04fc051223..95ab4390a0 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1864,7 +1864,6 @@ Hide the window (X11 semantics)
 
   [window setFrame: wr display: YES];
 
-  [view updateFrameSize: NO];
   unblock_input ();
 }
 
@@ -1913,7 +1912,6 @@ Hide the window (X11 semantics)
          so some key presses (TAB) are swallowed by the system.  */
       [window makeFirstResponder: view];
 
-      [view updateFrameSize: NO];
       unblock_input ();
     }
 }
@@ -5024,9 +5022,6 @@ in certain situations (rapid incoming events).
       if ([view judge])
         removed = YES;
     }
-
-  if (removed)
-    [eview updateFrameSize: NO];
 }
 
 /* ==========================================================================
@@ -6198,6 +6193,15 @@ - (void) setWindowClosing: (BOOL)closing
 - (void)dealloc
 {
   NSTRACE ("[EmacsView dealloc]");
+
+  /* Clear the view resize notification.  */
+  [[NSNotificationCenter defaultCenter]
+    removeObserver:self
+              name:NSViewFrameDidChangeNotification
+            object:nil];
+
+  CGContextRelease (drawingBuffer);
+
   [toolbar release];
   if (fs_state == FULLSCREEN_BOTH)
     [nonfs_window release];
@@ -7039,108 +7043,12 @@ - (BOOL)windowShouldClose: (id)sender
   return NO;
 }
 
-- (void) updateFrameSize: (BOOL) delay
-{
-  NSWindow *window = [self window];
-  NSRect wr = [window frame];
-  int extra = 0;
-  int oldc = cols, oldr = rows;
-  int oldw = FRAME_PIXEL_WIDTH (emacsframe);
-  int oldh = FRAME_PIXEL_HEIGHT (emacsframe);
-  int neww, newh;
-
-  NSTRACE ("[EmacsView updateFrameSize:]");
-  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
-  NSTRACE_RECT ("Original frame", wr);
-  NSTRACE_MSG  ("Original columns: %d", cols);
-  NSTRACE_MSG  ("Original rows: %d", rows);
-
-  if (! [self isFullscreen])
-    {
-      int toolbar_height;
-#ifdef NS_IMPL_GNUSTEP
-      // GNUstep does not always update the tool bar height.  Force it.
-      if (toolbar && [toolbar isVisible])
-          update_frame_tool_bar (emacsframe);
-#endif
-
-      toolbar_height = FRAME_TOOLBAR_HEIGHT (emacsframe);
-      if (toolbar_height < 0)
-        toolbar_height = 35;
-
-      extra = FRAME_NS_TITLEBAR_HEIGHT (emacsframe)
-        + toolbar_height;
-    }
-
-  if (wait_for_tool_bar)
-    {
-      /* The toolbar height is always 0 in fullscreen and undecorated
-         frames, so don't wait for it to become available.  */
-      if (FRAME_TOOLBAR_HEIGHT (emacsframe) == 0
-          && FRAME_UNDECORATED (emacsframe) == false
-          && ! [self isFullscreen])
-        {
-          NSTRACE_MSG ("Waiting for toolbar");
-          return;
-        }
-      wait_for_tool_bar = NO;
-    }
-
-  neww = (int)wr.size.width - emacsframe->border_width;
-  newh = (int)wr.size.height - extra;
-
-  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
-  NSTRACE_MSG ("FRAME_TOOLBAR_HEIGHT: %d", FRAME_TOOLBAR_HEIGHT (emacsframe));
-  NSTRACE_MSG ("FRAME_NS_TITLEBAR_HEIGHT: %d", FRAME_NS_TITLEBAR_HEIGHT (emacsframe));
-
-  cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
-  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
-
-  if (cols < MINWIDTH)
-    cols = MINWIDTH;
-
-  if (rows < MINHEIGHT)
-    rows = MINHEIGHT;
-
-  NSTRACE_MSG ("New columns: %d", cols);
-  NSTRACE_MSG ("New rows: %d", rows);
-
-  if (oldr != rows || oldc != cols || neww != oldw || newh != oldh)
-    {
-      NSView *view = FRAME_NS_VIEW (emacsframe);
-
-      change_frame_size (emacsframe,
-                         FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
-                         FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
-                         0, delay, 0, 1);
-      SET_FRAME_GARBAGED (emacsframe);
-      cancel_mouse_face (emacsframe);
-
-      /* The next two lines set the frame to the same size as we've
-         already set above.  We need to do this when we switch back
-         from non-native fullscreen, in other circumstances it appears
-         to be a noop.  (bug#28872) */
-      wr = NSMakeRect (0, 0, neww, newh);
-      [view setFrame: wr];
-#ifdef NS_DRAW_TO_BUFFER
-      [self createDrawingBuffer];
-#endif
-
-      // To do: consider using [NSNotificationCenter postNotificationName:].
-      [self windowDidMove: // Update top/left.
-	      [NSNotification notificationWithName:NSWindowDidMoveNotification
-					    object:[view window]]];
-    }
-  else
-    {
-      NSTRACE_MSG ("No change");
-    }
-}
 
 - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
 /* Normalize frame to gridded text size.  */
 {
   int extra = 0;
+  int cols, rows;
 
   NSTRACE ("[EmacsView windowWillResize:toSize: " NSTRACE_FMT_SIZE "]",
            NSTRACE_ARG_SIZE (frameSize));
@@ -7277,11 +7185,6 @@ - (void)windowDidResize: (NSNotification *)notification
   sz = [self windowWillResize: theWindow toSize: sz];
 #endif /* NS_IMPL_GNUSTEP */
 
-  if (cols > 0 && rows > 0)
-    {
-      [self updateFrameSize: YES];
-    }
-
   ns_send_appdefined (-1);
 }
 
@@ -7302,6 +7205,51 @@ - (void)viewDidEndLiveResize
 #endif /* NS_IMPL_COCOA */
 
 
+- (void)viewDidResize:(NSNotification *)notification
+{
+  NSRect frame = [self frame];
+  int oldw, oldh, neww, newh;
+
+  if (! FRAME_LIVE_P (emacsframe))
+    return;
+
+#ifdef NS_DRAW_TO_BUFFER
+  CGFloat scale = [[self window] backingScaleFactor];
+  oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
+  oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
+#else
+  oldw = FRAME_PIXEL_WIDTH (emacsframe);
+  oldh = FRAME_PIXEL_HEIGHT (emacsframe);
+#endif
+  neww = (int)NSWidth (frame);
+  newh = (int)NSHeight (frame);
+
+  NSTRACE ("[EmacsView viewDidResize]");
+
+  /* Don't want to do anything when the view size hasn't changed. */
+  if ((oldh == newh && oldw == neww))
+    {
+      NSTRACE_MSG ("No change");
+      return;
+    }
+
+  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
+  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
+
+  change_frame_size (emacsframe,
+                     FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
+                     FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
+                     0, YES, 0, 1);
+
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+  ns_clear_frame (emacsframe);
+  SET_FRAME_GARBAGED (emacsframe);
+  cancel_mouse_face (emacsframe);
+}
+
+
 - (void)windowDidBecomeKey: (NSNotification *)notification
 /* cf. x_detect_focus_change(), x_focus_changed(), x_new_focus_frame() */
 {
@@ -7463,10 +7411,6 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   maximizing_resize = NO;
 #endif
 
-#ifdef NS_DRAW_TO_BUFFER
-  [self createDrawingBuffer];
-#endif
-
   win = [[EmacsWindow alloc]
             initWithContentRect: r
                       styleMask: (FRAME_UNDECORATED (f)
@@ -7572,6 +7516,17 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+
+  /* Set up view resize notifications.  */
+  [self setPostsFrameChangedNotifications:YES];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:self
+         selector:@selector (viewDidResize:)
+             name:NSViewFrameDidChangeNotification object:nil];
+
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway.  */
@@ -7902,7 +7857,6 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     {
       [toolbar setVisible:YES];
       update_frame_tool_bar (emacsframe);
-      [self updateFrameSize:YES];
       [[self window] display];
     }
   else
@@ -8115,11 +8069,11 @@ - (void)toggleFullScreen: (id)sender
       // send notifications.
 
       [self windowWillExitFullScreen];
-      [fw setFrame: [w frame] display:YES animate:ns_use_fullscreen_animation];
+      [fw setFrame:[[w contentView] frame]
+                    display:YES animate:ns_use_fullscreen_animation];
       [fw close];
       [w makeKeyAndOrderFront:NSApp];
       [self windowDidExitFullScreen];
-      [self updateFrameSize:YES];
     }
 }
 
@@ -8628,13 +8582,6 @@ - (instancetype)setMiniwindowImage: (BOOL) setMini
 }
 
 
-- (void) setRows: (int) r andColumns: (int) c
-{
-  NSTRACE ("[EmacsView setRows:%d andColumns:%d]", r, c);
-  rows = r;
-  cols = c;
-}
-
 - (int) fullscreenState
 {
   return fs_state;
-- 
2.24.0


-- 
Alan Third

Message #26 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Fri, 27 Mar 2020 11:45:50 +0200

[Message part 1 (text/plain, inline)]

On 26 Mar 2020, at 23:21, Alan Third <alan <at> idiocy.org> wrote:
> 
> Hopefully this works better.
> 

It is!

But can the following line be removed?

> +  ns_clear_frame (emacsframe);

This removal resolves following issues for me:

1. Frames have white background color on creation.  This flashing is noticeable while using dark background for default face.

2. I'm experimenting with enabling macos native tabs by replacing NSWindowTabbingModeDisallowed with NSWindowTabbingModePreferred in nsterm.m.  Crash occurs on tabbed frame deletion (s-n s-w). Backtrace is attached.

Thanks!

[emacs-crash-tab-close.txt (text/plain, attachment)]

Message #29 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Fri, 27 Mar 2020 15:22:08 +0200

On 26 Mar 2020, at 23:21, Alan Third <alan <at> idiocy.org> wrote:
> Hopefully this works better.

Found another changed behavior:
1. emacs -Q
2. (setq test/frame (make-frame `((parent-frame . ,(selected-frame)))))
3. (modify-frame-parameters test/frame '((height . 1) (top . (- 40))))

Old behavior: child frame is at bottom of the parent frame.

New behavior: child frame is at top of the parent frame.
Child frame moves to bottom after code from step 3 applied one more time.

Step 3 can be replaced with these two steps to achieve old behavior:
(modify-frame-parameters test/frame '((height . 1)))
(modify-frame-parameters test/frame '((top . (- 40))))

Message #32 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Sat, 28 Mar 2020 02:56:59 +0100 (CET)

On Fri, Mar 27, 2020 at 03:22:08PM +0200, Andrii Kolomoiets wrote:
> On 26 Mar 2020, at 23:21, Alan Third <alan <at> idiocy.org> wrote:
> > Hopefully this works better.
> 
> Found another changed behavior:
> 1. emacs -Q
> 2. (setq test/frame (make-frame `((parent-frame . ,(selected-frame)))))
> 3. (modify-frame-parameters test/frame '((height . 1) (top . (- 40))))
> 
> Old behavior: child frame is at bottom of the parent frame.

This is wrong.

> New behavior: child frame is at top of the parent frame.

This is wrong too.

> Child frame moves to bottom after code from step 3 applied one more time.

Still wrong.

The bottom of the child frame should be 40 pixels from the bottom of
the parent frame.

This is quite annoying, it means Emacs 27 behaves wrongly. I wonder if
Emacs 26 does too, or has it been broken the whole time?

> Step 3 can be replaced with these two steps to achieve old behavior:
> (modify-frame-parameters test/frame '((height . 1)))
> (modify-frame-parameters test/frame '((top . (- 40))))

I inadvertently made order of operations matter when they shouldn’t.
I’ve got a fix for this, but it’s opening up a whole other can of
worms regarding frame positioning and the reporting of positions.

I think I know what to do but the last time I worked on this we had a
lot of edge cases that weren’t easy to test, with multiple monitors
and spaces. I think I’ll have to look at the old bug reports to work
out how we handle that.
-- 
Alan Third

Message #35 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: [PATCH v3] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Sat, 4 Apr 2020 15:17:49 +0100

* src/nsmenu.m (update_frame_tool_bar): Remove reference to
updateFrameSize.
* src/nsterm.h: ([EmacsView updateFrameSize]):
([EmacsView setRows:andColumns:]): Remove unused
method definitions.
(NS_PARENT_WINDOW_LEFT_POS):
(NS_PARENT_WINDOW_TOP_POS): Move to nsterm.m.
* src/nsterm.m (ns_parent_window_rect): New function.
(NS_PARENT_WINDOW_LEFT_POS):
(NS_PARENT_WINDOW_TOP_POS): Move to nsterm.m and simplify.
(ns_set_offset): Fix strange behaviours when using negative values.
(ns_set_window_size):
(ns_set_undecorated):
([EmacsView windowDidResize:]):
([EmacsView windowDidExitFullScreen]):
(ns_judge_scroll_bars): Remove references to updateFrameSize.
([EmacsView dealloc]): Unset resize notification and release buffer.
([EmacsView updateFrameSize:]): Remove function.
([EmacsView windowWillResize:toSize:]): Move some code to
viewDidResize.
([EmacsView viewDidResize]): New function.
([EmacsView initFrameFromEmacs:]): Set up resize notification and move
buffer creation until after the prerequisite objects are created.
([EmacsView toggleFullScreen:]): Set frame to the size of the
contentview, not the whole window, and remove reference to
updateFrameSize.
([EmacsView setRows:andColumns:]): Remove unused method.
([EmacsView windowDidMove:]): Tidy up.
---
Once again, I think this is right. Please test and let me know how you
get on.

src/nsmenu.m |   2 -
 src/nsterm.h |  15 ---
 src/nsterm.m | 309 +++++++++++++++++++++++----------------------------
 3 files changed, 142 insertions(+), 184 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 67f9a45a40..b7e4cbd565 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -1141,8 +1141,6 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct frame *)f
     }
 #endif
 
-  if (oldh != FRAME_TOOLBAR_HEIGHT (f))
-    [view updateFrameSize:YES];
   if (view->wait_for_tool_bar && FRAME_TOOLBAR_HEIGHT (f) > 0)
     {
       view->wait_for_tool_bar = NO;
diff --git a/src/nsterm.h b/src/nsterm.h
index 8396a542f7..e142dbd4f0 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -439,7 +439,6 @@ #define NS_DRAW_TO_BUFFER 1
 #endif
 @public
    struct frame *emacsframe;
-   int rows, cols;
    int scrollbarsNeedingUpdate;
    EmacsToolbar *toolbar;
    NSRect ns_userRect;
@@ -458,11 +457,9 @@ #define NS_DRAW_TO_BUFFER 1
 /* Emacs-side interface */
 - (instancetype) initFrameFromEmacs: (struct frame *) f;
 - (void) createToolbar: (struct frame *)f;
-- (void) setRows: (int) r andColumns: (int) c;
 - (void) setWindowClosing: (BOOL)closing;
 - (EmacsToolbar *) toolbar;
 - (void) deleteWorkingText;
-- (void) updateFrameSize: (BOOL) delay;
 - (void) handleFS;
 - (void) setFSValue: (int)value;
 - (void) toggleFullScreen: (id) sender;
@@ -1084,18 +1081,6 @@ #define NS_SCROLL_BAR_ADJUST_HORIZONTALLY(w, f)		\
    (FRAME_SCROLL_BAR_LINES (f) * FRAME_LINE_HEIGHT (f)	\
     - NS_SCROLL_BAR_HEIGHT (f)) : 0)
 
-/* Calculate system coordinates of the left and top of the parent
-   window or, if there is no parent window, the screen.  */
-#define NS_PARENT_WINDOW_LEFT_POS(f)                                    \
-  (FRAME_PARENT_FRAME (f) != NULL                                       \
-   ? [FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.origin.x : 0)
-#define NS_PARENT_WINDOW_TOP_POS(f)                                     \
-  (FRAME_PARENT_FRAME (f) != NULL                                       \
-   ? ([FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.origin.y    \
-      + [FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.size.height \
-      - FRAME_NS_TITLEBAR_HEIGHT (FRAME_PARENT_FRAME (f)))              \
-   : [[[NSScreen screens] objectAtIndex: 0] frame].size.height)
-
 #define FRAME_NS_FONT_TABLE(f) (FRAME_DISPLAY_INFO (f)->font_table)
 
 #define FRAME_FONTSET(f) ((f)->output_data.ns->fontset)
diff --git a/src/nsterm.m b/src/nsterm.m
index 04fc051223..461431622f 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -843,6 +843,32 @@ Free a pool and temporary objects it refers to (callable from C)
 }
 
 
+/* Get the frame rect, in system coordinates, of the parent window or,
+   if there is no parent window, the main screen.  */
+static inline NSRect
+ns_parent_window_rect (struct frame *f)
+{
+  NSRect parentRect;
+
+  if (FRAME_PARENT_FRAME (f) != NULL)
+    {
+      EmacsView *parentView = FRAME_NS_VIEW (FRAME_PARENT_FRAME (f));
+      parentRect = [parentView convertRect:[parentView frame]
+                                    toView:nil];
+      parentRect = [[parentView window] convertRectToScreen:parentRect];
+    }
+  else
+    parentRect = [[[NSScreen screens] objectAtIndex:0] frame];
+
+  return parentRect;
+}
+
+/* Calculate system coordinates of the left and top of the parent
+   window or, if there is no parent window, the main screen.  */
+#define NS_PARENT_WINDOW_LEFT_POS(f) NSMinX (ns_parent_window_rect (f))
+#define NS_PARENT_WINDOW_TOP_POS(f) NSMaxY (ns_parent_window_rect (f))
+
+
 static NSRect
 ns_row_rect (struct window *w, struct glyph_row *row,
                enum glyph_row_area area)
@@ -1741,61 +1767,64 @@ Hide the window (X11 semantics)
    -------------------------------------------------------------------------- */
 {
   NSView *view = FRAME_NS_VIEW (f);
-  NSScreen *screen = [[view window] screen];
+  NSRect windowFrame = [[view window] frame];
+  NSPoint topLeft;
 
   NSTRACE ("ns_set_offset");
 
   block_input ();
 
-  f->left_pos = xoff;
-  f->top_pos = yoff;
+  if (FRAME_PARENT_FRAME (f))
+    {
+      /* Convert the parent frame's view rectangle into screen
+         coords.  */
+      EmacsView *parentView = FRAME_NS_VIEW (FRAME_PARENT_FRAME (f));
+      NSRect parentRect = [parentView convertRect:[parentView frame]
+                                           toView:nil];
+      parentRect = [[parentView window] convertRectToScreen:parentRect];
+
+      if (f->size_hint_flags & XNegative)
+        topLeft.x = NSMaxX (parentRect) - NSWidth (windowFrame) + xoff;
+      else
+        topLeft.x = NSMinX (parentRect) + xoff;
 
-  if (view != nil)
+      if (f->size_hint_flags & YNegative)
+        topLeft.y = NSMinY (parentRect) + NSHeight (windowFrame) - yoff;
+      else
+        topLeft.y = NSMaxY (parentRect) - yoff;
+    }
+  else
     {
-      if (FRAME_PARENT_FRAME (f) == NULL && screen)
-        {
-          f->left_pos = f->size_hint_flags & XNegative
-            ? [screen visibleFrame].size.width + f->left_pos - FRAME_PIXEL_WIDTH (f)
-            : f->left_pos;
-          /* We use visibleFrame here to take menu bar into account.
-             Ideally we should also adjust left/top with visibleFrame.origin.  */
-
-          f->top_pos = f->size_hint_flags & YNegative
-            ? ([screen visibleFrame].size.height + f->top_pos
-               - FRAME_PIXEL_HEIGHT (f) - FRAME_NS_TITLEBAR_HEIGHT (f)
-               - FRAME_TOOLBAR_HEIGHT (f))
-            : f->top_pos;
-#ifdef NS_IMPL_GNUSTEP
-	  if (f->left_pos < 100)
-	    f->left_pos = 100;  /* don't overlap menu */
-#endif
-        }
-      else if (FRAME_PARENT_FRAME (f) != NULL)
-        {
-          struct frame *parent = FRAME_PARENT_FRAME (f);
+      /* If there is no parent frame then just convert to screen
+         coordinates, UNLESS we have negative values, in which case I
+         think it's best to position from the bottom and right of the
+         current screen rather than the main screen or whole
+         display.  */
+      NSRect screenFrame = [[[view window] screen] frame];
 
-          /* On X negative values for child frames always result in
-             positioning relative to the bottom right corner of the
-             parent frame.  */
-          if (f->left_pos < 0)
-            f->left_pos = FRAME_PIXEL_WIDTH (parent) - FRAME_PIXEL_WIDTH (f) + f->left_pos;
+      if (f->size_hint_flags & XNegative)
+        topLeft.x = NSMaxX (screenFrame) - NSWidth (windowFrame) + xoff;
+      else
+        topLeft.x = xoff;
 
-          if (f->top_pos < 0)
-            f->top_pos = FRAME_PIXEL_HEIGHT (parent) + FRAME_TOOLBAR_HEIGHT (parent)
-              - FRAME_PIXEL_HEIGHT (f) + f->top_pos;
-        }
+      if (f->size_hint_flags & YNegative)
+        topLeft.y = NSMinY (screenFrame) + NSHeight (windowFrame) - yoff;
+      else
+        topLeft.y = NSMaxY ([[[NSScreen screens] objectAtIndex:0] frame]) - yoff;
+
+#ifdef NS_IMPL_GNUSTEP
+      /* Don't overlap the menu.
 
-      /* Constrain the setFrameTopLeftPoint so we don't move behind the
-         menu bar.  */
-      NSPoint pt = NSMakePoint (SCREENMAXBOUND (f->left_pos
-                                                + NS_PARENT_WINDOW_LEFT_POS (f)),
-                                SCREENMAXBOUND (NS_PARENT_WINDOW_TOP_POS (f)
-                                                - f->top_pos));
-      NSTRACE_POINT ("setFrameTopLeftPoint", pt);
-      [[view window] setFrameTopLeftPoint: pt];
-      f->size_hint_flags &= ~(XNegative|YNegative);
+         FIXME: Surely there's a better way than just hardcoding 100
+         in here?  */
+      boundsRect.origin.x = 100;
+#endif
     }
 
+  NSTRACE_POINT ("setFrameTopLeftPoint", topLeft);
+  [[view window] setFrameTopLeftPoint:topLeft];
+  f->size_hint_flags &= ~(XNegative|YNegative);
+
   unblock_input ();
 }
 
@@ -1864,7 +1893,6 @@ Hide the window (X11 semantics)
 
   [window setFrame: wr display: YES];
 
-  [view updateFrameSize: NO];
   unblock_input ();
 }
 
@@ -1913,7 +1941,6 @@ Hide the window (X11 semantics)
          so some key presses (TAB) are swallowed by the system.  */
       [window makeFirstResponder: view];
 
-      [view updateFrameSize: NO];
       unblock_input ();
     }
 }
@@ -5024,9 +5051,6 @@ in certain situations (rapid incoming events).
       if ([view judge])
         removed = YES;
     }
-
-  if (removed)
-    [eview updateFrameSize: NO];
 }
 
 /* ==========================================================================
@@ -6198,6 +6222,15 @@ - (void) setWindowClosing: (BOOL)closing
 - (void)dealloc
 {
   NSTRACE ("[EmacsView dealloc]");
+
+  /* Clear the view resize notification.  */
+  [[NSNotificationCenter defaultCenter]
+    removeObserver:self
+              name:NSViewFrameDidChangeNotification
+            object:nil];
+
+  CGContextRelease (drawingBuffer);
+
   [toolbar release];
   if (fs_state == FULLSCREEN_BOTH)
     [nonfs_window release];
@@ -7039,108 +7072,12 @@ - (BOOL)windowShouldClose: (id)sender
   return NO;
 }
 
-- (void) updateFrameSize: (BOOL) delay
-{
-  NSWindow *window = [self window];
-  NSRect wr = [window frame];
-  int extra = 0;
-  int oldc = cols, oldr = rows;
-  int oldw = FRAME_PIXEL_WIDTH (emacsframe);
-  int oldh = FRAME_PIXEL_HEIGHT (emacsframe);
-  int neww, newh;
-
-  NSTRACE ("[EmacsView updateFrameSize:]");
-  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
-  NSTRACE_RECT ("Original frame", wr);
-  NSTRACE_MSG  ("Original columns: %d", cols);
-  NSTRACE_MSG  ("Original rows: %d", rows);
-
-  if (! [self isFullscreen])
-    {
-      int toolbar_height;
-#ifdef NS_IMPL_GNUSTEP
-      // GNUstep does not always update the tool bar height.  Force it.
-      if (toolbar && [toolbar isVisible])
-          update_frame_tool_bar (emacsframe);
-#endif
-
-      toolbar_height = FRAME_TOOLBAR_HEIGHT (emacsframe);
-      if (toolbar_height < 0)
-        toolbar_height = 35;
-
-      extra = FRAME_NS_TITLEBAR_HEIGHT (emacsframe)
-        + toolbar_height;
-    }
-
-  if (wait_for_tool_bar)
-    {
-      /* The toolbar height is always 0 in fullscreen and undecorated
-         frames, so don't wait for it to become available.  */
-      if (FRAME_TOOLBAR_HEIGHT (emacsframe) == 0
-          && FRAME_UNDECORATED (emacsframe) == false
-          && ! [self isFullscreen])
-        {
-          NSTRACE_MSG ("Waiting for toolbar");
-          return;
-        }
-      wait_for_tool_bar = NO;
-    }
-
-  neww = (int)wr.size.width - emacsframe->border_width;
-  newh = (int)wr.size.height - extra;
-
-  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
-  NSTRACE_MSG ("FRAME_TOOLBAR_HEIGHT: %d", FRAME_TOOLBAR_HEIGHT (emacsframe));
-  NSTRACE_MSG ("FRAME_NS_TITLEBAR_HEIGHT: %d", FRAME_NS_TITLEBAR_HEIGHT (emacsframe));
-
-  cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
-  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
-
-  if (cols < MINWIDTH)
-    cols = MINWIDTH;
-
-  if (rows < MINHEIGHT)
-    rows = MINHEIGHT;
-
-  NSTRACE_MSG ("New columns: %d", cols);
-  NSTRACE_MSG ("New rows: %d", rows);
-
-  if (oldr != rows || oldc != cols || neww != oldw || newh != oldh)
-    {
-      NSView *view = FRAME_NS_VIEW (emacsframe);
-
-      change_frame_size (emacsframe,
-                         FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
-                         FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
-                         0, delay, 0, 1);
-      SET_FRAME_GARBAGED (emacsframe);
-      cancel_mouse_face (emacsframe);
-
-      /* The next two lines set the frame to the same size as we've
-         already set above.  We need to do this when we switch back
-         from non-native fullscreen, in other circumstances it appears
-         to be a noop.  (bug#28872) */
-      wr = NSMakeRect (0, 0, neww, newh);
-      [view setFrame: wr];
-#ifdef NS_DRAW_TO_BUFFER
-      [self createDrawingBuffer];
-#endif
-
-      // To do: consider using [NSNotificationCenter postNotificationName:].
-      [self windowDidMove: // Update top/left.
-	      [NSNotification notificationWithName:NSWindowDidMoveNotification
-					    object:[view window]]];
-    }
-  else
-    {
-      NSTRACE_MSG ("No change");
-    }
-}
 
 - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
 /* Normalize frame to gridded text size.  */
 {
   int extra = 0;
+  int cols, rows;
 
   NSTRACE ("[EmacsView windowWillResize:toSize: " NSTRACE_FMT_SIZE "]",
            NSTRACE_ARG_SIZE (frameSize));
@@ -7277,11 +7214,6 @@ - (void)windowDidResize: (NSNotification *)notification
   sz = [self windowWillResize: theWindow toSize: sz];
 #endif /* NS_IMPL_GNUSTEP */
 
-  if (cols > 0 && rows > 0)
-    {
-      [self updateFrameSize: YES];
-    }
-
   ns_send_appdefined (-1);
 }
 
@@ -7302,6 +7234,51 @@ - (void)viewDidEndLiveResize
 #endif /* NS_IMPL_COCOA */
 
 
+- (void)viewDidResize:(NSNotification *)notification
+{
+  NSRect frame = [self frame];
+  int oldw, oldh, neww, newh;
+
+  if (! FRAME_LIVE_P (emacsframe))
+    return;
+
+#ifdef NS_DRAW_TO_BUFFER
+  CGFloat scale = [[self window] backingScaleFactor];
+  oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
+  oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
+#else
+  oldw = FRAME_PIXEL_WIDTH (emacsframe);
+  oldh = FRAME_PIXEL_HEIGHT (emacsframe);
+#endif
+  neww = (int)NSWidth (frame);
+  newh = (int)NSHeight (frame);
+
+  NSTRACE ("[EmacsView viewDidResize]");
+
+  /* Don't want to do anything when the view size hasn't changed. */
+  if ((oldh == newh && oldw == neww))
+    {
+      NSTRACE_MSG ("No change");
+      return;
+    }
+
+  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
+  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
+
+  change_frame_size (emacsframe,
+                     FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
+                     FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
+                     0, YES, 0, 1);
+
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+  ns_clear_frame (emacsframe);
+  SET_FRAME_GARBAGED (emacsframe);
+  cancel_mouse_face (emacsframe);
+}
+
+
 - (void)windowDidBecomeKey: (NSNotification *)notification
 /* cf. x_detect_focus_change(), x_focus_changed(), x_new_focus_frame() */
 {
@@ -7463,10 +7440,6 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   maximizing_resize = NO;
 #endif
 
-#ifdef NS_DRAW_TO_BUFFER
-  [self createDrawingBuffer];
-#endif
-
   win = [[EmacsWindow alloc]
             initWithContentRect: r
                       styleMask: (FRAME_UNDECORATED (f)
@@ -7572,6 +7545,17 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+
+  /* Set up view resize notifications.  */
+  [self setPostsFrameChangedNotifications:YES];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:self
+         selector:@selector (viewDidResize:)
+             name:NSViewFrameDidChangeNotification object:nil];
+
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway.  */
@@ -7601,9 +7585,8 @@ - (void)windowDidMove: sender
     return;
   if (screen != nil)
     {
-      emacsframe->left_pos = r.origin.x - NS_PARENT_WINDOW_LEFT_POS (emacsframe);
-      emacsframe->top_pos =
-        NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
+      emacsframe->left_pos = NSMinX (r) - NS_PARENT_WINDOW_LEFT_POS (emacsframe);
+      emacsframe->top_pos = NS_PARENT_WINDOW_TOP_POS (emacsframe) - NSMaxY (r);
 
       // FIXME: after event part below didExitFullScreen is not received
       // if (emacs_event)
@@ -7902,7 +7885,6 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     {
       [toolbar setVisible:YES];
       update_frame_tool_bar (emacsframe);
-      [self updateFrameSize:YES];
       [[self window] display];
     }
   else
@@ -8115,11 +8097,11 @@ - (void)toggleFullScreen: (id)sender
       // send notifications.
 
       [self windowWillExitFullScreen];
-      [fw setFrame: [w frame] display:YES animate:ns_use_fullscreen_animation];
+      [fw setFrame:[[w contentView] frame]
+                    display:YES animate:ns_use_fullscreen_animation];
       [fw close];
       [w makeKeyAndOrderFront:NSApp];
       [self windowDidExitFullScreen];
-      [self updateFrameSize:YES];
     }
 }
 
@@ -8628,13 +8610,6 @@ - (instancetype)setMiniwindowImage: (BOOL) setMini
 }
 
 
-- (void) setRows: (int) r andColumns: (int) c
-{
-  NSTRACE ("[EmacsView setRows:%d andColumns:%d]", r, c);
-  rows = r;
-  cols = c;
-}
-
 - (int) fullscreenState
 {
   return fs_state;
-- 
2.24.0


-- 
Alan Third

Message #38 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Mon, 06 Apr 2020 09:57:16 +0300

Alan Third <alan <at> idiocy.org> writes:

> Once again, I think this is right. Please test and let me know how you
> get on.

This code works differently in the patched version:

(progn
  (tool-bar-mode -1)
  (let ((child-frame (make-frame `((parent-frame . ,(selected-frame))
                                   (top . 0)
                                   (left . 0)
                                   (width . 1.0)
                                   (height . 1))))
        (new-frame (make-frame)))
    (set-frame-width new-frame (+ 20 (frame-parameter new-frame 'width)))
    (modify-frame-parameters child-frame `((parent-frame . ,new-frame)))
    (modify-frame-parameters child-frame '((top . 0)
                                           (left . 0)
                                           (width . 1.0)
                                           (height . 1)))))

Child frame doesn't occupy full width of the parent frame.

> +  ns_clear_frame (emacsframe);

Can this line be removed from the patch? I've described reasons to
remove it in another letter:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2020-03/msg01060.html

Thanks!

Message #41 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Mon, 6 Apr 2020 19:44:11 +0100

* src/nsmenu.m (update_frame_tool_bar): Remove reference to
updateFrameSize.
* src/nsterm.h: ([EmacsView updateFrameSize]):
([EmacsView setRows:andColumns:]): Remove unused
method definitions.
(NS_PARENT_WINDOW_LEFT_POS):
(NS_PARENT_WINDOW_TOP_POS): Move to nsterm.m.
* src/nsterm.m (ns_parent_window_rect): New function.
(NS_PARENT_WINDOW_LEFT_POS):
(NS_PARENT_WINDOW_TOP_POS): Move to nsterm.m and simplify.
(ns_set_offset): Fix strange behaviours when using negative values.
(ns_set_window_size):
(ns_set_undecorated):
([EmacsView windowDidResize:]):
([EmacsView windowDidExitFullScreen]):
(ns_judge_scroll_bars): Remove references to updateFrameSize.
([EmacsView dealloc]): Unset resize notification and release buffer.
([EmacsView updateFrameSize:]): Remove function.
([EmacsView windowWillResize:toSize:]): Move some code to
viewDidResize.
([EmacsView viewDidResize]): New function.
([EmacsView initFrameFromEmacs:]): Set up resize notification and move
buffer creation until after the prerequisite objects are created.
([EmacsView toggleFullScreen:]): Set frame to the size of the
contentview, not the whole window, and remove reference to
updateFrameSize.
([EmacsView setRows:andColumns:]): Remove unused method.
([EmacsView windowDidMove:]): Tidy up.
---
Out of interest, do you have a test suite, or are you just coming up
with these tests on the fly?

 src/nsmenu.m |   2 -
 src/nsterm.h |  15 ---
 src/nsterm.m | 318 ++++++++++++++++++++++++---------------------------
 3 files changed, 150 insertions(+), 185 deletions(-)

diff --git a/src/nsmenu.m b/src/nsmenu.m
index 67f9a45a40..b7e4cbd565 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -1141,8 +1141,6 @@ - (Lisp_Object)runMenuAt: (NSPoint)p forFrame: (struct frame *)f
     }
 #endif
 
-  if (oldh != FRAME_TOOLBAR_HEIGHT (f))
-    [view updateFrameSize:YES];
   if (view->wait_for_tool_bar && FRAME_TOOLBAR_HEIGHT (f) > 0)
     {
       view->wait_for_tool_bar = NO;
diff --git a/src/nsterm.h b/src/nsterm.h
index 8396a542f7..e142dbd4f0 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -439,7 +439,6 @@ #define NS_DRAW_TO_BUFFER 1
 #endif
 @public
    struct frame *emacsframe;
-   int rows, cols;
    int scrollbarsNeedingUpdate;
    EmacsToolbar *toolbar;
    NSRect ns_userRect;
@@ -458,11 +457,9 @@ #define NS_DRAW_TO_BUFFER 1
 /* Emacs-side interface */
 - (instancetype) initFrameFromEmacs: (struct frame *) f;
 - (void) createToolbar: (struct frame *)f;
-- (void) setRows: (int) r andColumns: (int) c;
 - (void) setWindowClosing: (BOOL)closing;
 - (EmacsToolbar *) toolbar;
 - (void) deleteWorkingText;
-- (void) updateFrameSize: (BOOL) delay;
 - (void) handleFS;
 - (void) setFSValue: (int)value;
 - (void) toggleFullScreen: (id) sender;
@@ -1084,18 +1081,6 @@ #define NS_SCROLL_BAR_ADJUST_HORIZONTALLY(w, f)		\
    (FRAME_SCROLL_BAR_LINES (f) * FRAME_LINE_HEIGHT (f)	\
     - NS_SCROLL_BAR_HEIGHT (f)) : 0)
 
-/* Calculate system coordinates of the left and top of the parent
-   window or, if there is no parent window, the screen.  */
-#define NS_PARENT_WINDOW_LEFT_POS(f)                                    \
-  (FRAME_PARENT_FRAME (f) != NULL                                       \
-   ? [FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.origin.x : 0)
-#define NS_PARENT_WINDOW_TOP_POS(f)                                     \
-  (FRAME_PARENT_FRAME (f) != NULL                                       \
-   ? ([FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.origin.y    \
-      + [FRAME_NS_VIEW (FRAME_PARENT_FRAME (f)) window].frame.size.height \
-      - FRAME_NS_TITLEBAR_HEIGHT (FRAME_PARENT_FRAME (f)))              \
-   : [[[NSScreen screens] objectAtIndex: 0] frame].size.height)
-
 #define FRAME_NS_FONT_TABLE(f) (FRAME_DISPLAY_INFO (f)->font_table)
 
 #define FRAME_FONTSET(f) ((f)->output_data.ns->fontset)
diff --git a/src/nsterm.m b/src/nsterm.m
index 04fc051223..4318622377 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -843,6 +843,32 @@ Free a pool and temporary objects it refers to (callable from C)
 }
 
 
+/* Get the frame rect, in system coordinates, of the parent window or,
+   if there is no parent window, the main screen.  */
+static inline NSRect
+ns_parent_window_rect (struct frame *f)
+{
+  NSRect parentRect;
+
+  if (FRAME_PARENT_FRAME (f) != NULL)
+    {
+      EmacsView *parentView = FRAME_NS_VIEW (FRAME_PARENT_FRAME (f));
+      parentRect = [parentView convertRect:[parentView frame]
+                                    toView:nil];
+      parentRect = [[parentView window] convertRectToScreen:parentRect];
+    }
+  else
+    parentRect = [[[NSScreen screens] objectAtIndex:0] frame];
+
+  return parentRect;
+}
+
+/* Calculate system coordinates of the left and top of the parent
+   window or, if there is no parent window, the main screen.  */
+#define NS_PARENT_WINDOW_LEFT_POS(f) NSMinX (ns_parent_window_rect (f))
+#define NS_PARENT_WINDOW_TOP_POS(f) NSMaxY (ns_parent_window_rect (f))
+
+
 static NSRect
 ns_row_rect (struct window *w, struct glyph_row *row,
                enum glyph_row_area area)
@@ -1741,61 +1767,64 @@ Hide the window (X11 semantics)
    -------------------------------------------------------------------------- */
 {
   NSView *view = FRAME_NS_VIEW (f);
-  NSScreen *screen = [[view window] screen];
+  NSRect windowFrame = [[view window] frame];
+  NSPoint topLeft;
 
   NSTRACE ("ns_set_offset");
 
   block_input ();
 
-  f->left_pos = xoff;
-  f->top_pos = yoff;
+  if (FRAME_PARENT_FRAME (f))
+    {
+      /* Convert the parent frame's view rectangle into screen
+         coords.  */
+      EmacsView *parentView = FRAME_NS_VIEW (FRAME_PARENT_FRAME (f));
+      NSRect parentRect = [parentView convertRect:[parentView frame]
+                                           toView:nil];
+      parentRect = [[parentView window] convertRectToScreen:parentRect];
+
+      if (f->size_hint_flags & XNegative)
+        topLeft.x = NSMaxX (parentRect) - NSWidth (windowFrame) + xoff;
+      else
+        topLeft.x = NSMinX (parentRect) + xoff;
 
-  if (view != nil)
+      if (f->size_hint_flags & YNegative)
+        topLeft.y = NSMinY (parentRect) + NSHeight (windowFrame) - yoff;
+      else
+        topLeft.y = NSMaxY (parentRect) - yoff;
+    }
+  else
     {
-      if (FRAME_PARENT_FRAME (f) == NULL && screen)
-        {
-          f->left_pos = f->size_hint_flags & XNegative
-            ? [screen visibleFrame].size.width + f->left_pos - FRAME_PIXEL_WIDTH (f)
-            : f->left_pos;
-          /* We use visibleFrame here to take menu bar into account.
-             Ideally we should also adjust left/top with visibleFrame.origin.  */
-
-          f->top_pos = f->size_hint_flags & YNegative
-            ? ([screen visibleFrame].size.height + f->top_pos
-               - FRAME_PIXEL_HEIGHT (f) - FRAME_NS_TITLEBAR_HEIGHT (f)
-               - FRAME_TOOLBAR_HEIGHT (f))
-            : f->top_pos;
-#ifdef NS_IMPL_GNUSTEP
-	  if (f->left_pos < 100)
-	    f->left_pos = 100;  /* don't overlap menu */
-#endif
-        }
-      else if (FRAME_PARENT_FRAME (f) != NULL)
-        {
-          struct frame *parent = FRAME_PARENT_FRAME (f);
+      /* If there is no parent frame then just convert to screen
+         coordinates, UNLESS we have negative values, in which case I
+         think it's best to position from the bottom and right of the
+         current screen rather than the main screen or whole
+         display.  */
+      NSRect screenFrame = [[[view window] screen] frame];
 
-          /* On X negative values for child frames always result in
-             positioning relative to the bottom right corner of the
-             parent frame.  */
-          if (f->left_pos < 0)
-            f->left_pos = FRAME_PIXEL_WIDTH (parent) - FRAME_PIXEL_WIDTH (f) + f->left_pos;
+      if (f->size_hint_flags & XNegative)
+        topLeft.x = NSMaxX (screenFrame) - NSWidth (windowFrame) + xoff;
+      else
+        topLeft.x = xoff;
 
-          if (f->top_pos < 0)
-            f->top_pos = FRAME_PIXEL_HEIGHT (parent) + FRAME_TOOLBAR_HEIGHT (parent)
-              - FRAME_PIXEL_HEIGHT (f) + f->top_pos;
-        }
+      if (f->size_hint_flags & YNegative)
+        topLeft.y = NSMinY (screenFrame) + NSHeight (windowFrame) - yoff;
+      else
+        topLeft.y = NSMaxY ([[[NSScreen screens] objectAtIndex:0] frame]) - yoff;
+
+#ifdef NS_IMPL_GNUSTEP
+      /* Don't overlap the menu.
 
-      /* Constrain the setFrameTopLeftPoint so we don't move behind the
-         menu bar.  */
-      NSPoint pt = NSMakePoint (SCREENMAXBOUND (f->left_pos
-                                                + NS_PARENT_WINDOW_LEFT_POS (f)),
-                                SCREENMAXBOUND (NS_PARENT_WINDOW_TOP_POS (f)
-                                                - f->top_pos));
-      NSTRACE_POINT ("setFrameTopLeftPoint", pt);
-      [[view window] setFrameTopLeftPoint: pt];
-      f->size_hint_flags &= ~(XNegative|YNegative);
+         FIXME: Surely there's a better way than just hardcoding 100
+         in here?  */
+      boundsRect.origin.x = 100;
+#endif
     }
 
+  NSTRACE_POINT ("setFrameTopLeftPoint", topLeft);
+  [[view window] setFrameTopLeftPoint:topLeft];
+  f->size_hint_flags &= ~(XNegative|YNegative);
+
   unblock_input ();
 }
 
@@ -1862,9 +1891,16 @@ Hide the window (X11 semantics)
 	   make_fixnum (FRAME_NS_TITLEBAR_HEIGHT (f)),
 	   make_fixnum (FRAME_TOOLBAR_HEIGHT (f))));
 
-  [window setFrame: wr display: YES];
+ /* Usually it seems safe to delay changing the frame size, but when a
+    series of actions are taken with no redisplay between them then we
+    can end up using old values so don't delay here.  */
+ change_frame_size (f,
+                    FRAME_PIXEL_TO_TEXT_WIDTH (f, pixelwidth),
+                    FRAME_PIXEL_TO_TEXT_HEIGHT (f, pixelheight),
+                    0, NO, 0, 1);
+
+  [window setFrame:wr display:NO];
 
-  [view updateFrameSize: NO];
   unblock_input ();
 }
 
@@ -1913,7 +1949,6 @@ Hide the window (X11 semantics)
          so some key presses (TAB) are swallowed by the system.  */
       [window makeFirstResponder: view];
 
-      [view updateFrameSize: NO];
       unblock_input ();
     }
 }
@@ -5024,9 +5059,6 @@ in certain situations (rapid incoming events).
       if ([view judge])
         removed = YES;
     }
-
-  if (removed)
-    [eview updateFrameSize: NO];
 }
 
 /* ==========================================================================
@@ -6198,6 +6230,15 @@ - (void) setWindowClosing: (BOOL)closing
 - (void)dealloc
 {
   NSTRACE ("[EmacsView dealloc]");
+
+  /* Clear the view resize notification.  */
+  [[NSNotificationCenter defaultCenter]
+    removeObserver:self
+              name:NSViewFrameDidChangeNotification
+            object:nil];
+
+  CGContextRelease (drawingBuffer);
+
   [toolbar release];
   if (fs_state == FULLSCREEN_BOTH)
     [nonfs_window release];
@@ -7039,108 +7080,12 @@ - (BOOL)windowShouldClose: (id)sender
   return NO;
 }
 
-- (void) updateFrameSize: (BOOL) delay
-{
-  NSWindow *window = [self window];
-  NSRect wr = [window frame];
-  int extra = 0;
-  int oldc = cols, oldr = rows;
-  int oldw = FRAME_PIXEL_WIDTH (emacsframe);
-  int oldh = FRAME_PIXEL_HEIGHT (emacsframe);
-  int neww, newh;
-
-  NSTRACE ("[EmacsView updateFrameSize:]");
-  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
-  NSTRACE_RECT ("Original frame", wr);
-  NSTRACE_MSG  ("Original columns: %d", cols);
-  NSTRACE_MSG  ("Original rows: %d", rows);
-
-  if (! [self isFullscreen])
-    {
-      int toolbar_height;
-#ifdef NS_IMPL_GNUSTEP
-      // GNUstep does not always update the tool bar height.  Force it.
-      if (toolbar && [toolbar isVisible])
-          update_frame_tool_bar (emacsframe);
-#endif
-
-      toolbar_height = FRAME_TOOLBAR_HEIGHT (emacsframe);
-      if (toolbar_height < 0)
-        toolbar_height = 35;
-
-      extra = FRAME_NS_TITLEBAR_HEIGHT (emacsframe)
-        + toolbar_height;
-    }
-
-  if (wait_for_tool_bar)
-    {
-      /* The toolbar height is always 0 in fullscreen and undecorated
-         frames, so don't wait for it to become available.  */
-      if (FRAME_TOOLBAR_HEIGHT (emacsframe) == 0
-          && FRAME_UNDECORATED (emacsframe) == false
-          && ! [self isFullscreen])
-        {
-          NSTRACE_MSG ("Waiting for toolbar");
-          return;
-        }
-      wait_for_tool_bar = NO;
-    }
-
-  neww = (int)wr.size.width - emacsframe->border_width;
-  newh = (int)wr.size.height - extra;
-
-  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
-  NSTRACE_MSG ("FRAME_TOOLBAR_HEIGHT: %d", FRAME_TOOLBAR_HEIGHT (emacsframe));
-  NSTRACE_MSG ("FRAME_NS_TITLEBAR_HEIGHT: %d", FRAME_NS_TITLEBAR_HEIGHT (emacsframe));
-
-  cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
-  rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
-
-  if (cols < MINWIDTH)
-    cols = MINWIDTH;
-
-  if (rows < MINHEIGHT)
-    rows = MINHEIGHT;
-
-  NSTRACE_MSG ("New columns: %d", cols);
-  NSTRACE_MSG ("New rows: %d", rows);
-
-  if (oldr != rows || oldc != cols || neww != oldw || newh != oldh)
-    {
-      NSView *view = FRAME_NS_VIEW (emacsframe);
-
-      change_frame_size (emacsframe,
-                         FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
-                         FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
-                         0, delay, 0, 1);
-      SET_FRAME_GARBAGED (emacsframe);
-      cancel_mouse_face (emacsframe);
-
-      /* The next two lines set the frame to the same size as we've
-         already set above.  We need to do this when we switch back
-         from non-native fullscreen, in other circumstances it appears
-         to be a noop.  (bug#28872) */
-      wr = NSMakeRect (0, 0, neww, newh);
-      [view setFrame: wr];
-#ifdef NS_DRAW_TO_BUFFER
-      [self createDrawingBuffer];
-#endif
-
-      // To do: consider using [NSNotificationCenter postNotificationName:].
-      [self windowDidMove: // Update top/left.
-	      [NSNotification notificationWithName:NSWindowDidMoveNotification
-					    object:[view window]]];
-    }
-  else
-    {
-      NSTRACE_MSG ("No change");
-    }
-}
 
 - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
 /* Normalize frame to gridded text size.  */
 {
   int extra = 0;
+  int cols, rows;
 
   NSTRACE ("[EmacsView windowWillResize:toSize: " NSTRACE_FMT_SIZE "]",
            NSTRACE_ARG_SIZE (frameSize));
@@ -7277,11 +7222,6 @@ - (void)windowDidResize: (NSNotification *)notification
   sz = [self windowWillResize: theWindow toSize: sz];
 #endif /* NS_IMPL_GNUSTEP */
 
-  if (cols > 0 && rows > 0)
-    {
-      [self updateFrameSize: YES];
-    }
-
   ns_send_appdefined (-1);
 }
 
@@ -7302,6 +7242,50 @@ - (void)viewDidEndLiveResize
 #endif /* NS_IMPL_COCOA */
 
 
+- (void)viewDidResize:(NSNotification *)notification
+{
+  NSRect frame = [self frame];
+  int oldw, oldh, neww, newh;
+
+  if (! FRAME_LIVE_P (emacsframe))
+    return;
+
+#ifdef NS_DRAW_TO_BUFFER
+  CGFloat scale = [[self window] backingScaleFactor];
+  oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
+  oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
+#else
+  oldw = FRAME_PIXEL_WIDTH (emacsframe);
+  oldh = FRAME_PIXEL_HEIGHT (emacsframe);
+#endif
+  neww = (int)NSWidth (frame);
+  newh = (int)NSHeight (frame);
+
+  NSTRACE ("[EmacsView viewDidResize]");
+
+  /* Don't want to do anything when the view size hasn't changed. */
+  if ((oldh == newh && oldw == neww))
+    {
+      NSTRACE_MSG ("No change");
+      return;
+    }
+
+  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
+  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
+
+  change_frame_size (emacsframe,
+                     FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
+                     FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
+                     0, YES, 0, 1);
+
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+  SET_FRAME_GARBAGED (emacsframe);
+  cancel_mouse_face (emacsframe);
+}
+
+
 - (void)windowDidBecomeKey: (NSNotification *)notification
 /* cf. x_detect_focus_change(), x_focus_changed(), x_new_focus_frame() */
 {
@@ -7463,10 +7447,6 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   maximizing_resize = NO;
 #endif
 
-#ifdef NS_DRAW_TO_BUFFER
-  [self createDrawingBuffer];
-#endif
-
   win = [[EmacsWindow alloc]
             initWithContentRect: r
                       styleMask: (FRAME_UNDECORATED (f)
@@ -7572,6 +7552,17 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
   [NSApp registerServicesMenuSendTypes: ns_send_types
                            returnTypes: [NSArray array]];
 
+#ifdef NS_DRAW_TO_BUFFER
+  [self createDrawingBuffer];
+#endif
+
+  /* Set up view resize notifications.  */
+  [self setPostsFrameChangedNotifications:YES];
+  [[NSNotificationCenter defaultCenter]
+      addObserver:self
+         selector:@selector (viewDidResize:)
+             name:NSViewFrameDidChangeNotification object:nil];
+
   /* macOS Sierra automatically enables tabbed windows.  We can't
      allow this to be enabled until it's available on a Free system.
      Currently it only happens by accident and is buggy anyway.  */
@@ -7601,9 +7592,8 @@ - (void)windowDidMove: sender
     return;
   if (screen != nil)
     {
-      emacsframe->left_pos = r.origin.x - NS_PARENT_WINDOW_LEFT_POS (emacsframe);
-      emacsframe->top_pos =
-        NS_PARENT_WINDOW_TOP_POS (emacsframe) - (r.origin.y + r.size.height);
+      emacsframe->left_pos = NSMinX (r) - NS_PARENT_WINDOW_LEFT_POS (emacsframe);
+      emacsframe->top_pos = NS_PARENT_WINDOW_TOP_POS (emacsframe) - NSMaxY (r);
 
       // FIXME: after event part below didExitFullScreen is not received
       // if (emacs_event)
@@ -7902,7 +7892,6 @@ - (void)windowDidExitFullScreen /* provided for direct calls */
     {
       [toolbar setVisible:YES];
       update_frame_tool_bar (emacsframe);
-      [self updateFrameSize:YES];
       [[self window] display];
     }
   else
@@ -8115,11 +8104,11 @@ - (void)toggleFullScreen: (id)sender
       // send notifications.
 
       [self windowWillExitFullScreen];
-      [fw setFrame: [w frame] display:YES animate:ns_use_fullscreen_animation];
+      [fw setFrame:[[w contentView] frame]
+                    display:YES animate:ns_use_fullscreen_animation];
       [fw close];
       [w makeKeyAndOrderFront:NSApp];
       [self windowDidExitFullScreen];
-      [self updateFrameSize:YES];
     }
 }
 
@@ -8628,13 +8617,6 @@ - (instancetype)setMiniwindowImage: (BOOL) setMini
 }
 
 
-- (void) setRows: (int) r andColumns: (int) c
-{
-  NSTRACE ("[EmacsView setRows:%d andColumns:%d]", r, c);
-  rows = r;
-  cols = c;
-}
-
 - (int) fullscreenState
 {
   return fs_state;
-- 
2.26.0


-- 
Alan Third

Message #44 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, aaronjensen <at> gmail.com, 28872 <at> debbugs.gnu.org
Subject: Re: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Tue, 07 Apr 2020 11:14:00 +0300

Alan Third <alan <at> idiocy.org> writes:

> Out of interest, do you have a test suite, or are you just coming up
> with these tests on the fly?

I'm using these minor modes which utilize child frame to display
minibuffer and completions buffer:
https://github.com/muffinmad/emacs-mini-frame
https://github.com/muffinmad/emacs-completions-frame

Single child frame moves across all frames so child frame positioning
issues are quickly comes in sight for me.  And then the fun part: narrow
down the code so the issue can be reproduced in `emacs -Q`.

The patch v4 works good so far, thank you!

Message #49 received at 40200-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Cc: 40200-done <at> debbugs.gnu.org, aaronjensen <at> gmail.com,
 28872-done <at> debbugs.gnu.org
Subject: Re: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)
Date: Thu, 16 Apr 2020 19:24:10 +0100

On Tue, Apr 07, 2020 at 11:14:00AM +0300, Andrii Kolomoiets wrote:
> The patch v4 works good so far, thank you!

Since I’ve not heard anything bad for over a week now I’ve pushed the
change to the master branch.

Thanks for your help.
-- 
Alan Third

Message #52 received at 40200-done <at> debbugs.gnu.org (full text, mbox):

From: HaiJun Zhang <netjune <at> outlook.com>
To: Andrii Kolomoiets <andreyk.mad <at> gmail.com>, Alan Third
 <alan <at> idiocy.org>
Cc: 40200-done <at> debbugs.gnu.org, aaronjensen <at> gmail.com,
 28872-done <at> debbugs.gnu.org
Subject: Re: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sat, 18 Apr 2020 09:52:47 +0800

[Message part 1 (text/plain, inline)]

There is problem here. After emacs startups, there is no mini buffer window and there is a blank rectangle at the right edge of the frame.

It is with my profile and it can’t be reproduced with emacs -Q.

No problem before the commit.

在 2020年4月17日 +0800 AM2:25，Alan Third <alan <at> idiocy.org>，写道：
> On Tue, Apr 07, 2020 at 11:14:00AM +0300, Andrii Kolomoiets wrote:
> > The patch v4 works good so far, thank you!
>
> Since I’ve not heard anything bad for over a week now I’ve pushed the
> change to the master branch.
>
> Thanks for your help.
> --
> Alan Third
>
>
>

[Message part 2 (text/html, inline)]

[WX20200417-153341@2x.png (image/png, attachment)]

Message #55 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: HaiJun Zhang <netjune <at> outlook.com>
Cc: 40200 <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200,
 bug#28872)
Date: Sat, 18 Apr 2020 13:07:14 +0100

On Sat, Apr 18, 2020 at 09:52:47AM +0800, HaiJun Zhang wrote:
> There is problem here. After emacs startups, there is no mini buffer
> window and there is a blank rectangle at the right edge of the
> frame.
> 
> It is with my profile and it can’t be reproduced with emacs -Q.
> 
> No problem before the commit.

Can you try working out what part of your configuration is causing
this?

Probably there’s a place in the code where I need to either force
Emacs to update the frame size, which is unlikely, or Emacs updates
the frame size and I need to force macOS to do the actual resize.

-- 
Alan Third

Message #58 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: HaiJun Zhang <netjune <at> outlook.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sat, 18 Apr 2020 22:12:27 +0800

[Message part 1 (text/plain, inline)]

I can reproduce it with the following lines in ~/.emacs:

(setq inhibit-startup-message t
   inhibit-startup-buffer-menu t)

(setq default-frame-alist ‘(
       (width . 85)
       (height . 35)
       (vertical-scroll-bars . nil)
       (horizontal-scroll-bars . nil)
       ))

(set-frame-font “Monospace 16”)


在 2020年4月18日 +0800 PM8:07，Alan Third <alan <at> idiocy.org>，写道：
> On Sat, Apr 18, 2020 at 09:52:47AM +0800, HaiJun Zhang wrote:
> > There is problem here. After emacs startups, there is no mini buffer
> > window and there is a blank rectangle at the right edge of the
> > frame.
> >
> > It is with my profile and it can’t be reproduced with emacs -Q.
> >
> > No problem before the commit.
>
> Can you try working out what part of your configuration is causing
> this?
>
> Probably there’s a place in the code where I need to either force
> Emacs to update the frame size, which is unlikely, or Emacs updates
> the frame size and I need to force macOS to do the actual resize.
>
> --
> Alan Third

[Message part 2 (text/html, inline)]

Message #61 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: HaiJun Zhang <netjune <at> outlook.com>
Cc: 40200 <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200,
 bug#28872)
Date: Sat, 18 Apr 2020 16:26:22 +0100

[Message part 1 (text/plain, inline)]

On Sat, Apr 18, 2020 at 10:12:27PM +0800, HaiJun Zhang wrote:
> I can reproduce it with the following lines in ~/.emacs:
> 
> (setq inhibit-startup-message t
>    inhibit-startup-buffer-menu t)
> 
> (setq default-frame-alist ‘(
>        (width . 85)
>        (height . 35)
>        (vertical-scroll-bars . nil)
>        (horizontal-scroll-bars . nil)
>        ))
> 
> (set-frame-font “Monospace 16”)

Can you please try the attached patch.
-- 
Alan Third

[0001-Fix-initial-frame-resizing-issue-on-NS-bug-40200.patch (text/plain, attachment)]

Message #64 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: HaiJun Zhang <netjune <at> outlook.com>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sun, 19 Apr 2020 09:49:58 +0800

[Message part 1 (text/plain, inline)]

The patch fixes the bug. Thanks.

Can you please look at another bug? bug#40541

在 2020年4月18日 +0800 PM11:26，Alan Third <alan <at> idiocy.org>，写道：
> On Sat, Apr 18, 2020 at 10:12:27PM +0800, HaiJun Zhang wrote:
> > I can reproduce it with the following lines in ~/.emacs:
> >
> > (setq inhibit-startup-message t
> >    inhibit-startup-buffer-menu t)
> >
> > (setq default-frame-alist ‘(
> >        (width . 85)
> >        (height . 35)
> >        (vertical-scroll-bars . nil)
> >        (horizontal-scroll-bars . nil)
> >        ))
> >
> > (set-frame-font “Monospace 16”)
>
> Can you please try the attached patch.
> --
> Alan Third

[Message part 2 (text/html, inline)]

Message #67 received at 40200-done <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: HaiJun Zhang <netjune <at> outlook.com>
Cc: 40200-done <at> debbugs.gnu.org, Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sun, 3 May 2020 17:26:22 +0100

On Sun, Apr 19, 2020 at 09:49:58AM +0800, HaiJun Zhang wrote:
> The patch fixes the bug. Thanks.

I’ve pushed it to master. Thanks!

-- 
Alan Third

Message #70 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, HaiJun Zhang <netjune <at> outlook.com>,
 Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sun, 03 May 2020 20:13:35 +0300

Hi, I've just filed bug#41055 which is also about NS frame resizing, it
may have something to do with this...

Filipp

Message #73 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Alan Third <alan <at> idiocy.org>
To: Filipp Gunbin <fgunbin <at> fastmail.fm>
Cc: 40200 <at> debbugs.gnu.org, HaiJun Zhang <netjune <at> outlook.com>,
 Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sun, 3 May 2020 19:41:44 +0100

On Sun, May 03, 2020 at 08:13:35PM +0300, Filipp Gunbin wrote:
> Hi, I've just filed bug#41055 which is also about NS frame resizing, it
> may have something to do with this...

Does this patch make any difference to your issue? Or is it the cause?

Most likely the commit that’s broken it is
24cb6908d70c14792c686679cb08091447b9c3b1

-- 
Alan Third

Message #76 received at 40200 <at> debbugs.gnu.org (full text, mbox):

From: Filipp Gunbin <fgunbin <at> fastmail.fm>
To: Alan Third <alan <at> idiocy.org>
Cc: 40200 <at> debbugs.gnu.org, HaiJun Zhang <netjune <at> outlook.com>,
 Andrii Kolomoiets <andreyk.mad <at> gmail.com>
Subject: Re: bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues
 (bug#40200, bug#28872)
Date: Sun, 03 May 2020 22:49:19 +0300

On 03/05/2020 19:41 +0100, Alan Third wrote:

> On Sun, May 03, 2020 at 08:13:35PM +0300, Filipp Gunbin wrote:
>> Hi, I've just filed bug#41055 which is also about NS frame resizing, it
>> may have something to do with this...
>
> Does this patch make any difference to your issue? Or is it the cause?

No, not really.

> Most likely the commit that’s broken it is
> 24cb6908d70c14792c686679cb08091447b9c3b1

Yes, I think so too.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.