Re: Bug in CVS Emacs frame positioning under X

[Jan D.]

Fran Litterio wrote:

> Frankly, this is not an elegant solution -- it is periliously close to
> busy-waiting (except that XSync() blocks). But my testing has shown
> that it guarantees that we know the up-to-date position of
> recently-moved frames, which is a requirement for detecting Type A
> window managers (i.e., WMs that misposition frames by the width and
> height of the WM decorations). Also, the loop in x_sync_with_move() is
> bounded so that it cannot excessively consume cycles.

It is the window manager that intercepts the X events to and from Emacs that 
introduces this timing problems.  Some WMs are better that others, with 
sawfish the loop count rarely goes over 1 or 2, with metacity 45-48 is not 
uncommon.  I didn't want a blocking solution, but if that is the way to go, so 
be it.

> I am currently running with this patch installed, and I'm seeing
> completely deterministic frame positioning that accurately
> compenstates for my Type A window manager (FVWM). I will test it
> against twm and mwm.  If others can test it against their favorite WM,
> I would greatly appreciate it.

I've cleaned up your patch so it is more in the Gnu coding style (attached). 
I'll test it on some more window managers later.

        Jan D.

Index: src/xterm.h
*** src/xterm.h.orig    2006-03-21 15:31:30.000000000 +0100
--- src/xterm.h 2006-04-03 09:01:38.000000000 +0200
***************
*** 637,654 ****
       FocusOut and LeaveNotify clears EXPLICIT/IMPLICIT. */
    int focus_state;
  
-   /* The latest move we made to FRAME_OUTER_WINDOW.  Saved so we can
-      compensate for type A WMs (see wm_type in dpyinfo above).  */
-   int expected_top;
-   int expected_left;
- 
    /* The offset we need to add to compensate for type A WMs.  */
    int move_offset_top;
    int move_offset_left;
  
!   /* Nonzero if we have made a move and needs to check if the WM placed us
!      at the right position.  */
!   int check_expected_move;
  };
  
  #define No_Cursor (None)
--- 637,650 ----
       FocusOut and LeaveNotify clears EXPLICIT/IMPLICIT. */
    int focus_state;
  
    /* The offset we need to add to compensate for type A WMs.  */
    int move_offset_top;
    int move_offset_left;
  
!   /* The frame's left/top offsets before we call XMoveWindow.  See
!      x_check_expected_move.  */
!   int left_before_move;
!   int top_before_move;
  };
  
  #define No_Cursor (None)
Index: src/xterm.c
*** src/xterm.c.orig    2006-04-03 08:30:23.000000000 +0200
--- src/xterm.c 2006-04-03 09:03:25.000000000 +0200
***************
*** 366,372 ****
                                            Lisp_Object *, Lisp_Object *,
                                            unsigned long *));
  static void x_check_fullscreen P_ ((struct frame *));
! static void x_check_expected_move P_ ((struct frame *));
  static int handle_one_xevent P_ ((struct x_display_info *, XEvent *,
                                  int *, struct input_event *));
  
--- 366,373 ----
                                            Lisp_Object *, Lisp_Object *,
                                            unsigned long *));
  static void x_check_fullscreen P_ ((struct frame *));
! static void x_check_expected_move P_ ((struct frame *, int, int));
! static void x_sync_with_move P_ ((struct frame *, int, int, int));
  static int handle_one_xevent P_ ((struct x_display_info *, XEvent *,
                                  int *, struct input_event *));
  
***************
*** 6661,6671 ****
                && GTK_WIDGET_MAPPED (FRAME_GTK_OUTER_WIDGET (f)))
  #endif
              {
-             /* What we have now is the position of Emacs's own window.
-                Convert that to the position of the window manager window.  */
              x_real_positions (f, &f->left_pos, &f->top_pos);
  
-             x_check_expected_move (f);
              if (f->want_fullscreen & FULLSCREEN_WAIT)
                f->want_fullscreen &= ~(FULLSCREEN_WAIT|FULLSCREEN_BOTH);
              }
--- 6662,6669 ----
***************
*** 8212,8219 ****
  {
    int modified_top, modified_left;
  
!   if (change_gravity > 0)
      {
        f->top_pos = yoff;
        f->left_pos = xoff;
        f->size_hint_flags &= ~ (XNegative | YNegative);
--- 8210,8220 ----
  {
    int modified_top, modified_left;
  
!   if (change_gravity != 0)
      {
+       FRAME_X_OUTPUT (f)->left_before_move = f->left_pos;
+       FRAME_X_OUTPUT (f)->top_before_move = f->top_pos;
+ 
        f->top_pos = yoff;
        f->left_pos = xoff;
        f->size_hint_flags &= ~ (XNegative | YNegative);
***************
*** 8231,8237 ****
    modified_left = f->left_pos;
    modified_top = f->top_pos;
  
!   if (FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_A)
      {
        /* Some WMs (twm, wmaker at least) has an offset that is smaller
           than the WM decorations.  So we use the calculated offset instead
--- 8232,8238 ----
    modified_left = f->left_pos;
    modified_top = f->top_pos;
  
!   if (change_gravity != 0 && FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_A)
      {
        /* Some WMs (twm, wmaker at least) has an offset that is smaller
           than the WM decorations.  So we use the calculated offset instead
***************
*** 8243,8255 ****
    XMoveWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
                 modified_left, modified_top);
  
!   if (FRAME_VISIBLE_P (f)
!       && FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_UNKNOWN)
!     {
!       FRAME_X_OUTPUT (f)->check_expected_move = 1;
!       FRAME_X_OUTPUT (f)->expected_top = f->top_pos;
!       FRAME_X_OUTPUT (f)->expected_left = f->left_pos;
!     }
  
    UNBLOCK_INPUT;
  }
--- 8244,8269 ----
    XMoveWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
                 modified_left, modified_top);
  
!   x_sync_with_move (f, f->left_pos, f->top_pos,
!                     FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_UNKNOWN
!                     ? 1 : 0);
! 
!   /* change_gravity is non-zero when this function is called from Lisp to
!      programmatically move a frame.  In that case, we call
!      x_check_expected_move to discover if we have a "Type A" or "Type B"
!      window manager, and, for a "Type A" window manager, adjust the position
!      of the frame.
! 
!      We call x_check_expected_move if a programmatic move occurred, and
!      either the window manager type (A/B) is unknown or it is Type A but we
!      need to compute the top/left offset adjustment for this frame.  */
! 
!   if (change_gravity != 0 &&
!       (FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_UNKNOWN
!        || (FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_A
!            && (FRAME_X_OUTPUT (f)->move_offset_left == 0
!                && FRAME_X_OUTPUT (f)->move_offset_top == 0))))
!     x_check_expected_move (f, modified_left, modified_top);
  
    UNBLOCK_INPUT;
  }
***************
*** 8284,8320 ****
      }
  }
  
! /* If frame parameters are set after the frame is mapped, we need to move
!    the window.
!    Some window managers moves the window to the right position, some
!    moves the outer window manager window to the specified position.
!    Here we check that we are in the right spot.  If not, make a second
!    move, assuming we are dealing with the second kind of window manager. */
  static void
! x_check_expected_move (f)
       struct frame *f;
  {
!   if (FRAME_X_OUTPUT (f)->check_expected_move)
!   {
!     int expect_top = FRAME_X_OUTPUT (f)->expected_top;
!     int expect_left = FRAME_X_OUTPUT (f)->expected_left;
  
!     if (expect_top != f->top_pos || expect_left != f->left_pos)
        {
          FRAME_X_DISPLAY_INFO (f)->wm_type = X_WMTYPE_A;
!         FRAME_X_OUTPUT (f)->move_offset_left = expect_left - f->left_pos;
!         FRAME_X_OUTPUT (f)->move_offset_top = expect_top - f->top_pos;
  
!         f->left_pos = expect_left;
!         f->top_pos = expect_top;
!         x_set_offset (f, expect_left, expect_top, 0);
        }
!     else if (FRAME_X_DISPLAY_INFO (f)->wm_type == X_WMTYPE_UNKNOWN)
        FRAME_X_DISPLAY_INFO (f)->wm_type = X_WMTYPE_B;
  
!     /* Just do this once */
!     FRAME_X_OUTPUT (f)->check_expected_move = 0;
    }
  }
  
  
--- 8298,8395 ----
      }
  }
  
! /* This function is called by x_set_offset to determine whether the window
!    manager interfered with the positioning of the frame.  Type A window
!    managers position the surrounding window manager decorations a small
!    amount above and left of the user-supplied position.  Type B window
!    managers position the surrounding window manager decorations at the
!    user-specified position.  If we detect a Type A window manager, we
!    compensate by moving the window right and down by the proper amount.  */
! 
  static void
! x_check_expected_move (f, expected_left, expected_top)
       struct frame *f;
+      int expected_left;
+      int expected_top;
  {
!   int count = 0, current_left = 0, current_top = 0;
  
!   /* x_real_positions returns the left and top offsets of the outermost
!      window manager window around the frame.  */
! 
!   x_real_positions (f, &current_left, &current_top);
! 
!   if (current_left != expected_left || current_top != expected_top)
      {
+       /* It's a "Type A" window manager. */
+ 
+       int adjusted_left;
+       int adjusted_top;
+ 
        FRAME_X_DISPLAY_INFO (f)->wm_type = X_WMTYPE_A;
!       FRAME_X_OUTPUT (f)->move_offset_left = expected_left - current_left;
!       FRAME_X_OUTPUT (f)->move_offset_top = expected_top - current_top;
  
!       /* Now fix the mispositioned frame's location. */
! 
!       adjusted_left = expected_left + FRAME_X_OUTPUT (f)->move_offset_left;
!       adjusted_top = expected_top + FRAME_X_OUTPUT (f)->move_offset_top;
! 
!       XMoveWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f),
!                    adjusted_left, adjusted_top);
! 
!       x_sync_with_move (f, expected_left, expected_top, 0);
      }
!   else
!     /* It's a "Type B" window manager.  We don't have to adjust the
!        frame's position. */
! 
      FRAME_X_DISPLAY_INFO (f)->wm_type = X_WMTYPE_B;
+ }
+ 
+ 
+ /* Wait for XGetGeometry to return up-to-date position information for a
+    recently-moved frame.  Call this immediately after calling XMoveWindow.
+    If FUZZY is non-zero, then LEFT and TOP are just estimates of where the
+    frame has been moved to, so we use a fuzzy position comparison instead
+    of an exact comparison.  */
  
! static void
! x_sync_with_move (f, left, top, fuzzy)
!     struct frame *f;
!     int left, top, fuzzy;
! {
!   int count = 0;
! 
!   while (count++ < 50)
!     {
!       int current_left = 0, current_top = 0;
! 
!       fprintf(stderr, "Count is %d\n", count);
! 
!       /* In theory, this call to XSync only needs to happen once, but in
!          practice, it doesn't seem to work, hence the need for the surrounding
!          loop.  */
! 
!       XSync (FRAME_X_DISPLAY (f), False);
!       x_real_positions (f, &current_left, &current_top);
! 
!       if (fuzzy)
!         {
!           /* The left fuzz-factor is 10 pixels.  The top fuzz-factor is 40
!              pixels.  */
! 
!           if (abs (current_left - left) <= 10 && abs (current_top - top) <= 
40)
!             return;
          }
+       else if (current_left == left && current_top == top)
+         return;
+     }
+ 
+   /* As a last resort, just wait 0.5 seconds and hope that XGetGeometry
+      will then return up-to-date position info. */
+ 
+   wait_reading_process_output (0, 500000, 0, 0, Qnil, NULL, 0);
  }