sort keys by circle of fifths #1000

Be-ing · 2016-09-02T23:39:44Z

Fixing https://bugs.launchpad.net/mixxx/+bug/1435628

The order starts with C at the top, then its relative minor (a), then adds 1 sharp (G), then G's relative minor (e), and so adding sharps/removing flats around the circle of fifths. For Lancelot notation, it starts with G#m at the top, which is 1A in that notation, and proceeds alphanumerically (minor keys appear before their relative major keys in this case).

I'm having trouble figuring out how to go about making it respond to changes in the Key Preferences. Presently, the sort order is only determined when Mixxx starts. Maybe emit a signal from DlgPrefKey::slotApply() and connect a slot to it in ColumnCache's constructor?

Woo! PR #1000!

The order starts with C at the top, then its relative minor (a), then adds 1 sharp (G), then G's relative minor (e), and so adding sharps/removing flats around the circle of fifths.

for readability

daschuer · 2016-09-04T13:16:59Z

Woo! PR #1000!

You got it! Congratulations ;-)

daschuer · 2016-09-04T13:44:51Z

This was:
#649

daschuer · 2016-09-04T13:45:38Z

src/library/columncache.cpp


    m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_LOCATION], sortNoCase);
    m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_ARTIST], sortNoCase);
    m_columnSortByIndex.insert(m_columnIndexByEnum[COLUMN_PLAYLISTTRACKSTABLE_TITLE], sortNoCase);
 }
+
+void ColumnCache::setKeySortOrder(QString const& notation) {
+    std::vector<mixxx::track::io::key::ChromaticKey> sortOrder;


Please add a comment, why we did not register a custom sorting function.

daschuer · 2016-09-04T13:48:29Z

I'm having trouble figuring out how to go about making it respond to changes in the Key Preferences. Presently, the sort order is only determined when Mixxx starts. Maybe emit a signal from DlgPrefKey::slotApply() and connect a slot to it in ColumnCache's constructor?

I think it will be the best, to use a ControlObject for it.

Be-ing · 2016-09-04T13:50:30Z

I think it will be the best, to use a ControlObject for it.

The question still remains, how to make ColumnCache react when the CO changes?

Be-ing · 2016-09-06T00:06:46Z

Okay, I figured out how to make use of a ControlObject and ControlProxy for this. It now responds to configuration changes. The new sorting order isn't applied until the user sorts the column again. I think this may be desirable to avoid unexpected changes in the sorting.

http://mixxx.org/wiki/doku.php/developer_guide_control could use a bit of updating.

daschuer · 2016-09-06T08:36:50Z

Travis fails, not your fault:
The job exceeded the maximum time limit for jobs, and has been terminated.

daschuer · 2016-09-06T08:43:17Z

src/preferences/dialog/dlgprefkey.cpp

@@ -119,6 +119,7 @@ void DlgPrefKey::loadSettings() {

    QString notation = m_pConfig->getValueString(
        ConfigKey(KEY_CONFIG_KEY, KEY_NOTATION));
+    m_pKeyNotation = new ControlProxy(ConfigKey("[Library]", "key_notation"));


Add "this" so the new ControlProxy will be deleted by the Qt Object tree

and add explanatory comment on use of KeyNotation CUSTOM

Be-ing · 2016-09-07T05:22:21Z

src/track/keyutils.h

@@ -12,8 +12,6 @@
 class KeyUtils {
  public:
    enum KeyNotation {
-        // The default notation (set with setNotation).
-        DEFAULT = 0,


I tried making CUSTOM = 0, but then the sort order wasn't being set up properly for that notation when Mixxx started, even when I added m_pKeyNotation->emitValueChanged(); after https://github.com/Be-ing/mixxx/blob/3a5142f3dd5db09955320f4b938cd9e921315c8c/src/preferences/dialog/dlgprefkey.cpp#L141

daschuer · 2016-09-07T06:52:06Z

A m_pKeyNotation->emitValueChanged(); here will issue the signal only slots bound in DlgPrefKey.

Just call slotSetKeySortOrder() directly like for a short solution.
.. or set bPersist = true in ControlObject and replace all related direct pConfig-> calles by ControlProxies.

daschuer · 2016-09-08T18:20:14Z

LGTM! Thank you.

Be-ing · 2016-09-08T18:21:42Z

Thanks for reviewing. I'm glad this is finally polished and merged. I'll sure be paying more attention to the key info in my library now.

esbrandt · 2016-09-08T18:39:42Z

Thanks for adding this useful feature.
Much appreciated.

On Sep 8, 2016, at 20:21, Be notifications@github.com wrote:

Thanks for reviewing. I'm glad this is finally polished and merged. I'll sure be paying more attention to the key info in my library now.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub #1000 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/AEUPSQ-Rj5dtctSYwKdCmF7TRWBBRrakks5qoFI5gaJpZM4J0Kg6.

esbrandt · 2016-09-08T19:15:30Z

Hmm, now there is this warning in the log

Warning [Main]: QString::arg: Argument missing: CASE key_id WHEN 0 THEN 0 WHEN 1 THEN 1 WHEN 22 THEN 2 WHEN 8 THEN 3 WHEN 17 THEN 4 WHEN 3 THEN 5 WHEN 24 THEN 6 WHEN 10 THEN 7 WHEN 19 THEN 8 WHEN 5 THEN 9 WHEN 14 THEN 10 WHEN 12 THEN 11 WHEN 21 THEN 12 WHEN 7 THEN 13 WHEN 16 THEN 14 WHEN 2 THEN 15 WHEN 23 THEN 16 WHEN 9 THEN 17 WHEN 18 THEN 18 WHEN 4 THEN 19 WHEN 13 THEN 20 WHEN 11 THEN 21 WHEN 20 THEN 22 WHEN 6 THEN 23 WHEN 15 THEN 24 END, key

FWIW , i have many short samples in my library, thad don´t have a key detected, even after analyzing.

Any hints?

Be-ing · 2016-09-08T19:43:15Z

Hm, I'm guessing that has to do with how undetected keys are stored in the DB. Perhaps a special case needs to be added to the SQL to handle that.

Be-ing · 2016-09-08T20:01:14Z

Ah, I see. The key field in the DB is NULL if no key is detected. I think adding something like WHEN NULL THEN 0 will get rid of the warning.

Be-ing added 6 commits July 16, 2015 19:07

sort keys by circle of fifths in library display

37cf42a

The order starts with C at the top, then its relative minor (a), then adds 1 sharp (G), then G's relative minor (e), and so adding sharps/removing flats around the circle of fifths.

construct key sorting query string from array of ChromaticKeys

5c6b56b

for readability

Merge remote-tracking branch 'upstream/1.12' into key_sort

eb48f16

Merge remote-tracking branch 'upstream/master' into key_sort

32f163e

Merge remote-tracking branch 'upstream/master' into key_sort

aa4ee67

sort keys appropriately when using Lancelot notation

97a9871

daschuer reviewed Sep 4, 2016
View reviewed changes

Be-ing added 2 commits September 4, 2016 11:31

Merge remote-tracking branch 'upstream/master' into key_sort

0f29ffd

make key sorting respond to notation changes

b9c7c1b

document GitHub discussion of slower implementation

836dcf2

daschuer reviewed Sep 6, 2016
View reviewed changes

Be-ing added 3 commits September 7, 2016 00:17

take care of memory leak

05c065d

add explanatory comment

29faef0

get rid of confusingly named KeyNotation DEFAULT

3a5142f

and add explanatory comment on use of KeyNotation CUSTOM

Be-ing reviewed Sep 7, 2016
View reviewed changes

0 index KeyUtils::KeyNotation

023e756

Be-ing force-pushed the key_sort branch from 7ba45c6 to 023e756 Compare September 7, 2016 13:46

daschuer merged commit 40babde into mixxxdj:master Sep 8, 2016

Be-ing mentioned this pull request Sep 9, 2016

rework keySortSQL to work with columnSortForFieldIndex() without a warning #1005

Merged

esbrandt mentioned this pull request Jun 24, 2017

WIP Manual 2.1.x updates mixxxdj/manual#59

Merged

37 tasks

esbrandt added a commit to esbrandt/manual that referenced this pull request Apr 14, 2018

Add info for sorting keys by circle of fifths, see mixxxdj/mixxx#1000

74f384e

rryan mentioned this pull request Jan 20, 2019

added release and zulip badget #2000

Merged

michelealbano mentioned this pull request Jun 12, 2023

script to create inverted colors skins, and a LateDay skin #11638

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sort keys by circle of fifths #1000

sort keys by circle of fifths #1000

Be-ing commented Sep 2, 2016 •

edited

Loading

daschuer commented Sep 4, 2016

daschuer commented Sep 4, 2016

daschuer Sep 4, 2016

daschuer commented Sep 4, 2016

Be-ing commented Sep 4, 2016

Be-ing commented Sep 6, 2016 •

edited

Loading

daschuer commented Sep 6, 2016

daschuer Sep 6, 2016

Be-ing Sep 7, 2016

daschuer commented Sep 7, 2016

daschuer commented Sep 8, 2016

Be-ing commented Sep 8, 2016

esbrandt commented Sep 8, 2016

esbrandt commented Sep 8, 2016

Be-ing commented Sep 8, 2016

Be-ing commented Sep 8, 2016

sort keys by circle of fifths #1000

sort keys by circle of fifths #1000

Conversation

Be-ing commented Sep 2, 2016 • edited Loading

daschuer commented Sep 4, 2016

daschuer commented Sep 4, 2016

daschuer Sep 4, 2016

Choose a reason for hiding this comment

daschuer commented Sep 4, 2016

Be-ing commented Sep 4, 2016

Be-ing commented Sep 6, 2016 • edited Loading

daschuer commented Sep 6, 2016

daschuer Sep 6, 2016

Choose a reason for hiding this comment

Be-ing Sep 7, 2016

Choose a reason for hiding this comment

daschuer commented Sep 7, 2016

daschuer commented Sep 8, 2016

Be-ing commented Sep 8, 2016

esbrandt commented Sep 8, 2016

esbrandt commented Sep 8, 2016

Be-ing commented Sep 8, 2016

Be-ing commented Sep 8, 2016

Be-ing commented Sep 2, 2016 •

edited

Loading

Be-ing commented Sep 6, 2016 •

edited

Loading