KeyboardController #966

Tomasito665 · 2016-06-21T01:39:37Z

Keyboard controller

This PR brings the KeyboardController to life, a controller whose life cycle is regulated in ControllerManager, just like every other Controller. Its task is to communicate with KeyboardEventFilter, which is instructed about current mappings when a new mapping is loaded.

When the user presses a key sequence known to KeyboardEventFilter, it passes the ConfigKey bound to the pressed key sequence to KeyboardController, which then does or does not set the Mixxx control, depending on whether the controller is enabled or not. The user can enable / disable it both via controller preferences and via main menu bar's Enable Keyboard Shortcuts checkbox.

Keyboard controller presets

Keyboard mappings are stored to disk as *.kbd.xml files and can hold mappings for multiple keyboard languages. However, it usually just contains info for one layout. That's because Mixxx is now able to translate key sequences from one keyboard layout to the other, runtime. Therefore, it is possible to start Mixxx with keyboard layout en_US, switch to ru_RU and then to el_GR and the keyboard mappings will keep working on each switch (naturally, tooltips will update as well).

Layouts tool

Mapping translations are made using keyboard layout tables found in layouts.cpp, to which layouts can be added or removed with the layouts_tool. The tool uses the X11 API to get the current keyboard map from X. Therefore, this tool does only work on Linux.

From config files to XML presets

The kbdcfg_to_kbdxml tool can be given multiple legacy *.kbd.cfg files (one per layout) and outputs one single *.kbd.xml file. The tool is also given the layouts.cpp file, so that it can explicitly add mappings for specific layouts or leave them out, depending on whether the specific mapping is translatable or not. This tool is used to create Keyboard-Default.kbd.xml, the default keyboard preset.

Other

If not obvious: restarting Mixxx is not longer required to switch to a custom mapping (there's no utility yet to make custom mappings, though. So, custom mappings will have to be made by hand, for now)
Keyboard works instantly on Mixxx startup, without having to manually give focus to one of the control widgets.
Key W and key S are now usable on Greek layouts (Explanation of the problem: http://pastebin.com/v4KVmrD4)

- Created KeyboardController class and implemented (though not functional yet) the pure virtual methods inherited from Controller. - Created mockup for KeyboardControllerPreset, that will contain (most likely) a ConfigObject containing the keymapping info. - Added ControllerVisitor::visit(KeyboardController*) and implemented this method also in HidController, BulkController and MidiController

…:visit(kbd)

This function will be used to get the Keyboard controller from other parts of Mixxx.

…ller slot

The KeyboardControllerPresetFileHandler is capable of reading this new format and loading the preset.

When a new keyboard preset is loaded, it emits a signal to KEF, sending a clone of the preset. KEF now reads that, instead of a ConfigObject. Since we don't make use anymore of the keyboard .cfg files, it doesn't make sense any more to keep using a ConfigObject the mapping.

Completely got rid of the "[Keyboard], enabled" config key. Everything is now done through "[Controller], Keyboard"

Add default keyseq to ShortcutChangeWatcher, so that it can change the shortcut of it's bound QAction to the default one, if it didn't find any ConfigValueKbd matching the bound ConfigKey.

All ShortcutChangeWatchers are currently stored in a QList in WMainMenuBar. This is a problem, because we also need those watchers in other parts of Mixxx. (i.e. in LegacySkinParser, in order to keep tooltips up to date). Make a member variable m_pShortcutController in Mixxx, which will then pass the shortcutcontroller to whatever object needs to know about shortcut updates. Signals will be received by the ShortcutController.

The creation of tooltips is currently a very linear process, that find place in LegacySkinParser::setupConnections(). We need to extract the tooltips setup and make it more generic, so that it can be refreshed when a new kbd preset is loaded. When a new keyboard preset is loaded, the TooltipUpdater is passed a clone of that kbd preset via a signal and refreshes all widgets, passed in by addWatcher().

There is a currently a bug, displaying tooltip shortcuts that are bound to a WBaseObject that is nor a WSliderComposed, nor a WPushButton. Fix by tweaking PushButtonTooltipWatcher::udate make correct use of setTooltipShortcut method, that was updated in the last commit, but the implementation in PushButtonTooltipWatcher was not updated yet.

By adding a new method to KeyboardControllerPreset returning all QKeySequences bound to a specific ConfigKey, separated by a given string separator. WidgetTooltipWatcher::setTooltipShortcut uses this method to separate the QKeySequences by a comma.

Making the code more readable, because it's less code.

Widgets can have multiple connections to multiple configKeys. With the current implementation, if there are two connections, and the second one's configKey has no keyboard shortcut defined, than it picks that one (empty string) and the tooltip shows any shortcut. With support for multiple config keys that problem is gone and the tooltip always displays the shortcuts.

Nothing special, mainly code quality things.

daschuer · 2016-08-29T21:10:05Z

res/controllers/Keyboard-Default.kbd.xml

+        <keyseq byPositionOf="en_US">.</keyseq>
+      </control>
+      <control action="hotcue_3_clear">
+        <keyseq byPositionOf="de_DE">Shift+:</keyseq>


Why we have here de_DE and not

<keyseq byPositionOf="en_US">Shift+></keyseq>

en_US is one of the two layouts that have two keys sharing the same keysym for the same modifier Check here for explanation.

This means that en_US users will have this action mapped to both keys, which I don't think it's a problem because the <LSGT> key is not autochthonal to American keyboards. It's the key between the left Shift and the Z key, which does only exist on European keyboards, a.k.a the 105th key.

Ah yes, thank you for the reminder!
Can you add a comment to the mapping file to avoid future confusion?

Sure! I'll be added in the next push :)

daschuer · 2016-08-30T19:05:53Z

@Tomasito665: What is missing to merge this into Master?
The most important point is no regressions.

The Keyboard mappings should work out of the Box without touching the preference.
** Can we link the "Enable Hotkeys" menu option with the Enable Keyboard controller?
There should be a doable way to convert custom mappings.
** can we "compile" the phyton script in a way that it runs on all targets when installed along with Mixxx? Than we may add a button somewhere that shows a help ...
** How much work would it be to transfer this to the C++ domain and do the convert on the fly?

Tomasito665 · 2016-08-31T13:23:10Z

@daschuer: The keyboard controller is already linked with the Enable keyboard shortcuts menu checkbox ;)

That python script is specially written to handle multiple xx_XX.cfg.kbd files, where xx_XX is a keyboard layout that the script knows about because of the layouts.cpp file it is given. With other words, this script is specially written to handle the files in mixxx/res/keyboard/. It is the tool I used for creating Default-Mapping.kbd.xm. I think it has too much unnecessary overhead to convert just one file.

The question is: Should Custom.kbd.xml, the conversion of Custom.kbd.cfg, be translatable or not?

I think we shouldn't, because we don't know the keyboard layout on which Custom.kbd.cfg was targeted by the user. We could guess... but if we guess incorrect, we mess the custom mappings up and the mappings might be unusable. And also, the user is already used to his custom mappings not being changed when the keyboard layout changes.

If we don't need it to be translatable, the conversion is pretty straight forward:

[Master]
crossfader_up h
crossfader_down g

﹀

<!-- Just map on raw 'h' and 'g', not giving any layout nor positional information -->
<group name="[Master]">
  <control action="crossfader_up">
    <keyseq>h</keyseq>
  </control>

  <control action="crossfader_down">
    <keyseq>g</keyseq>
  </control>
</group>

A C++ conversion tool for this won't be hard to write. We could use it in the Upgrade class. What are your thoughts on this?

daschuer · 2016-08-31T13:41:11Z

@daschuer: The keyboard controller is already linked with the Enable keyboard shortcuts menu checkbox ;)

Baybe there is a bug ... , I will retest more carefully.

A C++ conversion tool for this won't be hard to write. We could use it in the Upgrade class. What are your thoughts on this?

Perfect!

To all:
I think we could merge once the remaining issues are ironed out. How about merge this to 2.1?

Tomasito665 · 2016-09-09T10:13:17Z

@daschuer As you suggested, I included yvals.h for Windows. I tested it and it compiles :) However, KeyboardLayoutChange events are not coming through, at least on Windows 10. That means that mappings are not translated when switching from keyboard layout. I have not looked into it in detail yet, though. I will see what I can do this weekend.

daschuer · 2016-09-09T10:24:32Z

Thank you, very much for continue to care about this outside GSOC :-)
... and nice to know, you have a working Windows build environment.

daschuer · 2018-04-14T15:57:35Z

@Tomasito665:
It would be great if we can include this in Mixxx 2.2. Do you have fun and time to revive this PR?

Be-ing · 2018-04-18T04:37:48Z

I'll take the lack of response so far as a "no" and close this PR. @Tomasito665 if you are interested in resolving the merge conflicts and having this merged, please reopen this PR.

daschuer · 2018-04-18T22:05:15Z

I am, this is really a nice piece of software.

daschuer · 2018-09-05T06:10:56Z

This is superseded by #1746

Tomasito665 added 30 commits May 28, 2016 01:02

Added TODOs to KeyboardController, implemented DlgControllerLearning:…

1667621

…:visit(kbd)

Implemented last missing virtual methods to KeyboardController

2f140a5

Created keyboardenumerator and added to controllermanager

a027f09

Merge branch 'master' of https://github.com/mixxxdj/mixxx into develop

8e2cbf3

Added new functionality to Controllermanager. getControllerListOfType

2923b5f

This function will be used to get the Keyboard controller from other parts of Mixxx.

Got rid of getControllerListByType again

8ad64ce

KeyboardEventFilter now emits signals and connected to KeyboardContro…

80360be

…ller slot

KeyboardController now enumerated and preset info parsed

e7bfef0

Merge branch 'master' of https://github.com/mixxxdj/mixxx into develop

247517c

Made XML format for keyboard controller presets

df0ae4d

The KeyboardControllerPresetFileHandler is capable of reading this new format and loading the preset.

Bug fixed where Mixxx crashed when pressing keyboardkey without a preset

41acce5

Keyboardpreset filehandler is now capable of saving the preset

ea5614c

Removed TODO that was already done

4e1603f

Keyboard disablable and enablable in controller preferences

33ee563

Enable and disable keyboard controller from menubar

3d012ad

Completely got rid of the "[Keyboard], enabled" config key. Everything is now done through "[Controller], Keyboard"

KeyboardShortcuts working and updated when kbd preset loaded

10ca82d

Add default keyseq to shortcutchangewatcher

778a03a

Add default keyseq to ShortcutChangeWatcher, so that it can change the shortcut of it's bound QAction to the default one, if it didn't find any ConfigValueKbd matching the bound ConfigKey.

Merge branch 'master' of https://github.com/mixxxdj/mixxx into develop

46b870f

Split TooltipShortcutUpdater up in subclasses

0cd2c2c

Making the code more readable, because it's less code.

Take care of simple TODOs

4bdadec

Nothing special, mainly code quality things.

Removed auto-added <> includes where possible

3e6b47b

Format shortcutchangewatcher the same as tooltipupdater, for consistency

f743bbe

Added override keywoard where possible

a9e9177

daschuer reviewed Aug 29, 2016
View reviewed changes

Tomasito665 added 6 commits August 31, 2016 12:40

Remove 'virtual' keyword in overridden methods with 'override' keyword

86051df

Rename keyseqsRawI to keyseqsRawIterator

159eda4

Use static_cast for casting to QKeyEvent

42f7180

Extract prevLayout local static variable and make it a member var

8378cb3

Make constructor explicit

a84c982

Use LayoutNamePair struct instead of QStringList

1f580ff

Tomasito665 added 3 commits September 6, 2016 22:02

Add comments for actions based on de_DE positions

8abfc48

Make layouts.cpp compatible with VC2013

d615b9d

Merge branch 'develop' of github.com:Tomasito665/mixxx into develop

40ae1c9

Be-ing closed this Apr 18, 2018

daschuer reopened this Apr 18, 2018

Be-ing added stalled and removed stalled labels Apr 18, 2018

Be-ing added this to the stalled milestone Apr 18, 2018

daschuer mentioned this pull request Jul 9, 2018

Keyboard controller #1746

Draft

daschuer closed this Sep 5, 2018

uklotzde removed this from the stalled milestone Jun 12, 2019

mixxxbot mentioned this pull request Aug 23, 2022

Keyboard Layout Selection (Wishlist) #9701

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

KeyboardController #966

KeyboardController #966

Tomasito665 commented Jun 21, 2016 •

edited

Loading

daschuer Aug 29, 2016

Tomasito665 Aug 31, 2016

daschuer Aug 31, 2016

Tomasito665 Aug 31, 2016

daschuer commented Aug 30, 2016

Tomasito665 commented Aug 31, 2016

daschuer commented Aug 31, 2016

Tomasito665 commented Sep 9, 2016

daschuer commented Sep 9, 2016

daschuer commented Apr 14, 2018

Be-ing commented Apr 18, 2018

daschuer commented Apr 18, 2018

daschuer commented Sep 5, 2018

KeyboardController #966

KeyboardController #966

Conversation

Tomasito665 commented Jun 21, 2016 • edited Loading