Bug 409613

Summary: "Reset Canvas Rotation" set to "Space + R" breaks color picking after pan action
Product: [Applications] krita Reporter: Hector <misha.bossmass>
Component: Color SelectorsAssignee: Dmitry Kazakov <dimula73>
Status: RESOLVED FIXED    
Severity: normal CC: dimula73, halla, sti-naus
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: Microsoft Windows   
OS: Microsoft Windows   
Latest Commit: Version Fixed In:

Description Hector 2019-07-08 09:25:48 UTC
SUMMARY
With brush. after moving the canvas using the spacebar, the CTRL shortcut stops working. So i cant pick a color. This works again if you click on "b" to activate the brush again or undo something. Works after moving with middle button. 

STEPS TO REPRODUCE
1. With brush enabled move the canvas with Spacebar
2. Try to pick a color with Ctrl
Comment 1 Halla Rempt 2019-07-08 09:42:21 UTC
Hm, this works for me. Which version of Krita are you using?
Comment 2 Hector 2019-07-08 11:02:42 UTC
(In reply to Boudewijn Rempt from comment #1)
> Hm, this works for me. Which version of Krita are you using?

Im using 4.2.2. Observed since 4.2+ maybe. Or 4.1.7... I just tried it with a different keyboard layout. Sometimes it works, but sometimes the spacebar pan also stops working. Tried in virtual mint, everything works fine there. Maybe its a specific problem with my pc. About a week later I will have to install the system on a new drive. Maybe the problem will disappear.
Comment 3 Halla Rempt 2019-07-08 11:11:14 UTC
Okay, please keep us posted :-)
Comment 4 Hector 2019-07-14 12:00:07 UTC
(In reply to Boudewijn Rempt from comment #3)
> Okay, please keep us posted :-)

In the end, I found the reason. Shortcuts can be assigned to the spacebar + key. This breaks down the possibilities with ctrl and others after moving the canvas using the spacebar. So the bug is confirmed and resolved. I propose for now to disable the appointment of shortcuts with a space.
Comment 5 Stig (Rakurri) 2019-11-29 20:12:41 UTC
I also had the same problem. On all of my 3 Windows 10 computers the "Zoom Canvas" and "Alternate Invocation" did not work when I expected them to, and often were unresponsive for many seconds. It seemed to stop working at seemingly random times. Which was extremely frustrating. But now I know thanks to Hector, that this was caused by my keyboard shortcut for "Reset Canvas Rotation" being set to "Space + R" in my shortcut scheme which I used on all of my computers.
I also propose that Krita should disable the appointment of shortcuts with a space.
Comment 6 Dmitry Kazakov 2020-06-29 09:31:01 UTC
The bug has been fixed in this commit:

commit d454a04e713c0ffc4167021680f69be3698871ec
Author: Dmitry Kazakov <dimula73@gmail.com>, Wed Apr 15 23:46:55 2020 +0300 (3 months ago)

Fix pan/zoom shortcuts not working after accessing any docker

The reason for the bug was that KisExtendedModifiersMapper::
queryExtendedModifiers() has never been implemented for Windows.
Therefore all the shortcuts including Space could potentially be
lost when the user pressed it without focusing on the canvas.

We also need an implementation for OSX.

BUG:372646,373299,391088
Comment 7 Dmitry Kazakov 2020-06-29 09:36:00 UTC
No, I think I can reproduce it still
Comment 8 Dmitry Kazakov 2020-06-29 16:41:53 UTC
This bug is actually in Qt. When Space is set as a shortcut modifier, it doesn't handle KeyRelease for some reason, therefore internal state machine gets into a invalid state and eats some events.
Comment 9 Dmitry Kazakov 2020-06-29 18:52:13 UTC
Okay, the problem is the following:

Qt supports sequential shortcuts. That is, you can press "Space", release it, and then press "R" key and the shortcut will be triggered. It means that when you press Space the first time, Qt starts eating all the following ShortcutOverride events and they never get delivered to Krita until Qt matches the a complete shortcut or resets its state.

I need to think to understand if it is actually fixable or not.
Comment 10 Bug Janitor Service 2020-06-29 21:32:23 UTC
A possibly relevant merge request was started @ https://invent.kde.org/graphics/krita/-/merge_requests/403
Comment 11 Dmitry Kazakov 2020-06-29 21:45:27 UTC
Hi, Hector!

I've added a merge request to Krita with the fix to the bug. Could you please test it and check if the bug is fixed?

Merge request: https://invent.kde.org/graphics/krita/-/merge_requests/403
Testing package for Windows: https://yadi.sk/d/3PkOJpffkExBcQ
Comment 12 Hector 2020-06-30 07:41:03 UTC
(In reply to Dmitry Kazakov from comment #11)

> I've added a merge request to Krita with the fix to the bug. Could you
> please test it and check if the bug is fixed?

The bug that I had no longer appears in this package. It seems to have checked all the options that I had then.
Comment 13 Stig (Rakurri) 2020-06-30 13:38:39 UTC
I didn't experience the bug while testing in the new package, seems to work for me too! :)
Thank you! Dmitry
Comment 14 Dmitry Kazakov 2020-07-13 12:56:30 UTC
Git commit 80bdcbdf22a009c06523d535de9e2dd682ffdb34 by Dmitry Kazakov.
Committed on 13/07/2020 at 09:17.
Pushed by dkazakov into branch 'krita/4.3'.

Fix broken shortcuts when user uses Space key as a modifier

The Problem
-----------

Qt cannot use Space (or any non-modifier) key as a modifier. When
we assign "Space+R" shortcut to "Reset rotation" in Krita settings,
Qt actually assigns it not to "Space+R", but to "Space, R". That is,
the user is expected to press the two keys sequentially, not
simultaneously (though the latter option also works).

It means that after the user presses "Space" key, Qt transits into a
special state of an incomplete match and starts waiting for the following
key. While in this state, Qt (without this patch) eats all the shortcut
events.

Solution
--------

There is no proper solution for the problem, because Qt will still wait
for the second key to be pressed. That is, there is no way to instruct
Qt that Space and R keys should be pressed simultaneously (it is not
possible by design of QKeySequence.

This patch adds a workaround for the problem. It makes sure that all the
shortcut events are delivered to Krita, whatever the state Qt has
internally.

Test Plan
---------

Configuration:

1) Assign W as a primary shortcut for Zoom-In in **Canvas Input Settings**
2) Assign Space,R as a primary shortcut for Zoom-Out in **Keyboard Shortcuts**
3) Assign Space,W as a primary shortcut Rotate Canvas Left in **Keyboard Shortcuts**

Testcase 1:

1) Press Space, Release Space
2) Press W, Release W

Expected: canvas is zoomed in and not rotated, space+drag gesture still works fine

Testcase 2:

1) Press Space, Release Space
2) Press W, Release W
2) Press R, Release R

Expected: no zoom-out action happens (because Space modifier has already been consumed by the former action)

Testcase 3:

1) Press Space, Release Space
2) Press R, Release R

Expected: zoom-out action happens, space+drag gesture still works fine


Testcase 4:

Press Space, W and R randomly, it should still behave "in a sane way" :)

A  +68   -0    3rdparty/ext_qt/0101-Don-t-eat-ShortcutOverride-events-when-there-is-a-pa.patch
M  +4    -0    3rdparty/ext_qt/CMakeLists.txt
M  +6    -8    libs/ui/input/kis_input_manager.cpp

https://invent.kde.org/graphics/krita/commit/80bdcbdf22a009c06523d535de9e2dd682ffdb34
Comment 15 Dmitry Kazakov 2020-07-13 12:57:34 UTC
Git commit adc8c597dc2b10b02ec377e258e677361a0ebec3 by Dmitry Kazakov.
Committed on 13/07/2020 at 12:57.
Pushed by dkazakov into branch 'master'.

Fix broken shortcuts when user uses Space key as a modifier

The Problem
-----------

Qt cannot use Space (or any non-modifier) key as a modifier. When
we assign "Space+R" shortcut to "Reset rotation" in Krita settings,
Qt actually assigns it not to "Space+R", but to "Space, R". That is,
the user is expected to press the two keys sequentially, not
simultaneously (though the latter option also works).

It means that after the user presses "Space" key, Qt transits into a
special state of an incomplete match and starts waiting for the following
key. While in this state, Qt (without this patch) eats all the shortcut
events.

Solution
--------

There is no proper solution for the problem, because Qt will still wait
for the second key to be pressed. That is, there is no way to instruct
Qt that Space and R keys should be pressed simultaneously (it is not
possible by design of QKeySequence.

This patch adds a workaround for the problem. It makes sure that all the
shortcut events are delivered to Krita, whatever the state Qt has
internally.

Test Plan
---------

Configuration:

1) Assign W as a primary shortcut for Zoom-In in **Canvas Input Settings**
2) Assign Space,R as a primary shortcut for Zoom-Out in **Keyboard Shortcuts**
3) Assign Space,W as a primary shortcut Rotate Canvas Left in **Keyboard Shortcuts**

Testcase 1:

1) Press Space, Release Space
2) Press W, Release W

Expected: canvas is zoomed in and not rotated, space+drag gesture still works fine

Testcase 2:

1) Press Space, Release Space
2) Press W, Release W
2) Press R, Release R

Expected: no zoom-out action happens (because Space modifier has already been consumed by the former action)

Testcase 3:

1) Press Space, Release Space
2) Press R, Release R

Expected: zoom-out action happens, space+drag gesture still works fine


Testcase 4:

Press Space, W and R randomly, it should still behave "in a sane way" :)

A  +68   -0    3rdparty/ext_qt/0101-Don-t-eat-ShortcutOverride-events-when-there-is-a-pa.patch
M  +4    -0    3rdparty/ext_qt/CMakeLists.txt
M  +6    -8    libs/ui/input/kis_input_manager.cpp

https://invent.kde.org/graphics/krita/commit/adc8c597dc2b10b02ec377e258e677361a0ebec3