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

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

[Andrii Kolomoiets]

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

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

[Eli Zaretskii]

> From: Andrii Kolomoiets <andreyk.mad@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.

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

[Andrii Kolomoiets]

On 23 Mar 2020, at 21:22, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: Andrii Kolomoiets <andreyk.mad@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.

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

[Alan Third]

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

bug#40200: [PATCH] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

* 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

bug#28872: [PATCH] Fix NS frame resizing issues (bug#40200, bug#28872)

[Andrii Kolomoiets]

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

On 26 Mar 2020, at 00:28, Alan Third <alan@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.

[-- Attachment #2: emacs-crash-fs-close.txt --]
[-- Type: text/plain, Size: 6332 bytes --]

OS Version:            Mac OS X 10.15.3 (19D76)

Crashed Thread:        0  Dispatch queue: com.apple.main-thread

Exception Type:        EXC_BAD_ACCESS (SIGABRT)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000020
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0x20:
-->
    __TEXT                 000000010254b000-0000000102792000 [ 2332K] r-x/r-x SM=COW  /Users/USER/*/Emacs.app/Contents/MacOS/Emacs

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff6d2097fa __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fff6d2c6bc1 pthread_kill + 432
2   libsystem_c.dylib             	0x00007fff6d1203a2 raise + 26
3   org.gnu.Emacs                 	0x0000000102751769 terminate_due_to_signal + 153 (emacs.c:404)
4   org.gnu.Emacs                 	0x00000001027520ab emacs_abort + 15
5   org.gnu.Emacs                 	0x0000000102719e70 ns_term_shutdown + 80 (lisp.h:1586)
6   org.gnu.Emacs                 	0x00000001026097f4 shut_down_emacs + 340 (emacs.c:2462)
7   org.gnu.Emacs                 	0x0000000102751736 terminate_due_to_signal + 102 (emacs.c:385)
8   org.gnu.Emacs                 	0x0000000102629fce handle_fatal_signal + 14
9   org.gnu.Emacs                 	0x000000010262a051 deliver_thread_signal + 129
10  org.gnu.Emacs                 	0x0000000102628a09 deliver_fatal_thread_signal + 9
11  org.gnu.Emacs                 	0x000000010262a108 handle_sigsegv + 168
12  libsystem_platform.dylib      	0x00007fff6d2bb42d _sigtramp + 29
13  ???                           	000000000000000000 0 + 0
14  com.apple.CoreFoundation      	0x00007fff35a1435f __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
15  com.apple.CoreFoundation      	0x00007fff35a142f3 ___CFXRegistrationPost1_block_invoke + 63
16  com.apple.CoreFoundation      	0x00007fff35a14268 _CFXRegistrationPost1 + 372
17  com.apple.CoreFoundation      	0x00007fff35a13ebe ___CFXNotificationPost_block_invoke + 97
18  com.apple.CoreFoundation      	0x00007fff359e37e2 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1575
19  com.apple.CoreFoundation      	0x00007fff359e2c82 _CFXNotificationPost + 1351
20  com.apple.AppKit              	0x00007fff32c23719 -[NSView _postFrameChangeNotification] + 304
21  com.apple.AppKit              	0x00007fff32c1c1a9 -[NSView setFrameSize:] + 2917
22  com.apple.AppKit              	0x00007fff32c2dca2 -[NSView setFrame:] + 423
23  org.gnu.Emacs                 	0x000000010272027c -[EmacsView setFrame:] + 60 (nsterm.m:7346)
24  com.apple.AppKit              	0x00007fff32c412ee -[NSView resizeWithOldSuperviewSize:] + 1193
25  com.apple.AppKit              	0x00007fff32c40a13 -[NSView resizeSubviewsWithOldSize:] + 525
26  com.apple.AppKit              	0x00007fff32c1bcf6 -[NSView setFrameSize:] + 1714
27  com.apple.AppKit              	0x00007fff32c2dca2 -[NSView setFrame:] + 423
28  com.apple.AppKit              	0x00007fff32c3bd38 -[NSThemeFrame setStyleMask:] + 874
29  com.apple.AppKit              	0x00007fff32c3b89e __25-[NSWindow setStyleMask:]_block_invoke + 1939
30  com.apple.AppKit              	0x00007fff32c3b0b3 NSPerformVisuallyAtomicChange + 132
31  com.apple.AppKit              	0x00007fff32c3afbc -[NSWindow setStyleMask:] + 184
32  com.apple.AppKit              	0x00007fff3328d23d -[_NSWindowExitFullScreenTransitionController setupWindowForAfterFullScreenExit] + 106
33  com.apple.AppKit              	0x00007fff33509740 __46-[_NSExitFullScreenTransitionController start]_block_invoke_3 + 57
34  libdispatch.dylib             	0x00007fff6d068583 _dispatch_call_block_and_release + 12
35  libdispatch.dylib             	0x00007fff6d06950e _dispatch_client_callout + 8
36  libdispatch.dylib             	0x00007fff6d074bc4 _dispatch_main_queue_callback_4CF + 1105
37  com.apple.CoreFoundation      	0x00007fff35a5de00 __CFRUNLOOP_IS_SERVICING_THE_MAIN_DISPATCH_QUEUE__ + 9
38  com.apple.CoreFoundation      	0x00007fff35a1db8a __CFRunLoopRun + 2370
39  com.apple.CoreFoundation      	0x00007fff35a1cbd3 CFRunLoopRunSpecific + 499
40  com.apple.HIToolbox           	0x00007fff3457265d RunCurrentEventLoopInMode + 292
41  com.apple.HIToolbox           	0x00007fff345722a9 ReceiveNextEventCommon + 356
42  com.apple.HIToolbox           	0x00007fff34572127 _BlockUntilNextEventMatchingListInModeWithFilter + 64
43  com.apple.AppKit              	0x00007fff32be2ba4 _DPSNextEvent + 990
44  com.apple.AppKit              	0x00007fff32be1380 -[NSApplication(NSEvent) _nextEventMatchingEventMask:untilDate:inMode:dequeue:] + 1352
45  com.apple.AppKit              	0x00007fff32bd309e -[NSApplication run] + 658
46  org.gnu.Emacs                 	0x000000010271a02d -[EmacsApp run] + 333 (nsterm.m:5644)
47  org.gnu.Emacs                 	0x0000000102726bcc ns_read_socket + 668 (nsterm.m:4598)
48  org.gnu.Emacs                 	0x0000000102617035 gobble_input + 245 (keyboard.c:6887)
49  org.gnu.Emacs                 	0x000000010261a01a detect_input_pending + 250 (lisp.h:1272)
50  org.gnu.Emacs                 	0x00000001026e23ea wait_reading_process_output + 3194 (process.c:5491)
51  org.gnu.Emacs                 	0x00000001025525e8 sit_for + 312 (dispnew.c:6055)
52  org.gnu.Emacs                 	0x0000000102611f1a read_char + 5210 (keyboard.c:2738)
53  org.gnu.Emacs                 	0x000000010260ef2a read_key_sequence + 1722 (keyboard.c:9549)
54  org.gnu.Emacs                 	0x000000010260d72c command_loop_1 + 1340 (keyboard.c:1350)
55  org.gnu.Emacs                 	0x00000001026943d7 internal_condition_case + 263 (eval.c:1357)
56  org.gnu.Emacs                 	0x000000010261d740 command_loop_2 + 48 (lisp.h:1272)
57  org.gnu.Emacs                 	0x0000000102693beb internal_catch + 267 (eval.c:1118)
58  org.gnu.Emacs                 	0x0000000102751b35 command_loop.cold.1 + 69
59  org.gnu.Emacs                 	0x000000010260c7f3 command_loop + 131
60  org.gnu.Emacs                 	0x000000010260c723 recursive_edit_1 + 115 (keyboard.c:714)
61  org.gnu.Emacs                 	0x000000010260c97b Frecursive_edit + 347 (keyboard.c:787)
62  org.gnu.Emacs                 	0x000000010260b561 main + 7425 (emacs.c:2036)
63  libdyld.dylib                 	0x00007fff6d0c27fd start + 1

bug#28872: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

* 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

bug#28872: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)

[Andrii Kolomoiets]

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

On 26 Mar 2020, at 23:21, Alan Third <alan@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!

[-- Attachment #2: emacs-crash-tab-close.txt --]
[-- Type: text/plain, Size: 5618 bytes --]

Exception Type:        EXC_BAD_ACCESS (SIGABRT)
Exception Codes:       KERN_INVALID_ADDRESS at 0x0000000000000020
Exception Note:        EXC_CORPSE_NOTIFY

VM Regions Near 0x20:
-->
    __TEXT                 000000010aac2000-000000010ad09000 [ 2332K] r-x/r-x SM=COW  /Users/USER/*/Emacs.app/Contents/MacOS/Emacs

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0   libsystem_kernel.dylib        	0x00007fff6d2097fa __pthread_kill + 10
1   libsystem_pthread.dylib       	0x00007fff6d2c6bc1 pthread_kill + 432
2   libsystem_c.dylib             	0x00007fff6d1203a2 raise + 26
3   org.gnu.Emacs                 	0x000000010acc87a9 terminate_due_to_signal + 153 (emacs.c:404)
4   org.gnu.Emacs                 	0x000000010acc90eb emacs_abort + 15
5   org.gnu.Emacs                 	0x000000010ac90f60 ns_term_shutdown + 80 (lisp.h:1586)
6   org.gnu.Emacs                 	0x000000010ab808e4 shut_down_emacs + 340 (emacs.c:2462)
7   org.gnu.Emacs                 	0x000000010acc8776 terminate_due_to_signal + 102 (emacs.c:385)
8   org.gnu.Emacs                 	0x000000010aba10be handle_fatal_signal + 14
9   org.gnu.Emacs                 	0x000000010aba1141 deliver_thread_signal + 129
10  org.gnu.Emacs                 	0x000000010ab9faf9 deliver_fatal_thread_signal + 9
11  org.gnu.Emacs                 	0x000000010aba11f8 handle_sigsegv + 168
12  libsystem_platform.dylib      	0x00007fff6d2bb42d _sigtramp + 29
13  ???                           	000000000000000000 0 + 0
14  com.apple.CoreFoundation      	0x00007fff35a1435f __CFNOTIFICATIONCENTER_IS_CALLING_OUT_TO_AN_OBSERVER__ + 12
15  com.apple.CoreFoundation      	0x00007fff35a142f3 ___CFXRegistrationPost1_block_invoke + 63
16  com.apple.CoreFoundation      	0x00007fff35a14268 _CFXRegistrationPost1 + 372
17  com.apple.CoreFoundation      	0x00007fff35a13ebe ___CFXNotificationPost_block_invoke + 97
18  com.apple.CoreFoundation      	0x00007fff359e37e2 -[_CFXNotificationRegistrar find:object:observer:enumerator:] + 1575
19  com.apple.CoreFoundation      	0x00007fff359e2c82 _CFXNotificationPost + 1351
20  com.apple.AppKit              	0x00007fff32c23719 -[NSView _postFrameChangeNotification] + 304
21  com.apple.AppKit              	0x00007fff32c1c1a9 -[NSView setFrameSize:] + 2917
22  com.apple.AppKit              	0x00007fff32c2dca2 -[NSView setFrame:] + 423
23  org.gnu.Emacs                 	0x000000010ac972ac -[EmacsView setFrame:] + 60 (nsterm.m:7329)
24  com.apple.AppKit              	0x00007fff32c412ee -[NSView resizeWithOldSuperviewSize:] + 1193
25  com.apple.AppKit              	0x00007fff32c40a13 -[NSView resizeSubviewsWithOldSize:] + 525
26  com.apple.AppKit              	0x00007fff32c1bcf6 -[NSView setFrameSize:] + 1714
27  com.apple.AppKit              	0x00007fff32c2dca2 -[NSView setFrame:] + 423
28  com.apple.AppKit              	0x00007fff32c604f7 -[NSThemeFrame _relayoutAuxiliaryViewsOfType:] + 149
29  com.apple.AppKit              	0x00007fff32c6044f -[NSThemeFrame relayoutAuxiliaryViewsOfType:] + 27
30  com.apple.AppKit              	0x00007fff32e284fe -[NSTitlebarViewController removeChildViewControllerAtIndex:] + 430
31  com.apple.AppKit              	0x00007fff3341b9f1 -[NSWindowStackController _removeTabBarAccessoryViewControllerForWindow:] + 212
32  com.apple.AppKit              	0x00007fff3341cc7a -[NSWindowStackController removeWindow:] + 297
33  com.apple.AppKit              	0x00007fff32e988af -[NSWindow(NSWindowTabbing) _doTabbedWindowCleanupForOrderOut] + 97
34  com.apple.AppKit              	0x00007fff3347be55 -[NSWindow _finishClosingWindow] + 73
35  com.apple.AppKit              	0x00007fff32f590cd -[NSWindow _close] + 352
36  org.gnu.Emacs                 	0x000000010ac8dd4a ns_free_frame_resources + 202 (nsterm.m:1705)
37  org.gnu.Emacs                 	0x000000010ac9fd5a ns_destroy_window + 106 (nsterm.m:1733)
38  org.gnu.Emacs                 	0x000000010aad1faa delete_frame + 1642 (frame.c:2137)
39  org.gnu.Emacs                 	0x000000010ac0d891 funcall_subr + 257
40  org.gnu.Emacs                 	0x000000010ac0ce7b Ffuncall + 843
41  org.gnu.Emacs                 	0x000000010ac06459 Ffuncall_interactively + 73 (callint.c:254)
42  org.gnu.Emacs                 	0x000000010ac0ce7b Ffuncall + 843
43  org.gnu.Emacs                 	0x000000010ac07948 Fcall_interactively + 5336
44  org.gnu.Emacs                 	0x000000010ac0d8b1 funcall_subr + 289
45  org.gnu.Emacs                 	0x000000010ac0ce7b Ffuncall + 843
46  org.gnu.Emacs                 	0x000000010ac50737 exec_byte_code + 1815 (bytecode.c:633)
47  org.gnu.Emacs                 	0x000000010ac0ce19 Ffuncall + 745
48  org.gnu.Emacs                 	0x000000010ac0d4ec call1 + 44
49  org.gnu.Emacs                 	0x000000010ab84ab9 command_loop_1 + 2009 (lisp.h:1272)
50  org.gnu.Emacs                 	0x000000010ac0b4c7 internal_condition_case + 263 (eval.c:1357)
51  org.gnu.Emacs                 	0x000000010ab94830 command_loop_2 + 48 (lisp.h:1272)
52  org.gnu.Emacs                 	0x000000010ac0acdb internal_catch + 267 (eval.c:1118)
53  org.gnu.Emacs                 	0x000000010acc8b75 command_loop.cold.1 + 69
54  org.gnu.Emacs                 	0x000000010ab838e3 command_loop + 131
55  org.gnu.Emacs                 	0x000000010ab83813 recursive_edit_1 + 115 (keyboard.c:714)
56  org.gnu.Emacs                 	0x000000010ab83a6b Frecursive_edit + 347 (keyboard.c:787)
57  org.gnu.Emacs                 	0x000000010ab82651 main + 7425 (emacs.c:2036)
58  libdyld.dylib                 	0x00007fff6d0c27fd start + 1

bug#28872: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)

[Andrii Kolomoiets]

On 26 Mar 2020, at 23:21, Alan Third <alan@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))))

bug#28872: [PATCH v2] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

On Fri, Mar 27, 2020 at 03:22:08PM +0200, Andrii Kolomoiets wrote:
> On 26 Mar 2020, at 23:21, Alan Third <alan@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

bug#40200: [PATCH v3] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

* 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

bug#40200: [PATCH v3] Fix NS frame resizing issues (bug#40200, bug#28872)

[Andrii Kolomoiets]

Alan Third <alan@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!

bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

* 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

bug#40200: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Andrii Kolomoiets]

Alan Third <alan@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!

bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

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

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[HaiJun Zhang]

[-- Attachment #1.1: Type: text/plain, Size: 609 bytes --]

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@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
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 1321 bytes --]

[-- Attachment #2: WX20200417-153341@2x.png --]
[-- Type: image/png, Size: 177034 bytes --]

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

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

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[HaiJun Zhang]

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

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@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

[-- Attachment #2: Type: text/html, Size: 2095 bytes --]

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

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

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

[-- Attachment #2: 0001-Fix-initial-frame-resizing-issue-on-NS-bug-40200.patch --]
[-- Type: text/plain, Size: 2937 bytes --]

From 836c19eae4802a3aedee887669425f949971eb4a Mon Sep 17 00:00:00 2001
From: Alan Third <alan@idiocy.org>
Date: Sat, 18 Apr 2020 16:22:06 +0100
Subject: [PATCH] Fix initial frame resizing issue on NS (bug#40200)

* src/nsterm.m ([EmacsView viewDidResize:]): Don't try to determine
the old size when not drawing to the buffer.
---
 src/nsterm.m | 51 +++++++++++++++++++++------------------------------
 1 file changed, 21 insertions(+), 30 deletions(-)

diff --git a/src/nsterm.m b/src/nsterm.m
index 9cd1c9d860..d85a579e38 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7322,48 +7322,39 @@ - (void)viewDidEndLiveResize
 - (void)viewDidResize:(NSNotification *)notification
 {
   NSRect frame = [self frame];
-  int oldw, oldh, neww, newh;
+  int neww, newh;
 
   if (! FRAME_LIVE_P (emacsframe))
     return;
 
+  NSTRACE ("[EmacsView viewDidResize]");
+
+  neww = (int)NSWidth (frame);
+  newh = (int)NSHeight (frame);
+  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
+
 #ifdef NS_DRAW_TO_BUFFER
-#if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
   if ([self wantsUpdateLayer])
     {
-#endif
       CGFloat scale = [[self window] backingScaleFactor];
-      oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
-      oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
-#if MAC_OS_X_VERSION_MIN_REQUIRED < 101400
-    }
-  else
-    {
-#endif
-#endif /* NS_DRAW_TO_BUFFER */
-#if !defined (NS_DRAW_TO_BUFFER) || MAC_OS_X_VERSION_MIN_REQUIRED < 101400
-      oldw = FRAME_PIXEL_WIDTH (emacsframe);
-      oldh = FRAME_PIXEL_HEIGHT (emacsframe);
-#endif
-#if defined (NS_DRAW_TO_BUFFER) && MAC_OS_X_VERSION_MIN_REQUIRED < 101400
-    }
-#endif
-
-  neww = (int)NSWidth (frame);
-  newh = (int)NSHeight (frame);
+      int oldw = (CGFloat)CGBitmapContextGetWidth (drawingBuffer) / scale;
+      int oldh = (CGFloat)CGBitmapContextGetHeight (drawingBuffer) / scale;
 
-  NSTRACE ("[EmacsView viewDidResize]");
+      NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
 
-  /* Don't want to do anything when the view size hasn't changed. */
-  if ((oldh == newh && oldw == neww))
-    {
-      NSTRACE_MSG ("No change");
-      return;
+      /* Don't want to do anything when the view size hasn't changed. */
+      if ((oldh == newh && oldw == neww))
+        {
+          NSTRACE_MSG ("No change");
+          return;
+        }
     }
+#endif
 
-  NSTRACE_SIZE ("Original size", NSMakeSize (oldw, oldh));
-  NSTRACE_SIZE ("New size", NSMakeSize (neww, newh));
-
+  /* I'm not sure if it's safe to call this every time the view
+     changes size, as Emacs may already know about the change.
+     Unfortunately there doesn't seem to be a bullet-proof method of
+     determining whether we need to call it or not.  */
   change_frame_size (emacsframe,
                      FRAME_PIXEL_TO_TEXT_WIDTH (emacsframe, neww),
                      FRAME_PIXEL_TO_TEXT_HEIGHT (emacsframe, newh),
-- 
2.26.1

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[HaiJun Zhang]

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

The patch fixes the bug. Thanks.

Can you please look at another bug? bug#40541

在 2020年4月18日 +0800 PM11:26，Alan Third <alan@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

[-- Attachment #2: Type: text/html, Size: 1730 bytes --]

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

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

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Filipp Gunbin]

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

Filipp

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Alan Third]

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

bug#40200: bug#28872: [PATCH v4] Fix NS frame resizing issues (bug#40200, bug#28872)

[Filipp Gunbin]

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.