[PATCH v3] evdev-touchpad: Set some options using weston.ini

[Armin K.]

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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel
>