bug#51411: NS port cleanups

[Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Alan Third <alan@idiocy.org> writes:

> Is this the code in ns_draw_glyph_string?

Yes, it is.

> I think we normally accept what the system tells us is Alt, or the
> nearest equivalent. In this case GNUstep tells us what it's
> understanding of the nearest equivalent (option) is, and I don't see
> any reason to override that.

> So please remove the changes to modifier keys.

Fair enough, thanks for the feedback.  Done.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-General-improvements-to-NS-port.patch --]
[-- Type: text/x-patch, Size: 12119 bytes --]

From b4060ca7357091ef0ba6abf3faf0314321cd85be Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Sat, 23 Oct 2021 19:51:31 +0800
Subject: [PATCH] General improvements to NS port

* src/dispextern.h: Remove some !HAVE_NS conditionals around grab
related code.
* src/frame.c (gui_mouse_grabbed, gui_redo_mouse_highlight): Remove
!HAVE_NS conditionals around code.

* src/nsmenu.m (ns_update_menubar): Prevent recursive calls and enable
shallow updates on GNUstep.
(menuNeedsUpdate): Prevent recursive calls.
(ns_menu_show): Fix mysterious GC-related bug.
(update_frame_tool_bar_1): Work around mysterious toolbar sizing bug
on GNUstep.

* src/nsterm.h (struct ns_output): New field for tracking toolbar
visibility changes.

* src/nsterm.m (frame_set_mouse_pixel_position): Implement for
GNUstep.
(ns_redraw_scroll_bars): Enable for GNUstep.
(ns_clear_frame): Redraw scrollbars on GNUstep.
(ns_update_window_end): New function.
(ns_redisplay_interface): Add ns_update_window_end on GNUstep.
(- keyDown): Remove debug code that doesn't work on GNUstep.
(- mouseDown): Enable grab tracking on NS port.
(- resizeWithOldSuperviewSize): Fix build with NSTRACE.

* src/xdisp.c (note_tab_bar_highlight): Enable some code for NS port.
---
 src/dispextern.h |  2 --
 src/frame.c      |  4 ---
 src/nsmenu.m     | 76 +++++++++++++++++++++++++++++++++++++++---------
 src/nsterm.h     |  6 ++++
 src/nsterm.m     | 42 +++++++++++++++++++++-----
 src/xdisp.c      |  2 --
 6 files changed, 102 insertions(+), 30 deletions(-)

diff --git a/src/dispextern.h b/src/dispextern.h
index 08dac5d455..58e9048556 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -3722,10 +3722,8 @@ #define IMAGE_BACKGROUND_TRANSPARENT(img, f, mask)			      \
                                           const char *, const char *,
                                           enum resource_types);
 
-#ifndef HAVE_NS /* These both used on W32 and X only.  */
 extern bool gui_mouse_grabbed (Display_Info *);
 extern void gui_redo_mouse_highlight (Display_Info *);
-#endif /* HAVE_NS */
 
 #endif /* HAVE_WINDOW_SYSTEM */
 
diff --git a/src/frame.c b/src/frame.c
index 2b1cb452ef..79a7c89e0d 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -5028,8 +5028,6 @@ gui_set_no_special_glyphs (struct frame *f, Lisp_Object new_value, Lisp_Object o
 }
 
 
-#ifndef HAVE_NS
-
 /* Non-zero if mouse is grabbed on DPYINFO
    and we know the frame where it is.  */
 
@@ -5054,8 +5052,6 @@ gui_redo_mouse_highlight (Display_Info *dpyinfo)
 			  dpyinfo->last_mouse_motion_y);
 }
 
-#endif /* HAVE_NS */
-
 /* Subroutines of creating an X frame.  */
 
 /* Make sure that Vx_resource_name is set to a reasonable value.
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 05b89c2f56..b93d3a79bd 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -101,6 +101,15 @@
 static void
 ns_update_menubar (struct frame *f, bool deep_p)
 {
+#ifdef NS_IMPL_GNUSTEP
+  static int inside = 0;
+
+  if (inside)
+    return;
+
+  inside++;
+#endif
+
   BOOL needsSet = NO;
   id menu = [NSApp mainMenu];
   bool owfi;
@@ -120,7 +129,12 @@
   NSTRACE ("ns_update_menubar");
 
   if (f != SELECTED_FRAME () || FRAME_EXTERNAL_MENU_BAR (f) == 0)
+    {
+#ifdef NS_IMPL_GNUSTEP
+      inside--;
+#endif
       return;
+    }
   XSETFRAME (Vmenu_updating_frame, f);
 /*fprintf (stderr, "ns_update_menubar: frame: %p\tdeep: %d\tsub: %p\n", f, deep_p, submenu); */
 
@@ -144,10 +158,6 @@
   t = -(1000*tb.time+tb.millitm);
 #endif
 
-#ifdef NS_IMPL_GNUSTEP
-  deep_p = 1; /* See comment in menuNeedsUpdate.  */
-#endif
-
   if (deep_p)
     {
       /* Make a widget-value tree representing the entire menu trees.  */
@@ -275,6 +285,9 @@
 	  free_menubar_widget_value_tree (first_wv);
 	  discard_menu_items ();
 	  unbind_to (specpdl_count, Qnil);
+#ifdef NS_IMPL_GNUSTEP
+	  inside--;
+#endif
 	  return;
 	}
 
@@ -408,6 +421,10 @@
   if (needsSet)
     [NSApp setMainMenu: menu];
 
+#ifdef NS_IMPL_GNUSTEP
+  inside--;
+#endif
+
   unblock_input ();
 
 }
@@ -490,17 +507,34 @@ - (instancetype)initWithTitle: (NSString *)title
    call to ns_update_menubar.  */
 - (void)menuNeedsUpdate: (NSMenu *)menu
 {
+#ifdef NS_IMPL_GNUSTEP
+  static int inside = 0;
+#endif
+
   if (!FRAME_LIVE_P (SELECTED_FRAME ()))
     return;
 
-#ifdef NS_IMPL_COCOA
-/* TODO: GNUstep calls this method when the menu is still being built
-   which results in a recursive stack overflow.  One possible solution
-   is to use menuWillOpen instead, but the Apple docs explicitly warn
-   against changing the contents of the menu in it.  I don't know what
-   the right thing to do for GNUstep is.  */
+#ifdef NS_IMPL_GNUSTEP
+  /* GNUstep calls this method when the menu is still being built
+     which results in a recursive stack overflow, which this variable
+     prevents.  */
+
+  if (!inside)
+    ++inside;
+  else
+    return;
+#endif
+
   if (needsUpdate)
-    ns_update_menubar (SELECTED_FRAME (), true);
+    {
+#ifdef NS_IMPL_GNUSTEP
+      needsUpdate = NO;
+#endif
+      ns_update_menubar (SELECTED_FRAME (), true);
+    }
+
+#ifdef NS_IMPL_GNUSTEP
+  --inside;
 #endif
 }
 
@@ -827,6 +861,9 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
 
   p.x = x; p.y = y;
 
+  /* Don't GC due to a mysterious bug.  */
+  inhibit_garbage_collection ();
+
   /* now parse stage 2 as in ns_update_menubar */
   wv = make_widget_value ("contextmenu", NULL, true, Qnil);
   wv->button_type = BUTTON_TYPE_NONE;
@@ -998,15 +1035,17 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
 
   pmenu = [[EmacsMenu alloc] initWithTitle:
                    NILP (title) ? @"" : [NSString stringWithLispString: title]];
+  /* On GNUstep, this call makes menu_items nil for whatever reason
+     when displaying a context menu from `context-menu-mode'.  */
+  Lisp_Object items = menu_items;
   [pmenu fillWithWidgetValue: first_wv->contents];
+  menu_items = items;
   free_menubar_widget_value_tree (first_wv);
-  unbind_to (specpdl_count, Qnil);
-
   popup_activated_flag = 1;
   tem = [pmenu runMenuAt: p forFrame: f keymaps: keymaps];
   popup_activated_flag = 0;
   [[FRAME_NS_VIEW (SELECTED_FRAME ()) window] makeKeyWindow];
-
+  unbind_to (specpdl_count, Qnil);
   unblock_input ();
   return tem;
 }
@@ -1057,6 +1096,15 @@ - (void)menu:(NSMenu *)menu willHighlightItem:(NSMenuItem *)item
   [toolbar clearActive];
 #else
   [toolbar clearAll];
+  /* It takes at least 3 such adjustments to fix an issue where the
+     tool bar is 2x too tall when a frame's tool bar is first shown.
+     This is ugly, but I have no other solution for this problem.  */
+  if (FRAME_OUTPUT_DATA (f)->tool_bar_adjusted < 3)
+    {
+      [toolbar setVisible: NO];
+      FRAME_OUTPUT_DATA (f)->tool_bar_adjusted++;
+      [toolbar setVisible: YES];
+    }
 #endif
 
   /* Update EmacsToolbar as in GtkUtils, build items list.  */
diff --git a/src/nsterm.h b/src/nsterm.h
index 944dbd727c..8175f99664 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -978,6 +978,12 @@ #define BLUE16_FROM_ULONG(color) (BLUE_FROM_ULONG(color) * 0x101)
 
   /* Non-zero if we are doing an animation, e.g. toggling the tool bar.  */
   int in_animation;
+
+#ifdef NS_IMPL_GNUSTEP
+  /* Zero if this is the first time a toolbar has been updated on this
+     frame. */
+  int tool_bar_adjusted;
+#endif
 };
 
 /* This dummy declaration needed to support TTYs.  */
diff --git a/src/nsterm.m b/src/nsterm.m
index 957cd815a0..cb308ea23c 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -65,6 +65,7 @@ Updated by Christian Limpach (chris@nice.ch)
 
 #ifdef NS_IMPL_GNUSTEP
 #include "process.h"
+#import <GNUstepGUI/GSDisplayServer.h>
 #endif
 
 #ifdef NS_IMPL_COCOA
@@ -2259,13 +2260,19 @@ Hide the window (X11 semantics)
 {
   NSTRACE ("frame_set_mouse_pixel_position");
 
-  /* FIXME: what about GNUstep?  */
 #ifdef NS_IMPL_COCOA
   CGPoint mouse_pos =
     CGPointMake(f->left_pos + pix_x,
                 f->top_pos + pix_y +
                 FRAME_NS_TITLEBAR_HEIGHT(f) + FRAME_TOOLBAR_HEIGHT(f));
   CGWarpMouseCursorPosition (mouse_pos);
+#else
+  GSDisplayServer *server = GSServerForWindow ([FRAME_NS_VIEW (f) window]);
+  [server setMouseLocation: NSMakePoint (f->left_pos + pix_x,
+					 f->top_pos + pix_y
+					 + FRAME_NS_TITLEBAR_HEIGHT(f)
+					 + FRAME_TOOLBAR_HEIGHT(f))
+		  onScreen: [[[FRAME_NS_VIEW (f) window] screen] screenNumber]];
 #endif
 }
 
@@ -2578,8 +2585,7 @@ Hide the window (X11 semantics)
    ========================================================================== */
 
 
-#if 0
-/* FIXME: Remove this function. */
+#ifdef NS_IMPL_GNUSTEP
 static void
 ns_redraw_scroll_bars (struct frame *f)
 {
@@ -2624,10 +2630,9 @@ Hide the window (X11 semantics)
   NSRectFill (r);
   ns_unfocus (f);
 
-  /* as of 2006/11 or so this is now needed */
-  /* FIXME: I don't see any reason for this and removing it makes no
-     difference here.  Do we need it for GNUstep?  */
-  //ns_redraw_scroll_bars (f);
+#ifdef NS_IMPL_GNUSTEP
+  ns_redraw_scroll_bars (f);
+#endif
   unblock_input ();
 }
 
@@ -4933,6 +4938,17 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
 {
 }
 
+#ifdef NS_IMPL_GNUSTEP
+static void
+ns_update_window_end (struct window *w, bool cursor_on_p,
+		      bool mouse_face_overwritten_p)
+{
+  NSTRACE ("ns_update_window_end (cursor_on_p = %d)", cursor_on_p);
+
+  ns_redraw_scroll_bars (WINDOW_XFRAME (w));
+}
+#endif
+
 /* This and next define (many of the) public functions in this file.  */
 /* gui_* are generic versions in xdisp.c that we, and other terms, get away
          with using despite presence in the "system dependent" redisplay
@@ -4949,7 +4965,11 @@ static Lisp_Object ns_string_to_lispmod (const char *s)
   ns_scroll_run,
   ns_after_update_window_line,
   NULL, /* update_window_begin */
+#ifndef NS_IMPL_GNUSTEP
   NULL, /* update_window_end   */
+#else
+  ns_update_window_end,
+#endif
   0, /* flush_display */
   gui_clear_window_mouse_face,
   gui_get_glyph_overhangs,
@@ -6177,9 +6197,11 @@ In that case we use UCKeyTranslate (ns_get_shifted_character)
       Lisp_Object kind = fnKeysym ? QCfunction : QCordinary;
       emacs_event->modifiers = EV_MODIFIERS2 (flags, kind);
 
+#ifndef NS_IMPL_GNUSTEP
       if (NS_KEYLOG)
         fprintf (stderr, "keyDown: code =%x\tfnKey =%x\tflags = %x\tmods = %x\n",
                  code, fnKeysym, flags, emacs_event->modifiers);
+#endif
 
       /* If it was a function key or had control-like modifiers, pass
          it directly to Emacs.  */
@@ -6692,6 +6714,11 @@ - (void)mouseDown: (NSEvent *)theEvent
       emacs_event->code = EV_BUTTON (theEvent);
       emacs_event->modifiers = EV_MODIFIERS (theEvent)
                              | EV_UDMODIFIERS (theEvent);
+
+      if (emacs_event->modifiers & down_modifier)
+	FRAME_DISPLAY_INFO (emacsframe)->grabbed |= 1 << EV_BUTTON (theEvent);
+      else
+	FRAME_DISPLAY_INFO (emacsframe)->grabbed &= ~(1 << EV_BUTTON (theEvent));
     }
 
   XSETINT (emacs_event->x, lrint (p.x));
@@ -6992,7 +7019,6 @@ - (void)resizeWithOldSuperviewSize: (NSSize)oldSize
   height = (int)NSHeight (frame);
 
   NSTRACE_SIZE ("New size", NSMakeSize (width, height));
-  NSTRACE_SIZE ("Original size", size);
 
   /* Reset the frame size to match the bounds of the superview (the
      NSWindow's contentView).  We need to do this as sometimes the
diff --git a/src/xdisp.c b/src/xdisp.c
index 9998677262..bfe7c571ab 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -13891,7 +13891,6 @@ note_tab_bar_highlight (struct frame *f, int x, int y)
   clear_mouse_face (hlinfo);
 
   bool mouse_down_p = false;
-#ifndef HAVE_NS
   /* Mouse is down, but on different tab-bar item?  Or alternatively,
      the mouse might've been pressed somewhere we don't know about,
      and then have moved onto the tab bar.  In this case,
@@ -13904,7 +13903,6 @@ note_tab_bar_highlight (struct frame *f, int x, int y)
   if (mouse_down_p && f->last_tab_bar_item != prop_idx
       && f->last_tab_bar_item != -1)
     return;
-#endif
   draw = mouse_down_p ? DRAW_IMAGE_SUNKEN : DRAW_IMAGE_RAISED;
 
   /* If tab-bar item is not enabled, don't highlight it.  */
-- 
2.31.1