|Anonymous | Login||2016-07-24 15:04 CEST|
|My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0001895||Kdenlive||User Interface||public||2010-11-25 23:10||2010-12-07 21:35|
|Platform||32 bit intel and alike||OS||Kubuntu Linux||OS Version||8.10|
|Product Version||Recent git|
|Target Version||Fixed in Version|
|Summary||0001895: Add a JogAction class that takes care of the mapping of actions from Jog Shuttle devices|
|Description||This is my second step for improving the JogShuttle. |
I have introduced a two new classes:
* one takes care of the button-action mapping based on some configuration, and contains a list of all button-mappable actions with their user-visible strings (using i18n to make them translatable)
* another that takes care of the configuration. Currently, this second class just provides functionality to parse the mapping configuration, but will later also provide default configurations for devices we know about.
Note that I am not sure I have bound the actions to the right slots.
|Additional Information||Again, feel free to comment on any matter here, including style and used data structures etc. Also, if there are too many change requests, I will provide another patch, that will be simpler.|
|Tags||No tags attached.|
|Build/Install Method||Manual build from SVN|
|Attached Files|| shuttle_jog_actions.patch.gz [^] (6,726 bytes) 2010-11-25 23:10|
shuttle_jog_actions_take2.patch.gz [^] (12,571 bytes) 2010-12-03 00:43
fix_crash_and_logging.patch.gz [^] (1,026 bytes) 2010-12-03 01:26
edited on: 2010-11-27 21:16
There is one thing I would like to change in the way you implemented the jogshuttle buttons, it's the way the actions are created.
In your patch (jogaction.h), for each action you define a name and an i18n string for translation.
But all the actions already have a translatable string attached to them. FOr example if we take the "Set Zone In" action, it is defined in mainwindow.cpp around line 1216 like this:
KAction *markIn = collection->addAction("mark_in");
markIn->setText(i18n("Set Zone In"));
connect(markIn, SIGNAL(triggered(bool)), this, SLOT(slotSetInPoint()));
So I would prefer to use this action directly instead of recreating strings. It should be possible in mainwindow to list the action names that we want, in the case of "Set Zone In", the action name is "mark_in" (can also be found in kdenliveui.rc). Then, the (K)action can be reached by using:
and the action text can be found using:
So in mainwindow we could create a list of action names and translatable strings, then the jogshuttle would just emit the name of the action when triggered and a slot in mainwindow could trigger the action from the name.
Hope you understood me, if you need I can work on a sample patch.
Also, just for the record, I have a SpaceShuttle (basic model with 5 buttons), and the buttons are numbered from 5 to 9.
I understood you very well, and I like it too. Having the list of actions defined this way should also make it simpler when a new one is added, which would ideally need no change in the jogshuttle specific parts at all.
I will modify the code accordingly, and will submit a new patch.
Another question that came across my mind, as I will work on this a bit more: what is your approach with QString vs. std::string ? Do you prefer to go with the Qt infrastructure, or you don't mind the mix of std C++ and Qt ?
Please find a new patch (produced against current HEAD, @5127). This contains:
- a split between JogShuttle device events (jog fwd/back, shuttle pos from -7 .. +7, button #1) and the JogAction that send monitor forward/rewind and action events;
- modification to mainwindow to have a slot that can execute named actions
- adding a slot slotPause, as I did not find any that makes sure the monitor stops whatever its current state
- all the action names are now taken from the actions that get registered in the normal mainwindow::setupActions()
- changed the configuration to store the mapping from button to action-id (the internal name) that can handle an arbitrary number of buttons and is resitatn to UI language changes as we store the action-id not the index of the item in the combobox.
I have tested this on my machine, and it seems to work reasonably well. There is still some console logging left in there giving button mappings, this should probably go before any release.
My next step will be to modify the UI in the configuration so I can configure the other 10 buttons my device has.
Also, please give me feedback on the size of the patch, I am happy to split it if you feel that you need finer granularity for reverting purpose :-)
Added a new small patch to apply after the 'take2' one. This fixes the crash when the settings are not there yet or incomplete (like when upgrading kdenlive version).
It also removes the button activity console logging.
|I applied your patch with some small changes to improve the behavior in the Kdenlive Settings dialog (update the "apply" button status, etc).|
Great! Indeed, I missed the tracking of the changed state in the settings.
I have ran it from the repos, it looks like all is working as when I submitted it.
Thanks a lot!
|2010-11-25 23:10||fleury||New Issue|
|2010-11-25 23:10||fleury||File Added: shuttle_jog_actions.patch.gz|
|2010-11-27 21:15||j-b-m||Note Added: 0006125|
|2010-11-27 21:15||j-b-m||Assigned To||=> j-b-m|
|2010-11-27 21:15||j-b-m||Status||new => feedback|
|2010-11-27 21:16||j-b-m||Note Edited: 0006125||View Revisions|
|2010-11-28 10:07||fleury||Note Added: 0006128|
|2010-11-28 10:07||fleury||Status||feedback => assigned|
|2010-12-03 00:43||fleury||File Added: shuttle_jog_actions_take2.patch.gz|
|2010-12-03 00:50||fleury||Note Added: 0006141|
|2010-12-03 01:26||fleury||File Added: fix_crash_and_logging.patch.gz|
|2010-12-03 01:27||fleury||Note Added: 0006142|
|2010-12-07 20:44||j-b-m||Note Added: 0006149|
|2010-12-07 20:44||j-b-m||Status||assigned => feedback|
|2010-12-07 21:35||fleury||Note Added: 0006150|
|2010-12-07 21:35||fleury||Status||feedback => assigned|
|Copyright © 2000 - 2016 MantisBT Team|