libinput 1.23, Add accel config for pointer acceleration #72

rockerBOO · 2024-07-27T22:34:26Z

I was wanting to play with pointer acceleration added in 1.23. I gen'd the latest version 1.26.1.

Add AccelProfile::Custom
Add AccelType
Add AccelConfig

The example I have been testing with:

use std::{
    fs::{File, OpenOptions},
    os::fd::OwnedFd,
    path::Path,
};

use input::{AccelProfile, Libinput, LibinputInterface};

struct Interface;

impl LibinputInterface for Interface {
    fn open_restricted(&mut self, path: &Path, flags: i32) -> Result<OwnedFd, i32> {
        OpenOptions::new()
            .read(true)
            .write(true)
            .open(path)
            .map(|file| file.into())
            .map_err(|err| err.raw_os_error().unwrap())
    }
    fn close_restricted(&mut self, fd: OwnedFd) {
        let _file = File::from(fd);
    }
}

fn main() {
    let device = "/dev/input/event7";
    let mut input = Libinput::new_from_path(Interface);
    let mut device = input.path_add_device(device).expect("to get the device");

    dbg!(&device.name());

    if device.config_accel_is_available() {
        device
            .config_accel_set_profile(AccelProfile::Custom)
            .expect("to set profile to custom");

        let config = input::accel_config::AccelConfig::new(AccelProfile::Custom);

        config
            .set_points(
                input::accel_config::AccelType::Motion,
                0.5,
                4,
                vec![3., 4., 5., 6.],
            )
            .expect("to set points");

        dbg!(device.config_accel_profile().unwrap());

        device.config_accel_apply(config).expect("to apply config");
    }
}

Willing to make some updates of what may be better.

Thanks

Context

https://gitlab.freedesktop.org/libinput/libinput/-/merge_requests/775

Drakulix

Thanks for working on this!

Unfortunately there are some issues with this implementation mostly regarding the lifetime of the AccelConfig and the 1:1 copied implementation. But I would very much like to merge this, if you could take the time to tackle these issues!

Drakulix · 2024-07-29T12:30:30Z

Cargo.toml

 use_bindgen = ["input-sys/use_bindgen"]
 libinput_1_11 = ["input-sys/libinput_1_11"]
 libinput_1_14 = ["input-sys/libinput_1_14", "libinput_1_11"]
 libinput_1_15 = ["input-sys/libinput_1_15", "libinput_1_14"]
 libinput_1_19 = ["input-sys/libinput_1_19", "libinput_1_15"]
 libinput_1_21 = ["input-sys/libinput_1_21", "libinput_1_19"]
+libinput_1_26 = ["input-sys/libinput_1_26", "libinput_1_21"]


If this feature was added in 1.23 already, the feature should be named accordingly and the corresponding header file of it's tar-archive should be used for generating.

Thanks for pointing this out. Are we implying we should have a 1.23 version here instead of the 1.26 version? I'm not 100% sure what this is pointing out as a feature. Thanks.

In general these features are meant to be activated for the minimum-version of libinput users of these bindings want to support. So if you care to enable 1_21 because you need some methods introduced with that version, 1.21 is the minimum libinput version on any target system your crate needs.

Which is why this should be libinput_1_23, so that it signifies, that pointer acceleration profiles were added in that version and requiring these options, requires that version of libinput to be available.

Removed libinput_1_26 and added libinput_1_23

Drakulix · 2024-07-29T12:33:50Z

src/accel_config.rs

+    }
+
+    /// Get the pointer for the acceleration config
+    pub fn ptr(&self) -> *mut libinput_config_accel {


Please implement crate::AsRaw instead of rolling your own ptr-method.

Drakulix · 2024-07-29T12:35:27Z

src/accel_config.rs

+///
+/// @warning Unlike other structs pointer acceleration configuration is
+/// considered transient and <b>not</b> refcounted. Calling
+/// libinput_config_accel_destroy() <b>will</b> destroy the configuration.


Looking at these copied docs, we need a Drop implementation for AccelConfig that calls libinput_config_accel_destroy.

Also please reword the docs to not mention ref-counting or destroy calls, as this is handled transparently.

Drakulix · 2024-07-29T12:36:27Z

src/accel_config.rs

+/// configure the profile-specific acceleration properties.
+///
+/// In this version of libinput, this pointer acceleration configuration
+/// only provides configuration for @ref LIBINPUT_CONFIG_ACCEL_PROFILE_CUSTOM.


Also please replace LIBINPUT_CONFIG_ACCEL_PROFILE_CUSTOM with AccelProfile::Custom and generate proper doc-links.

Drakulix · 2024-07-29T12:37:06Z

src/accel_config.rs

+/// @ref libinput_config_accel_set_points.
+///
+/// Once set up, apply the configuration to a device using
+/// libinput_device_config_accel_apply(). Once applied,


Similarly please replace this with a link to Device::config_accel_apply.

Drakulix · 2024-07-29T12:37:25Z

src/accel_config.rs

+/// only provides configuration for @ref LIBINPUT_CONFIG_ACCEL_PROFILE_CUSTOM.
+///
+/// For @ref LIBINPUT_CONFIG_ACCEL_PROFILE_CUSTOM use
+/// @ref libinput_config_accel_set_points.


Similarly please replace this with a link to AccelConfig::set_points.

Drakulix · 2024-07-29T12:37:53Z

src/accel_config.rs

+                accel_type,
+                step,
+                npoints,
+                points.clone().as_mut_ptr(),


We own points, why do we clone?

Removed clone and set points as mut.

Drakulix · 2024-07-29T12:38:21Z

src/accel_config.rs

+/// Acceleration types are categories of movement by a device that may have
+/// specific acceleration functions applied. A device always supports the
+/// @ref LIBINPUT_ACCEL_TYPE_MOTION type (for regular pointer motion). Other
+/// types (e.g. scrolling) may be added in the future.


Please cleanup these docs as well.

Drakulix · 2024-07-29T12:39:20Z

src/device.rs

+    ///
+    /// # Safety
+    ///
+    /// The accel config could become dereferenced


How could it be? During the call it will be valid. Since this method transfers ownership it should be destroyed after this call though. It might be better to accept a reference here.

Set to a reference and removed the Safety.

rockerBOO · 2024-07-29T17:52:37Z

Thanks for taking the time to review this code. I will make the marked improvements. I minimally made these changes and wasn't sure if they would be accepted. I can now work to make this acceptable for merging.

Thanks.

TheOnlyMrCat · 2025-01-16T08:18:23Z

Is this ready for review? I'd like to see this merged (and am happy to do what I can to keep it going)

rockerBOO added 3 commits July 27, 2024 18:21

Add accel config for pointer acceleration

dfb72b1

Cleanup, unnecessary unsafe

b27e7cb

Update custom AccelProfile doc

61e0b84

Drakulix reviewed Jul 29, 2024

View reviewed changes

Remove 1.26.1, add 1.23. Fix pointer/reference

e0dae11

rockerBOO changed the title ~~libinput 1.26.1, Add accel config for pointer acceleration~~ libinput 1.23.1, Add accel config for pointer acceleration Aug 15, 2024

rockerBOO changed the title ~~libinput 1.23.1, Add accel config for pointer acceleration~~ libinput 1.23, Add accel config for pointer acceleration Aug 15, 2024

Remove poor comment

cd7b786

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

libinput 1.23, Add accel config for pointer acceleration #72

libinput 1.23, Add accel config for pointer acceleration #72

rockerBOO commented Jul 27, 2024 •

edited

Loading

Drakulix left a comment

Drakulix Jul 29, 2024

rockerBOO Jul 29, 2024

Drakulix Jul 29, 2024

rockerBOO Aug 15, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

rockerBOO Aug 15, 2024

Drakulix Jul 29, 2024

Drakulix Jul 29, 2024

rockerBOO Aug 15, 2024

rockerBOO commented Jul 29, 2024

TheOnlyMrCat commented Jan 16, 2025

libinput 1.23, Add accel config for pointer acceleration #72

Are you sure you want to change the base?

libinput 1.23, Add accel config for pointer acceleration #72

Conversation

rockerBOO commented Jul 27, 2024 • edited Loading

Drakulix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rockerBOO commented Jul 29, 2024

TheOnlyMrCat commented Jan 16, 2025

rockerBOO commented Jul 27, 2024 •

edited

Loading