StrataWM / strata

A cutting-edge, robust and sleek Wayland compositor with batteries included.
https://stratawm.github.io
GNU General Public License v3.0
179 stars 7 forks source link

Add keybinding detection #40

Closed anantnrg closed 1 year ago

anantnrg commented 1 year ago

Now that we have a working config which returns the keybinds, we need to detect those keybinds and then run the corresponding function. Will probably need to map the keys in the config to the actual keycodes so that they can be detected. I'll start working on it but for now the progress will be slow. Any help is greatly appreciated. Cheers :)

anantnrg commented 1 year ago

Hey @calops, a quick (maybe complicated) question. So for different functions (like close window or switch workspace) we need to call functions defined in the StrataState struct, right? So whats the best way to access an instance of this in our parse_config function? Either we would have to call it from a function which has access to an instance (for example, init_winit) and then pass it as a variable or we should make an instance of StrataState global. What do you think is the best way?

calops commented 1 year ago

I need to read the code a bit to answer that properly so I'll get back to you. Off the top of my head, we could simply include a reference (Arc and lock or whatever makes sense) to that struct in the Window object. And then implement mlua::UserData for it, which allows us to expose its methods to lua. Then we can call window:close() and it'll have access to all the context we need in the rust struct.

For the workspace thing, yeah maybe a global state like LUA is best after all.

anantnrg commented 1 year ago

Thats what I thought too. Well I'll get back to it when i get the time. Probably after the 24th. Cheers :)

anantnrg commented 1 year ago

Hey @calops, I've started working on implementing this. Could you please elaborate what you meant in your last comment? Cheers!

calops commented 1 year ago

I agree about the global state that will be declared alongside LUA, behind a ReentrantMutex as well (no need for Arc here).

As for the rest of my comment, I was talking about the UserData interface in mlua. It allows us to provide lua "classes" where the user can call methods. It lets them write window:close() instead of strata.api.close_window(window_handle), for example.

But I don't think that part is a priority yet. We can perfectly write close_window() for now and it won't be too bothersome to implement the more ergonomic way after (I can work on this this weekend).

anantnrg commented 1 year ago

Understood, thx :)

calops commented 1 year ago

Another important point for UserData: it lets us allocate lua-owned memory as rust objects. I don't think we have any use-case for this right now but it's good to keep in mind, as it allows us to share data between the runtime and rust with minimal (often zero) overhead.

anantnrg commented 1 year ago

Okay!

anantnrg commented 1 year ago

Hey @calops, I tried to implement the global state variable using ReentrantMutex but i noticed an issue. The StrataState variable is often updated and also its created in the init_winit function not in main.rs. And it also requires a lot of other variables, like an EventLoop and CalloopData. So how do we go about doing this?

If this is the case, then wouldn't the best idea to be to call the parse_config function from the init_winit (or init_udev after we implement that) and then pass the a variable of type StrataState to it as an argument? But then we would need to make the init_winit function async as well but I don't know how that'll hold up.

Please forgive my ignorance, this project was started as a project for me to learn Rust from. So I'm not that good at Rust as you are. Still learning.

calops commented 1 year ago

No worries!

For the use-case of initializing a global state depending on other runtime parameters, there is the once_cell crate. You can wrap the mutex in this and perform the one-time assignment wherever you see fit.

It could also make sense to wrap all this in a custom struct that implements Deref<Target=StrataState> and panics if the state isn't initialized (like the Loger::global() function in the example). That way we can use it seamlessly everywhere, we just need to make sure the first thing to use it is the function that will initialize it.

Something roughly like this:

struct StrataState;

impl StrataState {
    fn new() { ... }
}

struct StrataStateWrapper {
    inner: OnceCell<StrataState>,
}

impl StrataStateWrapper {
    pub fn set(&self, args) -> Result<(), Self> {
        self.inner.set(StrataState::new(args))
    }

    pub fn get(&self) -> &self {
        self.inner.get().expect("State not initialized")
    }

    pub fn get_mut(&self) -> &self {
        self.inner.get_mut().expect("State not initialized")
    }
}

impl Deref for StrataStateWrapper {
    type Target = StrataState;

    fn deref(&self) -> &Self::Target {
        &self.get()
    }
}

impl DerefMut for StrataStateWrrapper {
    fn deref_mut(&mut self) -> &mut Self::Target {
        &self.get_mut()
    }
}
anantnrg commented 1 year ago

Okay, I'll try to understand this and will get back to you. Cheers :)

anantnrg commented 1 year ago

Since we've done this (and it almost works), I'll close this issue.