linux-surface / iptsd

Userspace daemon for Intel Precise Touch & Stylus
GNU General Public License v2.0
86 stars 39 forks source link

MPP v2+, Slim Pen 2, Surface Pro 9 improvements #156

Closed iwanders closed 4 months ago

iwanders commented 4 months ago

Hi, this PR should address the long standing Slim Pen 2 button glitching from #102, #99 and https://github.com/quo/iptsd/issues/5.

Unfortunately I spent a lot of time at first trying to fix this by just using the current data and improving the detection / threshold. It all became easier when I took a step back and took a fresh look at all the data we get from the hardware with the MPP v2+ pens, without assuming DftButton actually holds button data. My analysis can be found at iwanders/analysis-ithc-iptsd, that page should probably be linked in one of the reverse engineering issues, but I don't know which one would be preferred. Fyi @quo, @qzed, @StollD, huge shoutout to the work you have been doing thus far.

In this PR:

Known limitations / deficiencies:

Filing this as a draft PR for discussion, current window naming is obviously a stop gap solution, hopefully this PR also gives others a chance to help test, I'll have limited time to test, but will (obviously) make time to incorporate feedback and answer questions.

StollD commented 4 months ago

First of all, thank you! This seems like it was a lot of work.

I added some comments and suggestions (and nitpicks). Unfortunately I don't have a device with a DFT based stylus, so I can't test it.

I don't really like the idea of having to make the model / type of the pen a config option, but I don't know enough about the hardware to suggest any alternative.

  • I tried to make the commits in the appropriate style, but it may be easier to just squash them, I don't think the individual commits pass the linter.

As long as the final commit passes, I don't really care. The linting is more me being obsessive anyways.

The button detection a approach means that I lose the eraser functionality for the Pen from AliExpress (eraser is done with a button, which also triggers the 0x0a row 4 & 5 switch), but I don't know how to fix that without using multiple frames instead of a single frame & window, we'd have to detect the alternating pattern.

If adding a temporal component to the DFT parser improves the situation, I'd be all for it.

iwanders commented 4 months ago

but I don't know how to fix that without using multiple frames instead of a single frame & window, we'd have to detect the alternating pattern.

If adding a temporal component to the DFT parser improves the situation, I'd be all for it.

Problem is that in the CN pen, the 4 to 5 switch still happens when the eraser button is pressed;

Image for depressing eraser, then make screen contact. :

2024_02_25_linux_cn_hover_erase_touch

For hover, press barrel, touch screen, release barrel, move, raise from contact:

2024_02_25_linux_cn_hover_button_click_touch_release_wait_hover

The only real distinction is the alternating signal, which is on like 0x0a 2&3 e&f (both first and second window). We could detect that for this pen and 'solve it'.

The problem is that the SP pen on linux creates a binary code (probably a pen identifier, just like the M2 on Windows, see this section and we don't want that binary code to trigger the eraser detection. We could do something ugly where we check if 6/6 frames were perfectly alternating or something, but I'm really worried that this binary code is like the 'marker color', a unique id for each pen to allow the screen to distinguish two of them and some pens may just have an id that creates a binary sequence that perfectly alternates. In short; I don't yet know how to solve this, only thing I can think of is making the eraser signal trump the button action... We could explore that, I don't think I've ever had an accidental eraser action, but the whole intent of this PR is to use the strong binary indicators available in v2+.

I had hoped the whole design in the CN pen with the whole eraser & button having almost the same signal because the emitter end is actually facing the screen was just a cost cutting measure, but according to this document the eraser in a side button is an approved design.

Funny thing is that this section then describes that the protocol can't actually handle that well:

Unlike tail-end eraser implementations, button-based implementations can physically allow the user to activate/deactivate the erase affordance without the pen transitioning through the "out of range" state. However, this is not supported by the underlying protocol.

iwanders commented 4 months ago

A few quick notes before work from my testing yesterday evening in comments.

Another limitation that currently affects mpp v2 pens is that the tilt is currently taken from Position, row 1, but that signal is only present when the stylus is touching the screen. I wonder if we should process Position2 if present instead. That has a solid signal on the tilt. Or we can - when we process Position2 for the contact detection store the dft values of the tilt row and consume that in the normal Position processing.

StollD commented 4 months ago

Another limitation that currently affects mpp v2 pens is that the tilt is currently taken from Position, row 1, but that signal is only present when the stylus is touching the screen.

Do you know how this is handled on Windows? On the "old" devices without a DFT stylus, the tilt is 0 when the pen tip is not touching the display.

iwanders commented 4 months ago

Another limitation that currently affects mpp v2 pens is that the tilt is currently taken from Position, row 1, but that signal is only present when the stylus is touching the screen.

Do you know how this is handled on Windows? On the "old" devices without a DFT stylus, the tilt is 0 when the pen tip is not touching the display.

I checked, I'm really surprised, but it does indeed become zero if the stylus isn't touching the screen. Even Microsoft's DigiInfo reports zero tiltx and tilty if the stylus isn't in contact. I installed Krita on windows, with the input API Windows 8+ Pointer Input (Windows Ink) this is also the behaviour. Same with the pointer events webapp.

There's no technical reason though... Like we do have the data in Position2 row 1 when the pen is raised, did a quick hack with just doing:

        case ipts::protocol::dft::Type::PositionMPP_2:
            this->handle_position_mpp_2(dft);
            this->handle_position(dft);
            break;

And in that case I do end up with a tilt that updates while the pen is raised, both in Krita as in the pointer events webapp. I'm astonished Windows doesn't do this.

Also, speaking about dft updates... we currently call on_stylus every dft window: https://github.com/linux-surface/iptsd/blob/f59b72646afd931e5a1013e6c76548d750979229/src/core/generic/application.hpp#L232

Should we think about a way where we can ensure that we only send that once for each group? Such that we don't have to worry about keeping the stylus data in a consistent state as we process the various dft windows in one group? Like, currently we consume the previous' cycle 0x0a DFT window on the next group's Button window. It's not a huge concern imo, but something to think about.

StollD commented 4 months ago

Tilt is calculated from two position signals, right? How does this work when one of the transmitters is out of range? Maybe thats why they only use the data when it is touching the display: To make sure both transmitters are in proximity.

Or this is just some weirdness that they inherited from N-Trig and never bothered to change.

I think by default iptsd should follow the Windows behaviour, but this sounds like a useful thing to put behind a config option. Although maybe that should be a seperate PR?

Should we think about a way where we can ensure that we only send that once for each group? Such that we don't have to worry about keeping the stylus data in a consistent state as we process the various dft windows in one group? Like, currently we consume the previous' cycle 0x0a DFT window on the next group's Button window. It's not a huge concern imo, but something to think about.

Another thing I noticed on my SB2 is that it only updates the tilt if I move the stylus. It sounds stupid with how precise these coordinates are supposed to be (1/9600 of the screen), but I can carefully tilt the stylus without the value changing.

Maybe that is something that the DFT parser could emulate.

iwanders commented 4 months ago

Tilt is calculated from two position signals, right? How does this work when one of the transmitters is out of range? Maybe thats why they only use the data when it is touching the display: To make sure both transmitters are in proximity.

If either signal strength goes below a threshold we just lose the pointer position and/or tilt signal. Thing is, in Krita you can see the outline of the cursor update to show the tilt and size of the brush before you start drawing, so there's a lot of value in updating the tilt before you touch the screen imo.

I think by default iptsd should follow the Windows behaviour, but this sounds like a useful thing to put behind a config option. Although maybe that should be a seperate PR?

Yeah, we can do that, only real outstanding items with this piece of work:

It sounds stupid with how precise these coordinates are supposed to be (1/9600 of the screen)

I expect that's part of the filtering done by the driver / hardware... I wish the hardware just did the whole position & tilt estimation, it would make things much easier. On my SP9 the current position estimate algorithm is not ideal, at times it seems to jump towards the edge of an antenna, particularly at some specific tilt angles. Seems less of a problem with other pens though, the Slim Pen 2 seems particularly susceptible to this problem.

StollD commented 4 months ago
  • The lack of eraser functionality with my Chinese pen, but the only solution I see there is being able to let the eraser disable the button state. Thoughts on this?

This is more like a hot thought and not really thought through, but can you let one method guide the other one? For example, if the new method detects a button press, you ignore the button magnitude setting but do the same phase calculations to determine button / rubber?

I don't know how reliable this would be though (because I don't really know the issue with the original method in the first place).

EDIT: Wait, or do you mean that you can detect the eraser already, but you want to press eraser and button at the same time? I am not actually sure what my SB2 would do in such case. But if necessary I would be fine with the eraser disabling the button.

StollD commented 4 months ago

From the SB2:

iwanders commented 4 months ago

I don't know how reliable this would be though (because I don't really know the issue with the original method in the first place).

Oh, mea culpa! I should've totally explained that in the description. So basically, the signal to noise ratio in the DftButton row 0 is kinda poor, for example in this test, the button isn't actually pressed, but the 0th row of the DftButton window clearly lights up. This is what causes the ghost button presses that slim pen 2 users have been reporting, I tried cranking the threshold up, as was suggested in one of the threads, while that resolved the ghost button presses it made the button detection only work when the tip was practically touching the screen for me. The 0th row is even more noisy at roughly 20 degrees towards the screen, and it is worse when the barrel button side is pointing towards the screen. I think it's still strong enough for the phase detection, I would've preferred a nice 'binary' signal, but maybe this is the best we have for now.

This is more like a hot thought and not really thought through, but can you let one method guide the other one? For example, if the new method detects a button press, you ignore the button magnitude setting but do the same phase calculations to determine button / rubber?

Yep! I truly appreciate your suggestion, I think this works really well. I just pushed this in 2d49ff7, I changed m_mppv2_button into m_mppv2_button_or_eraser, then we can just use the phase check to discern between barrel button and eraser button. I got too stuck on the 'button detection' and didn't think of that we can also just use it as a 'button or eraser' detection.

This seems to work for all 4 pens I have here, all of them allow eraser detection as well as button detection without any changes to settings.

but you want to press eraser and button at the same time?

Nope, you understood it right. I don't see much use in supporting both the button and the eraser.

Current solution is nice; the new changes are all additive. If they cause problems they can also all be disabled by increasing the new magnitudes in the configuration. I don't expect problems of course, but it's nice to know that we have the knobs to tune if it does. I made one more comment on a variable name, I think that should be prefixed. I'll try to do some more testing this evening and over the next few days. Aiming to move this out of draft at the beginning of the weekend if I don't discover any issues during my testing.

We should also really consider making an application that just prints the relevant dft magnitudes, doesn't need to be pretty, but currently it is impossible to say - print the button magnitudes - without doing any code changes. Happy to file a PR for this if you're ok with something simple in this direction.

StollD commented 4 months ago

I tried cranking the threshold up, as was suggested in one of the threads, while that resolved the ghost button presses it made the button detection only work when the tip was practically touching the screen for me.

Thanks for the explanation! I figured that it must have had something to do with signal strength, but I didn't consider the distance from the screen.

This seems to work for all 4 pens I have here, all of them allow eraser detection as well as button detection without any changes to settings.

Nice!

We should also really consider making an application that just prints the relevant dft magnitudes, doesn't need to be pretty, but currently it is impossible to say - print the button magnitudes - without doing any code changes. Happy to file a PR for this if you're ok with something simple in this direction.

I have been thinking about how I would do a DFT calibration tool for a while, but I never got around to working on it because of other stuff. A simple debugging app that just prints the magnitudes would be a great start in that regard.

iwanders commented 4 months ago

Marked this as ready for review, it's good to go in imo: Did some more testing yesterday evening and Wednesday evening. The code we arrived at seems to work well for all 4 pens.

quo commented 4 months ago

Amazing work! I never even considered that the type 0xa windows would contain a button signal, I assumed it was a pen ID or something, since it just seemed to cycle through the same three six values (and I don't have a MPP2 pen myself).

Code looks good to me. My only concern is that if the first type 0xa DFT window is missing for some reason, you will incorrectly read the button signal from the second window. So it would be better to just store the first 0xa DFT window, and delay the processing until you see the second one, so you know you're using the right window.

BTW, any chance you could share your raw data captures? When I have some more time I may try to figure out what the other 0xa bits mean. (Ideally separate captures of hovering, contact, eraser hover, eraser contact, and button press for each pen.)

iwanders commented 3 months ago

BTW, any chance you could share your raw data captures? When I have some more time I may try to figure out what the other 0xa bits mean.

Certainly @quo , I've added a lot of it just now in a release associated to my analysis repository: https://github.com/iwanders/analysis-ithc-iptsd/releases/tag/v0.0.1

I hope you'll find that sufficient, it's the raw IRPMon recordings from Windows, my repo contains scripts to convert those to the iptsd binary files.

I think the 0xa bits identify the pens with some unique id. I'm not sure how relevant that is. What currently still feels a bit wonky for the SP2 is the actual position estimates, in particular with the pen at an angle.

Another (probably more fun) thing to look at would be the way the pressure is encoded digitally, I looked at it briefly here, it does seem to contain a delta in each frame. I didn't figure it out though.

Lots of interesting things in this data, but ultimately I had to prioritize fixing the button glitching over exploring all the other information as time as limited. I'll have to step away from the Surface stuff to do some other things in my spare time, but if you do make some discoveries do ping me!