- Notifications
You must be signed in to change notification settings - Fork425
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Improve multi-monitor handling under wayland#1276
base:master
Are you sure you want to change the base?
Conversation
I'm down to merge this once someone has tested if the behavior breaks anything under X11, once the conficts are resolved ^^ |
When a monitor gets disconnected, the destroy event of all associatedwindows gets called, and the window gets removed.This patch changes that behavior: the window is still closed but theconfiguration is kept using the existing reload mechanism.In addition, a callback is added to listen for new monitors, triggeringa reload when a new monitor gets connected.This logic also reloads already running windows, which has a positive andnegative effect:- positive: if currently running e.g. on the second monitor specified in the list, the window can get moved to the first monitor- negative: if reloading starts it on the same monitor, it gets reset (e.g. graphs)I also had to work around an issue: the monitor model is not yet availableimmediately when a new monitor callback triggers. Waiting in the callbackdoes not help (I tried 10 seconds). However, waiting outside, it alwaysbecame available after 10ms.Tested with a dual monitor setup under KDE through a combinations of:- enabling/disabling individual monitors- switching between monitors- specifying a specific monitor in the yuck config- specifying a list of specific monitors in the yuck configIn all these cases the behavior is as expected, and the widget gets loadedon the first available monitor (or stays unloaded until one becomesavailable).It also works when opening a window without any of the configured monitorsbeing available.There is one remaining error from GTK when closing the window:GLib-GObject-CRITICAL **: 20:06:05.912: ../gobject/gsignal.c:2684: instance '0x55a4ab4be2d0' has no handler with id '136'This comes from the `self.gtk_window.disconnect(handler_id)` call. Toprevent that we'd have to reset `destroy_event_handler_id`.
39b18a9 to9aec9e9CompareIn case 'dirty' was set, we could end up with 'reuse_window = false', butnot closing the existing window.This moves the closing check to after 'if reuse_window', which allows tosimplify the logic.Fixes regression from29fa158
9aec9e9 to6b0f42eCompareI still have an old system with X11 installed, and was able to test there. It works as well with 2 differences in behavior:
I also rebased and resolved the conflicts. There was a regression in a recently added commit (29fa158) which I fixed in6b0f42e. |
@elkowar is there anything else I can or should do to get this merged? |
Flat commentedMar 13, 2025
I've been using this branch for a few days now, and when I re-connect monitors it causes a SIGABRT. Backtrace attached. Very consistently happens every time. |
@Flat which Linux distribution and desktop environment / window manager do you use? |
Flat commentedMar 17, 2025
Arch Linux with Hyprland (From git master) Attached is the yuck config as well. From the crash dump, it appears to be coming from thishttps://github.com/elkowar/eww/pull/1276/files#diff-12f72e340b5f9de88e64ebfc3df46f5d14a2bc5b0b2221eadf43ba087765d762R157-R158 as it is the only closure in wait_for_monitor_model I would assume it is hitting something on the monitors before they are fully initialized. |
It often works but sometimes eww crashes this happens with wayland (hyprland) |
Executors that poll a future cannot be called recursively (in this caseglib::main_context_futures::TaskSource::poll).So we cannot call gtk::main_iteration_do here, which in some cases led tothe future being polled again, which raised a panic in the form of:thread 'main' panicked at glib/src/main_context_futures.rs:238:56:called `Result::unwrap()` on an `Err` value: EnterErrorWe can just remove it as tokio::time::sleep() ensures the main threadcontinues to process (gtk) events during that time.
Thanks for the data points. I was able to find the problem and pushed a fix. |
Flat commentedMar 20, 2025
So far looks like it is now working as intended! |
Philipp-M commentedMar 21, 2025
I've tested it on both X as well as wayland where the bug indeed seems to be fixed. |
cstruck commentedMar 29, 2025
can confirm didn't happen since |
When a monitor gets disconnected, the destroy event of all associated windows gets called, and the window gets removed.
This PR changes that behavior: the window is still closed but the configuration is kept using the existing reload mechanism. In addition, a callback is added to listen for new monitors, triggering a reload when a new monitor gets connected.
This logic also reloads already running windows, which has a positive and negative effect:
I also had to work around an issue: the monitor model is not yet available immediately when a new monitor callback triggers. Waiting in the callback does not help (I tried 10 seconds). However, waiting outside, it always became available after 10ms.
Tested with a dual monitor setup under KDE through a combinations of:
In all these cases the behavior is as expected, and the widget gets loaded on the first available monitor (or stays unloaded until one becomes available).
It also works when opening a window without any of the configured monitors being available.
There is one remaining error from GTK when closing the window:
GLib-GObject-CRITICAL **: 20:06:05.912: ../gobject/gsignal.c:2684: instance '0x55a4ab4be2d0' has no handler with id '136'.This comes from the
self.gtk_window.disconnect(handler_id)call. To prevent that we'd have to resetdestroy_event_handler_id.I have not tested under X11, and I'm not sure if the behavior should be the same there.
Fixes#1158
Checklist
Please make sure you can check all the boxes that apply to this PR.
docs/content/maindirectory has been adjusted to reflect my changes.cargo fmtto automatically format all code before committing