Subject: bug#21415: 25.0.50; Emacs Trunk -- pixelwise width/height for x-create-frame

From: Anders Lindgren <[email-masked]>
Date: Tue, 27 Oct 2015 22:42:00 +0100

Hi,

here is a patch to fix the problems with tool-bar-mode, please try it out.

One thing that I have noticed lately is that Emacs often crash when
started. I managed to get a call stack by running gdb, but it doesn't mean
much to me. Do any of you have an idea what is going on?

    -- Anders

On Sat, Oct 24, 2015 at 11:43 PM, Anders Lindgren <[email-masked]> wrote:

> Hi,
>
> On Sat, Oct 24, 2015 at 8:57 PM, martin rudalics <[email-masked]> wrote:
> >
> > > Anyway, I think the problem you are seeing is due to the fact that I
> have
> > > replaced the code in "zoom" with custom code that simply resizes the
> frame.
> > > On OS X there is nothing special happening when maximizing the frame
> -- the
> > > outer frame (one pixel thick) is still there, no buttons should change
> >         ^^^^^
> > You mean the outer border, I presume.  Here the border (some 5 pixels)
> > is probably removed by the window manager.  Don't ask me why and how.
> > But this and most other problems I reported existed already before your
> > change.
>
> Was the outer frame removed and the icon changed before my rewrites? If
> so, try to use the old zoom system instead of the new.
>
>
> > I doubt it would help much.  One basic problem with GNUStep here is that
> > the workarea as returned by ‘display-monitor-attributes-list’ is the
> > same as the geometry returned by that function (are they the same on
> > OSX?).  Hence I see no way to have the GNUStep build recognize the
> > presence of my taskbar and not hide it when maximizing the frame.
>
> You can check if the NSScreen "frame" and "visibleFrame" would return
> different frame. In "ns_menu_bar_height" I use this. Maybe it can be used
> under GNUStep to find the height of the taskbar?
>
>
> > BTW
> >
> > (set-frame-parameter nil 'fullscreen 'fullboth)
> >
> > is broken here just as it was before your changes.  Does it work for you?
>
> Yes, it enters the real fullscreen mode.
>
> Does it work if you set `ns-use-native-fullscreen' to nil?
>
>
> > > When it comes to the double tool bar, I have unfortunately no idea
> what the
> > > problem is.
> >
> > The crazy thing is that with the old build I got it for a normal initial
> > frame and now I get for a maximized initial frame ;-)
>
> Bizzarre... I would check "update_frame_tool_bar" in nsmenu.m or
> initFrameFromEmacs in nsterm.m to see if any of those allocate more than
> one EmacsToolbar.
>
>
> I have located the toolbar error. When "setVisible" is called, OS X starts
> an animation. When this animation runs, windowWasResized tried to keep up
> and update rows and columns. On the way it got lost and the end result was
> a bad "row" could. When simply doing nothing when the animation runs,
> everything work OK. Hopefully, I will be able to push the end result within
> the next couple of days.
>
>     -- Anders
>
>
diff --git a/src/nsfns.m b/src/nsfns.m
index 1ed3e23..c243444 100644
--- a/src/nsfns.m
+++ b/src/nsfns.m
@@ -1298,6 +1298,8 @@ This function is an internal primitive--use `make-frame' instead.  */)
      = [NSCursor arrowCursor];
   f->output_data.ns->current_pointer = f->output_data.ns->text_cursor;
 
+  f->output_data.ns->in_animation = NO;
+
   [[EmacsView alloc] initFrameFromEmacs: f];
 
   x_icon (f, parms);
diff --git a/src/nsmenu.m b/src/nsmenu.m
index 2ef1223..ddc5dc2 100644
--- a/src/nsmenu.m
+++ b/src/nsmenu.m
@@ -998,10 +998,20 @@ free_frame_tool_bar (struct frame *f)
    -------------------------------------------------------------------------- */
 {
   EmacsView *view = FRAME_NS_VIEW (f);
+
+  NSTRACE ("free_frame_tool_bar");
+
   block_input ();
   view->wait_for_tool_bar = NO;
-  [[view toolbar] setVisible: NO];
+
   FRAME_TOOLBAR_HEIGHT (f) = 0;
+
+  /* Note: This trigger an animation, which calls windowDidResize
+     repeatedly. */
+  f->output_data.ns->in_animation = 1;
+  [[view toolbar] setVisible: NO];
+  f->output_data.ns->in_animation = 0;
+
   unblock_input ();
 }
 
@@ -1017,6 +1027,8 @@ update_frame_tool_bar (struct frame *f)
   EmacsToolbar *toolbar = [view toolbar];
   int oldh;
 
+  NSTRACE ("update_frame_tool_bar");
+
   if (view == nil || toolbar == nil) return;
   block_input ();
 
@@ -1096,7 +1108,11 @@ update_frame_tool_bar (struct frame *f)
     }
 
   if (![toolbar isVisible])
+    {
+      f->output_data.ns->in_animation = 1;
       [toolbar setVisible: YES];
+      f->output_data.ns->in_animation = 0;
+    }
 
 #ifdef NS_IMPL_COCOA
   if ([toolbar changed])
@@ -1150,6 +1166,8 @@ update_frame_tool_bar (struct frame *f)
 
 - initForView: (EmacsView *)view withIdentifier: (NSString *)identifier
 {
+  NSTRACE ("[EmacsToolbar initForView: withIdentifier:]");
+
   self = [super initWithIdentifier: identifier];
   emacsView = view;
   [self setDisplayMode: NSToolbarDisplayModeIconOnly];
@@ -1164,6 +1182,8 @@ update_frame_tool_bar (struct frame *f)
 
 - (void)dealloc
 {
+  NSTRACE ("[EmacsToolbar dealloc]");
+
   [prevIdentifiers release];
   [activeIdentifiers release];
   [identifierToItem release];
@@ -1172,6 +1192,8 @@ update_frame_tool_bar (struct frame *f)
 
 - (void) clearActive
 {
+  NSTRACE ("[EmacsToolbar clearActive]");
+
   [prevIdentifiers release];
   prevIdentifiers = [activeIdentifiers copy];
   [activeIdentifiers removeAllObjects];
@@ -1181,6 +1203,8 @@ update_frame_tool_bar (struct frame *f)
 
 - (void) clearAll
 {
+  NSTRACE ("[EmacsToolbar clearAll]");
+
   [self clearActive];
   while ([[self items] count] > 0)
     [self removeItemAtIndex: 0];
@@ -1188,6 +1212,8 @@ update_frame_tool_bar (struct frame *f)
 
 - (BOOL) changed
 {
+  NSTRACE ("[EmacsToolbar changed]");
+
   return [activeIdentifiers isEqualToArray: prevIdentifiers] &&
     enablement == prevEnablement ? NO : YES;
 }
@@ -1198,6 +1224,8 @@ update_frame_tool_bar (struct frame *f)
                         helpText: (const char *)help
                          enabled: (BOOL)enabled
 {
+  NSTRACE ("[EmacsToolbar addDisplayItemWithImage: ...]");
+
   /* 1) come up w/identifier */
   NSString *identifier
     = [NSString stringWithFormat: @"%lu", (unsigned long)[img hash]];
@@ -1231,6 +1259,7 @@ update_frame_tool_bar (struct frame *f)
    all items to enabled state (for some reason). */
 - (void)validateVisibleItems
 {
+  NSTRACE ("[EmacsToolbar validateVisibleItems]");
 }

@@ -1240,12 +1269,16 @@ update_frame_tool_bar (struct frame *f)
       itemForItemIdentifier: (NSString *)itemIdentifier
   willBeInsertedIntoToolbar: (BOOL)flag
 {
+  NSTRACE ("[EmacsToolbar toolbar: ...]");
+
   /* look up NSToolbarItem by identifier and return... */
   return [identifierToItem objectForKey: itemIdentifier];
 }
 
 - (NSArray *)toolbarDefaultItemIdentifiers: (NSToolbar *)toolbar
 {
+  NSTRACE ("[EmacsToolbar toolbarDefaultItemIdentifiers:]");
+
   /* return entire set.. */
   return activeIdentifiers;
 }
@@ -1253,6 +1286,8 @@ update_frame_tool_bar (struct frame *f)
 /* for configuration palette (not yet supported) */
 - (NSArray *)toolbarAllowedItemIdentifiers: (NSToolbar *)toolbar
 {
+  NSTRACE ("[EmacsToolbar toolbarAllowedItemIdentifiers:]");
+
   /* return entire set... */
   return activeIdentifiers;
   //return [identifierToItem allKeys];
diff --git a/src/nsterm.h b/src/nsterm.h
index 8d52dc6..3fb8cfc 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -65,9 +65,12 @@ typedef float EmacsCGFloat;
 
    ========================================================================== */
 
-/* Uncomment the following line to enable trace. */
+/* Uncomment the following line to enable trace.
 
-/* #define NSTRACE_ENABLED 1 */
+   Hint: keep the trailing whitespace -- the version control system
+   will reject accidental commits. */
+
+/* #define NSTRACE_ENABLED 1          */

 /* Print a call tree containing all annotated functions.
@@ -913,6 +916,9 @@ struct ns_output
 
   /* Non-zero if we are zooming (maximizing) the frame.  */
   int zooming;
+
+  /* Non-zero if we are doing an animation, e.g. toggling the tool bar. */
+  int in_animation;
 };
 
 /* this dummy decl needed to support TTYs */
diff --git a/src/nsterm.m b/src/nsterm.m
index e5eb8ca..ba205f5 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1497,7 +1497,7 @@ x_set_window_size (struct frame *f,
   if (view == nil)
     return;
 
-  NSTRACE_RECT ("input", wr);
+  NSTRACE_RECT ("current", wr);
 
 /*fprintf (stderr, "\tsetWindowSize: %d x %d, pixelwise %d, font size %d x %d\n", width, height, pixelwise, FRAME_COLUMN_WIDTH (f), FRAME_LINE_HEIGHT (f));*/
 
@@ -1559,7 +1559,6 @@ x_set_window_size (struct frame *f,
 	   make_number (FRAME_NS_TITLEBAR_HEIGHT (f)),
 	   make_number (FRAME_TOOLBAR_HEIGHT (f))));
 
-  [view setRows: rows andColumns: cols];
   NSTRACE_RECT ("setFrame", wr);
   [window setFrame: wr display: YES];
 
@@ -6142,6 +6141,8 @@ not_in_argv (NSString *arg)
   NSTRACE ("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])
     {
@@ -6158,13 +6159,19 @@ not_in_argv (NSString *arg)
   if (wait_for_tool_bar)
     {
       if (FRAME_TOOLBAR_HEIGHT (emacsframe) == 0)
-        return;
+        {
+          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 ("tool_bar_height: %d", emacsframe->tool_bar_height);
+
   cols = FRAME_PIXEL_WIDTH_TO_TEXT_COLS (emacsframe, neww);
   rows = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (emacsframe, newh);
 
@@ -6174,6 +6181,9 @@ not_in_argv (NSString *arg)
   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);
@@ -6191,6 +6201,10 @@ not_in_argv (NSString *arg)
       [view setFrame: wr];
       [self windowDidMove:nil];   // Update top/left.
     }
+  else
+    {
+      NSTRACE_MSG ("No change");
+    }
 }
 
 - (NSSize)windowWillResize: (NSWindow *)sender toSize: (NSSize)frameSize
@@ -6299,6 +6313,12 @@ not_in_argv (NSString *arg)
 {
   NSTRACE ("windowDidResize");
 
+  if (emacsframe->output_data.ns->in_animation)
+    {
+      NSTRACE_MSG ("Ignored (in animation)");
+      return;
+    }
+
   if (! [self fsIsNative])
     {
       NSWindow *theWindow = [notification object];
@@ -7396,6 +7416,7 @@ not_in_argv (NSString *arg)
 
 - (void) setRows: (int) r andColumns: (int) c
 {
+  NSTRACE ("[EmacsView setRows:%d andColumns:%d]", r, c);
   rows = r;
   cols = c;
 }