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

From: Pip Cet <[email-masked]>
Date: Sat, 22 Aug 2015 15:32:02 +0000

Let me just repeat my initial analysis, which I still believe is
correct and which might have gotten lost somewhere.

There are two problems:
(1) x_check_fullscreen (xterm.c) calls XResizeWindow without first
calling x_wm_set_size_hint (gtkutil.c), which would propagate the
`frame-resize-pixelwise' flag to have an effect on GTK/GDK
(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'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.

I take it something about that doesn't feel right to you, and that's
reason enough to investigate further.

On Sat, Aug 22, 2015 at 2:16 PM, martin rudalics <[email-masked]> wrote:
>> On Sat, Aug 22, 2015 at 6:40 AM, martin rudalics <[email-masked]> wrote:
>>>> When starting Emacs (GTK build) on an X server which has no window
>>>> manager (such as a newly-created Xnest session), setting
>>>> `frame-resize-pixelwise' to t followed by a resize operation often has
>>>> no effect.
>>>
>>> According to the manual
>>>
>>>       Setting this variable usually causes the next resize operation to
>>>       pass the corresponding size hints to the window manager.  This
>>>       means that this variable should be set only in a user's initial
>>>       file; applications should never bind it temporarily.
>>
>> That documentation is outdated and does not apply to GTK builds in all
>> cases, I'm afraid. It is not the window manager that decides to honor
>> or dishonor frame-resize-pixelwise but GDK.
>
> "Window manager" was an attempt to catch the behavior of all toolkits
> including Windows which doesn't have a window manager either.  Feel free
> to suggest a better term.

Oh, I see! That makes sense, thank you and sorry for my
narrow-mindedness in thinking only of X11 window managers.

>> See x_wm_set_size_hint and
>> gtk_window_move_resize (gtkwindow.c, in the GTK sources). In
>> particular, gtk_window_compute_configure_request calls
>> gtk_window_constrain_size which calls gdk_window_constrain_size which
>> calculates
>>
>>    width = base_width + FLOOR (width - base_width, xinc);
>>    height = base_height + FLOOR (height - base_height, yinc);
>>
>> (where FLOOR is defined as #define FLOOR(value, base)    ( ((gint)
>> ((value) / (base))) * (base) ) )
>
> I can't find that in the version from
>
> https://github.com/GNOME/gtk/blob/master/gtk/gtkwindow.c

Try https://github.com/GNOME/gtk/blob/master/gdk/gdkwindow.c ?

> and also must admit that I have forgotten most of the GTK code by now.
>
> IIRC base_width and base_height are somehow calculated from a minimum
> size and what Emacs requested earlier.  If frame_resize_pixelwise is
> true we _should_ have requested 1 from this
>
>   size_hints.width_inc = frame_resize_pixelwise ? 1 : FRAME_COLUMN_WIDTH
> (f);
>   size_hints.height_inc = frame_resize_pixelwise ? 1 : FRAME_LINE_HEIGHT
> (f);

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.

> Can you check how apparently an "integral" (the increment is a multiple
> of either FRAME_COLUMN_WIDTH or FRAME_LINE_HEIGHT) value gets passed via
> size_hints in your case?

Oh, I see what you mean now. I'll investigate that next.

>>> So it's possible that Xnest or some other X component refuses to resize
>>> your frame because the size hints were set up inappropriately.
>>
>> I'm pretty sure that's not what's happening, but I'll be happy to
>> provide traces to demonstrate my analysis is correct...or to prove it
>> wrong, of course! The attached gdb log shows quite clearly that it's
>> GDK making the second (erroneous) call to XResizeWindow, not Xnest
>> (there is no window manager).
>
> OK.  I won't doubt what you say here.

Feel free to, of course, though I understand sifting through gdb logs
is best avoided unless it's absolutely necessary.

>>> 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.

>>> Does it fail when you set `frame-resize-pixelwise' to t, request an
>>> integral resize first and a second non-integral one afterwards?
>>
>> I'm not sure I fully understand how you define "integral".
>
> See above.  Also the "first and a second" above was meant to do this in
> two consecutive commands (that is, with a redisplay in between).
> Passing different size requests in one and the same command can have
> unpredictable consequences (at least on other platforms).

Noted. Thanks for that piece of advice, that really helps me make
sense of what's going on.

>> In short,
>> non-full-screen resize + redisplay + full-screen resize works, the
>> other combinations do not.
>>
>> If I run this code (Xnest running on display :3):
>>
>> DISPLAY=:3 emacs -Q --eval "(progn (setq frame-resize-pixelwise t)
>> (set-frame-height (selected-frame) (1+ (frame-pixel-height
>> (selected-frame))) nil t) (redisplay) (set-frame-parameter
>> (selected-frame) 'fullscreen 'fullboth))"
>>
>> Things work, but without the "(redisplay)" they don't. (So that's a
>> non-full-screen resize first, then a full-screen resize). Doing the
>> full-screen resize first breaks things.
>
> "Breaks" in what sense?  That it does nothing or not fully resize?

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.

>> I must also point out that without the "(redisplay)", there are
>> unexpected results: the full screen resize appears to be ignored
>> entirely.
>
> I'd have expected that.

Good, that's one less thing to worry about then.

>> But, again, I currently stand by my initial analysis of what's
>> happening. The problem is that we cannot simply do the right thing
>> because of the KWin bug...
>
> What is the "KWin bug"?

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14627, which introduced
the conditional which prevents us from calling x_wm_size_hints

> I'm probably too silly to give you a good advice.

Not at all, it's much appreciated.

> 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.

> x_wm_set_size_hint (f, 0, 0);
>
> _before_ calling gtk_window_resize.  Could you play around with that?
>
> BTW if you set `frame-size-history' to a non-nil value you should get a
> list of frame resizing operations you can watch with the function
> `frame--size-history'.  IIRC I found GDB occasionally not 100% reliable
> to reproduce the actual history as of a normal, optimized build.

Thanks, I never considered that possibility. I'm running an
unoptimized debug build (-O0 -g3), but there might be useful
information there. So far it appears not to contain anything
unexpected (I'm looking at the variable directly, the function appears
to have mysteriously vanished from the current git tree, except for a
reference in the documentation...)

Thanks for all your help!
From 1f3f68b2a6da4c9d0b54f18b0bcc6c4dd2b243ad Mon Sep 17 00:00:00 2001
From: Philip <[email-masked]>
Date: Sat, 22 Aug 2015 14:59:10 +0000
Subject: [PATCH] Do not apply this patch.

See http://debbugs.gnu.org/cgi/bugreport.cgi?bug=14627
---
 src/gtkutil.c | 11 -----------
 src/xterm.c   |  2 ++
 2 files changed, 2 insertions(+), 11 deletions(-)

diff --git a/src/gtkutil.c b/src/gtkutil.c
index d684cd9..bdbfb0d 100644
--- a/src/gtkutil.c
+++ b/src/gtkutil.c
@@ -1364,7 +1364,6 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
   int base_width, base_height;
   int min_rows = 0, min_cols = 0;
   int win_gravity = f->win_gravity;
-  Lisp_Object fs_state, frame;
   int scale = xg_get_gdk_scale ();
 
   /* Don't set size hints during initialization; that apparently leads
@@ -1373,16 +1372,6 @@ x_wm_set_size_hint (struct frame *f, long int flags, bool user_position)
   if (NILP (Vafter_init_time) || !FRAME_GTK_OUTER_WIDGET (f))
     return;
 
-  XSETFRAME (frame, f);
-  fs_state = Fframe_parameter (frame, Qfullscreen);
-  if (EQ (fs_state, Qmaximized) || EQ (fs_state, Qfullboth))
-    {
-      /* Don't set hints when maximized or fullscreen.  Apparently KWin and
-         Gtk3 don't get along and the frame shrinks (!).
-      */
-      return;
-    }
-
   if (flags)
     {
       memset (&size_hints, 0, sizeof (size_hints));
diff --git a/src/xterm.c b/src/xterm.c
index b7aacfa..0fd2464 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -10189,6 +10189,8 @@ x_check_fullscreen (struct frame *f)
       frame_size_history_add
 	(f, Qx_check_fullscreen, width, height, Qnil);
 
+      x_wm_set_size_hint (f, 0, false);
+
       XResizeWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
 		     width, height);
 
-- 
2.5.0