Subject: bug#21317: 25.0.50; frame-resize-pixelwise has no effect (GTK, no window manager)

From: Pip Cet <[email-masked]>
Date: Sun, 23 Aug 2015 09:45:07 +0000

Sorry this is getting a bit long, but unfortunately, another issue appeared.

On Sat, Aug 22, 2015 at 5:46 PM, martin rudalics <[email-masked]> wrote:
> I understand that now.  You don't run do_ewmh_fullscreen.  So the bug
> _only_ occurs when you want to make a fullscreen frame?  This was not
> clear from you initial report.

Sorry about that. You're right, I've only seen the problem when I make
a fullscreen frame. That wasn't perfectly clear to me at the time, but
I should have tested better. Thank you for your patience.

>> (2) x_wm_set_size_hint returns without propagating the
>> `frame-resize-pixelwise' flag when the frame's fullscreen property is
>> 'maximized or 'fullboth.
>>
>> The first appears to be simple oversight, but the second is
>> intentional to work around a KWin bug.
>
> I faintly remember that bug now.  What I don't understand is the remedy.
> When we do
>
>   fs_state = Fframe_parameter (frame, Qfullscreen);
>   if (EQ (fs_state, Qmaximized) || EQ (fs_state, Qfullboth))
>
> the parameter has been already set but the frame is not yet fullscreen.
> Does this shrinking occur when the frame is already fullscreen or does
> it occur when a user wants to switch to fullscreen?

I'm not a hundred percent certain; from reading the thread, I think
it's the former: the window is in full-screen mode and starts
shrinking. I've installed KWin but have been unable to produce buggy
behavior, so far, without the workaround in gtkutil.c.

>> I've attached a patch which fixes things for me (at least when
>> tool-bar-mode is disabled :( ), but presumably breaks things for KWin
>> users, so it should not go in as it is until that matter has been
>> cleared; maybe that'll help clarify matters.
>
> When worse comes to worse we could condition that somehow: Invent an
> option which is off by default and call x_wm_set_size_hint when it's on.

I do wonder how useful it is to support the case without a window
manager; unfortunately, I think it is useful, much as I'd enjoy all
that code going away and leaving things to the window manager.

Anyway, if x_check_fullscreen is supposed to work the way it currently
does, bypassing xg_frame_set_char_size, there's a third issue to add
to my list:

(3) x_check_fullscreen does not call store_frame_param, unlike
x_set_window_size_1, so the frame has its 'fullscreen parameter
cleared to nil by x_net_wm_state; the next `set-frame-font' call then
results in an integral frame size rather than the full screen.

If my understanding is correct, the best way forward is this:

 - skip the hints in maximized/fullscreen state if
wm_supports(net_wm_state) || wm_supports(net_wm_state_fullscreen), it
might be KWin
 - otherwise, set the hints
 - call x_wm_set_size_hint from x_check_fullscreen
 - call store_frame_param from x_check_fullscreen

>> We do hit that code (in gtkutil.c) twice: once during early
>> initialization, when frame_resize_pixelwise is still false, and once
>> after GDK has decided to resize us to not-quite-full-screen size, when
>> it's too late. We should hit it once more, from x_check_fullscreen,
>> but don't do so for the two reasons listed above.
>
> I see.  If in frame.c you initialize frame_resize_pixelwise to 1 then
> everything works OK?

...Almost. Sorry. If I set frame_resize_pixelwise to 1 and run
`set-frame-font' afterwards, the frame loses its full-screen
resolution and changes to a similar resolution that's a multiple of
the character size, as far as I can tell. This is due to issue (3), I
believe. Similarly, we don't adjust for the tool bar size, which I
also believe is due to issue (3) (I keep forgetting about that one
since I don't usually use the tool bar).

I have attached the minimal patch (as far as I know) that works:
initialize frame_resize_pixelwise to 1, and fix issue (3). That works
both with a tool bar and with `set-frame-font' (I'm not in the habit
of running `set-frame-font', but it's run by the gconf code when
gnome-settings-daemon is active and specifies a system font).

>>>>> Does it also fail when `frame-resize-pixelwise' is set to t in your
>>>>> initial file?
>>>>
>>>> Yes, it does.
>>>
>>> That's bad.  It can only mean that we send an integral resize request
>>> _before_ Emacs reads the initial file and the subsequent "real" resize
>>> request fails because the size hints have been already set up wrongly.
>>
>> No, we call Fsetq and so on fine, I think it's just that we never call
>> x_set_wm_size_hints afterwards.
>
> We could invent a function `set-frame-resize-pixelwise' which sends the
> size hints (maybe iff when a frame is not fullscreen, or so to avoid the
> KWin bug).

How would that be different from x_wm_set_size_hints with FLAGS=0? I
suppose we could modify only the increments and leave the rest of the
size hints alone, but is that enough to prevent bad KWin behavior?

>> It appears to ignore the fullscreen resize; as you point out, that's
>> as expected, by the "don't resize twice without a redisplay" rule.
>
> It's rather a don't resize and change the fullscreen status without a
> redisplay.

Ah. Well, that doesn't really make sense anyway, so it's probably a
good rule :-)

>>> However, in
>>> xg_frame_set_char_size (which is responsible for the resize in the GTK
>>> case) we could do the
>>
>> That's a very interesting point, because xg_frame_set_char_size never
>> appears to get called here after early initialization. Instead, we
>> call XResizeWindow directly, then GDK catches the ConfigureNotify and
>> decides it doesn't like it, so it sends another (erroneous)
>> XResizeWindow.
> Interesting indeed.  Something's not right here, I'm afraid.  I'll have
> to look into this.

It seems like the code in x_check_fullscreen was intended to work
without a window manager (and without calling xg_frame_set_char_size),
but never did. It didn't call x_wm_set_size_hints and it didn't clean
up after x_net_wm_state by setting the 'fullscreen property again.

Thank you for your assistance in this, it's very much appreciated. If
you would like me to run more tests or provide debugging info, please
let me know, I'll do my best.
diff --git a/src/frame.c b/src/frame.c
index 2044048..cac966d 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -5161,7 +5161,7 @@ With some window managers you may have to set this to non-nil in order
 to set the size of a frame in pixels, to maximize frames or to make them
 fullscreen.  To resize your initial frame pixelwise, set this option to
 a non-nil value in your init file.  */);
-  frame_resize_pixelwise = 0;
+  frame_resize_pixelwise = 1;
 
   DEFVAR_LISP ("frame-inhibit-implied-resize", frame_inhibit_implied_resize,
 	       doc: /* Whether frames should be resized implicitly.
diff --git a/src/xterm.c b/src/xterm.c
index b7aacfa..4c7aec4 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -10169,6 +10169,7 @@ x_check_fullscreen (struct frame *f)
     {
       int width = FRAME_PIXEL_WIDTH (f), height = FRAME_PIXEL_HEIGHT (f);
       struct x_display_info *dpyinfo = FRAME_DISPLAY_INFO (f);
+      Lisp_Object lval;
 
       switch (f->want_fullscreen)
         {
@@ -10186,6 +10187,23 @@ x_check_fullscreen (struct frame *f)
           height = x_display_pixel_height (dpyinfo);
         }
 
+      lval = Qnil;
+      switch (f->want_fullscreen)
+	{
+	case FULLSCREEN_WIDTH:
+	  lval = Qfullwidth;
+	  break;
+	case FULLSCREEN_HEIGHT:
+	  lval = Qfullheight;
+	  break;
+	case FULLSCREEN_BOTH:
+	  lval = Qfullboth;
+	  break;
+	case FULLSCREEN_MAXIMIZED:
+	  lval = Qmaximized;
+	  break;
+	}
+
       frame_size_history_add
 	(f, Qx_check_fullscreen, width, height, Qnil);
 
@@ -10200,6 +10218,8 @@ x_check_fullscreen (struct frame *f)
 			     false, true, false, true);
 	  x_sync (f);
 	}
+
+      store_frame_param (f, Qfullscreen, lval);
     }
 }