bug#2546: toolbar bug

bug#2546: toolbar bug

[Adrian Robert]

The calculation method that was commented out in x_set_window_size()  
works here, and I've committed it to CVS.  As mentioned in the  
comment, this would sometimes fail previously if the toolbar had not  
been displayed yet.  I believe the relevant call would be [toolbar  
setVisible: true] and this normally happens in nsmenu.c:  
ns_update_toolbar().  When a new frame is created, it is initially  
set to NOT visible in EmacsView-initFrameFromEmacs, because it will  
be turned on if needed and after it is ready by ns_update_toolbar().   
However, if this gets called AFTER x_set_window_size(), it could  
cause the failure.  If you still see it in your environment, could  
you try throwing in a call to [[view toolbar] setVisible: true] in  
the 'if (tb)' section before calling frameRectForContentRect, and see  
if that helps?

BTW I changed the order of the COCOA and GNUstep implementations to  
agree with the XXX comment just above.  This was accidentally  
reversed during a time when emacs-devel was asking me to get rid of  
one of NS_IMPL_COCOA, NS_IMPL_GNUSTEP and I did not realize at first  
that this was impossible.

thanks,
Adrian

bug#2546: toolbar bug

[David Reitter]

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

On 6 Mar 2009, at 09:56, Adrian Robert wrote:

> The calculation method that was commented out in x_set_window_size()  
> works here, and I've committed it to CVS.  As mentioned in the  
> comment, this would sometimes fail previously if the toolbar had not  
> been displayed yet.  I believe the relevant call would be [toolbar  
> setVisible: true] and this normally happens in nsmenu.c:  
> ns_update_toolbar().  When a new frame is created, it is initially  
> set to NOT visible in EmacsView-initFrameFromEmacs, because it will  
> be turned on if needed and after it is ready by  
> ns_update_toolbar().  However, if this gets called AFTER  
> x_set_window_size(), it could cause the failure.  If you still see  
> it in your environment, could you try throwing in a call to [[view  
> toolbar] setVisible: true] in the 'if (tb)' section before calling  
> frameRectForContentRect, and see if that helps?
>
> BTW I changed the order of the COCOA and GNUstep implementations to  
> agree with the XXX comment just above.  This was accidentally  
> reversed during a time when emacs-devel was asking me to get rid of  
> one of NS_IMPL_COCOA, NS_IMPL_GNUSTEP and I did not realize at first  
> that this was impossible.


I've come to similar conclusions yesterday and tried a few things  
(including the setVisible, or a call to update_frame_toolbar to fill  
it) without much success.
However, what does seem to work is to only add the toolbar height if  
it is visible.  Try the patch below.

What would be better though is to set the toolbar before the frame is  
drawn, since the resizing effect is quite ugly.

Also, there is another bug that causes window content to be drawn  
briefly at the wrong position while the toolbar appears, i.e., after  
you click on the toolbar button to turn it on.  It's similar to the  
ugliness we see when resizing frames.  I haven't found that one yet.

- David






Index: nsterm.m
===================================================================
RCS file: /sources/emacs/emacs/src/nsterm.m,v
retrieving revision 1.58
diff -c -r1.58 nsterm.m
*** nsterm.m	7 Feb 2009 11:04:07 -0000	1.58
--- nsterm.m	6 Mar 2009 15:23:04 -0000
***************
*** 1162,1178 ****
   			    styleMask: [window styleMask]])
           - FRAME_NS_TITLEBAR_HEIGHT (f);
   #else
!     FRAME_NS_TOOLBAR_HEIGHT (f) = 32;
!       /* actually get wrong result here if toolbar not yet displayed
            NSHeight ([window frameRectForContentRect: NSMakeRect (0,  
0, 0, 0)])
            - FRAME_NS_TITLEBAR_HEIGHT (f); */
   #endif
     else
       FRAME_NS_TOOLBAR_HEIGHT (f) = 0;
!
     wr.size.width = pixelwidth + f->border_width;
!   wr.size.height = pixelheight + FRAME_NS_TITLEBAR_HEIGHT (f)
!                   + FRAME_NS_TOOLBAR_HEIGHT (f);

     /* constrain to screen if we can */
     if (screen)
--- 1169,1188 ----
   			    styleMask: [window styleMask]])
           - FRAME_NS_TITLEBAR_HEIGHT (f);
   #else
!       FRAME_NS_TOOLBAR_HEIGHT (f) = 32; // 47;
!       /* actually get wrong result here if toolbar not yet displayed
            NSHeight ([window frameRectForContentRect: NSMakeRect (0,  
0, 0, 0)])
            - FRAME_NS_TITLEBAR_HEIGHT (f); */
+
   #endif
     else
       FRAME_NS_TOOLBAR_HEIGHT (f) = 0;
!
     wr.size.width = pixelwidth + f->border_width;
!   wr.size.height = pixelheight + FRAME_NS_TITLEBAR_HEIGHT (f);
!
!   if ([[FRAME_NS_VIEW (f) toolbar] isVisible])
!     wr.size.height += FRAME_NS_TOOLBAR_HEIGHT (f);

     /* constrain to screen if we can */
     if (screen)


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]

bug#2546: toolbar bug

[David Reitter]

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

On 6 Mar 2009, at 09:56, Adrian Robert wrote:

> The calculation method that was commented out in x_set_window_size()  
> works here, and I've committed it to CVS.  As mentioned in the  
> comment, this would sometimes fail previously if the toolbar had not  
> been displayed yet.  I believe the relevant call would be [toolbar  
> setVisible: true] and this normally happens in nsmenu.c:  
> ns_update_toolbar().  When a new frame is created, it is initially  
> set to NOT visible in EmacsView-initFrameFromEmacs, because it will  
> be turned on if needed and after it is ready by  
> ns_update_toolbar().  However, if this gets called AFTER  
> x_set_window_size(), it could cause the failure.  If you still see  
> it in your environment, could you try throwing in a call to [[view  
> toolbar] setVisible: true] in the 'if (tb)' section before calling  
> frameRectForContentRect, and see if that helps?

OK, I've seen your patch now; this works for me.

The resizing problems persist.  Resizing happens only for each full  
column or row, but the it's jumping around badly, as if the whole  
window content is drawn one row too low first.



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 2193 bytes --]