Subject: Re: bug#74496: 30.0.91; fullscreen frame set with F11 is shifted when ctwm restarts

From: martin rudalics <[email-masked]>
Date: Sun, 8 Dec 2024 11:58:39 +0100

>> Maybe you can try the attached diff (it's against the release version)
 >> and set breakpoints at the four lines I marked with a
 >>
 >> // break-here
 >>
 >> comment.  Start gdb via run -Q, do F11 and restart CWTM.
 >
 > Here is the backtrace.
 >
 >     1  (gdb) info b
 >     2  Num     Type           Disp Enb Address            What
 >     3  1       breakpoint     keep y   0x000000000042edfd in gui_set_frame_parameters_1 at /u/xxx/src/emacs/29.4/src/frame.c:4461
 >     4  2       breakpoint     keep y   0x00000000004cf6fb in x_net_wm_state at /u/xxx/src/emacs/29.4/src/xterm.c:17504
 >     5  3       breakpoint     keep y   0x00000000004cf5c1 in x_handle_net_wm_state at /u/xxx/src/emacs/29.4/src/xterm.c:27249
 >     6  4       breakpoint     keep y   0x00000000004e52b5 in x_check_fullscreen at /u/xxx/src/emacs/29.4/src/xterm.c:27324
 >     7  (gdb) run -Q
 >     8  Starting program: /u/xxx/src/emacs/build-29-0/src/emacs -Q
 >     9  [New LWP 7051 of process 24519]
 >    10  [New LWP 795 of process 24519]
 >    11  [New LWP 1577 of process 24519]
 >    12  [New process 24519]
 >    13  [New process 24519]
 >    14
 >    15  Thread 1 "" hit Breakpoint 2, x_net_wm_state (f=f@entry=0x71354096ac10, window=<optimized out>) at /u/xxx/src/emacs/29.4/src/xterm.c:17504
 >    16  17504	    store_frame_param (f, Qfullscreen, lval); // break here
 >    17  (gdb) bt
 >    18  #0  x_net_wm_state (f=f@entry=0x71354096ac10, window=<optimized out>) at /u/xxx/src/emacs/29.4/src/xterm.c:17504
 >    19  #1  0x00000000004e9343 in handle_one_xevent (dpyinfo=dpyinfo@entry=0x713541033000, event=event@entry=0x7f7fff03c0d0, finish=finish@entry=0x7f7fff03c0cc, hold_quit=hold_quit@entry=0x7f7fff03c1c0) at /u/xxx/src/emacs/29.4/src/xterm.c:20998
 >    20  #2  0x00000000004f06cd in XTread_socket (terminal=<optimized out>, hold_quit=0x7f7fff03c1c0) at /u/xxx/src/emacs/29.4/src/xterm.c:24812
 >    21  #3  0x000000000051b7bd in gobble_input () at /u/xxx/src/emacs/29.4/src/keyboard.c:7427
 >    22  #4  0x000000000051b8d1 in handle_async_input () at /u/xxx/src/emacs/29.4/src/keyboard.c:7658
 >    23  #5  0x000000000051b8e7 in process_pending_signals () at /u/xxx/src/emacs/29.4/src/keyboard.c:7672
 >    24  #6  0x00000000005cb17a in wait_reading_process_output (time_limit=<optimized out>, nsecs=nsecs@entry=0, read_kbd=-1, do_display=do_display@entry=true, wait_for_cell=wait_for_cell@entry=0x0, wait_proc=wait_proc@entry=0x0, just_wait_proc=just_wait_proc@entry=0) at /u/xxx/src/emacs/29.4/src/process.c:5304
 >    25  #7  0x00000000004282e2 in sit_for (timeout=timeout@entry=0x7a, reading=reading@entry=true, display_option=display_option@entry=1) at /u/xxx/src/emacs/29.4/src/dispnew.c:6263
 >    26  #8  0x000000000051e53b in read_char (commandflag=1, map=map@entry=0x713540069963, prev_event=0x0, used_mouse_menu=used_mouse_menu@entry=0x7f7fff03c6bb, end_time=end_time@entry=0x0) at /u/xxx/src/emacs/29.4/src/lisp.h:767
 >    27  #9  0x000000000051f5ec in read_key_sequence (keybuf=keybuf@entry=0x7f7fff03c790, prompt=prompt@entry=0x0, dont_downcase_last=dont_downcase_last@entry=false, can_return_switch_frame=can_return_switch_frame@entry=true, fix_current_buffer=fix_current_buffer@entry=true, prevent_redisplay=prevent_redisplay@entry=false) at /u/xxx/src/emacs/29.4/src/keyboard.c:10084
 >    28  #10 0x000000000052090f in command_loop_1 () at /u/xxx/src/emacs/29.4/src/keyboard.c:1384
 >    29  #11 0x0000000000583676 in internal_condition_case (bfun=bfun@entry=0x520748 <command_loop_1>, handlers=handlers@entry=0x90, hfun=hfun@entry=0x5166aa <cmd_error>) at /u/xxx/src/emacs/29.4/src/eval.c:1474
 >    30  #12 0x00000000005113fb in command_loop_2 (handlers=handlers@entry=0x90) at /u/xxx/src/emacs/29.4/src/keyboard.c:1133
 >    31  #13 0x00000000005835ef in internal_catch (tag=tag@entry=0xfe40, func=func@entry=0x5113dd <command_loop_2>, arg=arg@entry=0x90) at /u/xxx/src/emacs/29.4/src/eval.c:1197
 >    32  #14 0x00000000005113ba in command_loop () at /u/xxx/src/emacs/29.4/src/keyboard.c:1111
 >    33  #15 0x00000000005162b8 in recursive_edit_1 () at /u/xxx/src/emacs/29.4/src/keyboard.c:720
 >    34  #16 0x00000000005165dd in Frecursive_edit () at /u/xxx/src/emacs/29.4/src/keyboard.c:803
 >    35  #17 0x0000000000510982 in main (argc=2, argv=0x7f7fff03cad8) at /u/xxx/src/emacs/29.4/src/emacs.c:2521

Thanks.  I attach a new diff to override the previous one which should
now trace all changes from fullheight to nil and vice-versa.  Please set
the breakpoints as you did before in the lines marked with // break here.

The new diff differs from the previous one in that I now trace the
fullheight behavior.  It's similar to the fullscreen one but one can run
it in one and the same virtual space and this make sure that no space
switches interfere.

Run Emacs as above, but zoom the Emacs window vertically verifying that
breakpoints are hit twice - type c to continue in each case - and then
restart CTWM.  Now things should get interesting.  At the first time a
breakpoint is hit do

p lval

It gets me 2 here.  Type c and wait if you get a second hit.  If so type

p lval

again.  This gets me 0 here and we reset the fullscreen parameter to
nil.  Please confirm whether you see the same.

Whatever the cause of this behavior is I observed the following here:

In x_net_wm_state we do

   int value = FULLSCREEN_NONE;

   x_get_current_wm_state (f, window, &value, &sticky, &shaded);

In x_get_current_wm_state we now get in the call for the first
breakpoint mentioned above an actual_size > 0 regardless of whether
xcb_get_property is used or XGetWindowProperty.  For the call of the
second breakpoint we get actual_size = 0 so the result of the call is
probably meaningless.  We use it nevertheless and so we reset the
fullscreen parameter to nil.

Unfortunately, fixing that behavior was of no help here.  Even if I do
not process calls where actual_size is 0 and the 'fullheight' value is
preserved (so Emacs does not reset fullheight by itself), it seems that
CTWM thinks that the window is no more fullheight with the old
fullheight sizes becoming the new normal sizes.

Note also that CTWM restart here triggers the following sequence of events

UnmapNotify, visible | iconified
MapNotify, not hidden & iconified, PS=754x976
UnmapNotify, visible | iconified
PropertyNotify, not hidden & iconified
MapNotify, not hidden & not iconified, PS=754x976

which means that the frame gets unmapped and iconified in between and
only then restored.  Whether and how these affect the fullscreen
behavior is yet unclear to me.

martin

diff --git a/src/xterm.c b/src/xterm.c
index fd3e58f85f6..ef354a54d1e 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -18041,7 +18041,12 @@ x_net_wm_state (struct frame *f, Window window)
       break;
     }
 
-  store_frame_param (f, Qfullscreen, lval);
+  if (EQ (get_frame_param (f, Qfullscreen), Qfullheight)
+      || EQ (lval, Qfullheight))
+    store_frame_param (f, Qfullscreen, lval); // break here
+  else
+    store_frame_param (f, Qfullscreen, lval);
+
   store_frame_param (f, Qsticky, sticky ? Qt : Qnil);
   store_frame_param (f, Qshaded, shaded ? Qt : Qnil);
 }
@@ -21573,6 +21578,14 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 
 #endif /* !defined USE_X_TOOLKIT && !defined USE_GTK */
 
+      if (f && CONSP (frame_size_history))
+	frame_size_history_extra
+	  (f, build_string ("ConfigureNotify"),
+	   FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f),
+	       configureEvent.xconfigure.width,
+	   configureEvent.xconfigure.height,
+	   f->new_width, f->new_height);
+
 #ifdef USE_GTK
       if (!f
 	  && (f = any)
@@ -21581,15 +21594,6 @@ handle_one_xevent (struct x_display_info *dpyinfo,
 	      || !(configureEvent.xconfigure.width <= 1
 		   && configureEvent.xconfigure.height <= 1)))
         {
-
-	  if (CONSP (frame_size_history))
-	    frame_size_history_extra
-	      (f, build_string ("ConfigureNotify"),
-	       FRAME_PIXEL_WIDTH (f), FRAME_PIXEL_HEIGHT (f),
-	       configureEvent.xconfigure.width,
-	       configureEvent.xconfigure.height,
-	       f->new_width, f->new_height);
-
 #ifdef HAVE_XDBE
           if (FRAME_X_DOUBLE_BUFFERED_P (f))
             x_drop_xrender_surfaces (f);
@@ -28272,7 +28276,12 @@ x_handle_net_wm_state (struct frame *f, const XPropertyEvent *event)
       break;
     }
 
-  store_frame_param (f, Qfullscreen, lval);
+  if (EQ (get_frame_param (f, Qfullscreen), Qfullheight)
+      || EQ (lval, Qfullheight))
+    store_frame_param (f, Qfullscreen, lval); // break here
+  else
+    store_frame_param (f, Qfullscreen, lval);
+
   store_frame_param (f, Qsticky, sticky ? Qt : Qnil);
   store_frame_param (f, Qshaded, shaded ? Qt : Qnil);
 
@@ -28340,7 +28349,11 @@ x_check_fullscreen (struct frame *f)
 
   /* `x_net_wm_state' might have reset the fullscreen frame parameter,
      restore it. */
-  store_frame_param (f, Qfullscreen, lval);
+  if (EQ (get_frame_param (f, Qfullscreen), Qfullheight)
+      || EQ (lval, Qfullheight))
+    store_frame_param (f, Qfullscreen, lval); // break here
+  else
+    store_frame_param (f, Qfullscreen, lval);
 }
 
 /* This function is called by x_set_offset to determine whether the window