bug#16013: 24.3.50; Rows in height is interpreted as pixels.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

Can't do this from -Q, but with a minimal .emacs:

(custom-set-variables
 '(default-frame-alist (quote ((height . 50))))
)

start emacs and it won't be 50 rows, but 50 pixels.

      Jan D.
      


In GNU Emacs 24.3.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.8.6)
 of 2013-11-30 on f19
Bzr revision: 115304 jan.h.d@swipnet.se-20131130123716-9sltid14wjk17zkc
Windowing system distributor `Fedora Project', version 11.0.11403000
Configured using:
 `configure --enable-checking --verbose --prefix=/opt/emacs-cvs
 CFLAGS=-g3'

Important settings:
  value of $LC_COLLATE: C
  value of $LANG: sv_SE.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
<escape> x r e p o r t . <backspace> - e m <tab> <
return>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
easymenu mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils time-date tooltip electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd tool-bar dnd
fontset image regexp-opt fringe tabulated-list newcomment lisp-mode
prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese case-table epa-hook jka-cmpr-hook help simple abbrev
minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

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

 > Can't do this from -Q, but with a minimal .emacs:
 >
 > (custom-set-variables
 >  '(default-frame-alist (quote ((height . 50))))
 > )
 >
 > start emacs and it won't be 50 rows, but 50 pixels.

Bad luck.  Please try the attached, pretty unripe patch.

Thanks, martin

[-- Attachment #2: frame-height.diff --]
[-- Type: text/plain, Size: 4492 bytes --]

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2013-11-30 09:25:31 +0000
+++ src/gtkutil.c	2013-11-30 14:44:13 +0000
@@ -938,27 +938,30 @@
    COLUMNS/ROWS is the size the edit area shall have after the resize.  */

 void
-xg_frame_set_char_size (struct frame *f, int width, int height)
+xg_frame_set_char_size (struct frame *f, int width, int height, bool pixelwise)
 {
-  int pixelwidth;
-  int pixelheight = FRAME_TEXT_TO_PIXEL_HEIGHT (f, height);
+  int pixelwidth = pixelwise ? width : 0;
+  int pixelheight = pixelwise ? height : FRAME_TEXT_TO_PIXEL_HEIGHT (f, height);

   if (FRAME_PIXEL_HEIGHT (f) == 0)
     return;

-  /* Take into account the size of the scroll bar.  Always use the
-     number of columns occupied by the scroll bar here otherwise we
-     might end up with a frame width that is not a multiple of the
-     frame's character width which is bad for vertically split
-     windows.  */
-  f->scroll_bar_actual_width
-    = FRAME_SCROLL_BAR_COLS (f) * FRAME_COLUMN_WIDTH (f);
-
-  compute_fringe_widths (f, 0);
-
-  /* FRAME_TEXT_COLS_TO_PIXEL_WIDTH uses scroll_bar_actual_width, so call it
-     after calculating that value.  */
-  pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
+  if (! pixelwise)
+    {
+      /* Take into account the size of the scroll bar.  Always use the
+	 number of columns occupied by the scroll bar here otherwise we
+	 might end up with a frame width that is not a multiple of the
+	 frame's character width which is bad for vertically split
+	 windows.  */
+      f->scroll_bar_actual_width
+	= FRAME_SCROLL_BAR_COLS (f) * FRAME_COLUMN_WIDTH (f);
+
+      compute_fringe_widths (f, 0);
+
+      /* FRAME_TEXT_COLS_TO_PIXEL_WIDTH uses scroll_bar_actual_width, so call it
+	 after calculating that value.  */
+      pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
+    }

   /* Do this before resize, as we don't know yet if we will be resized.  */
   xg_clear_under_internal_border (f);
@@ -988,7 +991,7 @@
     }
   else
     {
-      change_frame_size (f, width, height, 0, 1, 0, 1);
+      change_frame_size (f, pixelwidth, pixelheight, 0, 1, 0, 1);
       FRAME_PIXEL_WIDTH (f) = pixelwidth;
       FRAME_PIXEL_HEIGHT (f) = pixelheight;
      }
@@ -1095,7 +1098,7 @@
               && FRAME_X_DISPLAY (f) == dpy)
             {
               x_set_scroll_bar_default_width (f);
-              xg_frame_set_char_size (f, FRAME_COLS (f), FRAME_LINES (f));
+              xg_frame_set_char_size (f, FRAME_COLS (f), FRAME_LINES (f), 0);
             }
         }
     }

=== modified file 'src/gtkutil.h'
--- src/gtkutil.h	2013-09-17 07:06:42 +0000
+++ src/gtkutil.h	2013-11-30 14:43:32 +0000
@@ -139,7 +139,7 @@
 extern void xg_frame_resized (struct frame *f,
                               int pixelwidth,
                               int pixelheight);
-extern void xg_frame_set_char_size (struct frame *f, int cols, int rows);
+extern void xg_frame_set_char_size (struct frame *f, int cols, int rows, bool pixelwise);
 extern GtkWidget * xg_win_to_widget (Display *dpy, Window wdesc);

 extern void xg_display_open (char *display_name, Display **dpy);

=== modified file 'src/xterm.c'
--- src/xterm.c	2013-11-30 09:25:31 +0000
+++ src/xterm.c	2013-11-30 14:42:43 +0000
@@ -8743,8 +8743,12 @@
 void
 x_set_window_size (struct frame *f, int change_gravity, int width, int height, bool pixelwise)
 {
+  int pixelwidth, pixelheight;
+
   block_input ();

+  check_frame_size (f, &width, &height, pixelwise);
+
   if (NILP (tip_frame) || XFRAME (tip_frame) != f)
     {
       int r, c, text_width, text_height;
@@ -8773,14 +8777,25 @@
       change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
     }

+  if (pixelwise)
+    {
+      pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
+      pixelheight = FRAME_TEXT_TO_PIXEL_HEIGHT (f, height);
+    }
+  else
+    {
+      pixelwidth = FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width);
+      pixelheight = FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height);
+    }
+
 #ifdef USE_GTK
   if (FRAME_GTK_WIDGET (f))
-    xg_frame_set_char_size (f, width, height);
+    xg_frame_set_char_size (f, pixelwidth, pixelheight, 1);
   else
-    x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
+    x_set_window_size_1 (f, change_gravity, pixelwidth, pixelheight, 1);
 #else /* not USE_GTK */

-  x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
+  x_set_window_size_1 (f, change_gravity, pixelwidth, pixelheight, 1);

 #endif /* not USE_GTK */

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

2013-11-30 16:00, martin rudalics skrev:
>  > Can't do this from -Q, but with a minimal .emacs:
>  >
>  > (custom-set-variables
>  >  '(default-frame-alist (quote ((height . 50))))
>  > )
>  >
>  > start emacs and it won't be 50 rows, but 50 pixels.
>
> Bad luck.  Please try the attached, pretty unripe patch.
>

That works.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

> That works.

With correct height and width ?

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

1 dec 2013 kl. 10:44 skrev martin rudalics <rudalics@gmx.at>:

>> That works.
> 
> With correct height and width ?
> 

Width is 83 columns, so that is wrong, it should be 80 by default.
Height is missing 3 rows, but I think it did that before also, i.e. tool- and menu-bar.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

> Width is 83 columns, so that is wrong, it should be 80 by default.
> Height is missing 3 rows, but I think it did that before also, i.e. tool- and menu-bar.

What give

(/ (frame-text-height) (frame-char-height))
(/ (frame-text-width) (frame-char-width))

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

1 dec 2013 kl. 12:24 skrev martin rudalics <rudalics@gmx.at>:

>> Width is 83 columns, so that is wrong, it should be 80 by default.
>> Height is missing 3 rows, but I think it did that before also, i.e. tool- and menu-bar.
> 
> What give
> 
> (/ (frame-text-height) (frame-char-height))

47.

> (/ (frame-text-width) (frame-char-width))

This is a bit random.  On 5 starts, I got 79 once, 83 once and 84 three times.
And the number of columns match, 79, 83 and 84.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

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

 >> (/ (frame-text-height) (frame-char-height))
 >
 > 47.
 >
 >> (/ (frame-text-width) (frame-char-width))
 >
 > This is a bit random.  On 5 starts, I got 79 once, 83 once and 84 three times.
 > And the number of columns match, 79, 83 and 84.

My old patch was silly - it propagated the frame's pixel sizes instead
of the text sizes to xg_frame_set_char_size.  Please try the attached
one.  I still have two problems here: The height of the initial frame is
by six lines too small and a new frame made via C-x 5 2 is two lines
higher than the initial one.

Thanks, martin

[-- Attachment #2: change-frame-size.diff --]
[-- Type: text/plain, Size: 4481 bytes --]

=== modified file 'src/gtkutil.c'
--- src/gtkutil.c	2013-11-30 09:25:31 +0000
+++ src/gtkutil.c	2013-12-02 16:39:10 +0000
@@ -940,26 +940,12 @@
 void
 xg_frame_set_char_size (struct frame *f, int width, int height)
 {
-  int pixelwidth;
+  int pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
   int pixelheight = FRAME_TEXT_TO_PIXEL_HEIGHT (f, height);

   if (FRAME_PIXEL_HEIGHT (f) == 0)
     return;

-  /* Take into account the size of the scroll bar.  Always use the
-     number of columns occupied by the scroll bar here otherwise we
-     might end up with a frame width that is not a multiple of the
-     frame's character width which is bad for vertically split
-     windows.  */
-  f->scroll_bar_actual_width
-    = FRAME_SCROLL_BAR_COLS (f) * FRAME_COLUMN_WIDTH (f);
-
-  compute_fringe_widths (f, 0);
-
-  /* FRAME_TEXT_COLS_TO_PIXEL_WIDTH uses scroll_bar_actual_width, so call it
-     after calculating that value.  */
-  pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
-
   /* Do this before resize, as we don't know yet if we will be resized.  */
   xg_clear_under_internal_border (f);

@@ -987,11 +973,7 @@
       x_wait_for_event (f, ConfigureNotify);
     }
   else
-    {
-      change_frame_size (f, width, height, 0, 1, 0, 1);
-      FRAME_PIXEL_WIDTH (f) = pixelwidth;
-      FRAME_PIXEL_HEIGHT (f) = pixelheight;
-     }
+    change_frame_size (f, width, height, 0, 1, 0, 1);
 }

 /* Handle height/width changes (i.e. add/remove/move menu/toolbar).
@@ -1095,7 +1077,7 @@
               && FRAME_X_DISPLAY (f) == dpy)
             {
               x_set_scroll_bar_default_width (f);
-              xg_frame_set_char_size (f, FRAME_COLS (f), FRAME_LINES (f));
+              xg_frame_set_char_size (f, FRAME_TEXT_WIDTH (f), FRAME_TEXT_HEIGHT (f));
             }
         }
     }

=== modified file 'src/gtkutil.h'
--- src/gtkutil.h	2013-09-17 07:06:42 +0000
+++ src/gtkutil.h	2013-12-02 15:49:38 +0000
@@ -139,7 +139,7 @@
 extern void xg_frame_resized (struct frame *f,
                               int pixelwidth,
                               int pixelheight);
-extern void xg_frame_set_char_size (struct frame *f, int cols, int rows);
+extern void xg_frame_set_char_size (struct frame *f, int width, int height);
 extern GtkWidget * xg_win_to_widget (Display *dpy, Window wdesc);

 extern void xg_display_open (char *display_name, Display **dpy);

=== modified file 'src/xfns.c'
--- src/xfns.c	2013-11-30 09:25:31 +0000
+++ src/xfns.c	2013-12-02 14:36:33 +0000
@@ -3069,6 +3069,20 @@
      happen.  */
   init_frame_faces (f);

+  /* PXW: This is a duplicate from below.  We have to do it here since
+     otherwise x_set_tool_bar_lines will work with the character sizes
+     installed by init_frame_faces while the frame's pixel size is still
+     calculated from a character size of 1 and we subsequently hit the
+     eassert (height >= 0) assertion in window_box_height.  The
+     non-pixelwise code apparently worked around this because it had one
+     frame line vs one toolbar line which left us with a zero root
+     window height which was obviously wrong as well ...  */
+  width = FRAME_TEXT_WIDTH (f);
+  height = FRAME_TEXT_HEIGHT (f);
+  FRAME_TEXT_HEIGHT (f) = 0;
+  SET_FRAME_WIDTH (f, 0);
+  change_frame_size (f, width, height, 1, 0, 0, 1);
+
   /* Set the menu-bar-lines and tool-bar-lines parameters.  We don't
      look up the X resources controlling the menu-bar and tool-bar
      here; they are processed specially at startup, and reflected in

=== modified file 'src/xterm.c'
--- src/xterm.c	2013-11-30 16:25:51 +0000
+++ src/xterm.c	2013-12-02 17:16:35 +0000
@@ -8745,6 +8745,8 @@
 {
   block_input ();

+  check_frame_size (f, &width, &height, pixelwise);
+
   if (NILP (tip_frame) || XFRAME (tip_frame) != f)
     {
       int text_width, text_height;
@@ -8771,14 +8773,20 @@
       change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
     }

+  if (! pixelwise)
+    {
+      width = width * FRAME_COLUMN_WIDTH (f);
+      height = height * FRAME_LINE_HEIGHT (f);
+    }
+
 #ifdef USE_GTK
   if (FRAME_GTK_WIDGET (f))
     xg_frame_set_char_size (f, width, height);
   else
-    x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
+    x_set_window_size_1 (f, change_gravity, width, height, 1);
 #else /* not USE_GTK */

-  x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
+  x_set_window_size_1 (f, change_gravity, width, height, 1);

 #endif /* not USE_GTK */

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Stephen Berman]

On Mon, 02 Dec 2013 19:15:36 +0100 martin rudalics <rudalics@gmx.at> wrote:

>>> (/ (frame-text-height) (frame-char-height))
>>
>> 47.
>>
>>> (/ (frame-text-width) (frame-char-width))
>>
>> This is a bit random.  On 5 starts, I got 79 once, 83 once and 84
>> three times.  And the number of columns match, 79, 83 and 84.
>
> My old patch was silly - it propagated the frame's pixel sizes instead
> of the text sizes to xg_frame_set_char_size.  Please try the attached
> one.  I still have two problems here: The height of the initial frame
> is by six lines too small and a new frame made via C-x 5 2 is two
> lines higher than the initial one.

FYI I applied the new patch and started the rebuilt Emacs with my
initializations, which contain this:

(modify-frame-parameters nil '((width . 85) (fullscreen . fullheight)))

However, window-width in fact returned 73 (but window-height was the full
height).  Then I evalled this:

(modify-frame-parameters nil '((width . 85)))

Now, window-width was 78.  (Maybe the increase despite the same passed
width value is because I customize the tool bar to appear on the left
side, and maybe that takes effect after the initial frame width is set.)
To get a window-width of 85 columns, I had to give the width
frame-parameter a value of 92.

This is with the xft backend and font
-unknown-DejaVu Sans Mono-normal-normal-normal-*-12-*-*-*-m-0-iso10646-1

Steve Berman

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello. 

> 2 dec 2013 kl. 19:15 skrev martin rudalics <rudalics@gmx.at>:
> 
> >> (/ (frame-text-height) (frame-char-height))
> >
> > 47.
> >
> >> (/ (frame-text-width) (frame-char-width))
> >
> > This is a bit random.  On 5 starts, I got 79 once, 83 once and 84 three times.
> > And the number of columns match, 79, 83 and 84.
> 
> My old patch was silly - it propagated the frame's pixel sizes instead
> of the text sizes to xg_frame_set_char_size.  Please try the attached
> one.  I still have two problems here: The height of the initial frame is
> by six lines too small and a new frame made via C-x 5 2 is two lines
> higher than the initial one.

Why not check in the patch at once instead of posting it here?
The current trunk is unusable, so it must be an improvement. 

Also, did you test on the other X variants, i.e. Lucid, Motif and no toolkit?  This seems it only affects Gtk+..

    Jan D. 

> 
> Thanks, martin
> === modified file 'src/gtkutil.c'
> --- src/gtkutil.c    2013-11-30 09:25:31 +0000
> +++ src/gtkutil.c    2013-12-02 16:39:10 +0000
> @@ -940,26 +940,12 @@
> void
> xg_frame_set_char_size (struct frame *f, int width, int height)
> {
> -  int pixelwidth;
> +  int pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
>   int pixelheight = FRAME_TEXT_TO_PIXEL_HEIGHT (f, height);
> 
>   if (FRAME_PIXEL_HEIGHT (f) == 0)
>     return;
> 
> -  /* Take into account the size of the scroll bar.  Always use the
> -     number of columns occupied by the scroll bar here otherwise we
> -     might end up with a frame width that is not a multiple of the
> -     frame's character width which is bad for vertically split
> -     windows.  */
> -  f->scroll_bar_actual_width
> -    = FRAME_SCROLL_BAR_COLS (f) * FRAME_COLUMN_WIDTH (f);
> -
> -  compute_fringe_widths (f, 0);
> -
> -  /* FRAME_TEXT_COLS_TO_PIXEL_WIDTH uses scroll_bar_actual_width, so call it
> -     after calculating that value.  */
> -  pixelwidth = FRAME_TEXT_TO_PIXEL_WIDTH (f, width);
> -
>   /* Do this before resize, as we don't know yet if we will be resized.  */
>   xg_clear_under_internal_border (f);
> 
> @@ -987,11 +973,7 @@
>       x_wait_for_event (f, ConfigureNotify);
>     }
>   else
> -    {
> -      change_frame_size (f, width, height, 0, 1, 0, 1);
> -      FRAME_PIXEL_WIDTH (f) = pixelwidth;
> -      FRAME_PIXEL_HEIGHT (f) = pixelheight;
> -     }
> +    change_frame_size (f, width, height, 0, 1, 0, 1);
> }
> 
> /* Handle height/width changes (i.e. add/remove/move menu/toolbar).
> @@ -1095,7 +1077,7 @@
>               && FRAME_X_DISPLAY (f) == dpy)
>             {
>               x_set_scroll_bar_default_width (f);
> -              xg_frame_set_char_size (f, FRAME_COLS (f), FRAME_LINES (f));
> +              xg_frame_set_char_size (f, FRAME_TEXT_WIDTH (f), FRAME_TEXT_HEIGHT (f));
>             }
>         }
>     }
> 
> === modified file 'src/gtkutil.h'
> --- src/gtkutil.h    2013-09-17 07:06:42 +0000
> +++ src/gtkutil.h    2013-12-02 15:49:38 +0000
> @@ -139,7 +139,7 @@
> extern void xg_frame_resized (struct frame *f,
>                               int pixelwidth,
>                               int pixelheight);
> -extern void xg_frame_set_char_size (struct frame *f, int cols, int rows);
> +extern void xg_frame_set_char_size (struct frame *f, int width, int height);
> extern GtkWidget * xg_win_to_widget (Display *dpy, Window wdesc);
> 
> extern void xg_display_open (char *display_name, Display **dpy);
> 
> === modified file 'src/xfns.c'
> --- src/xfns.c    2013-11-30 09:25:31 +0000
> +++ src/xfns.c    2013-12-02 14:36:33 +0000
> @@ -3069,6 +3069,20 @@
>      happen.  */
>   init_frame_faces (f);
> 
> +  /* PXW: This is a duplicate from below.  We have to do it here since
> +     otherwise x_set_tool_bar_lines will work with the character sizes
> +     installed by init_frame_faces while the frame's pixel size is still
> +     calculated from a character size of 1 and we subsequently hit the
> +     eassert (height >= 0) assertion in window_box_height.  The
> +     non-pixelwise code apparently worked around this because it had one
> +     frame line vs one toolbar line which left us with a zero root
> +     window height which was obviously wrong as well ...  */
> +  width = FRAME_TEXT_WIDTH (f);
> +  height = FRAME_TEXT_HEIGHT (f);
> +  FRAME_TEXT_HEIGHT (f) = 0;
> +  SET_FRAME_WIDTH (f, 0);
> +  change_frame_size (f, width, height, 1, 0, 0, 1);
> +
>   /* Set the menu-bar-lines and tool-bar-lines parameters.  We don't
>      look up the X resources controlling the menu-bar and tool-bar
>      here; they are processed specially at startup, and reflected in
> 
> === modified file 'src/xterm.c'
> --- src/xterm.c    2013-11-30 16:25:51 +0000
> +++ src/xterm.c    2013-12-02 17:16:35 +0000
> @@ -8745,6 +8745,8 @@
> {
>   block_input ();
> 
> +  check_frame_size (f, &width, &height, pixelwise);
> +
>   if (NILP (tip_frame) || XFRAME (tip_frame) != f)
>     {
>       int text_width, text_height;
> @@ -8771,14 +8773,20 @@
>       change_frame_size (f, text_width, text_height, 0, 1, 0, 1);
>     }
> 
> +  if (! pixelwise)
> +    {
> +      width = width * FRAME_COLUMN_WIDTH (f);
> +      height = height * FRAME_LINE_HEIGHT (f);
> +    }
> +
> #ifdef USE_GTK
>   if (FRAME_GTK_WIDGET (f))
>     xg_frame_set_char_size (f, width, height);
>   else
> -    x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
> +    x_set_window_size_1 (f, change_gravity, width, height, 1);
> #else /* not USE_GTK */
> 
> -  x_set_window_size_1 (f, change_gravity, width, height, pixelwise);
> +  x_set_window_size_1 (f, change_gravity, width, height, 1);
> 
> #endif /* not USE_GTK */
> 
> 

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > FYI I applied the new patch and started the rebuilt Emacs with my
 > initializations, which contain this:
 >
 > (modify-frame-parameters nil '((width . 85) (fullscreen . fullheight)))
 >
 > However, window-width in fact returned 73 (but window-height was the full
 > height).  Then I evalled this:
 >
 > (modify-frame-parameters nil '((width . 85)))
 >
 > Now, window-width was 78.  (Maybe the increase despite the same passed
 > width value is because I customize the tool bar to appear on the left
 > side, and maybe that takes effect after the initial frame width is set.)
 > To get a window-width of 85 columns, I had to give the width
 > frame-parameter a value of 92.

I suppose this is the same issue I experience with my height value when
the toolbar is at the top.  Which toolkit do you use, if any?

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

> Why not check in the patch at once instead of posting it here?
> The current trunk is unusable, so it must be an improvement. 

Done.

> Also, did you test on the other X variants, i.e. Lucid, Motif and no toolkit?  This seems it only affects Gtk+..

Tested only with Gtk+.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Stephen Berman]

On Tue, 03 Dec 2013 08:56:04 +0100 martin rudalics <rudalics@gmx.at> wrote:

>> FYI I applied the new patch and started the rebuilt Emacs with my
>> initializations, which contain this:
>>
>> (modify-frame-parameters nil '((width . 85) (fullscreen . fullheight)))
>>
>> However, window-width in fact returned 73 (but window-height was the full
>> height).  Then I evalled this:
>>
>> (modify-frame-parameters nil '((width . 85)))
>>
>> Now, window-width was 78.  (Maybe the increase despite the same passed
>> width value is because I customize the tool bar to appear on the left
>> side, and maybe that takes effect after the initial frame width is set.)
>> To get a window-width of 85 columns, I had to give the width
>> frame-parameter a value of 92.
>
> I suppose this is the same issue I experience with my height value when
> the toolbar is at the top.

I think that's it, and this seems to be confirmed by starting my Emacs
like this:

emacs --eval '(tool-bar-mode -1)'

Now window-width returns the correct value of 85.  So it seems that the
metrics of your new patch are correct, but not the interaction of width
and height frame parameters with the tool bar.

>                             Which toolkit do you use, if any?

gtk3

Steve Berman

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Ok, now I get 80 columns and 48 rows.

	Jan D.

3 dec 2013 kl. 08:58 skrev martin rudalics <rudalics@gmx.at>:

>> Why not check in the patch at once instead of posting it here?
>> The current trunk is unusable, so it must be an improvement. 
> 
> Done.
> 
>> Also, did you test on the other X variants, i.e. Lucid, Motif and no toolkit?  This seems it only affects Gtk+..
> 
> Tested only with Gtk+.
> 
> martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

> I think that's it, and this seems to be confirmed by starting my Emacs
> like this:
> 
> emacs --eval '(tool-bar-mode -1)'
> 
> Now window-width returns the correct value of 85.  So it seems that the
> metrics of your new patch are correct, but not the interaction of width
> and height frame parameters with the tool bar.
> 
>>                             Which toolkit do you use, if any?
> 
> gtk3

I now count tool- and menubars with gtk.  Does that fix it?

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > Ok, now I get 80 columns and 48 rows.

Apparently not since you wanted 50 rows.  Please try once more.

Unfortunately a second frame still sometimes gets three rows less.  In
this regard the behavior here is the same as with 24.3.  Any ideas?

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

3 dec 2013 kl. 19:34 skrev martin rudalics <rudalics@gmx.at>:

> > Ok, now I get 80 columns and 48 rows.
> 
> Apparently not since you wanted 50 rows.  Please try once more.
> 

I now get 49 rows.  That is the same I get with 24.3.

> Unfortunately a second frame still sometimes gets three rows less.  In
> this regard the behavior here is the same as with 24.3.  Any ideas?

I don't see this in the trunk or 24.3.  Rows are consistently 49.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Some more info:

Lucid/Motif: 46 rows, 74 columns on initial frame,
            50 rows, 80 columns on subsequent frames.

No toolkit: 45 rows, 68 columns on initial frame,
                 47 rows, 75 columns on subsequent frames.

Gtk+3 and 2: 49 rows, 80 columns on initial frame.
                49 rows, 80 columns on subsequent frames.

	Jan D.

3 dec 2013 kl. 20:30 skrev Jan Djärv <jan.h.d@swipnet.se>:

> Hello.
> 
> 3 dec 2013 kl. 19:34 skrev martin rudalics <rudalics@gmx.at>:
> 
>>> Ok, now I get 80 columns and 48 rows.
>> 
>> Apparently not since you wanted 50 rows.  Please try once more.
>> 
> 
> I now get 49 rows.  That is the same I get with 24.3.
> 
>> Unfortunately a second frame still sometimes gets three rows less.  In
>> this regard the behavior here is the same as with 24.3.  Any ideas?
> 
> I don't see this in the trunk or 24.3.  Rows are consistently 49.
> 
> 	Jan D.
> 
> 

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Stephen Berman]

On Tue, 03 Dec 2013 19:34:07 +0100 martin rudalics <rudalics@gmx.at> wrote:

>> I think that's it, and this seems to be confirmed by starting my Emacs
>> like this:
>>
>> emacs --eval '(tool-bar-mode -1)'
>>
>> Now window-width returns the correct value of 85.  So it seems that the
>> metrics of your new patch are correct, but not the interaction of width
>> and height frame parameters with the tool bar.
>>
>>>                             Which toolkit do you use, if any?
>>
>> gtk3
>
> I now count tool- and menubars with gtk.  Does that fix it?

Yes; thanks.

Steve Berman

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > I now get 49 rows.  That is the same I get with 24.3.

Interesting.  50 rows are too large for my frame so I test with 30 rows
and 60 columns.  In this case I get what I specified.  Which are the
values passed to gtk_window_resize on your system?

 >> Unfortunately a second frame still sometimes gets three rows less.  In
 >> this regard the behavior here is the same as with 24.3.  Any ideas?
 >
 > I don't see this in the trunk or 24.3.  Rows are consistently 49.

Here the height decreases to 27 in, on the average, every second frame I
create via C-x 5 2.  Consistently, with 24.3 and current trunk on xfce.
I'll have to trace the values passed to gtk_window_resize myself.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

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

 > Lucid/Motif: 46 rows, 74 columns on initial frame,
 >             50 rows, 80 columns on subsequent frames.

I don't know which libraries are needed to build with Lucid.  I
installed open motif libraries but the build script doesn't appreciate
them.  So I can't test these here.  At least the "subsequent frames"
look correct.

 > No toolkit: 45 rows, 68 columns on initial frame,
 >                  47 rows, 75 columns on subsequent frames.

This can be fixed here using the attached patch.  But with a frame width
of 60 the echo area should resize but disappears on the initial frame.
I don't yet know why.

 > Gtk+3 and 2: 49 rows, 80 columns on initial frame.
 >                 49 rows, 80 columns on subsequent frames.

martin

[-- Attachment #2: x_set_frame_size.diff --]
[-- Type: text/plain, Size: 1099 bytes --]

=== modified file 'src/xterm.c'
--- src/xterm.c	2013-12-03 11:33:13 +0000
+++ src/xterm.c	2013-12-04 16:37:35 +0000
@@ -8552,12 +8552,15 @@
 
   compute_fringe_widths (f, 0);
 
-  pixelwidth =
-    (pixelwise ? width : FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width))
-    + FRAME_TOOLBAR_WIDTH (f);
-  pixelheight =
-    (pixelwise ? height : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height))
-    + FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f);
+  pixelwidth = ((pixelwise
+		 ? FRAME_TEXT_TO_PIXEL_WIDTH (f, width)
+		 : FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width))
+		+ FRAME_TOOLBAR_WIDTH (f));
+  pixelheight = ((pixelwise
+		  ? FRAME_TEXT_TO_PIXEL_HEIGHT (f, height)
+		  : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height))
+		 + FRAME_MENUBAR_HEIGHT (f)
+		 + FRAME_TOOLBAR_HEIGHT (f));
 
   if (change_gravity) f->win_gravity = NorthWestGravity;
   x_wm_set_size_hint (f, (long) 0, 0);
@@ -8593,8 +8596,6 @@
   else
     {
       change_frame_size (f, width, height, 0, 1, 0, 1);
-      FRAME_PIXEL_WIDTH (f) = pixelwidth;
-      FRAME_PIXEL_HEIGHT (f) = pixelheight;
       x_sync (f);
     }
 }

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

2013-12-04 19:06, martin rudalics skrev:
>  > Lucid/Motif: 46 rows, 74 columns on initial frame,
>  >             50 rows, 80 columns on subsequent frames.
>
> I don't know which libraries are needed to build with Lucid.  I
> installed open motif libraries but the build script doesn't appreciate
> them.  So I can't test these here.  At least the "subsequent frames"
> look correct.

livXaw is needed.

>
>  > No toolkit: 45 rows, 68 columns on initial frame,
>  >                  47 rows, 75 columns on subsequent frames.
>
> This can be fixed here using the attached patch.  But with a frame width
> of 60 the echo area should resize but disappears on the initial frame.
> I don't yet know why.

You should check in the patch.

>
>  > Gtk+3 and 2: 49 rows, 80 columns on initial frame.
>  >                 49 rows, 80 columns on subsequent frames.
>

I found the last line for Gtk+.  But it is a bit dependent on the window 
manager as size hints come into play.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > livXaw is needed.

libXaw I presume.  Built already.

 >>  > No toolkit: 45 rows, 68 columns on initial frame,
 >>  >                  47 rows, 75 columns on subsequent frames.
 >>
 >> This can be fixed here using the attached patch.  But with a frame width
 >> of 60 the echo area should resize but disappears on the initial frame.
 >> I don't yet know why.
 >
 > You should check in the patch.

Not yet.  I somehow don't add/subtract the "top margin" correctly.

 >>  > Gtk+3 and 2: 49 rows, 80 columns on initial frame.
 >>  >                 49 rows, 80 columns on subsequent frames.
 >>
 >
 > I found the last line for Gtk+.  But it is a bit dependent on the window
 > manager as size hints come into play.

I never lost it with xfce ;-)

Thanks, martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

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

I now managed to get the correct number of lines here with
the attached patch.  Tested with GTK3, Lucid and without
xtoolkit.  Not tested with Motif.  Please have a look.

Thanks, martin


[-- Attachment #2: frame-height.diff --]
[-- Type: text/plain, Size: 4978 bytes --]

martin@NOREST:~/emacs/quickfixes$ bzr diff
=== modified file 'src/dispnew.c'
--- src/dispnew.c	2013-11-30 09:25:31 +0000
+++ src/dispnew.c	2013-12-09 10:20:26 +0000
@@ -5535,6 +5535,10 @@
 	 manipulating video hardware.  */
       if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
 	FrameRows (FRAME_TTY (f)) = new_height;
+
+      FRAME_LINES (f) = new_lines;
+      FRAME_TEXT_HEIGHT (f) = new_text_height;
+      FRAME_PIXEL_HEIGHT (f) = FRAME_TEXT_TO_PIXEL_HEIGHT (f, new_text_height);
     }

   if (new_text_width != FRAME_TEXT_WIDTH (f)
@@ -5547,6 +5551,10 @@
       if ((FRAME_TERMCAP_P (f) && !pretend) || FRAME_MSDOS_P (f))
 	FrameCols (FRAME_TTY (f)) = new_cols;

+      SET_FRAME_COLS (f, new_cols);
+      FRAME_TEXT_WIDTH (f) = new_text_width;
+      FRAME_PIXEL_WIDTH (f) = FRAME_TEXT_TO_PIXEL_WIDTH (f, new_text_width);
+
 #if defined (HAVE_WINDOW_SYSTEM) && ! defined (USE_GTK) && ! defined (HAVE_NS)
       if (WINDOWP (f->tool_bar_window))
 	{
@@ -5556,13 +5564,6 @@
 #endif
     }

-  SET_FRAME_COLS (f, new_cols);
-  FRAME_LINES (f) = new_lines;
-  FRAME_TEXT_WIDTH (f) = new_text_width;
-  FRAME_TEXT_HEIGHT (f) = new_text_height;
-  FRAME_PIXEL_WIDTH (f) = FRAME_TEXT_TO_PIXEL_WIDTH (f, new_text_width);
-  FRAME_PIXEL_HEIGHT (f) = FRAME_TEXT_TO_PIXEL_HEIGHT (f, new_text_height);
-
   {
     struct window *w = XWINDOW (FRAME_SELECTED_WINDOW (f));
     int text_area_x, text_area_y, text_area_width, text_area_height;

=== modified file 'src/window.c'
--- src/window.c	2013-12-08 03:07:11 +0000
+++ src/window.c	2013-12-09 17:25:35 +0000
@@ -4056,7 +4056,8 @@
   r->top_line = FRAME_TOP_MARGIN (f);
   r->pixel_top = FRAME_TOP_MARGIN_HEIGHT (f);

-  if (new_pixel_size == old_pixel_size)
+  if (new_pixel_size == old_pixel_size
+      && XWINDOW (mini)->pixel_height == FRAME_LINE_HEIGHT (f))
     return;
   else if (WINDOW_LEAF_P (r))
     /* For a leaf root window just set the size.  */

=== modified file 'src/xfns.c'
--- src/xfns.c	2013-12-03 17:22:05 +0000
+++ src/xfns.c	2013-12-09 17:34:22 +0000
@@ -3069,7 +3069,6 @@
      happen.  */
   init_frame_faces (f);

-#ifdef USE_GTK
   /* PXW: This is a duplicate from below.  We have to do it here since
      otherwise x_set_tool_bar_lines will work with the character sizes
      installed by init_frame_faces while the frame's pixel size is still
@@ -3078,12 +3077,8 @@
      non-pixelwise code apparently worked around this because it had one
      frame line vs one toolbar line which left us with a zero root
      window height which was obviously wrong as well ...  */
-  width = FRAME_TEXT_WIDTH (f);
-  height = FRAME_TEXT_HEIGHT (f);
-  FRAME_TEXT_HEIGHT (f) = 0;
-  SET_FRAME_WIDTH (f, 0);
-  change_frame_size (f, width, height, 1, 0, 0, 1);
-#endif /* USE_GTK */
+  change_frame_size (f, FRAME_COLS (f) * FRAME_COLUMN_WIDTH (f),
+		     FRAME_LINES (f) * FRAME_LINE_HEIGHT (f), 1, 0, 0, 1);

   /* Set the menu-bar-lines and tool-bar-lines parameters.  We don't
      look up the X resources controlling the menu-bar and tool-bar

=== modified file 'src/xterm.c'
--- src/xterm.c	2013-12-07 23:04:10 +0000
+++ src/xterm.c	2013-12-09 17:39:28 +0000
@@ -6622,8 +6622,8 @@
               cancel_mouse_face (f);
             }

-          FRAME_PIXEL_WIDTH (f) = event->xconfigure.width;
-          FRAME_PIXEL_HEIGHT (f) = event->xconfigure.height;
+/**           FRAME_PIXEL_WIDTH (f) = event->xconfigure.width; **/
+/**           FRAME_PIXEL_HEIGHT (f) = event->xconfigure.height; **/
 #endif /* not USE_GTK */
 #endif

@@ -7689,6 +7689,9 @@
   FRAME_COLUMN_WIDTH (f) = font->average_width;
   FRAME_LINE_HEIGHT (f) = FONT_HEIGHT (font);

+  FRAME_TOOL_BAR_HEIGHT (f) = FRAME_TOOL_BAR_LINES (f) * FRAME_LINE_HEIGHT (f);
+  FRAME_MENU_BAR_HEIGHT (f) = FRAME_MENU_BAR_LINES (f) * FRAME_LINE_HEIGHT (f);
+
   compute_fringe_widths (f, 1);

   /* Compute the scroll bar width in character columns.  */
@@ -8541,13 +8544,15 @@

   compute_fringe_widths (f, 0);

-  pixelwidth =
-    (pixelwise ? width : FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width))
-    + FRAME_TOOLBAR_WIDTH (f);
-  pixelheight =
-    (pixelwise ? height : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height))
-    + FRAME_MENUBAR_HEIGHT (f) + FRAME_TOOLBAR_HEIGHT (f);
-
+  pixelwidth = ((pixelwise
+		 ? FRAME_TEXT_TO_PIXEL_WIDTH (f, width)
+		 : FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width))
+		+ FRAME_TOOLBAR_WIDTH (f));
+  pixelheight = ((pixelwise
+		  ? FRAME_TEXT_TO_PIXEL_HEIGHT (f, height)
+		  : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height))
+		 + FRAME_MENUBAR_HEIGHT (f)
+		 + FRAME_TOOLBAR_HEIGHT (f));
   if (change_gravity) f->win_gravity = NorthWestGravity;
   x_wm_set_size_hint (f, (long) 0, 0);
   XResizeWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
@@ -8582,8 +8587,6 @@
   else
     {
       change_frame_size (f, width, height, 0, 1, 0, 1);
-      FRAME_PIXEL_WIDTH (f) = pixelwidth;
-      FRAME_PIXEL_HEIGHT (f) = pixelheight;
       x_sync (f);
     }
 }

martin@NOREST:~/emacs/quickfixes$ 

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

> Some more info:
> 
> Lucid/Motif: 46 rows, 74 columns on initial frame,
>             50 rows, 80 columns on subsequent frames.
> 
> No toolkit: 45 rows, 68 columns on initial frame,
>                  47 rows, 75 columns on subsequent frames.
> 
> Gtk+3 and 2: 49 rows, 80 columns on initial frame.
>                 49 rows, 80 columns on subsequent frames.

What is the status of this?  Do you still get inconsistent results?

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

2014-01-11 15:01, martin rudalics skrev:
>> Some more info:
>>
>> Lucid/Motif: 46 rows, 74 columns on initial frame,
>>             50 rows, 80 columns on subsequent frames.
>>
>> No toolkit: 45 rows, 68 columns on initial frame,
>>                  47 rows, 75 columns on subsequent frames.
>>
>> Gtk+3 and 2: 49 rows, 80 columns on initial frame.
>>                 49 rows, 80 columns on subsequent frames.
>
> What is the status of this?  Do you still get inconsistent results?

Yes:

Toolkit         Initial frame    Subsequent frame
-------------------------------------------
Gtk+ 2/Gtk+ 3     50/80              50/80
Gnustep           50/80              49/80
Lucid             50/80              53/80  (Toolbar 3 lines).
Motif             50/80              53/80  (Toolbar 3 lines).
No toolkit        50/80              53/80  (Toolbar 3 lines, menu bar is 1)

Columns are correct in all cases so that is progress.
Rows correct only for Gtk.
Values for Lucid/Motif is with toolbar, i.e. there are 47 lines excluding 
toolbar for Lucid/Motif on initial fra,e. Ditto for no toolkit + 1 menu bar line.


Looks like toolbar is counted on initial frame, but not on subsequent frames 
for Lucid/Motif/No toolkit.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > Toolkit         Initial frame    Subsequent frame
 > -------------------------------------------
 > Gtk+ 2/Gtk+ 3     50/80              50/80
 > Gnustep           50/80              49/80
 > Lucid             50/80              53/80  (Toolbar 3 lines).
 > Motif             50/80              53/80  (Toolbar 3 lines).
 > No toolkit        50/80              53/80  (Toolbar 3 lines, menu bar
 > is 1)
 >
 > Columns are correct in all cases so that is progress.
 > Rows correct only for Gtk.
 > Values for Lucid/Motif is with toolbar, i.e. there are 47 lines
 > excluding toolbar for Lucid/Motif on initial fra,e. Ditto for no toolkit
 > + 1 menu bar line.
 >
 >
 > Looks like toolbar is counted on initial frame, but not on subsequent
 > frames for Lucid/Motif/No toolkit.

I'm still too silly to understand.  Should the initial frame have 53
rows (maybe 54 for the non-toolkit version) or should the subsequent
frames all have 50 rows?

I frequently asked on this list what `frame-height' and especially the
"number of lines available for display" stands for, but never got an
answer I could understand.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

12 jan 2014 kl. 10:54 skrev martin rudalics <rudalics@gmx.at>:

> > Toolkit         Initial frame    Subsequent frame
> > -------------------------------------------
> > Gtk+ 2/Gtk+ 3     50/80              50/80
> > Gnustep           50/80              49/80

BTW NS on OSX is 50/80 for both cases, so the GNUStep value 49/50 is probably GNUStep specific, I'll check that.

> > Lucid             50/80              53/80  (Toolbar 3 lines).
> > Motif             50/80              53/80  (Toolbar 3 lines).
> > No toolkit        50/80              53/80  (Toolbar 3 lines, menu bar
> > is 1)
> >
> > Columns are correct in all cases so that is progress.
> > Rows correct only for Gtk.
> > Values for Lucid/Motif is with toolbar, i.e. there are 47 lines
> > excluding toolbar for Lucid/Motif on initial fra,e. Ditto for no toolkit
> > + 1 menu bar line.
> >
> >
> > Looks like toolbar is counted on initial frame, but not on subsequent
> > frames for Lucid/Motif/No toolkit.
> 
> I'm still too silly to understand.  Should the initial frame have 53
> rows (maybe 54 for the non-toolkit version) or should the subsequent
> frames all have 50 rows?
> 
> I frequently asked on this list what `frame-height' and especially the
> "number of lines available for display" stands for, but never got an
> answer I could understand.


This has been inconsistent historically, i.e. Lucid/Motif/No toolkit counts differently than Gtk/NS.
I think the Gtk count makes more sense.  If a user requests 50 lines he probably means 50 editable lines, not 47.  So I think we should not count tool bar or menu bar.
The documentation says
"The height of the frame contents, in characters."
I don't think menu and tool bar is content.

This may break some lisp code that counts lines and does it differently for the two cases.  I don't know if there are any such code though.

BTW what values does the frame parameter height have now that pixelwise resize may show partial lines?  A floating point value?

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > This has been inconsistent historically, i.e. Lucid/Motif/No toolkit counts differently than Gtk/NS.
 > I think the Gtk count makes more sense.  If a user requests 50 lines he probably means 50 editable lines, not 47.  So I think we should not count tool bar or menu bar.

I agree.  Obviously, the fact that initial and subsequent frames have
different heights is a bug per se but I wanted a directive in either
direction.

 > The documentation says
 > "The height of the frame contents, in characters."
 > I don't think menu and tool bar is content.
 >
 > This may break some lisp code that counts lines and does it differently for the two cases.  I don't know if there are any such code though.
 >
 > BTW what values does the frame parameter height have now that pixelwise resize may show partial lines?  A floating point value?

No.  It's calculated thusly

   height = (f->new_height
	    ? (f->new_pixelwise
	       ? (f->new_height / FRAME_LINE_HEIGHT (f))
	       : f->new_height)
	    : FRAME_LINES (f));
   store_in_alist (&alist, Qheight, make_number (height));

so it's rounded down.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Stefan Monnier]

> If a user requests 50 lines he probably means 50 editable
> lines, not 47.

Right.  Of course, sometimes it's hard to do it right: if you first
create the 50-lines frame and then enable tool-bar-mode, you may have
a problem:
- OT1H enabling tool-bar-mode shouldn't resize the frame.
- OTOH if we don't resize the frame the resulting frame doesn't have 50
  lines any more.


        Stefan

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

12 jan 2014 kl. 21:25 skrev Stefan Monnier <monnier@iro.umontreal.ca>:

>> If a user requests 50 lines he probably means 50 editable
>> lines, not 47.
> 
> Right.  Of course, sometimes it's hard to do it right: if you first
> create the 50-lines frame and then enable tool-bar-mode, you may have
> a problem:
> - OT1H enabling tool-bar-mode shouldn't resize the frame.
> - OTOH if we don't resize the frame the resulting frame doesn't have 50
>  lines any more.

Quite right.  I once proposed using an invisible frame as the first frame.  Then we would know if tool bar mode is on when creating the second frame.  But there was problems with debug-init and other things, so it came to nothing.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

12 jan 2014 kl. 12:13 skrev Jan Djärv <jan.h.d@swipnet.se>:

> Hello.
> 
> 12 jan 2014 kl. 10:54 skrev martin rudalics <rudalics@gmx.at>:
> 
>>> Toolkit         Initial frame    Subsequent frame
>>> -------------------------------------------
>>> Gtk+ 2/Gtk+ 3     50/80              50/80
>>> Gnustep           50/80              49/80
> 
> BTW NS on OSX is 50/80 for both cases, so the GNUStep value 49/50 is probably GNUStep specific, I'll check that.

GNUStep has some bugs w.r.t. the tool bar, so the values for GNUStep is as good as it gets.

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 > GNUStep has some bugs w.r.t. the tool bar, so the values for GNUStep is as good as it gets.

As for the rest the main problem is this part in x_figure_window_size:

   /* This used to be done _before_ calling x_figure_window_size, but
      since the height is reset here, this was really a no-op.  I
      assume that moving it here does what Gerd intended (although he
      no longer can remember what that was...  ++KFS, 2003-03-25.  */

   /* Add the tool-bar height to the initial frame height so that the
      user gets a text display area of the size he specified with -g or
      via .Xdefaults.  Later changes of the tool-bar height don't
      change the frame size.  This is done so that users can create
      tall Emacs frames without having to guess how tall the tool-bar
      will get.  */
   if (toolbar_p && FRAME_TOOL_BAR_HEIGHT (f))
     {
       int margin, relief, bar_height;

       relief = (tool_bar_button_relief >= 0
		? tool_bar_button_relief
		: DEFAULT_TOOL_BAR_BUTTON_RELIEF);

       if (RANGED_INTEGERP (1, Vtool_bar_button_margin, INT_MAX))
	margin = XFASTINT (Vtool_bar_button_margin);
       else if (CONSP (Vtool_bar_button_margin)
	       && RANGED_INTEGERP (1, XCDR (Vtool_bar_button_margin), INT_MAX))
	margin = XFASTINT (XCDR (Vtool_bar_button_margin));
       else
	margin = 0;

       /* PXW: We should be able to not round here.  */
       bar_height = DEFAULT_TOOL_BAR_IMAGE_HEIGHT + 2 * margin + 2 * relief;
       FRAME_LINES (f) += (bar_height + FRAME_LINE_HEIGHT (f) - 1) / FRAME_LINE_HEIGHT (f);
     }

If I define it out, the sizes of the first and subsequent frames are
equal on Lucid.  IIUC x_figure_window_size is not called when changing
fonts or default heights.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

Michael Welsh Duggan schrieb:
 > Stefan Monnier <monnier@iro.umontreal.ca> writes:
 >
 >>> I know I am opening up a can of worms here, but I am going to argue that
 >>> `C-x SPC' be changed back to `gud-break'.
 >> Oddly enough there hasn't been the expected deluge of opinions.
 >
 > It surprises me, too.  My only guess is that people just don't use gdb
 > in emacs much, or they tend to use the mouse to set breakpoints.  (I
 > highly doubt people are using `C-x C-a C-b'.)  I guess I'll just have to
 > suggest rebinding it in gud-gdb-mode-hook to people who like it better
 > the way it was before.
 >
 > I know that the documentation updates are still a work in progress, but
 > please make sure current references to `C-x SPC' in the manual are
 > changed to `C-x C-a C-b' respectively.
 >

 >> I frequently asked on this list what `frame-height' and especially the
 >> "number of lines available for display" stands for, but never got an
 >> answer I could understand.
 >
 >
 > This has been inconsistent historically, i.e. Lucid/Motif/No toolkit counts differently than Gtk/NS.
 > I think the Gtk count makes more sense.  If a user requests 50 lines he probably means 50 editable lines, not 47.  So I think we should not count tool bar or menu bar.
 > The documentation says
 > "The height of the frame contents, in characters."
 > I don't think menu and tool bar is content.

I'm not sure what to do.  There's no problem for most elements of
`default-frame-alist' or when setting the default font.  The only real
offender is that of your init file - namely setting the default height.

A trivial scenario for Emacs 24.3 on Windows (I didn't try with that
version on Lucid/Motif but I suppose it's similar) is with emacs -Q:

(setq default-frame-alist '((height . 50)))

C-x 5 2

(set-frame-parameter nil 'height 50)

This changes the height of the new frame although it apparently is
already 50 lines high.  Such behavior constitutes a bug IMHO.  This
could be fixed but is certainly not trivial enough for inclusion in
24.4.

There are a few more arguments to count differently on Lucid/Motif/No
toolkit/Windows:

(1) When the window manager asks us to resize a frame, we do not
     subtract the toolbar height.  That is, the height of the toolbar is
     included in the frame's text height afterwards, defeating our
     illusion that it's counted separately.  This means an even less
     trivial fix than the one mentioned above.

(2) The real height of the toolbar is with tool_bar_height which might
     not fit the one we assume (in x_figure_window_size) anyway.  One
     more non-trivial fix since tool_bar_height is not available
     initially but only after the display engine handled it.  But the
     display engine wants the initial height of the frame so we have a
     chicken-and-egg problem here.

(3) Lucid/Motif/No toolkit/Windows can wrap the toolbar (something Gtk
     doesn't).  The display engine does this by stealing the necessary
     height from the editing area - that is, the root window - and
     autonomously updating the `tool-bar-lines' frame parameter.  This
     complicates subsequent frame resizing since we don't know a priori
     whether the toolbar will wrap again.

So while I agree with you that menu and tool bar should not be
considered content, I see no easy way to work around this assumption on
the systems in question.  Suggestions welcome.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

Sorry - my last mail contained unrelated text at the beginning.
Stripped now for better reading:

 >> I frequently asked on this list what `frame-height' and especially the
 >> "number of lines available for display" stands for, but never got an
 >> answer I could understand.
 >
 >
 > This has been inconsistent historically, i.e. Lucid/Motif/No toolkit counts differently than Gtk/NS.
 > I think the Gtk count makes more sense.  If a user requests 50 lines he probably means 50 editable lines, not 47.  So I think we should not count tool bar or menu bar.
 > The documentation says
 > "The height of the frame contents, in characters."
 > I don't think menu and tool bar is content.

I'm not sure what to do.  There's no problem for most elements of
`default-frame-alist' or when setting the default font.  The only real
offender is that of your init file - namely setting the default height.

A trivial scenario for Emacs 24.3 on Windows (I didn't try with that
version on Lucid/Motif but I suppose it's similar) is with emacs -Q:

(setq default-frame-alist '((height . 50)))

C-x 5 2

(set-frame-parameter nil 'height 50)

This changes the height of the new frame although it apparently is
already 50 lines high.  Such behavior constitutes a bug IMHO.  This
could be fixed but is certainly not trivial enough for inclusion in
24.4.

There are a few more arguments to count differently on Lucid/Motif/No
toolkit/Windows:

(1) When the window manager asks us to resize a frame, we do not
     subtract the toolbar height.  That is, the height of the toolbar is
     included in the frame's text height afterwards, defeating our
     illusion that it's counted separately.  This means an even less
     trivial fix than the one mentioned above.

(2) The real height of the toolbar is with tool_bar_height which might
     not fit the one we assume (in x_figure_window_size) anyway.  One
     more non-trivial fix since tool_bar_height is not available
     initially but only after the display engine handled it.  But the
     display engine wants the initial height of the frame so we have a
     chicken-and-egg problem here.

(3) Lucid/Motif/No toolkit/Windows can wrap the toolbar (something Gtk
     doesn't).  The display engine does this by stealing the necessary
     height from the editing area - that is, the root window - and
     autonomously updating the `tool-bar-lines' frame parameter.  This
     complicates subsequent frame resizing since we don't know a priori
     whether the toolbar will wrap again.

So while I agree with you that menu and tool bar should not be
considered content, I see no easy way to work around this assumption on
the systems in question.  Suggestions welcome.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Jan Djärv]

Hello.

16 jan 2014 kl. 11:03 skrev martin rudalics <rudalics@gmx.at>:

> 
> (1) When the window manager asks us to resize a frame, we do not
>    subtract the toolbar height.  That is, the height of the toolbar is
>    included in the frame's text height afterwards, defeating our
>    illusion that it's counted separately.  This means an even less
>    trivial fix than the one mentioned above.
> 
> (2) The real height of the toolbar is with tool_bar_height which might
>    not fit the one we assume (in x_figure_window_size) anyway.  One
>    more non-trivial fix since tool_bar_height is not available
>    initially but only after the display engine handled it.  But the
>    display engine wants the initial height of the frame so we have a
>    chicken-and-egg problem here.
> 
> (3) Lucid/Motif/No toolkit/Windows can wrap the toolbar (something Gtk
>    doesn't).  The display engine does this by stealing the necessary
>    height from the editing area - that is, the root window - and
>    autonomously updating the `tool-bar-lines' frame parameter.  This
>    complicates subsequent frame resizing since we don't know a priori
>    whether the toolbar will wrap again.
> 
> So while I agree with you that menu and tool bar should not be
> considered content, I see no easy way to work around this assumption on
> the systems in question.  Suggestions welcome.

Disable wrapping of the toolbar?

	Jan D.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

 >> (2) The real height of the toolbar is with tool_bar_height which might
 >>    not fit the one we assume (in x_figure_window_size) anyway.  One
 >>    more non-trivial fix since tool_bar_height is not available
 >>    initially but only after the display engine handled it.  But the
 >>    display engine wants the initial height of the frame so we have a
 >>    chicken-and-egg problem here.

BTW it's very difficult to know how much space drawing a toolbar really
takes because we should know its border size too.

 > Disable wrapping of the toolbar?

But this is customizable via `auto-resize-tool-bars'.

martin

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[martin rudalics]

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

I would proceed as follows: For the release, apply the attached patch.
This should give initial and subsequent frames the same size and thus
resolve most issues you raised in this thread and also fix bug 14795.
For Lucid/Motif/Windows the toolbar would remain part of the text area,
hence the number of lines available for editing would be less than
specified.  I see no way to provide a safe fix for this without
sufficient testing.  We could increase the number of DEFAULT_ROWS for
these builds though, so the default frames would retain their prior
sizes.

After the release I would try to change the behavior such that when a
frame is created, the toolbar is not counted in the text height on these
builds.  Hence `default-frame-alist' would be interpreted as for the
GTK build.  Subsequent changes (adding/removing/wrapping) of the toolbar
would not change the size of the frame but that of its text area, so in
this case the behavior would differ from GTK but still be different from
the prior behavior.

I soon intend to implement horizontal scrollbars anyway so the necessary
changes would mainly amount to something like defining:

#define FRAME_TEXT_LINES_TO_PIXEL_HEIGHT(f, lines) \
   ((lines) * FRAME_LINE_HEIGHT (f)		   \
    + FRAME_TOOL_BAR_HEIGHT (f)			   \
    + FRAME_SCROLL_BAR_AREA_HEIGHT (f)		   \
    + 2 * FRAME_INTERNAL_BORDER_WIDTH (f))

WDYT?

martin

[-- Attachment #2: frame-text-size.diff --]
[-- Type: text/plain, Size: 4888 bytes --]

=== modified file 'src/frame.c'
--- src/frame.c	2014-01-11 10:01:01 +0000
+++ src/frame.c	2014-01-28 18:21:35 +0000
@@ -4244,7 +4244,10 @@
   /* This used to be done _before_ calling x_figure_window_size, but
      since the height is reset here, this was really a no-op.  I
      assume that moving it here does what Gerd intended (although he
-     no longer can remember what that was...  ++KFS, 2003-03-25.  */
+     no longer can remember what that was...  ++KFS, 2003-03-25.
+
+     Defined out since it makes subsequent frames larger than the
+     initial one.  See Bug#16013.  martin, 2014-01-15.  */

   /* Add the tool-bar height to the initial frame height so that the
      user gets a text display area of the size he specified with -g or
@@ -4252,6 +4255,7 @@
      change the frame size.  This is done so that users can create
      tall Emacs frames without having to guess how tall the tool-bar
      will get.  */
+#if 0
   if (toolbar_p && FRAME_TOOL_BAR_HEIGHT (f))
     {
       int margin, relief, bar_height;
@@ -4272,6 +4276,7 @@
       bar_height = DEFAULT_TOOL_BAR_IMAGE_HEIGHT + 2 * margin + 2 * relief;
       FRAME_LINES (f) += (bar_height + FRAME_LINE_HEIGHT (f) - 1) / FRAME_LINE_HEIGHT (f);
     }
+#endif

   compute_fringe_widths (f, 0);


=== modified file 'src/widget.c'
--- src/widget.c	2014-01-01 07:43:34 +0000
+++ src/widget.c	2014-01-28 18:21:35 +0000
@@ -687,12 +687,18 @@
       int width, height;

       pixel_to_text_size (ew, ew->core.width, ew->core.height, &width, &height);
-      change_frame_size (f, width, height, 0, 1, 0, 1);
-
-      update_wm_hints (ew);
-      update_various_frame_slots (ew);
-
-      cancel_mouse_face (f);
+      if (width != FRAME_TEXT_WIDTH (f)
+	  || height != FRAME_TEXT_HEIGHT (f)
+	  || ew->core.width != FRAME_PIXEL_WIDTH (f)
+	  || ew->core.height + x->menubar_height != FRAME_PIXEL_HEIGHT (f))
+	{
+	  change_frame_size (f, width, height, 0, 1, 0, 1);
+
+	  update_wm_hints (ew);
+	  update_various_frame_slots (ew);
+
+	  cancel_mouse_face (f);
+	}
 #if 0  /* See comment above.  */
     }
   else
@@ -820,12 +826,12 @@

 /* Special entry points */
 void
-EmacsFrameSetCharSize (Widget widget, int columns, int rows)
+EmacsFrameSetCharSize (Widget widget, int width, int height)
 {
   EmacsFrame ew = (EmacsFrame) widget;
   struct frame *f = ew->emacs_frame.frame;

-  x_set_window_size (f, 0, columns, rows, 0);
+  x_set_window_size (f, 0, width, height, 1);
 }

 \f

=== modified file 'src/xmenu.c'
--- src/xmenu.c	2014-01-29 08:23:59 +0000
+++ src/xmenu.c	2014-01-29 09:09:51 +0000
@@ -682,7 +682,7 @@
   xg_update_frame_menubar (f);
 #else
   struct x_output *x;
-  int columns, rows;
+  int width, height;

   eassert (FRAME_X_P (f));

@@ -694,8 +694,8 @@
   block_input ();
   /* Save the size of the frame because the pane widget doesn't accept
      to resize itself. So force it.  */
-  columns = FRAME_COLS (f);
-  rows = FRAME_LINES (f);
+  width = FRAME_TEXT_WIDTH (f);
+  height = FRAME_TEXT_HEIGHT (f);

   /* Do the voodoo which means "I'm changing lots of things, don't try
      to refigure sizes until I'm done."  */
@@ -717,7 +717,7 @@
   lw_refigure_widget (x->column_widget, True);

   /* Force the pane widget to resize itself with the right values.  */
-  EmacsFrameSetCharSize (x->edit_widget, columns, rows);
+  EmacsFrameSetCharSize (x->edit_widget, width, height);
   unblock_input ();
 #endif
 }

=== modified file 'src/xterm.c'
--- src/xterm.c	2014-01-26 12:17:55 +0000
+++ src/xterm.c	2014-01-28 18:21:35 +0000
@@ -8573,21 +8573,26 @@

   compute_fringe_widths (f, 0);

-  pixelwidth = ((pixelwise
-		 ? FRAME_TEXT_TO_PIXEL_WIDTH (f, width)
-		 : FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width))
-		+ FRAME_TOOLBAR_WIDTH (f));
-  pixelheight = ((pixelwise
-		  ? FRAME_TEXT_TO_PIXEL_HEIGHT (f, height)
-		  : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height))
-		 + FRAME_MENUBAR_HEIGHT (f)
-		 + FRAME_TOOLBAR_HEIGHT (f));
+  pixelwidth = (pixelwise
+		? FRAME_TEXT_TO_PIXEL_WIDTH (f, width)
+		: FRAME_TEXT_COLS_TO_PIXEL_WIDTH (f, width));
+  pixelheight = (pixelwise
+		 ? FRAME_TEXT_TO_PIXEL_HEIGHT (f, height)
+		 : FRAME_TEXT_LINES_TO_PIXEL_HEIGHT (f, height));
+
+#if defined (USE_LUCID) || defined (USE_MOTIF)
+  {
+    struct x_output *x = f->output_data.x;
+
+    pixelheight += x->menubar_height;
+  }
+#endif
+
   if (change_gravity) f->win_gravity = NorthWestGravity;
   x_wm_set_size_hint (f, (long) 0, 0);
   XResizeWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
 		 pixelwidth, pixelheight);

-
   /* We've set {FRAME,PIXEL}_{WIDTH,HEIGHT} to the values we hope to
      receive in the ConfigureNotify event; if we get what we asked
      for, then the event won't cause the screen to become garbaged, so

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> I would proceed as follows: For the release, apply the attached patch.

[...]

> After the release I would try to change the behavior such that when a
> frame is created, the toolbar is not counted in the text height on these
> builds.

This was six years ago (and this was the final email in the thread), and
I seem to recall a large number of tweaks done to this area, so I'm not
sure whether this bug report should be closed?  It seems like the
original reported bug was fixed by Martin at the time, but then there
were some followup considerations...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 09 Sep 2020 15:07:19 +0200
> Cc: "16013@debbugs.gnu.org" <16013@debbugs.gnu.org>,
>  Jan Djärv <jan.h.d@swipnet.se>
> 
> martin rudalics <rudalics@gmx.at> writes:
> 
> > I would proceed as follows: For the release, apply the attached patch.
> 
> [...]
> 
> > After the release I would try to change the behavior such that when a
> > frame is created, the toolbar is not counted in the text height on these
> > builds.
> 
> This was six years ago (and this was the final email in the thread), and
> I seem to recall a large number of tweaks done to this area, so I'm not
> sure whether this bug report should be closed?  It seems like the
> original reported bug was fixed by Martin at the time, but then there
> were some followup considerations...

Martin should tell, but from where I stand, we already have a
satisfactory solution to these issues.  It took us (took Martin,
really) a lot of blood, sweat, and tears to get there, but the results
seems to be OK, as I've not seen any complaints about that for quite
some time.

bug#16013: 24.3.50; Rows in height is interpreted as pixels.

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> Martin should tell, but from where I stand, we already have a
> satisfactory solution to these issues.  It took us (took Martin,
> really) a lot of blood, sweat, and tears to get there, but the results
> seems to be OK, as I've not seen any complaints about that for quite
> some time.

OK, I'm closing this bug report, then.  If there's more to be done here,
please reopen.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no