Thread (37 messages) 37 messages, 4 authors, 2011-08-11

Re: [PATCH 7/9 v2] Input: synaptics - improved 2->3 finger transition handling

From: Daniel Kurtz <hidden>
Date: 2011-07-28 01:00:28
Also in: lkml

On Thu, Jul 28, 2011 at 5:13 AM, Chase Douglas
[off-list ref] wrote:
[...]
quoted
quoted
quoted
Would you prefer an implementation that continued to report count (via
BTN_TOUCH*) correctly, but dropped down to 0 or 1 MT-B slots when for
these cases where it could not determine the correct position or track_id
to report?
That may be doable, but I would prefer to just assume that tracking ids
are not valid when (tracked touches > reported touches).
Userspace is free to make this assumption, of course, but, in fact,
the image sensor trackpad actually does a pretty good job of tracking
the fingers - it just has serious issues reporting them!
Since a track_id change is how userspace is told that the identity of
the reported finger is changing, if the track_id of a finger position
datum is unknowable, I'd rather just discard it in the kernel than
report it to userspace with the wrong track_id.
Otherwise, the heuristics used in the userspace finger tracking
algorithms would need to be overly aggressively tuned to handle this
known error cases:
  2->3 and 3->2 finger transitions look like 2nd finger motion,
instead of reported finger changes.
quoted
quoted
It seems like it would be more work for userspace to handle this new way
than the simulated number-of-touch transitions, where the transient
states are all normal valid states.
This harkens back to my earlier statements where I said this new
Synaptics protocol is worse than the previous one :).

I agree that the implementation you gave here might be trickier for
userspace, so I'd rather table it unless you feel that the "tracking ids
are meaningless!" solution won't work. If you think there are problems
with that approach, we can re-evaluate.

Thanks!

-- Chase
Yes, I feel there are problems with this approach, as I tried to explain above.
Can you explain why you 'continuation gestures' can't handle 1->2->3
finger transitions looking like 1->2->1->3, and 3->2->3 looking like
3->2->0->3?

I think the only real point left to decide is what BTN_* events should
signify during these rare transition states:
  (a) the actually number of fingers on the pad,
  (b) the number of fingers being reported via the slots

The current patchset does (a).
We could do (b), if that would get these patches accepted sooner :)
I was thinking that the current patchset does (b). I think (a) is
better, and if it really works that way then I'm fine with it. It's hard
for me to keep track of the flow of the logic across the patches :).
Argh, my bad.  You are correct.  Current patchset does (b)!
It reports the number of active slots, not the number of touches.

In any case, I really don't know why you need (a).  We are talking
about some degenerate transitions here.  Your userspace driver should
work just fine with the (b) driver, it just loses some really
complicated continued gestures for hardware that can't support them.
That said, merging this patchset as is effectively means that the number
of slots is completely decoupled from the number of touches on the
device. Previously, one could say that the number of touches on the
device was equal to the number of open slots or more if all slots were
open. With this approach, we could have 0 slots open during a transition
when there are still touches down.

While the distinction makes sense for these synaptics devices, I don't
want the semantics to hold for full multitouch devices. Otherwise, we
would have to add many more BTN_*TAPs. If we go this route, we must have
a way to tell userspace that this is a special device where the number
of active touches can only be determined from BTN_*TAP. Thus, we would
need a property for this exception to normal behavior.
Henrik & Dmitry roughly suggested "do not define a new property; let
userspace detect max BTN_*TAP > ABS_MT_SLOT.max to indicate that
BTN_*TAP carries the total number of touches".  I wish they would
chime in on this patchset...
(PS: As I've thought more about it, I don't think we need the property I
was advocating for before. That property was for denoting that the
device tracks more than it reports. If we're going to get this complex
in the protocol, there's not much you can do with bitmask properties to
denote every specific special case.)

-- Chase
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help