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

[martin rudalics]

>> Meanwhile please install your changes.
>
> Done!

Thanks!

>> Now the only remaining issue we must fix before the release is that of
>> the non-shrinking echo area when toggling off the tool bar
>
> I wasn't aware of this problem. Can you point me to where it's described.

Citing Keith's experiments on OSX:

>>> (1) Toggle the tool bar off.  Does the overall frame height shrink or stay 
the same?
>>>
>>> A: Starting from Emacs -Q.  Using the mouse to toggle the toolbar off,
>>> the height of the frame shrinks -- i.e., top and left remain the same,
>>> and the bottom shrinks upward about one-half the height of the toolbar
>>> that disappears.  The minibuffer / echo area increases in size to about
>>> 3 lines of text, with just one line of text that reads: "menu-bar
>>> options showhide showhide-tool-bar".
>>>
>>>
>>> (2) Toggle the tool bar on again.  Does the overall frame height increase 
or stay the same?
>>>
>>> A: The frame height quickly shrinks and expands, but ultimately remains
>>> the same size -- i.e., slightly less than when Emacs -Q first began its
>>> GUI session.

I'll attach two screenshots.  The problem arose after Jan installed this
change:

diff --git a/src/ChangeLog b/src/ChangeLog
index 69da1c3..861ba91 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,8 @@
+2015-01-06  Jan Djärv  <jan.h.d@swipnet.se>
+
+    * nsterm.m (x_set_window_size): Call updateFrameSize to get real
+    size instead of using widht/height.  The frame may be constrained.
+
 2015-01-05  Paul Eggert  <eggert@cs.ucla.edu>

     * lisp.h (XSYMBOL): Parenthesize id in forward decl.
diff --git a/src/nsterm.m b/src/nsterm.m
index 2ccb7fe..bf3192b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1404,15 +1404,8 @@ x_set_window_size (struct frame *f,
     [view setBoundsOrigin: origin];
   }

-  change_frame_size (f, width, height, 0, 1, 0, pixelwise);
-/*  SET_FRAME_GARBAGED (f); // this short-circuits expose call in drawRect */
-
-  mark_window_cursors_off (XWINDOW (f->root_window));
-  cancel_mouse_face (f);
-
+  [view updateFrameSize: NO];
   unblock_input ();
-
-  do_pending_window_change (0);
 }

where the crucial aspect is the removal of the change_frame_size call.
Note that in updateFrameSize we call change_frame_size only

   if (oldr != rows || oldc != cols || neww != oldw || newh != oldh)

which usually won't hold when removing the tool bar.  Maybe we should
make the change_frame_size call in updateFrameSize unconditional after
being called from update_frame_tool_bar.

>> and the fact
>> that a frame keeps shrinking when turning off/on the tool bar.  I'll
>> look into that but if you have any ideas ...
>
> I though this was by design. On NextStep, the toolbar is excluded from
> `frame-pixel-height', so it makes sense that frame size change when enabled
> and disabled. Besides, it's height typically isn't a multiple of the text
> size, so the frame would need to be resized slightly anyway (unless
> `frame-resize-pixelwise' is set).

All true.  But the problem is that enabling the tool bar doesn't make
the overall frame height as large as it has been before disabling the
tool bar.  So eventually you might end up with a zero height frame.

> There are other things that we would need to fix:
>
> * Symbols in the fringe are inverted. I saw a patch for this on the
> emacs-dev mailing list a few weeks ago but I haven't tried it out.
>
> * When the cursor is rendered in the fringe, it's only drawn incorrectly.
> (This problem might be related to the previous.)

I've seen these but can't do much about them.  It would be great if you
looked into these.

> What is the time frame for the Emacs 25 release? I would be happy to work
> on this, but with my family situation, work will progress very, very slowly.

Bug fixes are practically always accepted.  There's a short period
immediately before the release when only regressions wrt the previous
release may be fixed.  So you won't have to bother about the time frame.

> I have thought about that as well. Currently, the package provides the
> macros NSTRACE_WHEN and NSTRACE_UNLESS. I don't use then right now,
> unfortunately they only silence the current function, not functions that
> may be called by it (which is something that we would want). Hopefully, I
> will be able to categorize the functions in such a way that the default
> setting would silence some of the more frequently called functions.

That would be fine.

Thanks, martin

tool-bar-mode-on.png
Description: PNG image

tool-bar-mode-off.png
Description: PNG image