enlightenment/enlightenment
enlightenment
/
enlightenment
10
9
Fork 12
Code Issues 24 Pull Requests 2 Projects Releases Wiki Activity

xinput: Add support for flat mouse acceleration and Hi-Res scrolling #36

Merged
raster merged 3 commits from nekobit/enlightenment:needs-moar-input into master 2023-03-18 01:37:02 -07:00
Conversation 2 Commits 3 Files Changed 7 +83 -9
nekobit commented 2023-03-09 19:17:40 -08:00
Contributor
Copy Link

Flat mouse accel is fairly obvious.

Hi-Res scrolling is an option that is useful on certain mice that technically support 'pixel-perfect' scrolling, but still 'click' like regular mice would when you scroll. Some mice are designed to use pixel-perfect scrolling, so keeping it optional is useful.

Remark: No Wayland support yet, maybe later.

Flat mouse accel is fairly obvious. Hi-Res scrolling is an option that is useful on certain mice that technically support 'pixel-perfect' scrolling, but still 'click' like regular mice would when you scroll. Some mice are designed to use pixel-perfect scrolling, so keeping it optional is useful. Remark: No Wayland support yet, maybe later.
nekobit added 1 commit 2023-03-09 19:17:43 -08:00
5b85700ada xinput: Add support for flat mouse acceleration and Hi-Res scrolling 
Flat mouse accel is fairly obvious.
Hi-Res scrolling is an option that is useful on certain mice that technically support 'pixel-perfect' scrolling, but still 'click' like regular mice would when you scroll. Some mice are designed to use pixel-perfect scrolling, so keeping it optional is useful.
nekobit started working 2023-03-09 19:28:58 -08:00
nekobit canceled time tracking 2023-03-09 19:29:01 -08:00
raster requested changes 2023-03-10 01:25:14 -08:00
raster left a comment
Owner
Copy Link

One more overall comment. All config values will be 0/NULL by default. Eet guarantees this. If the data file it decodes does not encode that field you get a 0.

You modify the config profile for a new default value. That's fine - someone with a NEW profile starting from scratch from the wizard will get this value you put in, but someone with an EXISTING user config will not - they will see the value as 0 - always.

E has config versioning to handle. this. there are EPOCH and GENERATION versions. A gnerartion version update has CONFIG_VERSION_CHECK() macros to upgrade a user's config when loaded - if config is less than version X then set some new values to something, then bump version up. you'll see a whole set of these at the end of the e_config.c loader code in e_config_load(). you can choose to be happy with what you have here knowing it'll be 0 by default, or you can truy bumping E_CONFIG_FILE_GENERATION. bumpibn EPOCH says "everything is now invalid - start again from nothing as we broke everything".

So a minor code style change and think of the above - are you happy with how it's going to work given the above?

One more overall comment. All config values will be 0/NULL by default. Eet guarantees this. If the data file it decodes does not encode that field you get a 0. You modify the config profile for a new default value. That's fine - someone with a NEW profile starting from scratch from the wizard will get this value you put in, but someone with an EXISTING user config will not - they will see the value as 0 - always. E has config versioning to handle. this. there are EPOCH and GENERATION versions. A gnerartion version update has CONFIG_VERSION_CHECK() macros to upgrade a user's config when loaded - if config is less than version X then set some new values to something, then bump version up. you'll see a whole set of these at the end of the e_config.c loader code in e_config_load(). you can choose to be happy with what you have here knowing it'll be 0 by default, or you can truy bumping E_CONFIG_FILE_GENERATION. bumpibn EPOCH says "everything is now invalid - start again from nothing as we broke everything". So a minor code style change and think of the above - are you happy with how it's going to work given the above?
src/bin/e_comp_x_devices.c Outdated
@ -152,0 +154,5 @@
cfval = e_config->touch_flat_accel;
else
cfval = e_config->mouse_flat_accel;
if ((val) && (size == 8) && (num == 2) && ((cfval == 1 && val[0] == 1) ||
raster commented 2023-03-10 01:13:21 -08:00
Owner
Copy Link

Just a style thing - I know it's not technically needed here but I always pefer to do:

if ((a == 1) && (b == 1))

instead of:

if (a == 1 && b == 1)

Why? The first is very explicit as to what the programmer INTENTED - you know they are not relying on a mistaken memory of operator precedence. We all know * and / happen before + and - .. but when you start throwing in &, | &&, || etc. it gets more hairy and so just as a style thing - always best to be crystal clear on what was intended to begin with. It's one of those "defensive programming" things that helps lead to fewere bugs.

Just a style thing - I know it's not technically needed here but I always pefer to do: ``` C if ((a == 1) && (b == 1)) ``` instead of: ``` C if (a == 1 && b == 1) ``` Why? The first is very explicit as to what the programmer INTENTED - you know they are not relying on a mistaken memory of operator precedence. We all know * and / happen before + and - .. but when you start throwing in &, | &&, || etc. it gets more hairy and so just as a style thing - always best to be crystal clear on what was intended to begin with. It's one of those "defensive programming" things that helps lead to fewere bugs.
nekobit commented 2023-03-10 07:44:17 -08:00
Author
Contributor
Copy Link

Fixed. Personally I find that a little overly defensive in some cases, since the == operator is fairly common knowledge, but I do see why that opinion is formed: bitmask operator usage, which can definitely trick people up, myself included (i.e. (x & 4 == 0) != ((x & 4) == 0)), and consistancy.

Fixed. Personally I find that a little overly defensive in some cases, since the == operator is fairly common knowledge, but I do see why that opinion is formed: bitmask operator usage, which can definitely trick people up, myself included (i.e. `(x & 4 == 0) != ((x & 4) == 0)`), and consistancy.
nekobit marked this conversation as resolved
nekobit added 1 commit 2023-03-10 07:38:37 -08:00
nekobit commented 2023-03-10 08:08:58 -08:00
Author
Contributor
Copy Link

Unrelated, but I feel like there should be a better approach to this CONFIG_VERSION_CHECK approach.

Notice in the _e_config_edd_init function, we initialize all values here.

E_CONFIG_VAL(D, T, mouse_accel, DOUBLE);
E_CONFIG_VAL(D, T, mouse_flat_accel, UCHAR);
E_CONFIG_VAL(D, T, mouse_accel_threshold, INT);

But what if we added the version (or even a UNIX timestamp) to the configuration initiation check using a variadic macro.

E_CONFIG_VAL(D, T, mouse_accel, DOUBLE);
E_CONFIG_VAL(D, T, mouse_flat_accel, UCHAR, 35); // New!
E_CONFIG_VAL(D, T, mouse_accel_threshold, INT, 36); // New!

Of course we can keep the number bump approach using a global variable / eet chunk, though that number could be hard to read out unless the E_CONFIG_VAL's are added well-ordered.

that way we can reduce some duplicated code spread out later on.

Thoughts? Nonetheless, ready to merge when you are.

Unrelated, but I feel like there should be a better approach to this `CONFIG_VERSION_CHECK` approach. Notice in the `_e_config_edd_init` function, we initialize all values here. ``` E_CONFIG_VAL(D, T, mouse_accel, DOUBLE); E_CONFIG_VAL(D, T, mouse_flat_accel, UCHAR); E_CONFIG_VAL(D, T, mouse_accel_threshold, INT); ``` But what if we added the version (or even a UNIX timestamp) to the configuration initiation check using a variadic macro. ``` E_CONFIG_VAL(D, T, mouse_accel, DOUBLE); E_CONFIG_VAL(D, T, mouse_flat_accel, UCHAR, 35); // New! E_CONFIG_VAL(D, T, mouse_accel_threshold, INT, 36); // New! ``` Of course we can keep the number bump approach using a global variable / eet chunk, though that number could be hard to read out unless the `E_CONFIG_VAL`'s are added well-ordered. that way we can reduce some duplicated code spread out later on. Thoughts? Nonetheless, ready to merge when you are.
nekobit requested review from raster 2023-03-10 08:14:48 -08:00
nekobit added 1 commit 2023-03-10 08:17:24 -08:00
raster commented 2023-03-18 01:35:45 -07:00
Owner
Copy Link

for your config comments.. .you need to follow the breadcrumbs a bit. what you are asking kind of isn't sane. those macros simply call another macro that adds an entry to eet's map of c struct contents so it knows "at offset 56 bytes in is an int type that will be called "version" ". that means eet when it walks memory knows there is an int there and how to encode/decode it. eet doesnt have anything to do with defaults because imagine that entry is a linked list instead... how do you express a default as a linked list in a macro? :) so version handling is done after struct decode knowing any un-encoded values in the data will be 0.

for your config comments.. .you need to follow the breadcrumbs a bit. what you are asking kind of isn't sane. those macros simply call another macro that adds an entry to eet's map of c struct contents so it knows "at offset 56 bytes in is an int type that will be called "version" ". that means eet when it walks memory knows there is an int there and how to encode/decode it. eet doesnt have anything to do with defaults because imagine that entry is a linked list instead... how do you express a default as a linked list in a macro? :) so version handling is done after struct decode knowing any un-encoded values in the data will be 0.
raster merged commit 766360359b into master 2023-03-18 01:37:02 -07:00
raster deleted branch needs-moar-input 2023-03-18 01:37:04 -07:00
Sign in to join this conversation.
Reviewers
No reviewers
raster
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: enlightenment/enlightenment#36
No description provided.
Delete Branch "nekobit/enlightenment:needs-moar-input"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?