rnote icon indicating copy to clipboard operation
rnote copied to clipboard

fix: workaround for windows ink

Open Doublonmousse opened this issue 1 year ago • 2 comments

This is a workaround for #785

  • force down state when the element is a stylus with a pressure value that is not zero. This way when a button is pressed before the pen is on the screen, this triggers proximity mode as rnote expects a left click when the pen touches the screen

  • forces a pen state to up upon a button release as once again we have our button as a buttonrelease instead of the left click or BUTTON_PRIMARY release

Tested and working on my surface pro. For now this is implemented using cfg flags only for windows builds.

From testing directly the pen input seen when using the Win32 API (what gtk is seeing) and WinRT, the original issue stems from limitations of the Win32 API. I'm reasonably confident then that the issue is a general issue on Windows, hence an os-specific workaround, so that applying it for all windows build shouldn't create issues.

Not sure this would make sense to apply elsewhere (or that this workaround may be needed outside windows).

I can add a toggle somewhere in the settings toolbar (hopefully in a way that's only shown on windows) if you want. I think it makes sense though to have the workaround enabled by default (xournal++ has a similar toggle that's disabled by default and isn't easy to find or understand that it fixes the issue when encountered).

Doublonmousse avatar Jun 07 '24 19:06 Doublonmousse

I haven't had issues on windows so I can't test this, but it looks like a reasonable fix

I can add a toggle somewhere in the settings toolbar (hopefully in a way that's only shown on windows) if you want.

Oh no please not. For fixes we should ensure it actually works and do a lot of testing instead of burdening the user with this.

flxzt avatar Jun 09 '24 05:06 flxzt

So, the original reason for the bug is this https://github.com/xournalpp/xournalpp/issues/2316#issuecomment-860083264

Now I have verified that this is what's happening on my end, and that it also would be possible to make it work correctly with some WinRT APIs instead of Win32 ones for the pens (I've tested this with the windows and windows-sys crate though doing this on gtk's code may be harder).

The thing I don't know is the following

  • does it affect every windows PC ? Did you not encounter any similar issue on Windows yourself ? I'd be interested to know on what hardware.

Seeing that there are technically different protocols (MPP, AES, USI with MPP being the most widely used) maybe there are slight differences.

I don't see a lot of windows-related input issues on this repo (https://github.com/flxzt/rnote/issues/243#issuecomment-1732542608 is on windows but buttons seems to register, the pen uses AES by default + extra drivers)

From my experience (me, some people that tried rnote, or some other reports on reddit) this issue affects at least all surface products.

I think the fix doesn't affect too much the UX even if all events are fired though, because it will still register things as expected for button presses, with a slight difference for button releases. Maybe I should test it on linux with the fix to see if and how it affects the working case (all events are there)

Doublonmousse avatar Jun 09 '24 06:06 Doublonmousse

let's try it, I think this is ready and can be merged. The proper fix to avoid this workaround would be in Gtk to provide a consistent event stream between all platforms and tablet technologies

flxzt avatar Jul 13 '24 10:07 flxzt

Yes, it would have to be done in gtk https://gitlab.gnome.org/GNOME/gtk/-/issues/6723 probably with a similar approach to this https://github.com/Doublonmousse/winit/tree/winit-pen though I'm not sure I'm ready to deal with potential issues porting this to gtk (according to this https://github.com/microsoft/WindowsAppSDK/issues/233).

Maybe I'm worrying for nothing and things have changed between 2020 and now and it would be possible to do this without too much issues (not sure gtk uses any WinRT API but if they do or start to do, it'll be possible to fix relatively easily).

Doublonmousse avatar Jul 13 '24 10:07 Doublonmousse

Alright, good to have this documented for future reference.

The workaround itself lgtm.

flxzt avatar Jul 22 '24 20:07 flxzt