APPROVE COMMIT 21.5 RECOMMEND 21.4 Vin, there's a bunch of "future work" cruft here, so I don't necessarily recommend this patch as-is. Also, I haven't actually checked if it applies, and the backwards-compatibility variable might be a little bogus in the stable line. Let me know what, if anything of this you want. Thanks to Glynn Clements, without whose discussion I never would have figured this out, to the good folks at Sun, HP, etc, who funded Real Documentation that Actually Helps, and to the various users who kept reporting the issue even after we thought we (mostly) had it licked. I provided an earlier patch, about which I wrote: sjt> It is possible to inhibit this, with no huge ill-effect on sjt> XEmacs functionality. I'm supplying a patch, with this sjt> action controlled from Lisp by the variable `wedge-metacity', sjt> defaulting to nil. This patch is *superseded* by the following patch, which may not apply to 21.4 (sorry), and if you applied the earlier patch you'll have to revert it (sorry again). If you can apply it and are willing to test it, it should "just work", no need to fiddle with anything. I will work up a 21.4 patch per Vin's comments (if any) within a day or so. And it's being committed to 21.5 immediately. If you run with the patch and have minor problems (but enough that you're willing to avoid maximizing under metacity et al), you can restore the old buggy behavior with (setq wedge-metacity t). I'm pretty sure this patch is "right", although I haven't been able to test with metacity. (1) The Xt conventions say "don't call geometry request functions from the resize callback". This patch moves the request from a resize callback into a function explicitly intended to change size. The theory is right. (2) With the old behavior, the resize callback called itself recursively, at least once. Now it doesn't. (3) The fluxbox window manager now gives "true maximization" where it used to flash to the true maximum, then drop back to the largest cell-oriented window. Index: src/ChangeLog =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/ChangeLog,v retrieving revision 1.954 diff -u -r1.954 ChangeLog --- src/ChangeLog 6 May 2006 08:46:38 -0000 1.954 +++ src/ChangeLog 6 May 2006 17:26:00 -0000 ＠＠ -0,0 +1,26 ＠＠ +2006-04-30 Stephen J. Turnbull <stephen(a)xemacs.org&gt; + + Move geometry management from EmacsFrameResize to x_set_frame_size. + Should fix "metacity maximization" bug. + Provide (deprecated, temporary) backward compatibility option. + + * EmacsFrame.c (EmacsFrameResize): + * frame-x.c (x_set_frame_size): + Move call of ChangeEmacsManagerSize. + Don't bogusly notify WM about size changes the WM asked for. + + * console-x.c (wedge-metacity): New builtin Boolean Lisp variable. + * console-x-impl.h (wedge_metacity): Declare C variable. + * console-x.c (vars_of_console_x): New function to initialize it. + * symsinit.h (vars_of_console_x): declare it. + * emacs.c (main_1): Call vars_of_console_x. + + * EmacsFrameP.h (struct EmacsFrame): + * EmacsManager.c (ChangeEmacsManagerSize): + * EmacsShell-sub.c (SuperClassRootGeometryManager): + * console-x-impl.h (wedge_metacity): + Various comments, some improved documentation, mostly sad comments + on the state of the art of Xt programming. + + * frame-x.c (defi): #undef it. (Random code cleanliness.) + Index: src/EmacsFrame.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsFrame.c,v retrieving revision 1.28 diff -u -r1.28 EmacsFrame.c --- src/EmacsFrame.c 25 Oct 2005 11:16:20 -0000 1.28 +++ src/EmacsFrame.c 6 May 2006 17:26:00 -0000 ＠＠ -360,19 +360,32 ＠＠ pixel_to_char_size (f, ew->core.width, ew->core.height, &columns, &rows); change_frame_size (f, rows, columns, 0); - /* Now we tell the EmacsShell that we've changed the size of the non-fixed - portion of the frame. Note that, if we the resize occurred as a result - of EmacsFrameSetCharSize(), this information will be stored twice. - This is not a big deal, as storing this information doesn't actually - do anything until the next resize. */ - if (FRAME_X_TOP_LEVEL_FRAME_P (f)) - x_wm_set_variable_size (FRAME_X_SHELL_WIDGET (f), columns, rows); + /* The code below is just plain wrong. If the EmacsShell or EmacsManager + needs to know, they should just ask. If needed information is being + updated here, then we should set a dirty flag and have it updated on an + as-needed basis. + For now, conditionalize so people can get work done if this breaks + something. */ + if (wedge_metacity) /* cf. x_set_frame_size */ + { + /* Now we tell the EmacsShell that we've changed the size of the + non-fixed portion of the frame. Note that, if the resize occurred + as a result of EmacsFrameSetCharSize(), this information will be + stored twice. This is not a big deal, as storing this information + doesn't actually do anything until the next resize. */ + if (FRAME_X_TOP_LEVEL_FRAME_P (f)) + x_wm_set_variable_size (FRAME_X_SHELL_WIDGET (f), columns, rows); - /* Kick the manager so that it knows we've changed size. */ - req.request_mode = 0; - XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); - EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), repl.width, - repl.height); + /* Kick the manager so that it knows we've changed size. + #### No, no, no! If this does anything at all, it will involve + changing the manager's size. That's not something that a child + widget should initialize as part of a purely informational + method!! */ + req.request_mode = 0; + XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); + EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), + repl.width, repl.height); + } } static Boolean Index: src/EmacsFrameP.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsFrameP.h,v retrieving revision 1.7 diff -u -r1.7 EmacsFrameP.h --- src/EmacsFrameP.h 4 May 2003 02:34:35 -0000 1.7 +++ src/EmacsFrameP.h 6 May 2006 17:26:00 -0000 ＠＠ -65,13 +65,16 ＠＠ Boolean iconic; /* whether this frame is iconic */ /* The rest of this is crap and should be deleted. + #### Comments that start with + are fields that actually get referred + to somewhere aside from the init function. + I guess the "crap" has mostly moved to specifiers? */ Boolean minibuffer; /* 0: normal frames with minibuffers. * 1: frames without minibuffers * 2: minibuffer only. */ Boolean unsplittable; /* frame can only have one window */ - int internal_border_width; /* internal borders */ + int internal_border_width; /* + internal borders */ int scrollbar_width; /* width of frame vertical sb's */ int scrollbar_height; /* height of frame horizontal sb's */ int top_toolbar_height; /* height of top toolbar */ ＠＠ -82,9 +85,9 ＠＠ int bottom_toolbar_border_width; /* ... of bottom toolbar */ int left_toolbar_border_width; /* ... of left toolbar */ int right_toolbar_border_width; /* ... of right toolbar */ - Dimension toolbar_shadow_thickness; + Dimension toolbar_shadow_thickness; /* + of shadows */ unsigned char scrollbar_placement; - int interline; /* skips between lines */ + int interline; /* + skips between lines */ XFontStruct* font; /* font */ Pixel foreground_pixel; /* foreground */ ＠＠ -97,7 +100,7 ＠＠ int bell_volume; /* how loud is beep */ Boolean menubar_p; /* initially show a menubar? */ - Boolean initially_unmapped; /* inhibit initial window mapping */ + Boolean initially_unmapped; /* + inhibit initial window mapping */ Boolean use_backing_store; /* backing store for menubar & ew? */ Dimension preferred_width; /* if non-zero, preferred size for */ Index: src/EmacsManager.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsManager.c,v retrieving revision 1.7 diff -u -r1.7 EmacsManager.c --- src/EmacsManager.c 22 Dec 2004 10:59:09 -0000 1.7 +++ src/EmacsManager.c 6 May 2006 17:26:00 -0000 ＠＠ -241,6 +241,12 ＠＠ if (height == 0) height = w->core.height; + /* #### AFAICT this gets called in two places. One is in ChangeManaged(), + above. The other is in EmacsFrameResize(). Perhaps ChangeManaged() + should initiate resize requests, but EmacsFrameResize() should not. + Unfortunately, I've tried making this conditional on whether we're + called from EmacsFrameResize() or not, but that results in an infloop + via the callback to x_layout_widgets() in Resize(). Whee! */ /* do nothing if we're already that size */ if (w->core.width != width || w->core.height != height) { Index: src/EmacsShell-sub.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/EmacsShell-sub.c,v retrieving revision 1.7 diff -u -r1.7 EmacsShell-sub.c --- src/EmacsShell-sub.c 24 Jan 2005 23:33:46 -0000 1.7 +++ src/EmacsShell-sub.c 6 May 2006 17:26:00 -0000 ＠＠ -278,7 +278,8 ＠＠ GenericClassExtRec *gcer; /* find the shell extension record that specifies the - root geometry manager method */ + root geometry manager method + #### We could use XtGetClassExtension here. */ for (gcer = (GenericClassExtRec *) swc->shell_class.extension; gcer; gcer = (GenericClassExtRec *) gcer->next_extension) ＠＠ -287,6 +288,9 ＠＠ break; } + /* #### The R11.6.4 Xt specification says if we don't find NULLQUARK here, + we should assume root_geometry_manager = XtInheritRootGeometryManager. + Is that actually callable? */ if (!gcer) ABORT (); Index: src/console-x-impl.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/console-x-impl.h,v retrieving revision 1.5 diff -u -r1.5 console-x-impl.h --- src/console-x-impl.h 26 Nov 2005 11:46:07 -0000 1.5 +++ src/console-x-impl.h 6 May 2006 17:26:00 -0000 ＠＠ -40,6 +40,8 ＠＠ DECLARE_CONSOLE_TYPE (x); +extern int wedge_metacity; + struct x_device { #ifdef NEW_GC ＠＠ -234,7 +236,8 ＠＠ /* The maximum number of widgets that can be displayed above the text area at one time. Currently no more than 3 will ever actually be - displayed (menubar, psheet, debugger panel). */ + displayed (menubar, psheet, debugger panel). + #### Are "psheet" and "debugger panel" relevant any more? */ #define MAX_CONCURRENT_TOP_WIDGETS 8 struct x_frame ＠＠ -243,12 +246,17 ＠＠ struct lrecord_header header; #endif /* NEW_GC */ - /* The widget of this frame. This is an EmacsShell or an - ExternalShell. */ + /* The widget of this frame. + This is an EmacsShell or an ExternalShell. + It negotiates with the window manager or containing app on behalf of + the container widget. Should be (but isn't) invisible to Emacs. */ Widget widget; /* The parent of the EmacsFrame, the menubar, and the scrollbars. - This is an EmacsManager. */ + This is an EmacsManager. + It is responsible for managing the geometry of the frame. This is what + Emacs mostly talks to. Anything that affects its geometry will be + reflected in the Shell widget, and thus cause WM interaction. */ Widget container; /* The widget of the menubar, of whatever widget class it happens to be. */ Index: src/console-x.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/console-x.c,v retrieving revision 1.16 diff -u -r1.16 console-x.c --- src/console-x.c 17 Dec 2005 19:47:02 -0000 1.16 +++ src/console-x.c 6 May 2006 17:26:00 -0000 ＠＠ -41,6 +41,8 ＠＠ DEFINE_CONSOLE_TYPE (x); +int wedge_metacity; /* nonzero means update WM_HINTS always */ + extern void x_has_keysym (KeySym, Lisp_Object, int); static int ＠＠ -399,6 +401,22 ＠＠ CONSOLE_HAS_METHOD (x, perhaps_init_unseen_key_defaults); } + +void +vars_of_console_x (void) +{ + DEFVAR_BOOL ("wedge-metacity", &wedge_metacity /* +When non-nil, frame geometry management is backward-compatible. +This is known to create inflooping window jitter in metacity, et al. +It also does not conform to Xt conventions for geometry management. +Specifically, all frame resizes, XEmacs-initiated or not, update WM_HINTS. +Furthermore, geometry changes occur in the widget resize method. + +The default is nil. This probably gives correct behavior regardless of the +window manager used. +This variable is deprecated and will be removed. +*/ ); +} void reinit_console_type_create_x (void) Index: src/emacs.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/emacs.c,v retrieving revision 1.166 diff -u -r1.166 emacs.c --- src/emacs.c 25 Apr 2006 14:02:08 -0000 1.166 +++ src/emacs.c 6 May 2006 17:26:01 -0000 ＠＠ -2186,6 +2186,7 ＠＠ #ifdef HAVE_BALLOON_HELP vars_of_balloon_x (); #endif + vars_of_console_x (); vars_of_device_x (); #ifdef HAVE_X_DIALOGS vars_of_dialog_x (); Index: src/frame-x.c =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/frame-x.c,v retrieving revision 1.72 diff -u -r1.72 frame-x.c --- src/frame-x.c 26 Nov 2005 11:46:08 -0000 1.72 +++ src/frame-x.c 6 May 2006 17:26:01 -0000 ＠＠ -507,6 +507,7 ＠＠ defi(Qtop, XtNy); #undef def +#undef defi } static Lisp_Object ＠＠ -2307,6 +2308,17 ＠＠ x_set_frame_size (struct frame *f, int cols, int rows) { EmacsFrameSetCharSize (FRAME_X_TEXT_WIDGET (f), cols, rows); + + if (!wedge_metacity) /* cf. EmacsFrameResize */ + { + /* Kick the manager so that it knows we've changed size. */ + XtWidgetGeometry req, repl; + req.request_mode = 0; + XtQueryGeometry (FRAME_X_CONTAINER_WIDGET (f), &req, &repl); + EmacsManagerChangeSize (FRAME_X_CONTAINER_WIDGET (f), + repl.width, repl.height); + } + #if 0 /* this is not correct. x_set_frame_size() is called from Fset_frame_size(), which may or may not have been called Index: src/symsinit.h =================================================================== RCS file: /pack/xemacscvs/XEmacs/xemacs/src/symsinit.h,v retrieving revision 1.57 diff -u -r1.57 symsinit.h --- src/symsinit.h 25 Apr 2006 14:02:09 -0000 1.57 +++ src/symsinit.h 6 May 2006 17:26:01 -0000 ＠＠ -306,6 +306,7 ＠＠ void vars_of_console_stream (void); void vars_of_console_mswindows (void); void vars_of_console_tty (void); +void vars_of_console_x (void); void vars_of_data (void); void vars_of_database (void); void vars_of_debug (void); -- School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN Ask not how you can "do" free software business; ask what your business can "do for" free software.