Bug 313218 - krita reset custom keyboard shortcut
Summary: krita reset custom keyboard shortcut
Status: RESOLVED FIXED
Alias: None
Product: krita
Classification: Applications
Component: Usability (show other bugs)
Version: git master (please specify the git hash!)
Platform: Compiled Sources Linux
: HI major
Target Milestone: ---
Assignee: Halla Rempt
URL:
Keywords: release_blocker
: 329148 342013 (view as bug list)
Depends on:
Blocks:
 
Reported: 2013-01-14 08:20 UTC by David REVOY
Modified: 2015-01-03 13:57 UTC (History)
7 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
Patch to fix shortcuts resets in a single Krita instance (987 bytes, patch)
2014-02-03 12:51 UTC, Dmitry Kazakov
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David REVOY 2013-01-14 08:20:56 UTC
When I attribute custom keyboard shortcut :

- Ctrl+U for Filter > Adjust > HSV adjustement ( shortcut attributed normally to text ; 'Underline' ) 
- Ctrl+L for Filter > Adjust > Level ( shortcut attributed normally to text ; 'Align Left' )

Krita reset them to the default after a bunch of restart or playing with workspace ( I still can't figure what is the exact action for reseting them ) .  A wish to make those keyboard shortcut by defaut is here bug #313217
Comment 1 David REVOY 2013-01-19 17:40:46 UTC
New informations about it  ; I opened Krita via konsole , till I got this message : 

krita(7420)/kdeui (kdelibs): Shortcut for KAction  "krita_filter_hsvadjustment" "&HSV Adjustment..." set with QShortcut::setShortcut()! See KAction documentation. 
krita(7420)/kdeui (kdelibs): Shortcut for KAction  "KritaTransform/KisToolMove" "Move a layer" set with QShortcut::setShortcut()! See KAction documentation.

I thought it maybe related.
Comment 2 David REVOY 2013-02-03 13:22:18 UTC
Marked as Resolved -> Worksforme

The keyboard short-cut proposed are now default ( thx Dmitry in bug #313217 ), so I can't test if Krita still reset the targeted shortcut on filters because it's now 'works for me' .  :)
Comment 3 Philippe Nicloux 2013-03-18 13:30:01 UTC
It seems that this bug is coming back : shortcuts on the 4,5,6 ( and only those keys ) on the numpad are reseted to default shortcuts at each launch. I work with the 2.7 git sources
Comment 4 David REVOY 2013-03-18 22:57:59 UTC
Ok, I reswitch it to unconfirmed. 
I note on my to-do list asap to try if I can reproduce it on numpad key.
Comment 5 Halla Rempt 2013-03-31 09:56:07 UTC
Ack. I've seen it happen again. We're looking into taking this issue into our own hands instead of relying on KDE.
Comment 6 Odysseas 2013-07-01 08:02:06 UTC
I have seen that the reset problem appears when modifying the default Shortcuts.
After creating a new scheme from scratch, the problem was solved for me.
Comment 7 Dmitry Kazakov 2013-08-09 18:20:51 UTC
Two people from the Russian community has reported this bug recently. So I'll raise the priority of the bug.
Comment 8 Dmitry Kazakov 2013-12-24 11:28:03 UTC
*** Bug 329148 has been marked as a duplicate of this bug. ***
Comment 9 Halla Rempt 2014-01-29 11:07:30 UTC
There is also a conflict between the canvas interaction shortcuts and the 'normal' shortcuts. If you redefine a normal shortcut to use a key used by the canvas interaction shortcuts, that won't work.
Comment 10 Dmitry Kazakov 2014-02-03 11:13:32 UTC
Steps to reproduce:

1) Open document in Krita [instance 1]
2) Change 1x1 shortcut to 'U'
3) Open document in Krita [instance 2] (just new view, not a separate instance)
4) Change 1x1 shortcut to 'Y'
5) Close Krita 1
6) Close Krita 2

<now 'U' will become Default shortcut, 'Y' will become current custom shortcut>

Repeat steps 1-6 and rerun Krita. There will be no shortcuts for 1x1 action.
Comment 11 Dmitry Kazakov 2014-02-03 11:18:32 UTC
Reversion of the order of steps 5-6 may lead to bug not appear.
Comment 12 Dmitry Kazakov 2014-02-03 12:37:04 UTC
The problem is that we load/save action shortcuts twice: firstly using KShortcutsDialog and secondly using our homegrown KActionCollection::allActionCollections(), which is actually not correct, since allActionCollections() contains duplicated shortcuts for all the views of Krita, which, yes, can contain not-coinciding shortcuts.
Comment 13 Dmitry Kazakov 2014-02-03 12:38:17 UTC
This small patch fixes the shortcuts resetting in a single Krita instance:

diff --git a/krita/ui/kis_view2.cpp b/krita/ui/kis_view2.cpp
index a0332ae..5a5b4ae 100644
--- a/krita/ui/kis_view2.cpp
+++ b/krita/ui/kis_view2.cpp
@@ -427,8 +427,8 @@ KisView2::KisView2(KoPart *part, KisDoc2 * doc, QWidget * parent)
 
     KConfigGroup group(KGlobal::config(), "krita/shortcuts");
     foreach(KActionCollection *collection, KActionCollection::allCollections()) {
-        collection->setConfigGroup("krita/shortcuts");
-        collection->readSettings(&group);
+        //collection->setConfigGroup("krita/shortcuts");
+        //collection->readSettings(&group);
     }
 
     KisInputProfileManager::instance()->loadProfiles();
@@ -471,8 +471,8 @@ KisView2::~KisView2()
     {
         KConfigGroup group(KGlobal::config(), "krita/shortcuts");
         foreach(KActionCollection *collection, KActionCollection::allCollections()) {
-            collection->setConfigGroup("krita/shortcuts");
-            collection->writeSettings(&group);
+            //collection->setConfigGroup("krita/shortcuts");
+            //collection->writeSettings(&group);
         }
     }
     delete m_d;
Comment 14 Dmitry Kazakov 2014-02-03 12:51:28 UTC
Created attachment 84968 [details]
Patch to fix shortcuts resets in a single Krita instance

I propose this patch as the initial attempt to fix the problem. It fixes random shortcuts resetting. Anyway this custom saving didn't solve the problem of concurrent shortcut editing in several views properly. After the patch is applied, the configuration will store the settings variant of the "last edited view instance"

Further development: add auto change-tracking of the kxml*rc configuration file and reload the shortcuts into own actions.

NOTE: we shouldn't use KActionCollection::allCollections() because it contains the collections of all the views, which duplicate the actions. Instead, we should use KXMLGUIFactory::clients(), or a predefined KXMLGUIFactory::refreshActionProperties().
Comment 15 Dmitry Kazakov 2014-02-03 13:40:54 UTC
Ok, now I know how to fix the bug. Will do it tomorrow or on Wednesday :)
Comment 16 Dmitry Kazakov 2014-02-05 08:50:01 UTC
Git commit 0eb57538670d615e89147c5346a3c93c5af3f645 by Dmitry Kazakov.
Committed on 05/02/2014 at 08:45.
Pushed by dkazakov into branch 'master'.

Fix interprocess updates of the configuration and a shortcut scheme

This patch does two things:
1) Removes handwriten saving of the shortcuts into the kritarc. Now the
   shortcuts are saved into XML gui tree as it is supposed to.
2) Added synchronization between different instances of Krita using
   QFileSystemNotifier on kritarc file.

M  +9    -0    krita/ui/kis_config.cc
M  +3    -0    krita/ui/kis_config.h
M  +67   -0    krita/ui/kis_config_notifier.cpp
M  +17   -0    krita/ui/kis_config_notifier.h
M  +8    -13   krita/ui/kis_view2.cpp
M  +1    -0    krita/ui/kis_view2.h
M  +2    -0    libs/main/KoMainWindow.cpp
M  +2    -0    libs/main/KoMainWindow.h

http://commits.kde.org/calligra/0eb57538670d615e89147c5346a3c93c5af3f645
Comment 17 Dmitry Kazakov 2014-02-05 09:34:14 UTC
Git commit 381a9d8d9e4a474d01553cac97cde0612b209099 by Dmitry Kazakov.
Committed on 05/02/2014 at 08:45.
Pushed by dkazakov into branch 'calligra/2.8'.

Fix interprocess updates of the configuration and a shortcut scheme

This patch does two things:
1) Removes handwriten saving of the shortcuts into the kritarc. Now the
   shortcuts are saved into XML gui tree as it is supposed to.
2) Added synchronization between different instances of Krita using
   QFileSystemNotifier on kritarc file.

M  +9    -0    krita/ui/kis_config.cc
M  +3    -0    krita/ui/kis_config.h
M  +67   -0    krita/ui/kis_config_notifier.cpp
M  +17   -0    krita/ui/kis_config_notifier.h
M  +8    -13   krita/ui/kis_view2.cpp
M  +1    -0    krita/ui/kis_view2.h
M  +2    -0    libs/main/KoMainWindow.cpp
M  +2    -0    libs/main/KoMainWindow.h

http://commits.kde.org/calligra/381a9d8d9e4a474d01553cac97cde0612b209099
Comment 18 Dmitry Kazakov 2014-02-09 10:45:48 UTC
Git commit bf05e0463853b211bba07cda8a504227d032fe1e by Dmitry Kazakov.
Committed on 09/02/2014 at 10:45.
Pushed by dkazakov into branch 'master'.

Fix the hangup on Linux when the LUT docker is present

This patch does two things:
1) Fixes LUT docker to update configuration only when some LUT-related
   config options really changed.
2) Adds a safe barrier locking in KisCanvas2::setDisplayFilter and
   KisCanvas2::setMonitorProfile. This is done using a new class
   KisDeferredSignal, which is based on boost::function + boost::bind.

TODO:
There is a still a "bug" that on every change of the settings (LUT-related
settings) the docker calls setMonitorProfile() four times! Well, it doesn't
lead to any problems right now, but this is surely no good.

M  +136  -73   krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp
M  +5    -9    krita/plugins/extensions/dockers/lut/lutdocker_dock.h
M  +1    -0    krita/ui/CMakeLists.txt
M  +18   -3    krita/ui/canvas/kis_canvas2.cpp
A  +39   -0    krita/ui/kis_deferred_signal.cpp     [License: GPL (v2+)]
A  +47   -0    krita/ui/kis_deferred_signal.h     [License: GPL (v2+)]

http://commits.kde.org/calligra/bf05e0463853b211bba07cda8a504227d032fe1e
Comment 19 Dmitry Kazakov 2014-03-11 12:23:18 UTC
The patches were reverted due to crashes and hangups
Comment 20 mc3dkid 2014-05-21 18:10:19 UTC
Just wanted to chime in that I am EAGERLY awaiting a fix for these shortcut issues. This is a pretty big deal breaker for me right now as my workflow requires highly customized input. Thanks.
Comment 21 Halla Rempt 2014-12-12 13:37:09 UTC
Git commit 84eced3cf6487405004d81037dd65b54329cb467 by Boudewijn Rempt.
Committed on 12/12/2014 at 13:34.
Pushed by rempt into branch 'calligra/2.9'.

Restore all shortcuts after we create the first view

Because many shortcuts aren't known until we explicitly create a view
and tools, restoring those shortcuts can only be done when we've got a
view. An alternative could be to create a hidden view on startup to
initialize the shortcuts.

M  +15   -17   krita/ui/KisPart.cpp
M  +0    -2    krita/ui/KisPart.h
M  +161  -138  krita/ui/KisView.cpp
M  +2    -0    krita/ui/KisView.h

http://commits.kde.org/calligra/84eced3cf6487405004d81037dd65b54329cb467
Comment 22 Halla Rempt 2014-12-15 13:23:42 UTC
Okay, now we've got a problem where having two open documents will overwrite the custom settings with the default settings for shortcuts.
Comment 23 Halla Rempt 2014-12-19 14:05:29 UTC
*** Bug 342013 has been marked as a duplicate of this bug. ***
Comment 24 Halla Rempt 2015-01-03 13:57:26 UTC
Git commit c02946c86c44a86a912b5c2100db8eaec5d92209 by Boudewijn Rempt.
Committed on 03/01/2015 at 11:06.
Pushed by rempt into branch 'calligra/2.9'.

Create a master action collection for the shortcut editor

This is still a half-way solution, ideally we'd just create actions from
the .action file and fetch those actions in the code where we need an
action, so there's still duplication. If we make the master action
collection the source for new actions, we will also have to make the
krita.action file translatable.

If you create an action which should have an editable shortcut, you need
to add that to a .action file. Make sure that all the text is exactly the
same, so it gets translated in the shortcut dialog.

On closing the shortcut dialog, changed shortcuts are saved. All
mainwindows's actioncollection will re-read the configuration so they
are synchronized. There's no local krita.rc file anymore, so the windows
won't be able to save the shortcuts to the local krita.rc, all shorcuts
are saved in the kritarc config file.
Related: bug 342270, bug 342184, bug 337368

A  +336  -0    krita/krita.actions
M  +1    -14   krita/ui/KisMainWindow.cpp
M  +92   -3    krita/ui/KisPart.cpp
M  +19   -0    krita/ui/KisPart.h
M  +0    -23   krita/ui/KisView.cpp
M  +0    -22   krita/ui/KisViewManager.cpp

http://commits.kde.org/calligra/c02946c86c44a86a912b5c2100db8eaec5d92209