Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use cosmic-config for input config; changeable at runtime #140

Merged
merged 7 commits into from
Sep 1, 2023

Conversation

ids1024
Copy link
Member

@ids1024 ids1024 commented Jul 28, 2023

With this, it's possible to change xkb-config and input-devices and immediately see them effect the compositor.

In an earlier draft for input configuration I had a method:

pub fn register_handler<T: serde::de::DeserializeOwned + 'static>(
    &mut self,
    name: &'static str,
    cb: fn(T, &mut State),
)

But I'm not sure that makes the code less entangled without more changes.

pop-os/libcosmic#126 would make a few things cleaner, but I'm still not sure on that design.

Leaving as a draft until cosmic-settings is updated to make use of this. And input-devices may not be the best way to configure input, but we'll need to think about the design there.

After input settings, settings from config.ron can also be migrated to cosmic-config and made changeable at runtime. outputs.ron can be dealt with later since we already have dynamic persistent output configuration that more or less works with wdisplays.

@Drakulix
Copy link
Member

Looks good to me code-wise.

But I'm not sure that makes the code less entangled without more changes.

Me neither, seems clean enough to me anyway.

And input-devices may not be the best way to configure input, but we'll need to think about the design there.

I think this is the biggest issue here. Looking at the cosmic-settings design, we need a way to configure defaults for a device type. I would like to retain the ability to special case the configuration of specific devices, though updating the defaults should probably also update all known devices, otherwise cosmic-settings UI wouldn't work as expected.

Thinking about this we might want to make a HashMap<DeviceCapability, InputConfig>?

outputs.ron can be dealt with later since we already have dynamic persistent output configuration that more or less works with wdisplays.

Though this depends on the wlr-output-configuration protocol, which doesn't necessarily expose everything we want to make configurable in the long term. E.g. it only recently added variable refresh rate, so for cosmic-settings I think we want to use the cosmic-config, while keeping wlr-output-configuration for compatibility with other apps.

@ids1024
Copy link
Member Author

ids1024 commented Jul 31, 2023

Thinking about this we might want to make a HashMap<DeviceCapability, InputConfig>?

Would that group mouse and touchpad acceleration? I think the designs have those separate, as is commonly done.

@Drakulix
Copy link
Member

Right... So maybe we need to match on some property of the UdevDevice to figure out the device "type".

@ids1024 ids1024 force-pushed the cosmic-comp-config branch 2 times, most recently from 41dc432 to 0fea3e4 Compare August 1, 2023 15:06
@Drakulix
Copy link
Member

So the solution is to just treat touchpads differently from everything else?

@ids1024
Copy link
Member Author

ids1024 commented Aug 30, 2023

Yes. It's awkward, but I don't see a better way to do it with the current design we have for settings. Unless we want to just have individual settings like touchpad-speed instead of this general mechanism.

We can't really have settings per capability because a device can (in principle) have any number of capabilities. Libinput doesn't have "device types" even if things like Gnome Shell try to act like it does. I guess settings are largely tied to a particular capability, looking at libinput I didn't see any explicit association.

@Drakulix
Copy link
Member

Just wanted to confirm, I don't think this is a terrible idea and I also don't see any obvious issues, it just seemed.. odd.

What is missing to get this out of "Draft"-status?

@ids1024
Copy link
Member Author

ids1024 commented Aug 30, 2023

I still need to make use of this for mouse/touchpad settings in cosmic-settings and use that to test that its working as expected, but otherwise this PR should be good. Moving other settings to cosmic-settings can be done after this is merged.

@Drakulix
Copy link
Member

I still need to make use of this for mouse/touchpad settings in cosmic-settings and use that to test that its working as expected, but otherwise this PR should be good. Moving other settings to cosmic-settings can be done after this is merged.

Sounds good. Please give me a ping once this is ready for final review and merge. I haven't found any glaring issues with the approach. Quite the opposite, I think this version looks quite clean! :)

This adds a `input-default` setting, which is used for input settings if a
device isn't set in `input-devices`. More awkwardly, it also adds an
`input-touchpad` setting, which is used instead of `input-default` for
touchpad devices, so we can separate mouse and touchpad settings even
though they use the same capability and settings.

This no longer sets the input config, and only reads it. If we add a UI
for per-device config, we'll need some IPC mechanism to list connected
devices. (Assuming cosmic-settings can't use libinput directly for that.)

This moves `InputConfig` and `XkbConfig` to a new `cosmic-comp-config`
crate, so they can be used in `cosmic-settings`.
@ids1024
Copy link
Member Author

ids1024 commented Aug 31, 2023

After a couple small changes to cosmic-comp-config, and making it not spew DeviceConfigError::Unsupported trying to set default config for devices that don't support it, (and remembering to delete an old input-devices that was overriding the defaults), this seems to be working and suitable for use in cosmic-settings.

So probably no need to do more on this branch, and we can merge it.

@ids1024 ids1024 marked this pull request as ready for review August 31, 2023 21:05
@Drakulix Drakulix merged commit 511ee8d into master_jammy Sep 1, 2023
1 check failed
@ids1024 ids1024 deleted the cosmic-comp-config branch October 5, 2023 16:46
ids1024 pushed a commit that referenced this pull request Jul 26, 2024
Adds a note about "Read-only filesystem" when testing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants