[PATCH libinput 07/11] touchpad: only try thumb detection in the lowest 15/8mm

[Peter Hutterer]

On Wed, Jul 22, 2015 at 03:51:09PM +0200, Hans de Goede wrote:
> Hi,
> 
> On 22-07-15 07:09, Peter Hutterer wrote:
> >That's the most likely area it will be resting in, if it's sitting anywhere
> >above that it's likely part of an interaction.
> >
> >A thumb in the lowest 15mm needs to trigger the pressure threshold before it's
> >labelled a thumb. A thumb in the lowest 8mm is considered a thumb if it
> >remains there for 300ms. Regardless of the pressure, since we can't reliably
> >get pressure here. If a thumb moves out of the area, or starts outside of that
> >area it is never a thumb.
> 
> "If a thumb moves out of the area" -> not so in the code:
> 
> <snip>
> 
> >  static void
> >-tp_thumb_detect(struct tp_dispatch *tp, struct tp_touch *t)
> >+tp_thumb_detect(struct tp_dispatch *tp, struct tp_touch *t, uint64_t time)
> >  {
> >-	/* once a thumb, always a thumb */
> >-	if (!tp->thumb.detect_thumbs || t->is_thumb)
> >+	enum tp_thumb_state state = t->thumb.state;
> >+
> >+	/* once a thumb, always a thumb, once ruled out always ruled out */
> >+	if (!tp->thumb.detect_thumbs ||
> >+	    t->thumb.state != THUMB_STATE_MAYBE)
> >  		return;
> >
> 
> This check make it so that a touch labeled a thumb (THUMB_STATE_YES)
> will stay a thumb even if it moves above the upper_thumb_line.

I'll change the commit message to clarify this. The reason for this check is
to simplify the code, once something is a thumb, it will remain a thumb even
when it moves about or changes pressure. it means we don't have to put code
into everything to check if something is _still_ a thumb and potentially get
odd behaviour. If that causes issues we can add the extra code required, but
until then I prefer the simple solution.

so the commit message should say "if a thumb moves out of the area _within a
timeout_ ..."

Cheers,
   Peter

> 
> >+	if (t->point.y < tp->thumb.upper_thumb_line) {
> >+		/* if a potential thumb is above the line, it won't ever
> >+		 * label as thumb */
> >+		t->thumb.state = THUMB_STATE_NO;
> >+		goto out;
> >+	}
> >+
> 
> This can be fixed by moving this check above the previous one.
> 
> Otherwise looks good.
> 
> Regards,
> 
> Hans