Improve multi-monitor handling under wayland #1276
Conversation
|
I'm down to merge this once someone has tested if the behavior breaks anything under X11, once the conficts are resolved ^^ |
bkueng
force-pushed
the
improve_multi_monitor_wayland
branch
2 times, most recently
from
9aec9e9 to
6b0f42e
Compare
|
I 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 in 6b0f42e. |
|
@elkowar is there anything else I can or should do to get this merged? |
|
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? |
Arch Linux with Hyprland (From git master) Attached is the yuck config as well. From the crash dump, it appears to be coming from this https://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) |
|
Thanks for the data points. I was able to find the problem and pushed a fix. |
So far looks like it is now working as intended! |
|
I've tested it on both X as well as wayland where the bug indeed seems to be fixed. |
|
can confirm didn't happen since |
|
That would really help for my setup, using a kvm switch means I need to restart eww each time I switch video input. |
|
@elkowar anything you still need to get this merged? |
|
@elkowar I'm running this PR branch since the 20.03. without any problems can I somehow support to get this merged? |
|
@bkueng could you rebase this onto the latest master? I'm down to merge! Sorry for being gone for so long ^^' |
When a monitor gets disconnected, the destroy event of all associated windows gets called, and the window gets removed. This patch 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: - 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 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: - enabling/disabling individual monitors - switching between monitors - specifying a specific monitor in the yuck config - specifying a list of specific monitors in the yuck config 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 reset `destroy_event_handler_id`.
Executors that poll a future cannot be called recursively (in this case glib::main_context_futures::TaskSource::poll). So we cannot call gtk::main_iteration_do here, which in some cases led to the 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: EnterError We can just remove it as tokio::time::sleep() ensures the main thread continues to process (gtk) events during that time.
bkueng
force-pushed
the
improve_multi_monitor_wayland
branch
from
b0bc461 to
c85c8e8
Compare
|
Hi @elkowar, sorry to ping you, but since this fix was quite wanted, would you consider doing a new release? |
6 participants
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