Tip state parsing logic is problematic

[AlexGuo1998]

This State parsing logic contains problems:

MagicTrackpad2ForWindows/AmtPtpHidFilter/Input.c

Lines 194 to 199 in f9c2be8

	// 0x1 = Transition between states
	// 0x2 = Floating finger
	// 0x4 = Contact/Valid
	// I've gotten 0x6 if I press on the trackpad and then keep my finger close
	// Note: These values come from my MBP9,2. These also are valid on my MT2
	ptpOutputReport.Contacts[i].TipSwitch = (f->State & 0x4) && (driverContext->IgnoreNearFingers == FALSE ? TRUE : !(f->State & 0x2));

After carefully reviewing some packet captures from a Magic Trackpad 2, the correct definition seems to be

Value	State	Notes
1	ToHover	(Transition from any state to Hover)
2	Hover	Finger approaching the trackpad, but have not (already) contacted
3	ToTouch	(Transition from any state to Touch)
4	Touch	Finger touching the trackpad
5	ToLeave	(Transition from any state to Leave)
6	Leave	After a touch, finger going back to hovering
7	ToOff	(Transition from any state to Off)
0	Off	The final state, when the finger is completely away from the trackpad

Captured data

(+ means the transition bit is set)

Swipe on the trackpad

1 +hover (This state may be missed)
3 +touch
4  touch
4  touch
...
4  touch
4  touch
5 +leave
7 +  off
0    off

Slow approach-touch-leave

2  hover (For the first finger, the first state is 2 instead of 1)
2  hover
2  hover
...
2  hover
2  hover
3 +touch
4  touch
4  touch
...
4  touch
4  touch
4  touch
5 +leave
6  leave
7 +  off
0    off

Slow approach-touch-leave while another finger touching

#1 4  touch
#1 4  touch
#1 4  touch
#1 4  touch #9 1 +hover (For the second finger, the first state is 1)
#1 4  touch #9 2  hover
#1 4  touch #9 2  hover
...
#1 4  touch #9 2  hover
#1 4  touch #9 2  hover
#1 4  touch #9 3 +touch
#1 4  touch #9 4  touch
#1 4  touch #9 4  touch
...
#1 4  touch #9 4  touch
#1 4  touch #9 4  touch
#1 4  touch #9 5 +leave
#1 4  touch #9 6  leave
#1 4  touch #9 6  leave
...
#1 4  touch #9 6  leave
#1 4  touch #9 6  leave
#1 4  touch #9 7 +  off
#1 4  touch #9 0    off
#1 4  touch #9 0    off (Yes, there are 2 frames with state 0, for the second finger)
#1 4  touch
#1 4  touch
#1 4  touch

Weaving a USB cable over the trackpad

2  hover
2  hover
2  hover
...
2  hover
2  hover
7 +  off
0    off

The current logic treats, for example, 4 (Touch) and 5 (ToLeave) the same, but 4 (Touch) should actually be treated the same as 3 (ToTouch). This causes a 1-frame lag when transitioning between states.

Also, states 2 (Hover) and 6 (Leave) should be differentiated. During normal use, if a finger accidentally lifts slightly off the trackpad while moving, the state sequence becomes ...44566...663444..., while a cable hovering over the trackpad only produces state 2. Thus it may be a good idea to treat 6 (Leave) as touching, but 2 (Hover) as not touching.

I can create a patch for the driver and control panel, if we agree to update the current implementation.

BTW, thanks for the signed drivers!

[vitoplantamura]

hi @AlexGuo1998, It seems the Linux kernel only does the equivalent of "f->State == 0x4". Here: https://github.com/torvalds/linux/blob/254f49634ee16a731174d2ae34bc50bd5f45e731/drivers/hid/hid-magicmouse.c#L244 NOTE: It seems Linux completely ignores the equivalent of our "Finger" field. I'm tagging @averycblack in case he wants to give his opinion. PS: I wonder if we still need the "IgnoreNearFingers" flag in the Control Panel. The intent of this flag was to differentiate the behavior of the original driver (imbushuo) from ours. This is the relevant code in imbushuo: https://github.com/imbushuo/mac-precision-touchpad/blob/29d277880f167a3cead6c1fe8b4eba58bc168a05/src/AmtPtpDeviceUsbUm/InputInterrupt.c#L518 If you'd like to create a PR, I'll test it in Windows Test Mode for a couple of days on my work PC before merging, signing, and releasing it. Thank you, Vito

[averycblack]

The flag details I gave were bit flags. 0x1 | 0x4 = 5, transition from valid contact to another state (probably hovering since 0x2 isn't set). Makes a bit more sense when you look at it that way. I never treated anything with bit 2 (0x4)/valid contact unasserted as valid.

I believe my MBP9,2 behaved somewhat differently too and never deasserted bit 2 (contact) when the finger went from contact to a very close hover. That's why I check both bits 2 and 1. I would have issues scrolling, doing gestures, etc without those.

[vitoplantamura]

Thanks @averycblack. Since Linux does "f->State == 0x4" instead of "(f->State & 0x4) && !(f->State & 0x2)", I think a reasonable way to proceed is to build a version of the driver with the changes proposed by @AlexGuo1998 and see how it behaves (and feels) before signing and releasing it to users. Thanks, Vito

[AlexGuo1998]

The flag details I gave were bit flags.

I'm not sure if treating this as a bit flag is correct. The transition bit, when set, is indicating transition to the next state (N+1), not transition from the previous state (N-1), since press-hover-press gets 4-5-6-3-4 instead of 4-5-6-7-4.

With this interpretation, as long as we hit a odd state (or, a state with the transition bit set) e.g. 3, we are sure it will becomes 4 in the next frame. And the hardware also must know it will become 4, in order to report this 3 state. Thus it's better to treat it as "transition to state 4" and the same as state 4, instead of the same as state 2.

[AlexGuo1998]

Possibly the most flexible solution is to store a bitflag of treated-as-touched states in the registry, so anyone can set their preferred value. For example:

  76543210
0b11111110 = effectively the old implementation by imbushuo
0b11110000 = current implementation
0b00110000 = current implementation with IgnoreNearFingers set
0b01111000 = proposed by me
0b00011000 = if touch->hover is treated as not touching

[vitoplantamura]

I think giving the user the ability to customize this behavior in the registry is overkill... and not useful IMO. My goal is to have behaviour equivalent to Linux, which is a reasonable reference implementation :-) Since Linux does "state = tdata[3] & 0xC0; down = state == 0x80;" which is equivalent to our "(f->State & 0x4) && !(f->State & 0x2)" (please correct me if I'm wrong), I think the current code is fine :-) Thank you, Vito **EDIT**: In my previous messages, I incorrectly wrote that Linux does "f->State == 0x4": this created a lot of confusion in the discussion. **The correct code is in this message.** This is a reminder to myself not to look at complex source code on a smartphone 😅