bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Kelly Dean]

With 24.4, emacs -Q, then M-x split-window-horizontally, then open any file with a lot of long logical lines (each a few visual lines long), so that as you page down through the file, every screenful of text has some wrapped lines on it. Adjust the vertical size of your Emacs frame so that the vertical size of your Emacs windows is not an integer multiple of the visual line height, so that the bottom visual line of text is partially covered by the top of the mode line.
Scroll forward a couple pages using pgdn (scroll-up-command), then notice exactly which visual line is at the top, and which one is at the bottom. Press pgdn, then pgup, and notice that the text doesn't return to the previous position; instead, it's scrolled backward by one visual line. This bug wasn't in 24.3.

The bug is in window.c of the 24.4 release, line 5042. The fix is to change ⌜window_box_height (w)⌝ on that line to ⌜window_box_height (w) / dy * dy⌝ in order to align the result to an integer multiple of the visual line height, since as of 24.4, Emacs no longer ensures that window_box_height is such a multiple. Patch excluded, per request.

There might be a related bug (with the same fix) on line 4959, but I haven't tested that.

BTW on line 471 (copied from line 466) of window.h, ‟width” should be ‟height”.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Eli Zaretskii]

> From: Kelly Dean <kelly@prtime.org>
> Date: Sat, 15 Nov 2014 06:57:45 +0000
> 
> With 24.4, emacs -Q, then M-x split-window-horizontally, then open any file with a lot of long logical lines (each a few visual lines long), so that as you page down through the file, every screenful of text has some wrapped lines on it. Adjust the vertical size of your Emacs frame so that the vertical size of your Emacs windows is not an integer multiple of the visual line height, so that the bottom visual line of text is partially covered by the top of the mode line.
> Scroll forward a couple pages using pgdn (scroll-up-command), then notice exactly which visual line is at the top, and which one is at the bottom. Press pgdn, then pgup, and notice that the text doesn't return to the previous position; instead, it's scrolled backward by one visual line. This bug wasn't in 24.3.
> 
> The bug is in window.c of the 24.4 release, line 5042. The fix is to change ⌜window_box_height (w)⌝ on that line to ⌜window_box_height (w) / dy * dy⌝ in order to align the result to an integer multiple of the visual line height, since as of 24.4, Emacs no longer ensures that window_box_height is such a multiple. Patch excluded, per request.
> 
> There might be a related bug (with the same fix) on line 4959, but I haven't tested that.


Martin, I propose the following patch:

diff --git a/src/window.c b/src/window.c
index b002423..7462fdc 100644
--- a/src/window.c
+++ b/src/window.c
@@ -4956,8 +4956,8 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
 	  int px;
 	  int dy = frame_line_height;
 	  if (whole)
-	    dy = max ((window_box_height (w)
-		       - next_screen_context_lines * dy),
+	    dy = max ((window_box_height (w) / dy
+		       - next_screen_context_lines) * dy,
 		      dy);
 	  dy *= n;
 
@@ -5039,8 +5039,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
     {
       ptrdiff_t start_pos = IT_CHARPOS (it);
       int dy = frame_line_height;
-      dy = max ((window_box_height (w)
-		 - next_screen_context_lines * dy),
+      dy = max ((window_box_height (w) / dy - next_screen_context_lines) * dy,
 		dy) * n;
 
       /* Note that move_it_vertically always moves the iterator to the


> BTW on line 471 (copied from line 466) of window.h, ‟width” should be ‟height”.

Indeed, and "column" should be "line".  Thanks, fixed.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

First of all many thanks to Kelly Dean for finding the problem and
proposing a solution.

 > Martin, I propose the following patch:
 >
 > diff --git a/src/window.c b/src/window.c
 > index b002423..7462fdc 100644
 > --- a/src/window.c
 > +++ b/src/window.c
 > @@ -4956,8 +4956,8 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
 >   	  int px;
 >   	  int dy = frame_line_height;
 >   	  if (whole)
 > -	    dy = max ((window_box_height (w)
 > -		       - next_screen_context_lines * dy),
 > +	    dy = max ((window_box_height (w) / dy
 > +		       - next_screen_context_lines) * dy,
 >   		      dy);
 >   	  dy *= n;
 >
 > @@ -5039,8 +5039,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
 >       {
 >         ptrdiff_t start_pos = IT_CHARPOS (it);
 >         int dy = frame_line_height;
 > -      dy = max ((window_box_height (w)
 > -		 - next_screen_context_lines * dy),
 > +      dy = max ((window_box_height (w) / dy - next_screen_context_lines) * dy,
 >   		dy) * n;
 >
 >         /* Note that move_it_vertically always moves the iterator to the

If it works for you, please apply it.  I'm too silly to understand it,
though.  IIUC we have a rounding issue which makes us go by one line too
far or too few when scrolling up and your patch compensates that
rounding issue by putting it back into that window_box_height (w) / dy
calculation.  Could you add comments which tell more or less how you
corrected the issue?

Thanks, martin

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Eli Zaretskii]

> Date: Sat, 15 Nov 2014 12:13:17 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: 19060@debbugs.gnu.org
> 
>  > diff --git a/src/window.c b/src/window.c
>  > index b002423..7462fdc 100644
>  > --- a/src/window.c
>  > +++ b/src/window.c
>  > @@ -4956,8 +4956,8 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
>  >   	  int px;
>  >   	  int dy = frame_line_height;
>  >   	  if (whole)
>  > -	    dy = max ((window_box_height (w)
>  > -		       - next_screen_context_lines * dy),
>  > +	    dy = max ((window_box_height (w) / dy
>  > +		       - next_screen_context_lines) * dy,
>  >   		      dy);
>  >   	  dy *= n;
>  >
>  > @@ -5039,8 +5039,7 @@ window_scroll_pixel_based (Lisp_Object window, int n, bool whole, int noerror)
>  >       {
>  >         ptrdiff_t start_pos = IT_CHARPOS (it);
>  >         int dy = frame_line_height;
>  > -      dy = max ((window_box_height (w)
>  > -		 - next_screen_context_lines * dy),
>  > +      dy = max ((window_box_height (w) / dy - next_screen_context_lines) * dy,
>  >   		dy) * n;
>  >
>  >         /* Note that move_it_vertically always moves the iterator to the
> 
> If it works for you, please apply it.

It evidently works for Kelly.

> I'm too silly to understand it, though.  IIUC we have a rounding
> issue which makes us go by one line too far or too few when
> scrolling up and your patch compensates that rounding issue by
> putting it back into that window_box_height (w) / dy calculation.

Yes.  Why is it hard to understand?

> Could you add comments which tell more or less how you corrected the
> issue?

Will do, once we agree that this is the right fix.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

 > Yes.  Why is it hard to understand?

When I try to correct an off-by-one error I usually never understand
what I'm doing.  I "fix" it by rounding in one direction first and if
this doesn't seem to work as intended I try the other direction.

 >> Could you add comments which tell more or less how you corrected the
 >> issue?
 >
 > Will do, once we agree that this is the right fix.

First of all I'd need to understand the bug itself.  Do we assume that
scrolling down is "correct" in some sense or does the bug manifest
itself there already?  Does scrolling up always move to the line before
the "previously established as correct" one or to the line after it?
(I'm asking because I cannot reproduce the problem with Kelly Dean's
pgup/pgdn recipe.)

martin

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Eli Zaretskii]

> Date: Sat, 15 Nov 2014 13:12:47 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: kelly@prtime.org, 19060@debbugs.gnu.org
> 
> First of all I'd need to understand the bug itself.  Do we assume that
> scrolling down is "correct" in some sense or does the bug manifest
> itself there already?  Does scrolling up always move to the line before
> the "previously established as correct" one or to the line after it?
> (I'm asking because I cannot reproduce the problem with Kelly Dean's
> pgup/pgdn recipe.)

Kelly, please help Martin reproduce the problem.  Perhaps you need to
provide more details, like maybe the file you used and the geometry of
the frame needed to see the issue.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

 > Kelly, please help Martin reproduce the problem.  Perhaps you need to
 > provide more details, like maybe the file you used and the geometry of
 > the frame needed to see the issue.

Just a few informations.  I tried on Windows XP with the current trunk
(the release doesn't allow to resize frames pixelwise easily).  I did
emacs -Q, displayed xdisp.c, did C-x 3 and resized the frame with the
mouse to make sure that the last lines of the windows showing xdisp.c do
get displayed only partially.  Now pgup/pgdn in the right window always
gets me to the same line it had before when scrolling in the other
direction.  Am I missing something?

martin

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Eli Zaretskii]

> Date: Sat, 15 Nov 2014 14:29:12 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: kelly@prtime.org, 19060@debbugs.gnu.org
> 
> Just a few informations.  I tried on Windows XP with the current trunk
> (the release doesn't allow to resize frames pixelwise easily).  I did
> emacs -Q, displayed xdisp.c, did C-x 3 and resized the frame with the
> mouse to make sure that the last lines of the windows showing xdisp.c do
> get displayed only partially.  Now pgup/pgdn in the right window always
> gets me to the same line it had before when scrolling in the other
> direction.  Am I missing something?

Since the issue is rounding vs truncating, perhaps try playing with
the portion of the last displayed line, trying both less and greater
than half the canonical line height.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Angelo Graziosi]

Martin Rudalics wrote:
> IIUC we have a rounding issue which makes us go by one line too
> far or too few when scrolling up and your patch compensates that
> rounding issue by putting it back into that window_box_height (w) / dy
> calculation.

Apparently, this seem related, in some way, to what I flagged here:

  http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00621.html

..or not?

Ciao,
  Angelo.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

 > Since the issue is rounding vs truncating, perhaps try playing with
 > the portion of the last displayed line, trying both less and greater
 > than half the canonical line height.

Got it.  The last line must show more than a half of the frame's
character height.  And the patch fixes it AFAICT.  So please apply it
with as much comment as you can provide.

Thanks, martin

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

 > Apparently, this seem related, in some way, to what I flagged here:
 >
 >   http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00621.html
 >
 > ..or not?

Maybe.  But I have no idea what your report describes.  IIUC you are
using `desktop-save' to save the window configuration across sessions
and when you restart Emacs its frame's height is one line less.  Is that
correct?  Then please tell us how much the height decreases in pixels
(you can use the function `window--dump-frame' for that) and what the
height stored by `desktop-save' is.  I suppose this is somewhere in a
cons whose car is something like `height' or `frame-height' or the like.

martin

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Eli Zaretskii]

> Date: Sat, 15 Nov 2014 15:32:41 +0100
> From: martin rudalics <rudalics@gmx.at>
> CC: kelly@prtime.org, 19060@debbugs.gnu.org
> 
>  > Since the issue is rounding vs truncating, perhaps try playing with
>  > the portion of the last displayed line, trying both less and greater
>  > than half the canonical line height.
> 
> Got it.  The last line must show more than a half of the frame's
> character height.  And the patch fixes it AFAICT.  So please apply it
> with as much comment as you can provide.

Done on the emacs-24 branch.

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[Angelo Graziosi]

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



Il 15/11/2014 15:33, martin rudalics ha scritto:
>  > Apparently, this seem related, in some way, to what I flagged here:
>  >
>  >   http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00621.html
>  >
>  > ..or not?
>
> Maybe.  But I have no idea what your report describes.  IIUC you are
> using `desktop-save' to save the window configuration across sessions
> and when you restart Emacs its frame's height is one line less.  Is that
> correct?  Then please tell us how much the height decreases in pixels

Yes, even if each time not always the same size: sometimes more, 
sometimes less. I have quantified this in about one line (or, if you 
prefer, about the height of the minibuffer). I would say 10-20 pixel.

> (you can use the function `window--dump-frame' for that) and what the
> height stored by `desktop-save' is.  I suppose this is somewhere in a
> cons whose car is something like `height' or `frame-height' or the like.

I am afraid, I wasn't able to use that function.. How to evaluate it? I 
tried a few command but they didn't work.

I have attached a tar-ball with a few examples of desktop file. They 
have been obtained in this way.

First I removed the current desktop and history files. So I started 
Emacs and moved it so that it was centered in the Windows desktop (I 
tried this with the Cygwin build..). Then I closed Emacs and it asked to 
save the desktop. Yes - and this generated the "desktop-00" file. Then I 
restarted Emacs and closed it obtaining the "desktop-01" file. I 
repeated this 2 times obtaining "desktop-03". After repeating this a few 
times I obtained the "desktop" file: now Emacs frame is conspicuously short.


Ciao,
  Angelo.

[-- Attachment #2: desktop_files.tar.gz --]
[-- Type: application/x-gzip, Size: 1324 bytes --]

bug#19060: [FIX INCLUDED] Off-by-one-line scrolling bug in window_scroll_pixel_based

[martin rudalics]

 >> (you can use the function `window--dump-frame' for that) and what the
 >> height stored by `desktop-save' is.  I suppose this is somewhere in a
 >> cons whose car is something like `height' or `frame-height' or the like.
 >
 > I am afraid, I wasn't able to use that function.. How to evaluate it? I tried a few command but they didn't work.

M-: (window--dump-frame)

The results appear in a buffer called *window-frame-dump*.  Do this
please once before exiting one session and once after entering the
subsequent session and post the respective buffer contents here.  And
please do that in the appropriate thread, namely for bug#19048.

 > First I removed the current desktop and history files. So I started
 > Emacs and moved it so that it was centered in the Windows desktop (I
 > tried this with the Cygwin build..). Then I closed Emacs and it asked
 > to save the desktop. Yes - and this generated the "desktop-00"
 > file. Then I restarted Emacs and closed it obtaining the "desktop-01"
 > file. I repeated this 2 times obtaining "desktop-03". After repeating
 > this a few times I obtained the "desktop" file: now Emacs frame is
 > conspicuously short.

I suppose the frame height is stored in the (height . xx) conses.  In
desktop-00 you have (height . 34), in desktop-01 (height . 33) and in
desktop-03 (height . 31).  So your frame height seems to decrease by one
line every time you restore it.  Using `window--dump-frame' we should be
able to see what the frame height was before exiting one session and
after restoring it in another one and hopefully why the frame decreases
by that line.

Thanks, martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

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


Il 16/11/2014 12:37, martin rudalics ha scritto:
>  >> (you can use the function `window--dump-frame' for that) and what the
>  >> height stored by `desktop-save' is.  I suppose this is somewhere in a
>  >> cons whose car is something like `height' or `frame-height' or the
> like.
>  >
>  > I am afraid, I wasn't able to use that function.. How to evaluate it?
> I tried a few command but they didn't work.
>
> M-: (window--dump-frame)
>

OK. I removed the desktop and history files, than I started Emacs with 
this simple init file:

$ cat init.el
;; Instead to save in ~/.emacs.desktop
(setq desktop-base-file-name "~/.emacs.d/desktop")

;; Save desktop
(desktop-save-mode t)


I did M-: (window--dump-frame), saved the window-frame-dump in 
window-frame-dump-00.txt file. Then I closed Emacs saving the desktop at 
the request.

Restarted Emacs repeating the M-: (window--dump-frame) command and saved 
the window-frame-dump in window-frame-dump-01.txt file.

Repeating the same saving in window-frame-dump-02.txt file. These files 
are in the attached tar-ball.


Ciao,
  Angelo.


[-- Attachment #2: window-frame-dump.tar.gz --]
[-- Type: application/x-gzip, Size: 615 bytes --]

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 > OK. I removed the desktop and history files, than I started Emacs with this simple init file:
 >
 > $ cat init.el
 > ;; Instead to save in ~/.emacs.desktop
 > (setq desktop-base-file-name "~/.emacs.d/desktop")
 >
 > ;; Save desktop
 > (desktop-save-mode t)
 >
 >
 > I did M-: (window--dump-frame), saved the window-frame-dump in window-frame-dump-00.txt file. Then I closed Emacs saving the desktop at the request.
 >
 > Restarted Emacs repeating the M-: (window--dump-frame) command and saved the window-frame-dump in window-frame-dump-01.txt file.
 >
 > Repeating the same saving in window-frame-dump-02.txt file. These files are in the attached tar-ball.

Fine.  Just that now I can't relate this to the saved desktop.  To avoid
an incorrect guess please do the following:

(1) Save the result of `window--dump-frame' in window-frame-dump-00.

(2) Exit Emacs.

(3) Save the contents of you desktop file in desktop-00.

(4) Restart Emacs.

(5) Save the result of `window--dump-frame' in window-frame-dump-01.

And please also try to do the same with `tool-bar-mode' turned off.  I
suspect that restoring frames has problems reestablishing the correct
height of the tool bar.

Thanks, martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

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

Il 16/11/2014 14:54, martin rudalics ha scritto:
>
> Fine.  Just that now I can't relate this to the saved desktop.  To avoid
> an incorrect guess please do the following:
>
> (1) Save the result of `window--dump-frame' in window-frame-dump-00.
>
> (2) Exit Emacs.
>
> (3) Save the contents of you desktop file in desktop-00.
>
> (4) Restart Emacs.
>
> (5) Save the result of `window--dump-frame' in window-frame-dump-01.
>
> And please also try to do the same with `tool-bar-mode' turned off.  I
> suspect that restoring frames has problems reestablishing the correct
> height of the tool bar.

Attached the result. As before, I used the same minimal init.el file and 
have removed the desktop file before to repeat with `tool-bar-mode' 
turned off (no-TB)


Ciao,
Angelo.

[-- Attachment #2: window-frame-dump2.tar.gz --]
[-- Type: application/x-gzip, Size: 1842 bytes --]

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 > Attached the result. As before, I used the same minimal init.el file and have removed the desktop file before to repeat with `tool-bar-mode' turned off (no-TB)

So IIUC the problem doesn't happen without the tool bar.  Right?

martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 16/11/2014 15:22, martin rudalics ha scritto:
>  > Attached the result. As before, I used the same minimal init.el file
> and have removed the desktop file before to repeat with `tool-bar-mode'
> turned off (no-TB)
>
> So IIUC the problem doesn't happen without the tool bar.  Right?

Confirmed!

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Hi Martin,

Il 16/11/2014 15:33, Angelo Graziosi ha scritto:
>
>
> Il 16/11/2014 15:22, martin rudalics ha scritto:
>>  > Attached the result. As before, I used the same minimal init.el file
>> and have removed the desktop file before to repeat with `tool-bar-mode'
>> turned off (no-TB)
>>
>> So IIUC the problem doesn't happen without the tool bar.  Right?
>
> Confirmed!

just for completeness...

Here: http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00582.html

I flagged a similar issue on OSX which has been fixed:

http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00582.html

I don't know if this could be useful also for you...

Ciao,
  Angelo.

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 >> So IIUC the problem doesn't happen without the tool bar.  Right?
 >
 > Confirmed!

Damn!  In a normal frame with tool bar, please evaluate the following
three forms

(tool-bar-height nil t)
(tool-bar-height)
(frame-parameter nil 'tool-bar-lines)

and tell me what they give.  I fail to understand why `tool-bar-lines'
is 2 in your saved desktop.

And while you're there: In a saved desktop find the (tool-bar-lines . 2)
entry, replace it with (tool-bar-lines . 3), fire another instance of
Emacs and tell me whether the frame height decreased.

martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 16/11/2014 16:14, martin rudalics ha scritto:
>  >> So IIUC the problem doesn't happen without the tool bar.  Right?
>  >
>  > Confirmed!
>
> Damn!  In a normal frame with tool bar, please evaluate the following
> three forms
>
> (tool-bar-height nil t)
> (tool-bar-height)
> (frame-parameter nil 'tool-bar-lines)

Always with a minimal init.el, I have

M-: (tool-bar-height nil t)
36 (#o44, #x24, ?$)

M-: (tool-bar-height)
3 (#o3, #x3, ?\C-c)


M-: (frame-parameter nil 'tool-bar-lines)
3 (#o3, #x3, ?\C-c)


>
> and tell me what they give.  I fail to understand why `tool-bar-lines'
> is 2 in your saved desktop.

?? No, Martin I have : (tool-bar-lines . 3)

(the same on desktop-00 file I sent previously, and (tool-bar-lines . 0) 
in desktop-no-TB-00).

I havd done many tries: Emacs frame decreases as described but 
(tool-bar-lines . 3) NOT (tool-bar-lines . 2). So I am not sure how to 
do the test you require... :(

> And while you're there: In a saved desktop find the (tool-bar-lines . 2)
> entry, replace it with (tool-bar-lines . 3), fire another instance of
> Emacs and tell me whether the frame height decreased.
>
> martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 16/11/2014 16:14, martin rudalics ha scritto:
>  >> So IIUC the problem doesn't happen without the tool bar.  Right?
>  >
>  > Confirmed!

Since this issue is recent, I wonder if it caused by this change:

src/ChangeLog:

2014-11-07  Martin Rudalics  <rudalics@gmx.at>

	* dispnew.c (change_frame_size_1): Fix call of
	adjust_frame_size.
	
[...]

	* window.c (Fset_window_configuration): Adjust adjust_frame_size call.
	* xfns.c (x_set_menu_bar_lines, x_set_internal_border_width)
	(Fx_create_frame): Adjust adjust_frame_size calls.
	(x_set_tool_bar_lines, x_change_tool_bar_height): Make sure that
	frame can get resized when tool-bar-lines parameter changes from
	or to zero.
	(Fx_frame_geometry): New function.
	* xmenu.c (update_frame_menubar): On Lucid call
	adjust_frame_size with one pixel less height to avoid that
	repeatedly adding/removing the menu bar grows the frame.
	(free_frame_menubar): On Motif arrange to optionally preserve
	the old frame height when removing the menu bar.
	* xterm.c (x_new_font): Adjust adjust_frame_size call.


  Angelo

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 17/11/2014 11:56, Angelo Graziosi ha scritto:
>
>
> Il 16/11/2014 16:14, martin rudalics ha scritto:
>>  >> So IIUC the problem doesn't happen without the tool bar.  Right?
>>  >
>>  > Confirmed!
>
> Since this issue is recent, I wonder if it caused by this change:

Bingo!

I have build this commit

author	Tassilo Horn 2014-11-07 10:48:25 (GMT)

download	emacs-a067ef9a5ddc9812e35734e8c027684e01d684ef.tar.gz

doc/misc/{gnus.texi,gnus-faq.texi}: Add link to EWW manual


...and the frame is restored with the same size..


And this commit (just one minute lather the previous):

author	Martin Rudalics 2014-11-07 10:49:22 (GMT)
download	emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz

Improve inhibiting of implied frame resizes.


..and the frame decreases in height each time one starts Emacs...


Ciao,
Angelo.

>
> src/ChangeLog:
>
> 2014-11-07  Martin Rudalics  <rudalics@gmx.at>
>
>      * dispnew.c (change_frame_size_1): Fix call of
>      adjust_frame_size.
>
> [...]
>
>      * window.c (Fset_window_configuration): Adjust adjust_frame_size call.
>      * xfns.c (x_set_menu_bar_lines, x_set_internal_border_width)
>      (Fx_create_frame): Adjust adjust_frame_size calls.
>      (x_set_tool_bar_lines, x_change_tool_bar_height): Make sure that
>      frame can get resized when tool-bar-lines parameter changes from
>      or to zero.
>      (Fx_frame_geometry): New function.
>      * xmenu.c (update_frame_menubar): On Lucid call
>      adjust_frame_size with one pixel less height to avoid that
>      repeatedly adding/removing the menu bar grows the frame.
>      (free_frame_menubar): On Motif arrange to optionally preserve
>      the old frame height when removing the menu bar.
>      * xterm.c (x_new_font): Adjust adjust_frame_size call.
>
>
>   Angelo

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 > And this commit (just one minute lather the previous):
 >
 > author    Martin Rudalics 2014-11-07 10:49:22 (GMT)
 > download    emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz
 >
 > Improve inhibiting of implied frame resizes.
 >
 >
 > ..and the frame decreases in height each time one starts Emacs...

OK.  But you didn't answer my earlier questions:

In a normal frame with tool bar, please evaluate the following
three forms

(tool-bar-height nil t)
(tool-bar-height)
(frame-parameter nil 'tool-bar-lines)

and tell me what they give.  I fail to understand why `tool-bar-lines'
is 2 in your saved desktop.

And while you're there: In a saved desktop find the (tool-bar-lines . 2)
entry, replace it with (tool-bar-lines . 3), fire another instance of
Emacs and tell me whether the frame height decreased.

Please do that now.

Thanks, martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 17/11/2014 19:25, martin rudalics ha scritto:
>  > And this commit (just one minute lather the previous):
>  >
>  > author    Martin Rudalics 2014-11-07 10:49:22 (GMT)
>  > download    emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz
>  >
>  > Improve inhibiting of implied frame resizes.
>  >
>  >
>  > ..and the frame decreases in height each time one starts Emacs...
>
> OK.  But you didn't answer my earlier questions:

I DID! See:

http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00792.html

Ciao,
  Angelo.

>
> In a normal frame with tool bar, please evaluate the following
> three forms
>
> (tool-bar-height nil t)
> (tool-bar-height)
> (frame-parameter nil 'tool-bar-lines)
>
> and tell me what they give.  I fail to understand why `tool-bar-lines'
> is 2 in your saved desktop.
>
> And while you're there: In a saved desktop find the (tool-bar-lines . 2)
> entry, replace it with (tool-bar-lines . 3), fire another instance of
> Emacs and tell me whether the frame height decreased.
>
> Please do that now.
>
> Thanks, martin

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 >> OK.  But you didn't answer my earlier questions:
 >
 > I DID! See:
 >
 > http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00792.html

Sorry, my bad.  That message must have got drowned by the GIT postings.

Quoting from that URL now:

 > Always with a minimal init.el, I have
 >
 > M-: (tool-bar-height nil t)
 > 36 (#o44, #x24, ?$)
 >
 > M-: (tool-bar-height)
 > 3 (#o3, #x3, ?\C-c)
 >
 >
 > M-: (frame-parameter nil 'tool-bar-lines)
 > 3 (#o3, #x3, ?\C-c)

That's fine.  I was confused because your first attachment (the one
containing desktop, desktop-00, desktop-01 and desktop-03) had a two
lines tool bar for each saved desktop and I wasn't able to reconcile
that with the frame dumps you sent later.  You apparently used a larger
font in your earlier posting.

I'll now look into why that change

 > And this commit (just one minute lather the previous):
 >
 > author    Martin Rudalics 2014-11-07 10:49:22 (GMT)
 > download    emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz
 >
 > Improve inhibiting of implied frame resizes.

may have caused the problems you see.  What is the value of
`frame-inhibit-implied-resize' on your system?

BTW, has

 > I flagged a similar issue on OSX which has been fixed:
 >
 > http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00582.html
 >
 > I don't know if this could be useful also for you...

been fixed for good now?  Did that issue appear at the same time as
bug#19048?

martin

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 18/11/2014 08:51, martin rudalics ha scritto:
>
> I'll now look into why that change
>
>  > And this commit (just one minute lather the previous):
>  >
>  > author    Martin Rudalics 2014-11-07 10:49:22 (GMT)
>  > download    emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz
>  >
>  > Improve inhibiting of implied frame resizes.
>
> may have caused the problems you see.  What is the value of
> `frame-inhibit-implied-resize' on your system?

C-h v frame-inhibit-implied-resize

frame-inhibit-implied-resize is a variable defined in `C source code'.
Its value is (tool-bar-lines)

Documentation:
Whether frames should be resized implicitly.
[...]

> BTW, has
>
>  > I flagged a similar issue on OSX which has been fixed:
>  >
>  > http://lists.gnu.org/archive/html/bug-gnu-emacs/2014-11/msg00582.html
>  >
>  > I don't know if this could be useful also for you...
>
> been fixed for good now?  Did that issue appear at the same time as
> bug#19048?

Yes it has been fixed:

author	Jan D 2014-11-15 13:35:15 (GMT)
[...]
Fix getting frame size wrong when restoring desktop.

from ChangeLog

+	* nsmenu.m (update_frame_tool_bar): If tool bar changes height,
+	call updateFrameSize.

Maybe these two issue were similar in appearance but different in 
substance...

Ciao, Angelo.

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 18/11/2014 08:51, martin rudalics ha scritto:
>
> I'll now look into why that change
>
>  > And this commit (just one minute lather the previous):
>  >
>  > author    Martin Rudalics 2014-11-07 10:49:22 (GMT)
>  > download    emacs-1c50b3adb636addc4244942e8f0e33b1e557ec07.tar.gz
>  >
>  > Improve inhibiting of implied frame resizes.
>
> may have caused the problems you see.

BTW, I see the same issue in today build of Windows build (MSYS2-MinGW64)...


   Angelo

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

> BTW, I see the same issue in today build of Windows build (MSYS2-MinGW64)...

I see it too, now.

martin

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

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

 >> BTW, I see the same issue in today build of Windows build (MSYS2-MinGW64)...

Please try the attached patch.

Thanks, martin

[-- Attachment #2: x_set_tool_bar_lines.patch --]
[-- Type: text/plain, Size: 3228 bytes --]

diff --git a/src/w32fns.c b/src/w32fns.c
index 1b290b7..e514970 100644
--- a/src/w32fns.c
+++ b/src/w32fns.c
@@ -1721,15 +1721,7 @@ x_set_tool_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
   else
     nlines = 0;

-  if (nlines == 0)
-    x_change_tool_bar_height (f, nlines * FRAME_LINE_HEIGHT (f));
-  else
-    {
-      f->n_tool_bar_rows = 0;
-      FRAME_TOOL_BAR_LINES (f) = nlines;
-      adjust_frame_glyphs (f);
-      SET_FRAME_GARBAGED (f);
-    }
+  x_change_tool_bar_height (f, nlines * FRAME_LINE_HEIGHT (f));
 }


@@ -1749,11 +1741,7 @@ x_change_tool_bar_height (struct frame *f, int height)
   /* Recalculate tool bar and frame text sizes.  */
   FRAME_TOOL_BAR_HEIGHT (f) = height;
   FRAME_TOOL_BAR_LINES (f) = lines;
-/**   FRAME_TEXT_HEIGHT (f) **/
-/**     = FRAME_PIXEL_TO_TEXT_HEIGHT (f, FRAME_PIXEL_HEIGHT (f)); **/
-/**   FRAME_LINES (f) **/
-/**     = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, FRAME_PIXEL_HEIGHT (f)); **/
-  /* Store the `tool-bar-lines' and `height' frame parameters.  */
+  /* Store `tool-bar-lines' and `height' frame parameters.  */
   store_frame_param (f, Qtool_bar_lines, make_number (lines));
   store_frame_param (f, Qheight, make_number (FRAME_LINES (f)));

@@ -1772,6 +1760,10 @@ x_change_tool_bar_height (struct frame *f, int height)
   adjust_frame_size (f, -1, -1, (old_height == 0 || height == 0) ? 2 : 4, 0,
 		     Qtool_bar_lines);

+  /* adjust_frame_size might not have done anything, garbage frame
+     here.  */
+  adjust_frame_glyphs (f);
+  SET_FRAME_GARBAGED (f);
   if (FRAME_X_WINDOW (f))
     x_clear_under_internal_border (f);
 }
diff --git a/src/xfns.c b/src/xfns.c
index aaa75f2..59715d0 100644
--- a/src/xfns.c
+++ b/src/xfns.c
@@ -1075,19 +1075,7 @@ x_set_tool_bar_lines (struct frame *f, Lisp_Object value, Lisp_Object oldval)
   else
     nlines = 0;

-#ifdef USE_GTK
   x_change_tool_bar_height (f, nlines * FRAME_LINE_HEIGHT (f));
-#else /* !USE_GTK */
-  if (nlines == 0)
-    x_change_tool_bar_height (f, nlines * FRAME_LINE_HEIGHT (f));
-  else
-    {
-      f->n_tool_bar_rows = 0;
-      FRAME_TOOL_BAR_LINES (f) = nlines;
-      adjust_frame_glyphs (f);
-      SET_FRAME_GARBAGED (f);
-    }
-#endif /* USE_GTK */
 }


@@ -1124,10 +1112,6 @@ x_change_tool_bar_height (struct frame *f, int height)
   /* Recalculate tool bar and frame text sizes.  */
   FRAME_TOOL_BAR_HEIGHT (f) = height;
   FRAME_TOOL_BAR_LINES (f) = lines;
-/**   FRAME_TEXT_HEIGHT (f) **/
-/**     = FRAME_PIXEL_TO_TEXT_HEIGHT (f, FRAME_PIXEL_HEIGHT (f)); **/
-/**   FRAME_LINES (f) **/
-/**     = FRAME_PIXEL_HEIGHT_TO_TEXT_LINES (f, FRAME_PIXEL_HEIGHT (f)); **/
   /* Store the `tool-bar-lines' and `height' frame parameters.  */
   store_frame_param (f, Qtool_bar_lines, make_number (lines));
   store_frame_param (f, Qheight, make_number (FRAME_LINES (f)));
@@ -1153,6 +1137,10 @@ x_change_tool_bar_height (struct frame *f, int height)
   adjust_frame_size (f, -1, -1, (old_height == 0 || height == 0) ? 2 : 4, 0,
 		     Qtool_bar_lines);

+  /* adjust_frame_size might not have done anything, garbage frame
+     here.  */
+  adjust_frame_glyphs (f);
+  SET_FRAME_GARBAGED (f);
   if (FRAME_X_WINDOW (f))
     x_clear_under_internal_border (f);

bug#19048: Frame height decreases at Emacs start

[Angelo Graziosi]

Il 19/11/2014 09:30, martin rudalics ha scritto:
>  >> BTW, I see the same issue in today build of Windows build
> (MSYS2-MinGW64)...
>
> Please try the attached patch.

It seems to work both with Cygwin and MSYS2-MinGW64 builds.

Thanks,
  Angelo.

>
> Thanks, martin

bug#19048: Frame height decreases at Emacs start

[martin rudalics]

 > It seems to work both with Cygwin and MSYS2-MinGW64 builds.

OK.  If you don't have further problems I'll check it in in a couple of
days.

Thanks, martin