Hi,
On 06/04/2014 10:57 PM, Jonas Ådahl wrote:
> On Wed, Jun 04, 2014 at 11:10:35AM +0200, Hans de Goede wrote:
>> Hi,
>>
>> On 06/03/2014 07:34 AM, Peter Hutterer wrote:
>>> Now that we have run-time changes of the tap.enabled state move the check
>>> to the IDLE state only. Otherwise the tap machine may hang if tapping is
>>> disabled while a gesture is in progress.
>>>
>>> Two basic tests are added to check for the tap default setting - which is
>>> now
>>> "tap disabled by default", because a bike shed's correct color is green.
>>>
>>> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
>>> ---
>>> src/evdev-mt-touchpad-tap.c | 56
>>> +++++++++++++++++++++++++++++++++++++++------
>>> src/evdev-mt-touchpad.h | 1 +
>>> test/touchpad.c | 32 ++++++++++++++++++++++++++
>>> 3 files changed, 82 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/src/evdev-mt-touchpad-tap.c b/src/evdev-mt-touchpad-tap.c
>>> index eee334f..24ea8c3 100644
>>> --- a/src/evdev-mt-touchpad-tap.c
>>> +++ b/src/evdev-mt-touchpad-tap.c
>>> @@ -438,13 +438,13 @@ static void
>>> tp_tap_handle_event(struct tp_dispatch *tp, enum tap_event event, uint64_t
>>> time)
>>> {
>>> enum tp_tap_state current;
>>> - if (!tp->tap.enabled)
>>> - return;
>>>
>>> current = tp->tap.state;
>>>
>>> switch(tp->tap.state) {
>>> case TAP_STATE_IDLE:
>>> + if (!tp->tap.enabled)
>>> + break;
>>> tp_tap_idle_handle_event(tp, event, time);
>>> break;
>>> case TAP_STATE_TOUCH:
>>> @@ -572,9 +572,6 @@ tp_tap_timeout_handler(void *data)
>>> unsigned int
>>> tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t time)
>>> {
>>> - if (!tp->tap.enabled)
>>> - return 0;
>>> -
>>> if (tp->tap.timeout && tp->tap.timeout <= time) {
>>> tp_tap_clear_timer(tp);
>>> tp_tap_handle_event(tp, TAP_EVENT_TIMEOUT, time);
>>> @@ -583,9 +580,56 @@ tp_tap_handle_timeout(struct tp_dispatch *tp, uint64_t
>>> time)
>>> return tp->tap.timeout;
>>> }
>>>
>>> +static int
>>> +tp_tap_config_count(struct libinput_device *device)
>>> +{
>>> + struct evdev_dispatch *dispatch = ((struct evdev_device
>>> *)device)->dispatch;
>>> + struct tp_dispatch *tp = container_of(dispatch, tp, base);
>>> +
>>> + return min(tp->ntouches, 3); /* we only do up to 3 finger tap */
>>> +}
>>> +
>>> +static enum libinput_config_status
>>> +tp_tap_config_enable(struct libinput_device *device, int enabled)
>>> +{
>>> + struct evdev_dispatch *dispatch = ((struct evdev_device
>>> *)device)->dispatch;
>>
>> Please user container_of here for proper type checking rather then
>> just a blatant cast of $random_type_foo to $random_type_bar. Same for all
>> the other occurrences of the same pattern below.
>
> Would just like to point out that there are several places this cast is
> already in place, and starting to use container_of would introduce
> inconsistencies. I think this kind of casting isn't necessarily bad for
> single (or primary) inheritance types of objects.
I really believe that we should make use of compiler type checking were
possible. Lets go with Peters suggestion to do a separate patch to move
all these kind of casts over to container_of in one big patch.
Regards,
Hans
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
