On 07/30/2013 10:28 PM, Jonas Ådahl wrote:
> On Tue, Jul 30, 2013 at 09:36:25PM +0200, Armin K wrote:
>> This patch adds 3 new options to weston.ini to allow
>> the user to change default constant_accel_factor,
>> min_accel_factor and max_accel_factor. If no options
>> are set, it falls back using defaults as it did before.
>>
>> v2: create weston_config_section_get_double and use it
>> instead of manualy converting string to double.
>>
>> v3: add default values in weston_config_get_double
>> instead of using conditionals.
>
> One comments on a minor details below:
>
>> ---
>> shared/config-parser.c | 26 ++++++++++++++++++++++++++
>> shared/config-parser.h | 4 ++++
>> src/evdev-touchpad.c | 40 +++++++++++++++++++++++++++++++++++-----
>> weston.ini | 5 +++++
>> 4 files changed, 70 insertions(+), 5 deletions(-)
>>
>> diff --git a/shared/config-parser.c b/shared/config-parser.c
>> index 4e6cf7f..f98209c 100644
>> --- a/shared/config-parser.c
>> +++ b/shared/config-parser.c
>> @@ -337,6 +337,32 @@ weston_config_section_get_uint(struct
>> weston_config_section *section,
>>
>> WL_EXPORT
>> int
>> +weston_config_section_get_double(struct weston_config_section *section,
>> + const char *key,
>> + double *value, double default_value)
>> +{
>> + struct weston_config_entry *entry;
>> + char *end;
>> +
>> + entry = config_section_get_entry(section, key);
>> + if (entry == NULL) {
>> + *value = default_value;
>> + errno = ENOENT;
>> + return -1;
>> + }
>> +
>> + *value = strtod(entry->value, &end);
>> + if (*end != '\0') {
>> + *value = default_value;
>> + errno = EINVAL;
>> + return -1;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +WL_EXPORT
>> +int
>> weston_config_section_get_string(struct weston_config_section *section,
>> const char *key,
>> char **value, const char *default_value)
>> diff --git a/shared/config-parser.h b/shared/config-parser.h
>> index 794e09c..410a7ef 100644
>> --- a/shared/config-parser.h
>> +++ b/shared/config-parser.h
>> @@ -88,6 +88,10 @@ weston_config_section_get_uint(struct
>> weston_config_section *section,
>> const char *key,
>> uint32_t *value, uint32_t default_value);
>> int
>> +weston_config_section_get_double(struct weston_config_section *section,
>> + const char *key,
>> + double *value, double default_value);
>> +int
>> weston_config_section_get_string(struct weston_config_section *section,
>> const char *key,
>> char **value,
>> diff --git a/src/evdev-touchpad.c b/src/evdev-touchpad.c
>> index 53300ce..e727e49 100644
>> --- a/src/evdev-touchpad.c
>> +++ b/src/evdev-touchpad.c
>> @@ -26,10 +26,12 @@
>> #include <math.h>
>> #include <string.h>
>> #include <stdbool.h>
>> +#include <unistd.h>
>> #include <linux/input.h>
>>
>> #include "filter.h"
>> #include "evdev.h"
>> +#include "../shared/config-parser.h"
>>
>> /* Default values */
>> #define DEFAULT_CONSTANT_ACCEL_NUMERATOR 50
>> @@ -670,6 +672,38 @@ struct evdev_dispatch_interface touchpad_interface = {
>> touchpad_destroy
>> };
>>
>> +static void
>> +touchpad_parse_config(struct touchpad_dispatch *touchpad, double *diagonal)
>
> Why make the diagonal parameter a pointer? Looks to be like it shouldn't
> be.
>
Because it is calculated in the touchpad_init function and is used after
this function call. Instead of copying the parameter (reserving more
memory), we just indirectly access it. I don't see the problem.
>> +{
>> + struct weston_config *config;
>> + struct weston_config_section *s;
>> + int config_fd;
>> +
>> + double constant_accel_factor;
>> + double min_accel_factor;
>> + double max_accel_factor;
>> +
>> + config_fd = open_config_file("weston.ini");
>> + config = weston_config_parse(config_fd);
>> + close(config_fd);
>> +
>> + s = weston_config_get_section(config, "touchpad", NULL, NULL);
>> + weston_config_section_get_double(s, "constant_accel_factor",
>> + &constant_accel_factor,
>> + DEFAULT_CONSTANT_ACCEL_NUMERATOR);
>> + weston_config_section_get_double(s, "min_accel_factor",
>> + &min_accel_factor,
>> + DEFAULT_MIN_ACCEL_FACTOR);
>> + weston_config_section_get_double(s, "max_accel_factor",
>> + &max_accel_factor,
>> + DEFAULT_MAX_ACCEL_FACTOR);
>> +
>> + touchpad->constant_accel_factor =
>> + constant_accel_factor / *diagonal;
>> + touchpad->min_accel_factor = min_accel_factor;
>> + touchpad->max_accel_factor = max_accel_factor;
>> +}
>> +
>> static int
>> touchpad_init(struct touchpad_dispatch *touchpad,
>> struct evdev_device *device)
>> @@ -710,11 +744,7 @@ touchpad_init(struct touchpad_dispatch *touchpad,
>> height = abs(device->abs.max_y - device->abs.min_y);
>> diagonal = sqrt(width*width + height*height);
>>
>> - touchpad->constant_accel_factor =
>> - DEFAULT_CONSTANT_ACCEL_NUMERATOR / diagonal;
>> -
>> - touchpad->min_accel_factor = DEFAULT_MIN_ACCEL_FACTOR;
>> - touchpad->max_accel_factor = DEFAULT_MAX_ACCEL_FACTOR;
>> + touchpad_parse_config(touchpad, &diagonal);
>>
>> touchpad->hysteresis.margin_x =
>> diagonal / DEFAULT_HYSTERESIS_MARGIN_DENOMINATOR;
>> diff --git a/weston.ini b/weston.ini
>> index f2abceb..ff0f3ba 100644
>> --- a/weston.ini
>> +++ b/weston.ini
>> @@ -57,3 +57,8 @@ path=/usr/libexec/weston-keyboard
>> #name=X1
>> #mode=1024x768
>> #transform=flipped-270
>> +
>> +#[touchpad]
>> +#constant_accel_factor = 50
>> +#min_accel_factor = 0.16
>> +#max_accel_factor = 1.0
>> --
>> 1.8.3.4
>>
>
> Jonas
> _______________________________________________
> wayland-devel mailing list
> wayland-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
