11:21 a.m.
21.5 APPROVED
Sometimes I'm fine with the usual geometry of frames, sometimes I want
them vertically maximized (which the quartzwm doesn't provide an easy
way to do), so I have a Lisp function to do it. That function was
unreliable because #'device-system-metric was. This patch fixes it.
History and analysis:
Several months ago I got an external monitor, which when plugged in to
my notebook automatically changes the dimensions of the X screen from
1280x800 to something like 2560x1420. #'device-system-metric doesn't
recognize those changes. I believe that's because Xlib caches screen
metrics in the screen struct, and doesn't catch the change on the fly.
The information on the root window, which of course fills the screen,
*is* updated (I assume this costs a protocol request, but this
function is not used often -- not at all in core Lisp as far as I
could tell). The patch changes x_device_system_metrics to get the
geometry of the root window rather than using the ScreenWidth and
ScreenHeight macros. It does the trick in light testing.
It also changes x_set_frame_position and balloon_help_create to use
Fdevice_system_metric for the same reason (specifically, if the screen
shrinks and a small window is positioned against the right or bottom
edges, it could easily end up off-screen!)
I considered selecting ConfigureNotify and caching in XEmacs, but
decided against it. The rationale is in the source (see attached patch).
diff -r 2d6c322fbf80 src/ChangeLog
--- a/src/ChangeLog Thu Mar 28 19:10:32 2019 +0000
+++ b/src/ChangeLog Fri Apr 05 19:02:12 2019 +0900
＠＠ -1,3 +1,16 ＠＠
+2019-04-02 Stephen J. Turnbull <turnbull(a)sk.tsukuba.ac.jp>
+
+ * device-x.c (x_device_system_metrics):
+ Use geometry of root window to get pixel size of display, not the
+ Display* macros. The Xlib macros (and functions!) seem to cache
+ old values, which gives wrong results on displays that can change
+ size on the fly.
+
+ * frame-x.c (x_set_frame_position):
+ * balloon_help.c (balloon_help_create):
+ Use Fdevice_system_metric for width and height in case screen size
+ has changed. Change "frame_[wh]" to "screen_[wh]".
+
2019-03-23 Jerry James <james(a)xemacs.org>
* array.h (XD_DYNARR_DESC,XD_LISP_DYNARR_DESC): fix segfaults when
diff -r 2d6c322fbf80 src/balloon_help.c
--- a/src/balloon_help.c Thu Mar 28 19:10:32 2019 +0000
+++ b/src/balloon_help.c Fri Apr 05 19:02:12 2019 +0900
＠＠ -507,6 +507,8 ＠＠
Pixel fg, Pixel bg, Pixel shine, Pixel shadow,
XFontStruct* font)
{
+ Lisp_Object root_size;
+
if (b_dpy) balloon_help_destroy ();
b_dpy = dpy;
＠＠ -526,8 +528,11 ＠＠
b_timer = None;
b_delay = 500;
- b_screenWidth = DisplayWidth (b_dpy, DefaultScreen(b_dpy));
- b_screenHeight = DisplayHeight (b_dpy, DefaultScreen(b_dpy));
+ root_size = Fdevice_system_metric (get_device_from_display (dpy),
+ Qsize_device,
+ Qnil);
+ b_screenWidth = XFIXNUM (XCAR (root_size));
+ b_screenHeight = XFIXNUM (XCDR (root_size));
b_lastShape = SHAPE_CONE_FREE;
}
diff -r 2d6c322fbf80 src/device-x.c
--- a/src/device-x.c Thu Mar 28 19:10:32 2019 +0000
+++ b/src/device-x.c Fri Apr 05 19:02:12 2019 +0900
＠＠ -1755,12 +1755,29 ＠＠
switch (m)
{
+ /* In Xlib, both macros and functions checking configuration
+ seem to cache the results from the first call. So we check on
+ the root window instead. I considered selecting ConfigureNotify
+ on the root window, checking the size on that event, and caching
+ the result in struct x_device. However, ConfigureNotify doesn't
+ seem to be used that way for any other device metrics, so
+ responding to the event immediately doesn't seem necessary, and
+ it would involve complexity I don't understand. I guess this is
+ because Xlib does so much caching itself, so that using Xlib
+ calls is not very expensive for rarely changing metrics. */
case DM_size_device:
- return Fcons (make_fixnum (DisplayWidth (dpy, DefaultScreen (dpy))),
- make_fixnum (DisplayHeight (dpy, DefaultScreen (dpy))));
+ {
+ int x, y;
+ /* Nobody would be running X on a smaller screen, right? */
+ Dimension w = 1024, h = 768, b, d;
+ Window root = RootWindow(dpy, DefaultScreen(dpy));
+ /* If this fails, Xlib promises w and h are not updated. */
+ XGetGeometry(dpy, root, &root, &x, &y, &w, &h, &b, &d);
+ Fcons (make_fixnum (w), make_fixnum (h));
+ }
case DM_size_device_mm:
- return Fcons (make_fixnum (DisplayWidthMM (dpy, DefaultScreen (dpy))),
- make_fixnum (DisplayHeightMM (dpy, DefaultScreen (dpy))));
+ return Fcons (make_fixnum (XDisplayWidthMM (dpy, DefaultScreen (dpy))),
+ make_fixnum (XDisplayHeightMM (dpy, DefaultScreen (dpy))));
case DM_num_bit_planes:
return make_fixnum (DisplayPlanes (dpy, DefaultScreen (dpy)));
case DM_num_color_cells:
diff -r 2d6c322fbf80 src/frame-x.c
--- a/src/frame-x.c Thu Mar 28 19:10:32 2019 +0000
+++ b/src/frame-x.c Fri Apr 05 19:02:12 2019 +0900
＠＠ -2228,8 +2228,11 ＠＠
{
Widget w = FRAME_X_SHELL_WIDGET (f);
Display *dpy = XtDisplay (w);
- Dimension frame_w = DisplayWidth (dpy, DefaultScreen (dpy));
- Dimension frame_h = DisplayHeight (dpy, DefaultScreen (dpy));
+ Lisp_Object root_size = Fdevice_system_metric (FRAME_DEVICE (f),
+ Qsize_device,
+ Qnil);
+ Dimension screen_w = XFIXNUM (XCAR (root_size));
+ Dimension screen_h = XFIXNUM (XCDR (root_size));
Dimension shell_w, shell_h, shell_bord;
int win_gravity;
Arg al[3];
＠＠ -2245,9 +2248,9 ＠＠
yoff >= 0 ? NorthEastGravity :
SouthEastGravity;
if (xoff < 0)
- xoff += frame_w - shell_w - 2*shell_bord;
+ xoff += screen_w - shell_w - 2*shell_bord;
if (yoff < 0)
- yoff += frame_h - shell_h - 2*shell_bord;
+ yoff += screen_h - shell_h - 2*shell_bord;
/* Update the hints so that, if this window is currently iconified, it will
come back at the right place. We can't look at s->visible to determine
1:30 p.m.
New subject: [COMMIT] Get Stephen's last change compiling, fix a clear bug.
Thanks for the change, Steve. As far as I can see your reasoning applies
equally to Display{Width,Height}MM and probably the other system metrics, but
I don’t see an easy way of forcing X11 to return an up-to-date value (as you
comment, the functions just return the same cached values that the macros do).
APPROVE COMMIT
NOTE: This patch has been committed.
# HG changeset patch
# User Aidan Kehoe <kehoea(a)parhasard.net>
# Date 1555244583 -3600
# Sun Apr 14 13:23:03 2019 +0100
# Node ID a975482ad82db38730ffaaf33e0db6092f7cf779
# Parent 60e40afb6fbede36786e8fe8c3f74025c798723a
Get Stephen's last change compiling, fix a clear bug.
src/ChangeLog addition:
2019-04-14 Aidan Kehoe <kehoea(a)parhasard.net>
Get Stephen's last change compiling, fix a clear bug.
* balloon_help.c:
Make the types, macros from from lisp.h, device.h, console-x.h
available here.
Use the XEmacs Boolint rather than making a new boolean.
* balloon_help.c (show_help):
* balloon_help.c (balloon_help_create):
Wrap the pointer to the device returned by
get_device_from_display(), making it a Lisp_Object, as we want.
* balloon_help.c (balloon_help_hide):
* device-x.c (x_device_system_metrics):
Declare the parameters to XGetGeometry using the documented types.
Return the pixel size as intended, rather than falling through to
the DM_size_device_mm case.
* device.c:
Move Qsize_device to general-slots.h.
* frame-x.c (x_set_frame_position):
DPY isn't used, removed it.
* general-slots.h:
Move Qsize_device here.
* lisp.h:
Declare Fdevice_system_metric(), now it's used in frame_x.c,
balloon_help.c.
diff -r 60e40afb6fbe -r a975482ad82d src/ChangeLog
--- a/src/ChangeLog Fri Apr 05 18:55:50 2019 +0900
+++ b/src/ChangeLog Sun Apr 14 13:23:03 2019 +0100
＠＠ -1,3 +1,29 ＠＠
+2019-04-14 Aidan Kehoe <kehoea(a)parhasard.net>
+
+ Get Stephen's last change compiling, fix a clear bug.
+ * balloon_help.c:
+ Make the types, macros from from lisp.h, device.h, console-x.h
+ available here.
+ Use the XEmacs Boolint rather than making a new boolean.
+ * balloon_help.c (show_help):
+ * balloon_help.c (balloon_help_create):
+ Wrap the pointer to the device returned by
+ get_device_from_display(), making it a Lisp_Object, as we want.
+ * balloon_help.c (balloon_help_hide):
+ * device-x.c (x_device_system_metrics):
+ Declare the parameters to XGetGeometry using the documented types.
+ Return the pixel size as intended, rather than falling through to
+ the DM_size_device_mm case.
+ * device.c:
+ Move Qsize_device to general-slots.h.
+ * frame-x.c (x_set_frame_position):
+ DPY isn't used, removed it.
+ * general-slots.h:
+ Move Qsize_device here.
+ * lisp.h:
+ Declare Fdevice_system_metric(), now it's used in frame_x.c,
+ balloon_help.c.
+
2019-04-02 Stephen J. Turnbull <turnbull(a)sk.tsukuba.ac.jp>
* device-x.c (x_device_system_metrics):
diff -r 60e40afb6fbe -r a975482ad82d src/balloon_help.c
--- a/src/balloon_help.c Fri Apr 05 18:55:50 2019 +0900
+++ b/src/balloon_help.c Sun Apr 14 13:23:03 2019 +0100
＠＠ -31,27 +31,22 ＠＠
*/
#include <config.h>
+#include "lisp.h"
+#include "device.h"
+#include "console-x.h"
+
#include <string.h>
#include <stdlib.h>
#include <stdio.h>
-#include <assert.h>
#include <X11/Xlib.h>
#include <X11/Xutil.h>
#include <X11/extensions/shape.h>
-#include "compiler.h"
#include "xintrinsic.h"
#include "balloon_help.h"
-#ifndef max
-#define max(x,y) (x>y?x:y)
-#endif
-
-#undef bool
-#define bool int
-
#define MARGIN_WIDTH 4
#define POINTER_OFFSET 8
#define BORDER_WIDTH 2
＠＠ -78,7 +73,7 ＠＠
static GC b_shadowGC;
static Window b_win;
-static bool b_winMapped;
+static Boolint b_winMapped;
static Pixmap b_mask;
static int b_maskWidth, b_maskHeight;
＠＠ -377,7 +372,7 ＠＠
XClearWindow (b_dpy, b_win);
XMapRaised (b_dpy, b_win);
- b_winMapped = True;
+ b_winMapped = 1;
draw_text (b_dpy, b_win, b_gc, b_fontStruct,
BORDER_WIDTH + MARGIN_WIDTH,
＠＠ -524,13 +519,13 ＠＠
create_pixmap_mask (1, 1);
b_maskGC = create_gc (dpy, b_mask, bg, fg, b_fontStruct);
- b_winMapped = False;
+ b_winMapped = 0;
b_timer = None;
b_delay = 500;
- root_size = Fdevice_system_metric (get_device_from_display (dpy),
- Qsize_device,
- Qnil);
+ root_size
+ = Fdevice_system_metric (wrap_device (get_device_from_display (dpy)),
+ Qsize_device, Qnil);
b_screenWidth = XFIXNUM (XCAR (root_size));
b_screenHeight = XFIXNUM (XCDR (root_size));
＠＠ -572,7 +567,7 ＠＠
b_text = NULL;
XUnmapWindow (b_dpy, b_win);
- b_winMapped = False;
+ b_winMapped = 0;
if (b_timer)
{
XtRemoveTimeOut (b_timer);
diff -r 60e40afb6fbe -r a975482ad82d src/device-x.c
--- a/src/device-x.c Fri Apr 05 18:55:50 2019 +0900
+++ b/src/device-x.c Sun Apr 14 13:23:03 2019 +0100
＠＠ -1769,11 +1769,11 ＠＠
{
int x, y;
/* Nobody would be running X on a smaller screen, right? */
- Dimension w = 1024, h = 768, b, d;
+ unsigned int w = 1024, h = 768, b, dd;
Window root = RootWindow(dpy, DefaultScreen(dpy));
/* If this fails, Xlib promises w and h are not updated. */
- XGetGeometry(dpy, root, &root, &x, &y, &w, &h, &b, &d);
- Fcons (make_fixnum (w), make_fixnum (h));
+ XGetGeometry(dpy, root, &root, &x, &y, &w, &h, &b, &dd);
+ return Fcons (make_fixnum (w), make_fixnum (h));
}
case DM_size_device_mm:
return Fcons (make_fixnum (XDisplayWidthMM (dpy, DefaultScreen (dpy))),
diff -r 60e40afb6fbe -r a975482ad82d src/device.c
--- a/src/device.c Fri Apr 05 18:55:50 2019 +0900
+++ b/src/device.c Sun Apr 14 13:23:03 2019 +0100
＠＠ -72,7 +72,7 ＠＠
Qcolor_scrollbar, Qcolor_desktop, Qcolor_workspace, Qfont_default,
Qfont_menubar, Qfont_dialog, Qsize_cursor, Qsize_scrollbar,
Qsize_menu, Qsize_toolbar, Qsize_toolbar_button,
- Qsize_toolbar_border, Qsize_icon, Qsize_icon_small, Qsize_device,
+ Qsize_toolbar_border, Qsize_icon, Qsize_icon_small,
Qsize_workspace, Qoffset_workspace, Qsize_device_mm, Qdevice_dpi,
Qnum_bit_planes, Qnum_color_cells, Qnum_screens, Qmouse_buttons,
Qsave_under, Qswap_buttons, Qshow_sounds, Qslow_device, Qsecurity,
diff -r 60e40afb6fbe -r a975482ad82d src/frame-x.c
--- a/src/frame-x.c Fri Apr 05 18:55:50 2019 +0900
+++ b/src/frame-x.c Sun Apr 14 13:23:03 2019 +0100
＠＠ -2227,7 +2227,6 ＠＠
x_set_frame_position (struct frame *f, int xoff, int yoff)
{
Widget w = FRAME_X_SHELL_WIDGET (f);
- Display *dpy = XtDisplay (w);
Lisp_Object root_size = Fdevice_system_metric (FRAME_DEVICE (f),
Qsize_device,
Qnil);
diff -r 60e40afb6fbe -r a975482ad82d src/general-slots.h
--- a/src/general-slots.h Fri Apr 05 18:55:50 2019 +0900
+++ b/src/general-slots.h Sun Apr 14 13:23:03 2019 +0100
＠＠ -275,6 +275,7 ＠＠
SYMBOL (Qsignal);
SYMBOL_MODULE_API (Qsimple);
SYMBOL (Qsize);
+SYMBOL (Qsize_device);
SYMBOL (Qsound);
SYMBOL (Qspace);
SYMBOL (Qspecifier);
diff -r 60e40afb6fbe -r a975482ad82d src/lisp.h
--- a/src/lisp.h Fri Apr 05 18:55:50 2019 +0900
+++ b/src/lisp.h Sun Apr 14 13:23:03 2019 +0100
＠＠ -4708,6 +4708,7 ＠＠
Lisp_Object object);
extern Lisp_Object Qdevice_live_p;
+EXFUN (Fdevice_system_metric, 3);
/* Defined in device-x.c */
extern Lisp_Object Vx_initial_argv_list;
Ar an cúigiú lá de mí Aibréan, scríobh Stephen J. Turnbull:
21.5 APPROVED
Sometimes I'm fine with the usual geometry of frames, sometimes I want
them vertically maximized (which the quartzwm doesn't provide an easy
way to do), so I have a Lisp function to do it. That function was
unreliable because #'device-system-metric was. This patch fixes it.
History and analysis:
Several months ago I got an external monitor, which when plugged in to
my notebook automatically changes the dimensions of the X screen from
1280x800 to something like 2560x1420. #'device-system-metric doesn't
recognize those changes. I believe that's because Xlib caches screen
metrics in the screen struct, and doesn't catch the change on the fly.
The information on the root window, which of course fills the screen,
*is* updated (I assume this costs a protocol request, but this
function is not used often -- not at all in core Lisp as far as I
could tell). The patch changes x_device_system_metrics to get the
geometry of the root window rather than using the ScreenWidth and
ScreenHeight macros. It does the trick in light testing.
It also changes x_set_frame_position and balloon_help_create to use
Fdevice_system_metric for the same reason (specifically, if the screen
shrinks and a small window is positioned against the right or bottom
edges, it could easily end up off-screen!)
I considered selecting ConfigureNotify and caching in XEmacs, but
decided against it. The rationale is in the source (see attached patch).
[...]
--
‘As I sat looking up at the Guinness ad, I could never figure out /
How your man stayed up on the surfboard after forty pints of stout’
(C. Moore)
2079
days inactive
2088
days old
1 comments
2 participants
participants (2)
-
Aidan Kehoe -
Stephen J. Turnbull