Detect light/dark OS theme

[nickolasclarke]

This adds an option to use the System preferences to set the theme at launch. It uses the dark-light crate to do so.

This is my very first time touching Rust, Druid, and frankly any sort of native GUI framework. While it should be relatively simple to have a long-running process to call setup_system_theme or similar continuously, it is not clear to me where that is best done within Druid. Unfortunately, dark-light does not have a method check and yield state change asynchronously. If you can point me in the right direction for where to put an infinite loop like that, I'll gladly update this PR.

[jpochyla]

Hi @nickolasclarke, thanks a lot for the PR! We were trying hard to zvoid having zbus in the dependency tree, as it drags a ton a stuff inside, slowing down the build :( Maybe we could add an off-by-default feature for this, or use a different library? What do you think?

[nickolasclarke]

I think either would work. I'll poke around for another library that does this cross-platform.

[nickolasclarke]

@jpochyla coming back to this, it looks like zbus + zvariant are only used in the freedesktop based GUIs, which we could use the the dbus crate which may be a bit lighter? Otherwise, I am unsure on how to implement this cleaner.

[nickolasclarke]

@jpochyla bumping this. Any thoughts on my latest comment?

[jacksongoode]

@nickolasclarke I can have a look at this and see if we can merge it!

Edit: I made some small code structure changes based off a previous dark/light theme PR. The last thing I would want to see if its possible is to detect the dark/light theme when selected. If you change the system setting and then reselect "System" it doesn't reflect the new system setting.

[nickolasclarke]

@jacksongoode yeah, I called that out in my PR. I wasnt sure at the time on how to get dark-light to asynchronously yield whenever the system state changes. AS such, I was going to just put an infinite watcher loop in, but was not sure on the best place for it.

Also @jpochyla didn't want zbus, so I did some research for alternative crates and found dbus. I could refactor this PR to use that, though I am not sure if that addresses the concerns about making build times blow up.

[jacksongoode]

Comparing build times from main, (~7, ~30, ~12min) I don't think this PR actually increases the build times dramatically? If you want to compare this a little bit more you can but I feel somewhat comfortable allowing dark-light for now. I think the one thing that would be nice, instead of a watcher, could you make it such that the selection of system and the theme options causes a trigger to check the current system light or dark state? I don't think we need constant polling, just maybe when a selection is made.

[nickolasclarke]

@jacksongoode fine by me. Iirc, it did do that at the time, I'll try this again to see current state. But if you did select system, it would not update again once the system theme changes. I ran into this with my system automatically changing theme and light temperature based on sunrise and sunset. As such, I think a constant poller would be required.

[nickolasclarke]

Chatgpt suggests something along these lines for zbus:

use zbus::{Connection, Result, proxy};

#[proxy(
    interface = "org.freedesktop.appearance",
    default_service = "org.freedesktop.appearance",
    default_path = "/org/freedesktop/appearance"
)]
trait Appearance {
    async fn get(&self, property: &str) -> Result<String>;

    #[dbus_proxy(signal)]
    async fn property_changed(&self, property: &str, value: &str);
}

// Although we use `async-std` here, you can use any async runtime of choice.
#[async_std::main]
async fn main() -> Result<()> {
    let connection = Connection::session().await?;
    // `proxy` macro creates `AppearanceProxy` based on `Appearance` trait.
    let proxy = AppearanceProxy::new(&connection).await?;
    // Subscribe to the `PropertyChanged` signal and provide a callback function.
    proxy.connect_property_changed(|property, value| {
        println!("{} changed to {}", property, value);
        Ok(())
    })?;
    // Wait forever.
    loop {
        async_std::task::sleep(std::time::Duration::from_secs(1)).await;
    }
}

I'll see if light-dark supports this already.

[jacksongoode]

I was just reviewing other rust projects using zbus. I think a lot are migrating from dbus to zbus. I think this is fine generally. I think we need to use the use_zbus feature of the souvlaki package.

[jacksongoode]

I'd like someone to test this version on Linux and make sure it works!

[jacksongoode]

I'll actually test it today on arm Linux.

[nickolasclarke]

I did sleuth through the light-dark crate to see if it had this property_changed watcher functionality we would need, but I did not see it. If I get some time this weekend, I'll try to implement it here.

[jacksongoode]

@nickolasclarke I believe we need to wait for this to get merged frewsxcv/rust-dark-light#26

[nickolasclarke]

@jacksongoode ahh, good find.

[jacksongoode]

Well that PR got merged so once they update, we should try again!

[SO9010]

I just tried it on my device, and it worked well. The one thing is that we may want to have some Async function that can tell when the theme has changed, as it doesn't dynamically change; you have to restart the window.

[nickolasclarke]

@SO9010 that PR we were waiting on was merged that should permit this. I believe all we need to do is bump dark-light to 1.0.1. We are pinned at 1.0.0

[SO9010]

Just let me know if you want me to test it on Linux again :)

Edit: I just tested it on my computer by bumping the package to 1.0.1, as suspected you are required to use an async function or something else to dynamically change the colour scheme as that function is only run at the start of the program or when someone has changed the colour scheme options.

[nickolasclarke]

@jacksongoode getting around to testing this out, dark-light does pull in tokiyo, which is required for this subscribe method. Tokiyo has not dependency in the past and is called out specifically as such in the README. We ok with that?

[jacksongoode]

@nickolasclarke That's actually interesting because the only instance I see tokio being used in their repo is in the example. Do they actually need it?

Edit: Looking at the PR makes me a bit confused. A lot of the packages changed.

[nickolasclarke]

@jacksongoode are you talking about the lock file? I did a gh sync to sync my fork with origin, but perhaps that went bad? I can hard reset that file and re-run.

Looking at the underlying code, it is an async function that relies on the future trait. I'm not familiar enough with rust aysnc dev to say wether we need tokiyo for that or not. Seems simple enough to rely on std lib. If we do just use std lib, do you want to cut a PR upstream or fork the project here and remove the dependency?

[jacksongoode]

Oh sorry, I just clarified with the author of that PR. tokio is only set as a dev dependency so we shouldn't be pulling it in (in theory) when we build the release.

[nickolasclarke]

@jacksongoode the latest release does not include that method, I've asked them to do a new crate release.