From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: "Fran Litterio" Newsgroups: gmane.emacs.devel Subject: Re: Bug in CVS Emacs frame positioning under X Date: Sat, 1 Apr 2006 10:38:55 -0500 Message-ID: References: NNTP-Posting-Host: main.gmane.org Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable X-Trace: sea.gmane.org 1143905964 25191 80.91.229.2 (1 Apr 2006 15:39:24 GMT) X-Complaints-To: usenet@sea.gmane.org NNTP-Posting-Date: Sat, 1 Apr 2006 15:39:24 +0000 (UTC) Cc: =?ISO-8859-1?Q?Jan_Dj=E4rv?= , Richard Stallman Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Sat Apr 01 17:39:20 2006 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([199.232.76.165]) by ciao.gmane.org with esmtp (Exim 4.43) id 1FPiBz-00084t-4z for ged-emacs-devel@m.gmane.org; Sat, 01 Apr 2006 17:39:11 +0200 Original-Received: from localhost ([127.0.0.1] helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1FPiBy-0005xx-Gn for ged-emacs-devel@m.gmane.org; Sat, 01 Apr 2006 10:39:10 -0500 Original-Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1FPiBn-0005xg-48 for emacs-devel@gnu.org; Sat, 01 Apr 2006 10:38:59 -0500 Original-Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1FPiBl-0005xU-GD for emacs-devel@gnu.org; Sat, 01 Apr 2006 10:38:57 -0500 Original-Received: from [199.232.76.173] (helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1FPiBl-0005xR-BC for emacs-devel@gnu.org; Sat, 01 Apr 2006 10:38:57 -0500 Original-Received: from [64.233.182.185] (helo=nproxy.gmail.com) by monty-python.gnu.org with esmtp (Exim 4.52) id 1FPiEO-0002yK-Vt for emacs-devel@gnu.org; Sat, 01 Apr 2006 10:41:41 -0500 Original-Received: by nproxy.gmail.com with SMTP id l36so345308nfa for ; Sat, 01 Apr 2006 07:38:55 -0800 (PST) DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=h7IQ/LQXdI+gNUMNOdmbO+awEJN9qZ1K/fcBzTmj0eJMXbYr1OCANfN8+34KtoFVHMhFdmnIjD/MOsukUc9yzh7HimA1TBvzVOGiG3gsVQ87ZaxVBCI2rh/kNS5a+hp8/rFnyz4EBgSEOt7K/ZZb6oQSuha08sF2ycOZ39euSAA= Original-Received: by 10.48.161.6 with SMTP id j6mr683925nfe; Sat, 01 Apr 2006 07:38:55 -0800 (PST) Original-Received: by 10.49.23.5 with HTTP; Sat, 1 Apr 2006 07:38:55 -0800 (PST) Original-To: emacs-devel@gnu.org In-Reply-To: Content-Disposition: inline X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:52290 Archived-At: On 3/29/06, Richard Stallman wrote: > It could be that we could solve this by completely reorganizing the > way that code works. The person who wrote it was not the best > programmer, and I then tried to fix it up with just a partial > understanding of how to use Xt. I can't claim to be an expert at X programming, but I've changed the frame positioning algorithm in src/xterm.c so that it copes with the non-deterministic nature of X event timing, and so that it fixes the frame positioning bug I reported earlier in this thread. Patches to src/xterm.h and src/xterm.c are below. Here's what I did: First, I tried Jan Dj=E4rv's suggestion to call XSync() to force Xlib to wait for the X server to handle the XMoveWindow() request, but that failed to make XGetGeometry() see up-to-date position information for the just-moved frame. Instead, I wrote a new function, x_sync_with_move() that uses XSync() in a loop to make sure XGetGeometry() sees up-to-date position information after a frame is moved. 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. Then I changed x_set_offset() to perform these calls in this order: XMoveWindow() x_sync_with_move() x_check_expected_move() I rewrote x_check_expected_move() so that it calls XMoveWindow() directly to compensate for Type A window managers and the top/left adjustments are recorded for future use with that frame. The adjustments are stored in struct x_output, so the behavior is correct even if the user's window manager draws differently sized decorations around different Emacs frames (e.g., FVWM can be configured to do this). I removed the call to x_check_expected_move() from the code that handles ConfigureNotify events, because it is now called synchronously immediately after XMoveWindow(). 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. A good acid test is to evaluate the following Elisp: (dolist (i '(1 2 3 4 5 6 7 8 9 a b c d e)) (let ((frame (make-frame '((top . 50) (left . 50))))) (set-frame-position frame 200 200) (set-frame-position frame 300 300))) All of the frames should end up exactly at x/y position 300/300. -- Francis Litterio flitterio@gmail.com http://world.std.com/~franl/ GPG and PGP public keys available on keyservers. Index: xterm.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsroot/emacs/emacs/src/xterm.h,v retrieving revision 1.183 diff -w -u -r1.183 xterm.h --- xterm.h=0918 Mar 2006 13:49:11 -0000=091.183 +++ xterm.h=091 Apr 2006 13:56:52 -0000 @@ -637,18 +637,14 @@ 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; + /* 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: xterm.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /cvsroot/emacs/emacs/src/xterm.c,v retrieving revision 1.909 diff -w -u -r1.909 xterm.c --- xterm.c=0924 Mar 2006 15:24:20 -0000=091.909 +++ xterm.c=091 Apr 2006 13:58:21 -0000 @@ -366,7 +366,8 @@ =09=09=09=09=09 Lisp_Object *, Lisp_Object *, =09=09=09=09=09 unsigned long *)); static void x_check_fullscreen P_ ((struct frame *)); -static void x_check_expected_move P_ ((struct frame *)); +static void x_check_expected_move P_ ((struct frame *, int, int)); +static void x_sync_with_move(struct frame *, int, int, int); static int handle_one_xevent P_ ((struct x_display_info *, XEvent *, =09=09=09=09 int *, struct input_event *)); @@ -6661,11 +6662,8 @@ && GTK_WIDGET_MAPPED (FRAME_GTK_OUTER_WIDGET (f))) #endif { -=09 /* What we have now is the position of Emacs's own window. -=09=09 Convert that to the position of the window manager window. */ =09 x_real_positions (f, &f->left_pos, &f->top_pos); -=09 x_check_expected_move (f); =09 if (f->want_fullscreen & FULLSCREEN_WAIT) =09=09f->want_fullscreen &=3D ~(FULLSCREEN_WAIT|FULLSCREEN_BOTH); } @@ -8212,8 +8210,11 @@ { int modified_top, modified_left; - if (change_gravity > 0) + if (change_gravity !=3D 0) { + FRAME_X_OUTPUT (f)->left_before_move =3D f->left_pos; + FRAME_X_OUTPUT (f)->top_before_move =3D f->top_pos; + f->top_pos =3D yoff; f->left_pos =3D xoff; f->size_hint_flags &=3D ~ (XNegative | YNegative); @@ -8231,7 +8232,7 @@ modified_left =3D f->left_pos; modified_top =3D f->top_pos; - if (FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D X_WMTYPE_A) + if (change_gravity !=3D 0 && FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D 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,12 +8244,26 @@ 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 =3D=3D X_WMTYPE_UNKNOWN) + x_sync_with_move(f, f->left_pos, f->top_pos, +=09=09 FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D 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 positi= on + 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 w= e + need to compute the top/left offset adjustment for this frame. + */ + + if (change_gravity !=3D 0 && + (FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D X_WMTYPE_UNKNOWN || + (FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D X_WMTYPE_A && +=09(FRAME_X_OUTPUT (f)->move_offset_left =3D=3D 0 && FRAME_X_OUTPUT (f)->move_offset_top =3D=3D 0)))) { - FRAME_X_OUTPUT (f)->check_expected_move =3D 1; - FRAME_X_OUTPUT (f)->expected_top =3D f->top_pos; - FRAME_X_OUTPUT (f)->expected_left =3D f->left_pos; + x_check_expected_move(f, modified_left, modified_top); } UNBLOCK_INPUT; @@ -8284,37 +8299,93 @@ } } -/* 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. *= / +/* This function is called by x_set_offset() to determine whether the wind= ow + 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) +x_check_expected_move (f, expected_left, expected_top) struct frame *f; + int expected_left; + int expected_top; { - if (FRAME_X_OUTPUT (f)->check_expected_move) - { - int expect_top =3D FRAME_X_OUTPUT (f)->expected_top; - int expect_left =3D FRAME_X_OUTPUT (f)->expected_left; + int count =3D 0, current_left =3D 0, current_top =3D 0; + + /* x_real_positions() returns the left and top offsets of the outermost + window manager window around the frame. */ + + x_real_positions (f, ¤t_left, ¤t_top); - if (expect_top !=3D f->top_pos || expect_left !=3D f->left_pos) + if (current_left !=3D expected_left || current_top !=3D expected_top) { + /* It's a "Type A" window manager. */ + FRAME_X_DISPLAY_INFO (f)->wm_type =3D X_WMTYPE_A; - FRAME_X_OUTPUT (f)->move_offset_left =3D expect_left - f->left_pos= ; - FRAME_X_OUTPUT (f)->move_offset_top =3D expect_top - f->top_pos; + FRAME_X_OUTPUT (f)->move_offset_left =3D expected_left - current_left; + FRAME_X_OUTPUT (f)->move_offset_top =3D expected_top - current_top; + + /* Now fix the mispositioned frame's location. */ - f->left_pos =3D expect_left; - f->top_pos =3D expect_top; - x_set_offset (f, expect_left, expect_top, 0); + int adjusted_left =3D expected_left + FRAME_X_OUTPUT (f)->move_offset_= left; + int adjusted_top =3D expected_top + FRAME_X_OUTPUT (f)->move_offset_to= p; + + XMoveWindow (FRAME_X_DISPLAY (f), FRAME_OUTER_WINDOW (f), +=09=09 adjusted_left, adjusted_top); + + x_sync_with_move(f, expected_left, expected_top, 0); } - else if (FRAME_X_DISPLAY_INFO (f)->wm_type =3D=3D X_WMTYPE_UNKNOWN) + 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 =3D X_WMTYPE_B; + } +} + - /* Just do this once */ - FRAME_X_OUTPUT (f)->check_expected_move =3D 0; +/* 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 =3D 0; + + while (count++ < 50) + { + int current_left =3D 0, current_top =3D 0; + + /* 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, ¤t_left, ¤t_top); + + if (fuzzy) + { + /* The left fuzz-factor is 10 pixels. The top fuzz-factor is 40 pixels. */ + + if (abs(current_left - left) <=3D 10 && abs(current_top - top) <=3D = 40) +=09return; } + else if (current_left =3D=3D left && current_top =3D=3D 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); }