Skip to content

fix canvas inputs breaking due to inconsistent key states

Simon Ra requested to merge simonra/krita:broken_cavas_input into master

https://bugs.kde.org/show_bug.cgi?id=451424

There are some remain issues in this bug report: On Windows, shortcuts like [Windows Key]+D and [Windows Key]+[0~9] still break canvas input due to keys that register as pressed but never released.

The root cause is that krita uses both QT key events, and native Windows/X11 API "polling" calls. This creates situations where keys are polled and saved to KisShortcutMatcher's state, but no matching keyReleased events are generated. Blame may or may not be shared beween the parties, Windows, QT and krita.

The solution proved tricky (to me), because I wanted to preserve the behaviour of krita wherein it still matches shortcuts while the application has no focus. Making changes to the affected code is very prone to breaking things in subtle and horrible ways, and I have not made it prettier. Perhaps there are better solutions. I will keep thinking on it.

In essence, I tracked keys that were thus polled from native API calls, and re-querry these keys until they are released. Performance impact on systems that run Krita fluidly should be minimal, but it is an additional API call that occurs on every input, until all such keys have been cleared.

I have tested this a fair bit on windows and linux, though additional eyes would be appreciated. I cannot test OSX, though I suspect that some of the functionality described here never worked on OSX to begin with.

Its possible this obsoletes some other special cases, but the ones I looked at were still required as best as I could tell.

A case might be made to limit these changes to a platform (Windows), but I felt that since we do the same thing on linux, it just doesnt happen to break, I would not go that route. I think of it as future proofing.

I will try to be available on irc for dialogue (simonra) if desired!

Test Plan

  1. In the canvas input settings, bind canvas zoom to both: -shift + middle mouse button -0 (zero) + left mouse button
  2. Focus and unfocus the application with clicking outside of it and back inside of it. Confirm both input methods still work.
  3. Focus and unfocus the application with 2x alt+tab (or equivalent window manager hotkey). Confirm both input methods still work.
  4. mix 2) and 3)
  5. Focus and unfocus the application with 2x win+d (or equivalent window manager hotkey to minimize/maximize all applications). Confirm both input methods still work.
  6. Unfocus the application, hold down shift, then mouse over the application and begin dragging the mouse. Confirm zoom begins immediately.
  7. Unfocus the application, hold down 0 (Zero), then mouse over the application and begin dragging the mouse. Confirm zoom begins immediately.
  8. (window manager specific) Focus Krita, then hit [Meta]+D twice. Confirm both input methods still work.
  9. (windows only, may require specific settings) on the task bar, arrange things so that krita is the first application in the list, and another running application is second. Focus krita. Press [Windows Key]+2 followed by [Windows Key]+1. Krita should have focus again. Confirm both input methods still work.
  10. Vary the timings and order of key releases in 9). Confirm both input methods still work.
  11. Unfocus the application. Hold down the Meta/Windows key, then click on the application to focus it. Release the Meta key. Confirm both input methods still work.

Formalities Checklist

  • I confirmed this builds.
  • I confirmed Krita ran and the relevant functions work.
  • I tested the relevant unit tests and can confirm they are not broken. (looks like just sporadic failures to me?)
  • I made sure my commits build individually and have good descriptions as per KDE guidelines.
  • I made sure my code conforms to the standards set in the HACKING file.
  • I can confirm the code is licensed and attributed appropriately, and that unattributed code is mine, as per KDE Licensing Policy.

Reminder: the reviewer is responsible for merging the patch, this is to ensure at the least two people can build the patch. In case a patch breaks the build, both the author and the reviewer should be contacted to fix the build. If this is not possible, the commits shall be reverted, and a notification with the reasoning and any relevant logs shall be sent to the mailing list, kimageshop@kde.org.

Edited by Simon Ra

Merge request reports