Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: workaround for windows ink #1113

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Doublonmousse
Copy link
Collaborator

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).

- 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
@Doublonmousse Doublonmousse changed the title workaround for windows ink fix: workaround for windows ink Jun 7, 2024
@flxzt
Copy link
Owner

flxzt commented Jun 9, 2024

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.

@Doublonmousse
Copy link
Collaborator Author

Doublonmousse commented Jun 9, 2024

So, the original reason for the bug is this
xournalpp/xournalpp#2316 (comment)

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 (#243 (comment) 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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants