On 05/22/2014 05:35 AM, Peter Hutterer wrote:
> On Tue, May 20, 2014 at 04:34:57PM +0200, Hans de Goede wrote:
>> We don't want touches in the button area to cause the pointer to move. So
>> instead of making a touch the pointer when it moves to TOUCH_BEGIN, wait
>> with making it the pointer until its buttons state moves to
>> BUTTON_STATE_AREA.
>>
>> Note that a touch in the main area of the touchpad will move to
>> BUTTON_STATE_AREA immediately.
>
> I stared at that code for a while before I realised why this wouldn't
> regress older touchpads - please add here that "If software-buttons are not
> enabled, any finger is in the BUTTON_STATE_AREA".
Updated the commit msg with this.
> two more style-only fixes below.
>
>> While at it also refactor the is_pointer setting in general, removing
>> code duplicition wrt checking that another touch is not already
>> the pointer on unpinning a finger, and add safeguards that unpinning
>> does not make a finger which is not in button state BUTTON_STATE_AREA the
>> pointer, nor that the button code makes a pinned finger the pointer.
>>
>> All these sanity checks are combined into a new tp_button_active function,
>> since they should be taken into account for 2 finger scrolling, etc. too.
>>
>> Signed-off-by: Hans de Goede <hdego...@redhat.com>
>> Signed-off-by: Peter Hutterer <peter.hutte...@who-t.net>
>> ---
>> src/evdev-mt-touchpad-buttons.c | 7 ++++++
>> src/evdev-mt-touchpad.c | 49
>> +++++++++++++++++++++--------------------
>> src/evdev-mt-touchpad.h | 6 +++++
>> test/touchpad.c | 8 +++----
>> 4 files changed, 42 insertions(+), 28 deletions(-)
>>
>> diff --git a/src/evdev-mt-touchpad-buttons.c
>> b/src/evdev-mt-touchpad-buttons.c
>> index 61055ac..e47a55e 100644
>> --- a/src/evdev-mt-touchpad-buttons.c
>> +++ b/src/evdev-mt-touchpad-buttons.c
>> @@ -142,6 +142,7 @@ tp_button_set_state(struct tp_dispatch *tp, struct
>> tp_touch *t,
>> break;
>> case BUTTON_STATE_AREA:
>> t->button.curr = BUTTON_EVENT_IN_AREA;
>> + tp_set_pointer(tp, t);
>> break;
>> case BUTTON_STATE_BOTTOM:
>> break;
>> @@ -598,3 +599,9 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t
>> time)
>>
>> return rc;
>> }
>> +
>> +int
>> +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t)
>> +{
>> + return t->button.state == BUTTON_STATE_AREA;
>> +}
>> diff --git a/src/evdev-mt-touchpad.c b/src/evdev-mt-touchpad.c
>> index 7f73f6e..924ba16 100644
>> --- a/src/evdev-mt-touchpad.c
>> +++ b/src/evdev-mt-touchpad.c
>> @@ -152,8 +152,6 @@ tp_get_touch(struct tp_dispatch *tp, unsigned int slot)
>> static inline void
>> tp_begin_touch(struct tp_dispatch *tp, struct tp_touch *t)
>> {
>> - struct tp_touch *tmp = NULL;
>> -
>> if (t->state != TOUCH_UPDATE) {
>> tp_motion_history_reset(t);
>> t->dirty = true;
>> @@ -162,15 +160,6 @@ tp_begin_touch(struct tp_dispatch *tp, struct tp_touch
>> *t)
>> tp->nfingers_down++;
>> assert(tp->nfingers_down >= 1);
>> tp->queued |= TOUCHPAD_EVENT_MOTION;
>> -
>> - tp_for_each_touch(tp, tmp) {
>> - if (tmp->is_pointer)
>> - break;
>> - }
>> -
>> - if (!tmp->is_pointer) {
>> - t->is_pointer = true;
>> - }
>> }
>> }
>>
>> @@ -342,7 +331,6 @@ static void
>> tp_unpin_finger(struct tp_dispatch *tp, struct tp_touch *t)
>> {
>> unsigned int xdist, ydist;
>> - struct tp_touch *tmp = NULL;
>>
>> if (!t->pinned.is_pinned)
>> return;
>> @@ -350,19 +338,11 @@ tp_unpin_finger(struct tp_dispatch *tp, struct
>> tp_touch *t)
>> xdist = abs(t->x - t->pinned.center_x);
>> ydist = abs(t->y - t->pinned.center_y);
>>
>> - if (xdist * xdist + ydist * ydist <
>> - tp->buttons.motion_dist * tp->buttons.motion_dist)
>> - return;
>> -
>> - t->pinned.is_pinned = false;
>> -
>> - tp_for_each_touch(tp, tmp) {
>> - if (tmp->is_pointer)
>> - break;
>> + if (xdist * xdist + ydist * ydist >=
>> + tp->buttons.motion_dist * tp->buttons.motion_dist) {
>
> don't indent the line here please.
No idea what exactly you want me to do here, note the indentation is unchanged
from the original check a few lines higher up. Not fixed.
>
>> + t->pinned.is_pinned = false;
>> + tp_set_pointer(tp, t);
>> }
>> -
>> - if (t->state != TOUCH_END && !tmp->is_pointer)
>> - t->is_pointer = true;
>> }
>>
>> static void
>> @@ -378,6 +358,27 @@ tp_pin_fingers(struct tp_dispatch *tp)
>> }
>> }
>>
>> +static int
>> +tp_touch_active(struct tp_dispatch *tp, struct tp_touch *t)
>> +{
>> + return (t->state == TOUCH_BEGIN || t->state == TOUCH_UPDATE) &&
>> + !t->pinned.is_pinned && tp_button_touch_active(tp, t);
>> +}
>> +
>> +void tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t)
>
> please add a line-break after void
Done.
Regards,
Hans
>
> Cheers,
> Peter
>
>> +{
>> + struct tp_touch *tmp = NULL;
>> +
>> + /* Only set the touch as pointer if we don't have one yet */
>> + tp_for_each_touch(tp, tmp) {
>> + if (tmp->is_pointer)
>> + return;
>> + }
>> +
>> + if (tp_touch_active(tp, t))
>> + t->is_pointer = true;
>> +}
>> +
>> static void
>> tp_process_state(struct tp_dispatch *tp, uint32_t time)
>> {
>> diff --git a/src/evdev-mt-touchpad.h b/src/evdev-mt-touchpad.h
>> index 8d8dd84..5509e93 100644
>> --- a/src/evdev-mt-touchpad.h
>> +++ b/src/evdev-mt-touchpad.h
>> @@ -200,6 +200,9 @@ struct tp_dispatch {
>> void
>> tp_get_delta(struct tp_touch *t, double *dx, double *dy);
>>
>> +void
>> +tp_set_pointer(struct tp_dispatch *tp, struct tp_touch *t);
>> +
>> int
>> tp_tap_handle_state(struct tp_dispatch *tp, uint32_t time);
>>
>> @@ -229,4 +232,7 @@ tp_post_button_events(struct tp_dispatch *tp, uint32_t
>> time);
>> int
>> tp_button_handle_state(struct tp_dispatch *tp, uint32_t time);
>>
>> +int
>> +tp_button_touch_active(struct tp_dispatch *tp, struct tp_touch *t);
>> +
>> #endif
>> diff --git a/test/touchpad.c b/test/touchpad.c
>> index bbae6cd..959978e 100644
>> --- a/test/touchpad.c
>> +++ b/test/touchpad.c
>> @@ -70,10 +70,10 @@ START_TEST(touchpad_2fg_no_motion)
>>
>> litest_drain_events(li);
>>
>> - litest_touch_down(dev, 0, 50, 50);
>> - litest_touch_down(dev, 1, 70, 70);
>> - litest_touch_move_to(dev, 0, 50, 50, 80, 50, 5);
>> - litest_touch_move_to(dev, 1, 70, 70, 80, 50, 5);
>> + litest_touch_down(dev, 0, 20, 20);
>> + litest_touch_down(dev, 1, 70, 20);
>> + litest_touch_move_to(dev, 0, 20, 20, 80, 80, 5);
>> + litest_touch_move_to(dev, 1, 70, 20, 80, 50, 5);
>> litest_touch_up(dev, 0);
>> litest_touch_up(dev, 1);
>>
>> --
>> 1.9.0
>>
_______________________________________________
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
