On 07/28/2011 06:56 AM, Daniel Kurtz wrote: > On Thu, Jul 28, 2011 at 10:07 AM, Chase Douglas > <chase.douglas@xxxxxxxxxxxxx> wrote: >> On 07/27/2011 06:00 PM, Daniel Kurtz wrote: >>> On Thu, Jul 28, 2011 at 5:13 AM, Chase Douglas >>> <chase.douglas@xxxxxxxxxxxxx> wrote: >>> [...] >>>>>> >>>>>>> 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. >>>>> >>>>>> >>>>>>> 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. >> >> To give userspace incorrect information about the number of touches on >> the device is always bad. Lets say the degenerate case is when you go >> from two touches to three (maybe Synaptics doesn't do this, but someone >> else might). When the user performs a three finger tap, we'd see two >> touches down, two lifted up, three touches down, three lifted up in >> short succession. Userspace is gonna get pretty confused about that :). >> >> (Please don't make me try to come up with a use case we already have in >> Unity that would be broken for Synaptics due to this. I could probably >> find one, but it would take quite a bit of thinking. :) > > Just to be clear: > > Debouncing num-fingers at the start of a tap works just fine. > For a 3f-tap, you would see one of: > 0->1->2->3 > 0->2->3 > 0->3 > > Not: > 0->1->2->0->3 > 0->2->0->3 > > You only see 2->0->3 if there had previously been 3 fingers down, and > some of those fingers are removed: > 0->3->0*->2*->0*->3* > 0->3->0*->1* > > Where the "*" states are when mt_state_lost = true. > So, with method (b), you might see 3->0->2->0 if the release of the > other two fingers was really slow (longer than 37.5 ms), or, more > likely on a tap, just 3->0. > > In any case, I don't think we are making progress arguing (a) vs. (b). > Let me implement method (a), and upload for review. > I agree that it makes some sense to always accurately report number of > touches with the BTN_*, independent of number of slots. > > A true MT-B userspace app would always do the right thing with the > slots, legacy apps can always do the right thing in legacy mode, and a > hybrid app get do 2-touch multitouch & use BTN_* to determine total > number of touches. > > Was there anything else I should add/change while I'm at it? No, I think functionally I would be happy with the new version. I still want the property bit :). Dmitry, can you weigh in here? What I remember was you were disinclined towards a new property bit, but I'd like to see a firm decision taken and the reasoning behind it. > You mention documentation below, was there something in particular > that you'd like to see documented better? Where? Documentation of anything that goes against the normal protocol should be in Documentation/input/event-codes.txt and/or Documentation/input/multi-touch-protocol.txt. The latter seems the most appropriate place for this. If you want extra credit you could document SEMI_MT as well, I think that slipped through the cracks; this isn't a requirement for me, though :). Thanks! >>>> 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... >> >> I think it sets a really bad precedent to obey the stated protocol in >> most cases, but disregard it in others if specific conditions are met, >> which are not documented and are not presented in a clear manner to >> userspace. At the *very least*, this change would need documentation to >> go in at the same time, but I strongly believe a property is merited here. >> >> -- Chase > -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
