Skip to content

Commit

Permalink
x11/wayland: always try the portal for appearance
Browse files Browse the repository at this point in the history
Right now the initial x11 appearance retrieval uses the specific
connection interface, which completely circumvents the already existing
more complete implementation in x_and_wayland.rs.
The latter implementation is strictly better, because it first attempts
getting the appearance from the XDG desktop portal and then falls back
to the X11 interface.

Before this patch there was a very weird issue for folks using the OS
system dark mode with the following config snippet:

```
color_scheme = scheme_for_appearance(wezterm.gui.get_appearance())
```

The color_scheme on startup would be correct, but there would be a very
weird problem where sometimes wezterm ignores the first time that the
portal notifies about an appearance update.

The source of the bug was an inconsistent retrieval of the appearance
setting:
- The Lua API used the XDG desktop portal
- The internal appearance used the X11 specific connection at startup

For example due to this, the internal appearance variable could have
stored "Dark" from the X11 connection, but the actual appearance from
the XDG desktop portal was "Light".
If then the XDG desktop portal changes to "Dark", the
appearance_changed() method would dismiss the update because
self.appearance was already "Dark".

It is only after that, that the internal inconsistency would have been
solved and following appearance changes would succeed and update the
colorscheme correctly.

To fix this problem, we now use the portal directly in both the x11 and
wayland connections,  which is consistent with the Lua
wezterm.gui.get_appearance() API.

refs: wez#2258
  • Loading branch information
vimpostor committed Jul 15, 2022
1 parent 80e72fa commit a22ade6
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
13 changes: 12 additions & 1 deletion window/src/os/wayland/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::os::wayland::output::OutputHandler;
use crate::os::x11::keyboard::Keyboard;
use crate::screen::{ScreenInfo, Screens};
use crate::spawn::*;
use crate::{Connection, ScreenRect, WindowEvent};
use crate::{Appearance, Connection, ScreenRect, WindowEvent};
use anyhow::{bail, Context};
use mio::unix::SourceFd;
use mio::{Events, Interest, Poll, Token};
Expand Down Expand Up @@ -425,6 +425,17 @@ impl ConnectionOps for WaylandConnection {
*self.should_terminate.borrow_mut() = true;
}

fn get_appearance(&self) -> Appearance {
match promise::spawn::block_on(crate::os::xdg_desktop_portal::get_appearance()) {
Ok(appearance) => return appearance,
Err(err) => {
log::debug!("Unable to resolve appearance using xdg-desktop-portal: {err:#}");
}
}
// fallback
Appearance::Light
}

fn run_message_loop(&self) -> anyhow::Result<()> {
let res = self.run_message_loop_impl();
// Ensure that we drop these eagerly, to avoid
Expand Down
6 changes: 6 additions & 0 deletions window/src/os/x11/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@ impl ConnectionOps for XConnection {
}

fn get_appearance(&self) -> Appearance {
match promise::spawn::block_on(crate::os::xdg_desktop_portal::get_appearance()) {
Ok(appearance) => return appearance,
Err(err) => {
log::debug!("Unable to resolve appearance using xdg-desktop-portal: {err:#}");
}
}
if let Some(XSetting::String(name)) = self.xsettings.borrow().get("Net/ThemeName") {
let lower = name.to_ascii_lowercase();
match lower.as_str() {
Expand Down
6 changes: 0 additions & 6 deletions window/src/os/x_and_wayland.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,12 +142,6 @@ impl ConnectionOps for Connection {
}

fn get_appearance(&self) -> Appearance {
match promise::spawn::block_on(crate::os::xdg_desktop_portal::get_appearance()) {
Ok(appearance) => return appearance,
Err(err) => {
log::debug!("Unable to resolve appearance using xdg-desktop-portal: {err:#}");
}
}
match self {
Self::X11(x) => x.get_appearance(),
#[cfg(feature = "wayland")]
Expand Down

0 comments on commit a22ade6

Please sign in to comment.