From 9bcb27f2e8f6f4a42b2c78344a9061f933ff5d8f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 25 Feb 2019 13:52:10 +0100 Subject: [PATCH 01/10] Pass the RRScreenChangeNotify event to libxrandr From Xrandr(3): > Clients must call back into Xlib using XRRUpdateConfiguration when > screen configuration change notify events are generated [...]. --- src/x11/screen.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index a9ac74a..357b9c5 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -25,8 +25,6 @@ int screens_len; bool dunst_follow_errored = false; -int randr_event_base = 0; - static int randr_major_version = 0; static int randr_minor_version = 0; @@ -90,8 +88,8 @@ void alloc_screen_ar(int n) void randr_init(void) { - int randr_error_base = 0; - if (!XRRQueryExtension(xctx.dpy, &randr_event_base, &randr_error_base)) { + int ignored; + if (!XRRQueryExtension(xctx.dpy, &ignored, &ignored)) { LOG_W("Could not initialize the RandR extension. " "Falling back to single monitor mode."); return; @@ -145,7 +143,7 @@ static int autodetect_dpi(struct screen_info *scr) void screen_check_event(XEvent event) { - if (event.type == randr_event_base + RRScreenChangeNotify) { + if (XRRUpdateConfiguration(&event)) { LOG_D("XEvent: processing 'RRScreenChangeNotify'"); randr_update(); From 121ddd2b94c46bf6f8c8a8a4769430a3e832c2a1 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 25 Feb 2019 14:49:18 +0100 Subject: [PATCH 02/10] Always throw out the whole screen array If n < screens_len, screens_len did not get updated. So boundary checks wouldn't catch, if screens between n and screens_len are accessed. This will prevent such an error type in the future. --- src/x11/screen.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index 357b9c5..fd23c56 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -77,12 +77,8 @@ void init_screens(void) void alloc_screen_ar(int n) { assert(n > 0); - if (n <= screens_len) return; - - screens = g_realloc(screens, n * sizeof(struct screen_info)); - - memset(screens, 0, n * sizeof(struct screen_info)); - + g_free(screens); + screens = g_malloc0(n * sizeof(struct screen_info)); screens_len = n; } From 4f510e170366862f52043db8d5a40872b07c1ea2 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Mon, 25 Feb 2019 19:01:34 +0100 Subject: [PATCH 03/10] Query the X11 screen's DPI instead of monitor When changing the DPI via xrandr --dpi , xrandr will send a RRScreenChangeEvent and the DPI value should get adjusted. Falsely, we thought randr_update() would catch up and query the right monitor values. But nothing changes, because we query the XRRMonitorInfo. The monitor info contains the real physical width and height of the monitor's screen. But xrandr --dpi only changes the - let's say - virtual screen size of the virtual overall screen (and therefore changing the DPI to the matching value). Important commands to understand: - Changes dpi of the virtual screen xrandr --dpi - Gives info about the "virtual" screen size (used by DisplayWidth) xdpyinfo | grep -B1 resolution - Gives info about the "physical" screen size (used by XRRMonitorInfo) xrandr -q I know, that I'm probably not right and might not understand the topic in its full size yet[0]. But I'm 100% sure, that the terms "monitor", "screen", "screens", "output" and "display" do not have a consistent naming scheme. [0] https://twitter.com/dechampsgu/status/857924498280124416 Fixes #382 --- src/draw.c | 2 +- src/x11/screen.c | 43 +++++++++++++++++++++++++------------------ src/x11/screen.h | 2 +- 3 files changed, 27 insertions(+), 20 deletions(-) diff --git a/src/draw.c b/src/draw.c index e1032c0..7dbe55f 100644 --- a/src/draw.c +++ b/src/draw.c @@ -237,7 +237,7 @@ static PangoLayout *layout_create(cairo_t *c) struct screen_info *screen = get_active_screen(); PangoContext *context = pango_cairo_create_context(c); - pango_cairo_context_set_resolution(context, get_dpi_for_screen(screen)); + pango_cairo_context_set_resolution(context, screen_dpi_get(screen)); PangoLayout *layout = pango_layout_new(context); diff --git a/src/x11/screen.c b/src/x11/screen.c index fd23c56..8afde38 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -37,7 +37,7 @@ static int x_follow_tear_down_error_handler(void); static int FollowXErrorHandler(Display *display, XErrorEvent *e); static Window get_focused_window(void); -static double get_xft_dpi_value(void) +static double screen_dpi_get_from_xft(void) { static double dpi = -1; //Only run this once, we don't expect dpi changes during runtime @@ -64,6 +64,30 @@ static double get_xft_dpi_value(void) return dpi; } +static double screen_dpi_get_from_monitor(struct screen_info *scr) +{ + return (double)scr->h * 25.4 / (double)scr->mmh; +} + +double screen_dpi_get(struct screen_info *scr) +{ + if ( ! settings.force_xinerama + && settings.per_monitor_dpi) + return screen_dpi_get_from_monitor(scr); + + if (screen_dpi_get_from_xft() > 0) + return screen_dpi_get_from_xft(); + + // Calculate the DPI on the overall screen size. + // xrandr --dpi does only change the overall screen's millimeters, + // but not the physical screen's sizes. + // + // The screen parameter is XDefaultScreen(), as our scr->id references + // the xrandr monitor and not the xrandr screen + return ((((double)DisplayWidth (xctx.dpy, XDefaultScreen(xctx.dpy))) * 25.4) / + ((double)DisplayWidthMM(xctx.dpy, XDefaultScreen(xctx.dpy)))); +} + void init_screens(void) { if (!settings.force_xinerama) { @@ -132,11 +156,6 @@ void randr_update(void) XRRFreeMonitors(m); } -static int autodetect_dpi(struct screen_info *scr) -{ - return (double)scr->h * 25.4 / (double)scr->mmh; -} - void screen_check_event(XEvent event) { if (XRRUpdateConfiguration(&event)) { @@ -349,18 +368,6 @@ sc_cleanup: return &screens[ret]; } -double get_dpi_for_screen(struct screen_info *scr) -{ - double dpi = 0; - if ((!settings.force_xinerama && settings.per_monitor_dpi && - (dpi = autodetect_dpi(scr)))) - return dpi; - else if ((dpi = get_xft_dpi_value())) - return dpi; - else - return 96; -} - /* * Return the window that currently has * the keyboard focus. diff --git a/src/x11/screen.h b/src/x11/screen.h index 864c230..b8d8dc4 100644 --- a/src/x11/screen.h +++ b/src/x11/screen.h @@ -20,7 +20,7 @@ void init_screens(void); void screen_check_event(XEvent event); struct screen_info *get_active_screen(void); -double get_dpi_for_screen(struct screen_info *scr); +double screen_dpi_get(struct screen_info *scr); /** * Find the currently focused window and check if it's in From 37e580e8576ed411e6a682218f20e037e8e58d4f Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Thu, 28 Feb 2019 12:45:17 +0100 Subject: [PATCH 04/10] (style) swap conditional blocks --- src/x11/screen.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index 8afde38..d23cf45 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -90,11 +90,11 @@ double screen_dpi_get(struct screen_info *scr) void init_screens(void) { - if (!settings.force_xinerama) { + if (settings.force_xinerama) { + xinerama_update(); + } else { randr_init(); randr_update(); - } else { - xinerama_update(); } } From ca7234b1c88a89ad151d5d6350adfbfc286928e6 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Thu, 28 Feb 2019 12:49:18 +0100 Subject: [PATCH 05/10] Cache the Xft.dpi value outside The Xft.dpi value is one way to set the dpi of the X11 display. Querying this value requires much overhead. Therefore we have to cache this. Previously we did just query it at the beginning and ignored further changes. As there is no native signal to catch a change in its xrdb value, we have to rely on the root window's PropertyNotify event and filter there the RESOURCE_MANAGER atom. This will get hooked up later. --- src/x11/screen.c | 37 ++++++++++++++++++++++++------------- src/x11/screen.h | 1 + 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index d23cf45..9648c7b 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -37,31 +37,42 @@ static int x_follow_tear_down_error_handler(void); static int FollowXErrorHandler(Display *display, XErrorEvent *e); static Window get_focused_window(void); + +/** + * A cache variable to cache the Xft.dpi xrdb values. + * We do not expect to change xrdb often, but there's much + * overhead to query it once. + * + * @retval -DBL_MAX: uncached + * @retval <=0: Invalid and unusable value + * @retval >0: valid + */ +double screen_dpi_xft_cache = -DBL_MAX; + +void screen_dpi_xft_cache_purge(void) +{ + screen_dpi_xft_cache = -DBL_MAX; +} + static double screen_dpi_get_from_xft(void) { - static double dpi = -1; - //Only run this once, we don't expect dpi changes during runtime - if (dpi <= -1) { + if (screen_dpi_xft_cache == -DBL_MAX) { + screen_dpi_xft_cache = 0; + XrmInitialize(); char *xRMS = XResourceManagerString(xctx.dpy); - if (!xRMS) { - dpi = 0; - return 0; - } + ASSERT_OR_RET(xRMS, screen_dpi_xft_cache); XrmDatabase xDB = XrmGetStringDatabase(xRMS); char *xrmType; XrmValue xrmValue; - if (XrmGetResource(xDB, "Xft.dpi", "Xft.dpi", &xrmType, &xrmValue)) { - dpi = strtod(xrmValue.addr, NULL); - } else { - dpi = 0; - } + if (XrmGetResource(xDB, "Xft.dpi", "Xft.dpi", &xrmType, &xrmValue)) + screen_dpi_xft_cache = strtod(xrmValue.addr, NULL); XrmDestroyDatabase(xDB); } - return dpi; + return screen_dpi_xft_cache; } static double screen_dpi_get_from_monitor(struct screen_info *scr) diff --git a/src/x11/screen.h b/src/x11/screen.h index b8d8dc4..1fc2033 100644 --- a/src/x11/screen.h +++ b/src/x11/screen.h @@ -17,6 +17,7 @@ struct screen_info { }; void init_screens(void); +void screen_dpi_xft_cache_purge(void); void screen_check_event(XEvent event); struct screen_info *get_active_screen(void); From e9e199c4eced90716ea884a68d53d8abba4a2e0a Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Mar 2019 19:21:03 +0100 Subject: [PATCH 06/10] Pass pointer to check_screen_event --- src/x11/screen.c | 8 ++++---- src/x11/screen.h | 2 +- src/x11/x.c | 5 ++++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index 9648c7b..0b590ea 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -167,15 +167,15 @@ void randr_update(void) XRRFreeMonitors(m); } -void screen_check_event(XEvent event) +bool screen_check_event(XEvent *ev) { - if (XRRUpdateConfiguration(&event)) { + if (XRRUpdateConfiguration(ev)) { LOG_D("XEvent: processing 'RRScreenChangeNotify'"); randr_update(); - } else { - LOG_D("XEvent: Ignoring '%d'", event.type); + return true; } + return false; } void xinerama_update(void) diff --git a/src/x11/screen.h b/src/x11/screen.h index 1fc2033..fb83bb7 100644 --- a/src/x11/screen.h +++ b/src/x11/screen.h @@ -18,7 +18,7 @@ struct screen_info { void init_screens(void); void screen_dpi_xft_cache_purge(void); -void screen_check_event(XEvent event); +bool screen_check_event(XEvent *ev); struct screen_info *get_active_screen(void); double screen_dpi_get(struct screen_info *scr); diff --git a/src/x11/x.c b/src/x11/x.c index 07d658f..c26ed32 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -355,7 +355,10 @@ gboolean x_mainloop_fd_dispatch(GSource *source, GSourceFunc callback, gpointer } break; default: - screen_check_event(ev); + if (!screen_check_event(&ev)) { + LOG_D("XEvent: Ignoring '%d'", ev.type); + } + break; } } From 9961efd10cabba03a0673a6dcce21d2236d3fced Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Mar 2019 19:26:15 +0100 Subject: [PATCH 07/10] Copy the database from the display connection This allows to update the DB on the xctx.dpy object. Crawling the string from xctx.dpy will always return the xdefaults string, which represents the defaults from the initialisation, but not with any other updates. --- src/x11/screen.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index 0b590ea..ce89702 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -60,17 +60,13 @@ static double screen_dpi_get_from_xft(void) screen_dpi_xft_cache = 0; XrmInitialize(); - char *xRMS = XResourceManagerString(xctx.dpy); - ASSERT_OR_RET(xRMS, screen_dpi_xft_cache); - - XrmDatabase xDB = XrmGetStringDatabase(xRMS); char *xrmType; XrmValue xrmValue; - - if (XrmGetResource(xDB, "Xft.dpi", "Xft.dpi", &xrmType, &xrmValue)) + XrmDatabase db = XrmGetDatabase(xctx.dpy); + ASSERT_OR_RET(db, screen_dpi_xft_cache); + if (XrmGetResource(db, "Xft.dpi", "Xft.dpi", &xrmType, &xrmValue)) screen_dpi_xft_cache = strtod(xrmValue.addr, NULL); - XrmDestroyDatabase(xDB); } return screen_dpi_xft_cache; } From 4b06d67605cf89d22e2a0e8197ca1907c6eee1d8 Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Mar 2019 19:48:06 +0100 Subject: [PATCH 08/10] Initialize xrm during init Initializing xrm is required only once. Everything else would be overhead. --- src/x11/screen.c | 2 -- src/x11/x.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/x11/screen.c b/src/x11/screen.c index ce89702..7c3347f 100644 --- a/src/x11/screen.c +++ b/src/x11/screen.c @@ -59,8 +59,6 @@ static double screen_dpi_get_from_xft(void) if (screen_dpi_xft_cache == -DBL_MAX) { screen_dpi_xft_cache = 0; - XrmInitialize(); - char *xrmType; XrmValue xrmValue; XrmDatabase db = XrmGetDatabase(xctx.dpy); diff --git a/src/x11/x.c b/src/x11/x.c index c26ed32..e069beb 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -471,6 +471,8 @@ void x_setup(void) init_screens(); x_shortcut_grab(&settings.history_ks); + + XrmInitialize(); } struct geometry x_parse_geometry(const char *geom_str) From 812d5a3b84c093adfbdab9939ee57545e442090c Mon Sep 17 00:00:00 2001 From: Benedikt Heine Date: Wed, 6 Mar 2019 20:13:56 +0100 Subject: [PATCH 09/10] Replace the xresources DB on PropertyNotify When changing values with xrdb on the command line, an XEvent is triggered on the "RESOURCE_MANAGER" atom. But xlib just doesn't care. XResourceManagerString() will still report the old values. Also XrmGetDatabase() won't help out. So, when we receive a PropertyNotify for the resource manager atom, we have to manually query its contents, convert it to a DB and replace it in the display object. This allows, that any method just can call XrmGetDatabase() and get the latest values. --- src/x11/x.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 54 insertions(+), 1 deletion(-) diff --git a/src/x11/x.c b/src/x11/x.c index e069beb..8540b51 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include "../dbus.h" @@ -55,6 +56,8 @@ bool dunst_grab_errored = false; static bool fullscreen_last = false; +static void XRM_update_db(void); + static void x_shortcut_init(struct keyboard_shortcut *ks); static int x_shortcut_grab(struct keyboard_shortcut *ks); static void x_shortcut_ungrab(struct keyboard_shortcut *ks); @@ -333,9 +336,18 @@ gboolean x_mainloop_fd_dispatch(GSource *source, GSourceFunc callback, gpointer ev.xcreatewindow.override_redirect == 0) XRaiseWindow(xctx.dpy, win->xwin); break; + case PropertyNotify: + if (ev.xproperty.atom == XA_RESOURCE_MANAGER) { + LOG_D("XEvent: processing PropertyNotify for Resource manager"); + XRM_update_db(); + screen_dpi_xft_cache_purge(); + draw(); + break; + } + /* Explicitly fallthrough. Other PropertyNotify events, e.g. catching + * _NET_WM get handled in the Focus(In|Out) section */ case FocusIn: case FocusOut: - case PropertyNotify: LOG_D("XEvent: Checking for active screen changes"); fullscreen_now = have_fullscreen_window(); scr = get_active_screen(); @@ -440,6 +452,47 @@ void x_free(void) XCloseDisplay(xctx.dpy); } +static int XErrorHandlerDB(Display *display, XErrorEvent *e) +{ + char err_buf[BUFSIZ]; + XGetErrorText(display, e->error_code, err_buf, BUFSIZ); + LOG_W("%s", err_buf); + return 0; +} + +static void XRM_update_db(void) +{ + XrmDatabase db; + XTextProperty prop; + Window root; + // We shouldn't destroy the first DB coming + // from the display object itself + static bool runonce = false; + + XFlush(xctx.dpy); + XSetErrorHandler(XErrorHandlerDB); + + root = RootWindow(xctx.dpy, DefaultScreen(xctx.dpy)); + + XLockDisplay(xctx.dpy); + if (XGetTextProperty(xctx.dpy, root, &prop, XA_RESOURCE_MANAGER)) { + if (runonce) { + db = XrmGetDatabase(xctx.dpy); + XrmDestroyDatabase(db); + } + + db = XrmGetStringDatabase((const char*)prop.value); + XrmSetDatabase(xctx.dpy, db); + } + XUnlockDisplay(xctx.dpy); + + runonce = true; + + XFlush(xctx.dpy); + XSync(xctx.dpy, false); + XSetErrorHandler(NULL); +} + /* * Setup X11 stuff */ From 1bc3237a359f37905426012c0cca90d71c4b3b18 Mon Sep 17 00:00:00 2001 From: Nikos Tsipinakis Date: Sat, 23 Mar 2019 19:00:19 +0200 Subject: [PATCH 10/10] Subscribe to PropertyChangeMask regardless of follow_mode PropertyNotify events are used primarily to detect active screen changes when follow mode is used but now we also need them to receive resource manager events in order to update the dpi value. --- src/x11/x.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/x11/x.c b/src/x11/x.c index 8540b51..55267e8 100644 --- a/src/x11/x.c +++ b/src/x11/x.c @@ -674,9 +674,17 @@ struct window_x11 *x_win_create(void) win->esrc = x_win_reg_source(win); - long root_event_mask = SubstructureNotifyMask; + /* SubstructureNotifyMask is required for receiving CreateNotify events + * in order to raise the window when something covers us. See #160 + * + * PropertyChangeMask is requred for getting screen change events when follow_mode != none + * and it's also needed to receive + * XA_RESOURCE_MANAGER events to update the dpi when + * the xresource value is updated + */ + long root_event_mask = SubstructureNotifyMask | PropertyChangeMask; if (settings.f_mode != FOLLOW_NONE) { - root_event_mask |= FocusChangeMask | PropertyChangeMask; + root_event_mask |= FocusChangeMask; } XSelectInput(xctx.dpy, root, root_event_mask);