Subject: Re: bug#72986: Disabling menu-bar-mode changes size of new frames

From: martin rudalics <[email-masked]>
Date: Mon, 9 Sep 2024 18:52:27 +0200

> This makes an obviously visible difference: C-x 5 2 opens a small frame
 > which immediately becomes large.
 >
 > Here's the *frame-size-history* buffer:
...
 > ConfigureNotify, PS=1328x1260, XS=400x374, DS=1328x1260
 > xg_frame_resized, rejected, PS=1328x1260, XS=400x374, DS=664x630
 > tool-bar-lines (2), MS=160x175
 > xg_frame_set_char_size, visible, PS=1328x1260, XS=1328x1260, DS=1328x1260
 > ConfigureNotify, PS=1328x1260, XS=1328x1258, DS=1328x1260
 > xg_frame_resized, rejected, PS=1328x1260, XS=1328x1258, DS=664x671
 > set_window_configuration (4), MS=160x175 IH IV
 >
 > Note that once more I retained the simple patch to frame.c to set
 > frame-size-history to (100).

Setting 'frame-size-history' to '(100), doing C-x 5 2 and evaluating
'frame--size-history' should be enough.  The frame.c change is needed
only for the initial frame because at that time no Lisp was evaluated.

The result is not yet what I hoped for.  We reject two ConfigureNotify
events but the ensuing xg_frame_set_char_size calls do not get us a
ConfigureNotify we'd accept.  IIUC the final height may be off by two
pixels.  What does (frame-text-height) return for the first and the
second frame?  Also I hope you didn't get the GTK assertion failure.
And how does removing the menubar behave?

Now something completely different.  A couple of years ago I noticed
that our size hints calculations are fundamentally wrong.  I tried to
fix them and I attach some parts of that fix together with the previous
changes I proposed.  Attached as size_hints.diff.  If the patch does not
apply, complain.  I edited it by hand to exclude other changes I made
and I might have made mistakes.  Again I'm interested in the history of
the second frame.

martin

diff --git a/src/gtkutil.c b/src/gtkutil.c
index d57627f152f..7a5cf8e32e1 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1129,11 +1129,45 @@ xg_set_geometry (struct frame *f)
     }
 }
 
+static struct frame *last_resize_frame = NULL;
+static int last_resize_height = -1;
+static int last_resize_width = -1;
+static int last_resize_count = 0;
+
 /** Function to handle resize of native frame F to WIDTH and HEIGHT
     pixels after we got a ConfigureNotify event.  */
 void
 xg_frame_resized (struct frame *f, int width, int height)
 {
+#ifndef HAVE_PGTK
+  if (f == last_resize_frame
+      && (width != last_resize_width
+	  || height != last_resize_height)
+      && last_resize_count <= 3)
+    /* We did not get what we wanted, retry.  */
+    {
+      if (CONSP (frame_size_history))
+	frame_size_history_extra
+	  (f, build_string ("xg_frame_resized, rejected"),
+	   FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f), width, height,
+	   last_resize_width, last_resize_height);
+
+      gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
+			 last_resize_width, last_resize_height);
+
+      last_resize_count++;
+
+      return;
+    }
+  else
+    /* We either got what we asked for or lost the battle.  */
+    {
+      last_resize_frame = NULL;
+      last_resize_height = -1;
+      last_resize_width = -1;
+      last_resize_count = 0;
+    }
+#endif
   /* Ignore case where size of native rectangle didn't change.  */
   if (width != FRAME_PIXEL_WIDTH (f)
       || height != FRAME_PIXEL_HEIGHT (f)
@@ -1199,7 +1233,7 @@ xg_frame_set_char_size (struct frame *f, int width, int height)
   outer_height /= xg_get_scale (f);
   outer_width /= xg_get_scale (f);
 
-  xg_wm_set_size_hint (f, 0, 0);
+  xg_wm_set_size_hint (f, 0, 0, width, height);
 
   /* Resize the top level widget so rows and columns remain constant.
 
@@ -1297,19 +1331,20 @@ xg_frame_set_char_size (struct frame *f, int width, int height)
   else
     {
 #ifndef HAVE_PGTK
+      last_resize_frame = f;
+      last_resize_height = outer_height;
+      last_resize_width = outer_width;
+      last_resize_count = 0;
+
       gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
 			 outer_width, outer_height);
 #else
       if (FRAME_GTK_OUTER_WIDGET (f))
-	{
-	  gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
-			     outer_width, outer_height);
-	}
+	gtk_window_resize (GTK_WINDOW (FRAME_GTK_OUTER_WIDGET (f)),
+			   outer_width, outer_height);
       else
-	{
-	  gtk_widget_set_size_request (FRAME_GTK_WIDGET (f),
-				       outer_width, outer_height);
-	}
+	gtk_widget_set_size_request (FRAME_GTK_WIDGET (f),
+				     outer_width, outer_height);
 #endif
       fullscreen = Qnil;
     }
@@ -1910,14 +1945,16 @@ xg_free_frame_widgets (struct frame *f)
     }
 }
 
-/* Set the normal size hints for the window manager, for frame F.
-   FLAGS is the flags word to use--or 0 meaning preserve the flags
-   that the window now has.
-   If USER_POSITION, set the User Position
-   flag (this is useful when FLAGS is 0).  */
+/** Set the normal size hints for the window manager, for frame F.
+    FLAGS is the flags word to use--or 0 meaning preserve the flags
+    that the window now has.
+    If USER_POSITION, set the User Position
+    flag (this is useful when FLAGS is 0).
+    WIDTH and HEIGHT are the native pixel sizes for F.  */
 
 void
-xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
+xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position,
+		     int width, int height)
 {
   /* Must use GTK routines here, otherwise GTK resets the size hints
      to its own defaults.  */
@@ -1927,6 +1964,9 @@ xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
   int win_gravity = f->win_gravity;
   Lisp_Object fs_state, frame;
   int scale = xg_get_scale (f);
+  int hor_fix = FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, 0);
+  int ver_fix = FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, 0);
+  int rest_width = 0, rest_height = 0;
 
   /* Don't set size hints during initialization; that apparently leads
      to a race condition.  See the thread at
@@ -1936,6 +1976,16 @@ xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
       || FRAME_PARENT_FRAME (f))
     return;
 
+  if (width < 0)
+    width = FRAME_PIXEL_WIDTH (f);
+
+  width /= scale;
+
+  if (height < 0)
+    height = FRAME_PIXEL_HEIGHT (f);
+
+  height /= scale;
+
   XSETFRAME (frame, f);
   fs_state = Fframe_parameter (frame, Qfullscreen);
   if ((EQ (fs_state, Qmaximized) || EQ (fs_state, Qfullboth))
@@ -1963,22 +2013,26 @@ xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
   size_hints = f->output_data.xp->size_hints;
   hint_flags = f->output_data.xp->hint_flags;
 
-  hint_flags |= GDK_HINT_RESIZE_INC | GDK_HINT_MIN_SIZE;
-  size_hints.width_inc = frame_resize_pixelwise ? 1 : FRAME_COLUMN_WIDTH (f);
-  size_hints.height_inc = frame_resize_pixelwise ? 1 : FRAME_LINE_HEIGHT (f);
+  hint_flags |= GDK_HINT_BASE_SIZE | GDK_HINT_RESIZE_INC | GDK_HINT_MIN_SIZE;
+  size_hints.width_inc
+    = frame_resize_pixelwise ? 1 : (FRAME_COLUMN_WIDTH (f) / scale);
+  size_hints.height_inc
+    = frame_resize_pixelwise ? 1 : (FRAME_LINE_HEIGHT (f) / scale);
+
+  base_width = (hor_fix + FRAME_TOOLBAR_WIDTH (f)) / scale;
+  base_height = ((ver_fix + FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f))
+		 / scale);
 
-  hint_flags |= GDK_HINT_BASE_SIZE;
-  /* Use one row/col here so base_height/width does not become zero.
-     Gtk+ and/or Unity on Ubuntu 12.04 can't handle it.
-     Obviously this makes the row/col value displayed off by 1.  */
-  base_width = FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, 1) + FRAME_TOOLBAR_WIDTH (f);
-  base_height = FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, 1)
-    + FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f);
+  /* Now correct the base sizes by what the text equivalents of WIDTH
+     and HEIGHT are not multiples of the size increments.  */
+  rest_width = (width - hor_fix / scale) % size_hints.width_inc;
+  rest_height = (height - ver_fix / scale) % size_hints.height_inc;
+  size_hints.base_width = base_width + rest_width;
+  size_hints.base_height = base_height + rest_height;
 
-  size_hints.base_width = base_width;
-  size_hints.base_height = base_height;
-  size_hints.min_width  = base_width;
-  size_hints.min_height = base_height;
+  /* For GTK2 include the size of external decorations in the minimum
+     sizes, for GTK3 don't do that.  I have no idea why these behave
+     differently (on xfwm4 at least).  */
 
   /* These currently have a one to one mapping with the X values, but I
      don't think we should rely on that.  */
@@ -2018,11 +2072,6 @@ xg_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
       hint_flags |= GDK_HINT_USER_POS;
     }
 
-  size_hints.base_width /= scale;
-  size_hints.base_height /= scale;
-  size_hints.width_inc /= scale;
-  size_hints.height_inc /= scale;
-
   if (hint_flags != f->output_data.xp->hint_flags
       || memcmp (&size_hints,
 		 &f->output_data.xp->size_hints,
diff --git a/src/gtkutil.h b/src/gtkutil.h
index b689053b4b8..cf1adf0d606 100644
--- a/src/gtkutil.h
+++ b/src/gtkutil.h
@@ -153,7 +153,7 @@ #define XG_ITEM_DATA "emacs_menuitem"
 extern int xg_get_default_scrollbar_width (struct frame *f);
 extern int xg_get_default_scrollbar_height (struct frame *f);
 
-extern void xg_wm_set_size_hint (struct frame *, long int, bool);
+extern void xg_wm_set_size_hint (struct frame *, long int, bool, int, int);
 
 extern void update_frame_tool_bar (struct frame *f);
 extern void free_frame_tool_bar (struct frame *f);
diff --git a/src/pgtkfns.c b/src/pgtkfns.c
index f0fd3000965..c6379280a1d 100644
--- a/src/pgtkfns.c
+++ b/src/pgtkfns.c
@@ -1645,7 +1645,7 @@ #define INSTALL_CURSOR(FIELD, NAME) \
      badly we want them.  This should be done after we have the menu
      bar so that its size can be taken into account.  */
   block_input ();
-  xg_wm_set_size_hint (f, window_prompting, false);
+  xg_wm_set_size_hint (f, window_prompting, false, -1, -1);
   unblock_input ();
 
   adjust_frame_size (f, FRAME_TEXT_WIDTH (f), FRAME_TEXT_HEIGHT (f),
diff --git a/src/pgtkterm.c b/src/pgtkterm.c
index 079945126e0..d16b61b3e9d 100644
--- a/src/pgtkterm.c
+++ b/src/pgtkterm.c
@@ -639,7 +639,7 @@ pgtk_set_offset (struct frame *f, int xoff, int yoff, int change_gravity)
   pgtk_calc_absolute_position (f);
 
   block_input ();
-  xg_wm_set_size_hint (f, 0, false);
+  xg_wm_set_size_hint (f, 0, false, -1, -1);
 
   if (change_gravity != 0)
     {
@@ -686,7 +686,7 @@ pgtk_set_window_size (struct frame *f, bool change_gravity,
 
   f->output_data.pgtk->preferred_width = pixelwidth;
   f->output_data.pgtk->preferred_height = pixelheight;
-  xg_wm_set_size_hint (f, 0, 0);
+  xg_wm_set_size_hint (f, 0, 0, -1, -1);
   xg_frame_set_char_size (f, pixelwidth, pixelheight);
   gtk_widget_queue_resize (FRAME_WIDGET (f));
 
@@ -974,7 +974,7 @@ pgtk_set_parent_frame (struct frame *f, Lisp_Object new_value,
 			      fixed, TRUE, TRUE, 0);
 	  f->output_data.pgtk->preferred_width = alloc.width;
 	  f->output_data.pgtk->preferred_height = alloc.height;
-	  xg_wm_set_size_hint (f, 0, 0);
+	  xg_wm_set_size_hint (f, 0, 0, -1, -1);
 	  xg_frame_set_char_size (f, FRAME_PIXEL_TO_TEXT_WIDTH (f, alloc.width),
 				  FRAME_PIXEL_TO_TEXT_HEIGHT (f, alloc.height));
 	  gtk_widget_queue_resize (FRAME_WIDGET (f));
diff --git a/src/xfns.c b/src/xfns.c
index 3f0d8f3fcd0..92f1567b66a 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -4845,7 +4845,7 @@ DEFUN ("x-wm-set-size-hint", Fx_wm_set_size_hint, Sx_wm_set_size_hint,
   struct frame *f = decode_window_system_frame (frame);
 
   block_input ();
-  x_wm_set_size_hint (f, 0, false);
+  x_wm_set_size_hint (f, 0, false, -1, -1);
   unblock_input ();
   return Qnil;
 }
@@ -5340,7 +5340,7 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
   gui_default_parameter (f, parms, Qno_accept_focus, Qnil,
                          NULL, NULL, RES_TYPE_BOOLEAN);
 
-#if defined (USE_X_TOOLKIT) || defined (USE_GTK)
+#if defined (USE_X_TOOLKIT) && !defined (USE_GTK)
   /* Create the menu bar.  */
   if (!minibuffer_only && FRAME_EXTERNAL_MENU_BAR (f))
     {
@@ -5365,7 +5365,7 @@ DEFUN ("x-create-frame", Fx_create_frame, Sx_create_frame,
      badly we want them.  This should be done after we have the menu
      bar so that its size can be taken into account.  */
   block_input ();
-  x_wm_set_size_hint (f, window_prompting, false);
+  x_wm_set_size_hint (f, window_prompting, false, -1, -1);
   unblock_input ();
 
   adjust_frame_size (f, FRAME_TEXT_WIDTH (f), FRAME_TEXT_HEIGHT (f),
diff --git a/src/xterm.c b/src/xterm.c
index 0c20d38b0f7..4dc50213622 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -27617,7 +27617,7 @@ x_set_offset (struct frame *f, int xoff, int yoff, int change_gravity)
   x_calc_absolute_position (f);
 
   block_input ();
-  x_wm_set_size_hint (f, 0, false);
+  x_wm_set_size_hint (f, 0, false, -1, -1);
 
 #ifdef USE_GTK
   if (x_gtk_use_window_move)
@@ -28318,7 +28318,7 @@ x_check_fullscreen (struct frame *f)
 	  emacs_abort ();
         }
 
-      x_wm_set_size_hint (f, 0, false);
+      x_wm_set_size_hint (f, 0, false,-1, -1);
 
       XResizeWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
 		     width, height);
@@ -28489,7 +28489,7 @@ x_set_window_size_1 (struct frame *f, bool change_gravity,
 {
   if (change_gravity)
     f->win_gravity = NorthWestGravity;
-  x_wm_set_size_hint (f, 0, false);
+  x_wm_set_size_hint (f, 0, false, width, height);
 
   XResizeWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
 		 width, height + FRAME_MENUBAR_HEIGHT (f));
@@ -29326,7 +29326,7 @@ x_make_frame_invisible (struct frame *f)
      program-specified, so that when the window is mapped again, it will be
      placed at the same location, without forcing the user to position it
      by hand again (they have already done that once for this window.)  */
-  x_wm_set_size_hint (f, 0, true);
+  x_wm_set_size_hint (f, 0, true, -1, -1);
 
 #ifdef USE_GTK
   if (FRAME_GTK_OUTER_WIDGET (f))
@@ -30049,7 +30049,8 @@ x_embed_frame (struct x_display_info *dpyinfo, struct frame *f)
    The GTK version is in gtkutils.c.  */
 
 void
-x_wm_set_size_hint (struct frame *f, long flags, bool user_position)
+x_wm_set_size_hint (struct frame *f, long flags, bool user_position,
+		    int width, int height)
 {
 #ifndef USE_GTK
   XSizeHints size_hints;
@@ -30228,7 +30229,7 @@ x_wm_set_size_hint (struct frame *f, long flags, bool user_position)
 
   XSetWMNormalHints (FRAME_X_DISPLAY (f), window, &size_hints);
 #else
-  xg_wm_set_size_hint (f, flags, user_position);
+  xg_wm_set_size_hint (f, flags, user_position, width, height);
 #endif /* USE_GTK */
 }
 
diff --git a/src/xterm.h b/src/xterm.h
index 8d5c9917749..2b6145a2b14 100644
--- a/src/xterm.h
+++ b/src/xterm.h
@@ -1774,7 +1774,7 @@ #define SELECTION_EVENT_TIME(eventp)	\
 extern void x_make_frame_invisible (struct frame *);
 extern void x_iconify_frame (struct frame *);
 extern void x_free_frame_resources (struct frame *);
-extern void x_wm_set_size_hint (struct frame *, long, bool);
+extern void x_wm_set_size_hint (struct frame *, long, bool, int, int);
 #if defined HAVE_XSYNCTRIGGERFENCE && !defined USE_GTK \
   && defined HAVE_CLOCK_GETTIME
 extern void x_sync_init_fences (struct frame *);