MIDI Controller preferences: improve UX with MIDI options

src/controllers/delegates/midioptionsdelegate.cpp

@@ -81,13 +81,21 @@ QWidget* MidiOptionsDelegate::createEditor(QWidget* parent,
     // * clicking an option or pressing Enter on a selected option toggles it,
     //   closes the list view and commits the updated data
     // * pressing Space on a selected option toggles it, list remains open
-    // * clicking outside the listview closes it, and another click causing the
+    // * clicking outside the list view closes it, and another click causing the
     //   combobox to lose focus closes that and commits pending changes
     connect(pComboBox,
             QOverload<int>::of(&QComboBox::activated),
             this,
             &MidiOptionsDelegate::commitAndCloseEditor);
 
+    // Uncheck other exclusive options whenever one is checked, whether by
+    // clicking, pressing Enter, or toggling with Space (which keeps the list
+    // open and doesn't emit activated()).
+    connect(pModel,
+            &QStandardItemModel::itemChanged,
+            this,
+            &MidiOptionsDelegate::slotItemChanged);
+
     return pComboBox;
 }
 
@@ -161,21 +169,46 @@ void MidiOptionsDelegate::commitAndCloseEditor(int index) {
     DEBUG_ASSERT(pItem);
     if (pItem->isCheckable()) {
         pItem->setCheckState(pItem->checkState() == Qt::Checked ? Qt::Unchecked : Qt::Checked);
-        // TODO Concurrent option scan be selected. Implement a compatibility
-        // matrix and uncheck all options that are incompatible with the last
-        // checked option. Store initial check state for/in each item and
-        // hook up to QStandardItemModel::itemChanged()
     } else {
         // Clear was selected. Uncheck all other items
         for (int row = 0; row < pModel->rowCount() - 1; row++) {
             if (row == index) { // Actually it's the last item, but this is safer.
                 continue;
             }
-            pItem = pModel->item(row, 0);
-            pItem->setCheckState(Qt::Unchecked);
+            auto* pOtherItem = pModel->item(row, 0);
+            pOtherItem->setCheckState(Qt::Unchecked);
         }
     }
 
     emit commitData(pComboBox);
     emit closeEditor(pComboBox);
 }
+
+void MidiOptionsDelegate::slotItemChanged(QStandardItem* pItem) {
+    // Return if we unchecked an item
+    if (!pItem || !pItem->isCheckable() || pItem->checkState() != Qt::Checked) {
+        return;
+    }
+    auto option = static_cast<MidiOption>(pItem->data().toUInt());
+    // Nothing to do for non-exclusive options
+    if (!midiOptionIsExclusive(option)) {
+        return;
+    }
+
+    // This is an exclusive option, so uncheck all other exclusive options,
+    // but keep state of "modifiers" (Invert, SoftTakeover).
+    // This happens on the fly so it covers toggling items with Spacebar,
+    // as well as clicks committed via activated().
+    const auto* pModel = qobject_cast<QStandardItemModel*>(pItem->model());
+    DEBUG_ASSERT(pModel);
+    for (int row = 0; row < pModel->rowCount() - 1; row++) {
+        auto* pOtherItem = pModel->item(row, 0);
+        if (pOtherItem == pItem || !pOtherItem->isCheckable()) {
+            continue;
+        }
+        auto otherOption = static_cast<MidiOption>(pOtherItem->data().toUInt());
+        if (midiOptionIsExclusive(otherOption)) {
+            pOtherItem->setCheckState(Qt::Unchecked);
+        }
+    }
+}

src/controllers/delegates/midioptionsdelegate.h

@@ -2,6 +2,8 @@
 
 #include <QStyledItemDelegate>
 
+class QStandardItem;
+
 class MidiOptionsDelegate : public QStyledItemDelegate {
     Q_OBJECT
   public:
@@ -20,4 +22,5 @@ class MidiOptionsDelegate : public QStyledItemDelegate {
 
   private slots:
     void commitAndCloseEditor(int index);
+    void slotItemChanged(QStandardItem* pItem);
 };

src/controllers/midi/midicontroller.cpp

@@ -502,7 +502,7 @@ double MidiController::computeValue(
     }
 
     if (options.testFlag(MidiOption::Invert)) {
-        return 127. - newmidivalue;
+        newmidivalue = 127. - newmidivalue;
     }
 
     if (options & (MidiOption::Rot64 | MidiOption::Rot64Invert)) {
@@ -518,73 +518,53 @@ double MidiController::computeValue(
         } else {
             tempval -= diff;
         }
-        return (tempval < 0. ? 0. : (tempval > 127. ? 127.0 : tempval));
-    }
-
-    if (options.testFlag(MidiOption::Rot64Fast)) {
+        newmidivalue = tempval < 0. ? 0. : (tempval > 127. ? 127.0 : tempval);
+    } else if (options.testFlag(MidiOption::Rot64Fast)) {
         tempval = prevmidivalue;
         diff = newmidivalue - 64.;
         diff *= 1.5;
         tempval += diff;
-        return (tempval < 0. ? 0. : (tempval > 127. ? 127.0 : tempval));
-    }
-
-    if (options.testFlag(MidiOption::Diff)) {
-        //Interpret 7-bit signed value using two's compliment.
+        newmidivalue = tempval < 0. ? 0. : (tempval > 127. ? 127.0 : tempval);
+    } else if (options.testFlag(MidiOption::Diff)) {
+        // Interpret 7-bit signed value using two's compliment.
         if (newmidivalue >= 64.) {
             newmidivalue = newmidivalue - 128.;
         }
-        //Apply sensitivity to signed value. FIXME
-       // if(sensitivity > 0)
+        // Apply sensitivity to signed value. FIXME
+        // if(sensitivity > 0)
         //    _newmidivalue = _newmidivalue * ((double)sensitivity / 50.);
-        //Apply new value to current value.
+        // Apply new value to current value.
         newmidivalue = prevmidivalue + newmidivalue;
-    }
-
-    if (options.testFlag(MidiOption::SelectKnob)) {
-        //Interpret 7-bit signed value using two's compliment.
+    } else if (options.testFlag(MidiOption::SelectKnob)) {
+        // Interpret 7-bit signed value using two's compliment.
+        // Since this is a selection knob, we do not want to inherit previous values.
         if (newmidivalue >= 64.) {
             newmidivalue = newmidivalue - 128.;
         }
-        //Apply sensitivity to signed value. FIXME
-        //if(sensitivity > 0)
+        // FIXME Apply sensitivity to signed value
+        // if (sensitivity > 0)
         //    _newmidivalue = _newmidivalue * ((double)sensitivity / 50.);
-        //Since this is a selection knob, we do not want to inherit previous values.
-    }
-
-    if (options.testFlag(MidiOption::Button)) {
-        newmidivalue = newmidivalue != 0;
-    }
-
-    if (options.testFlag(MidiOption::Switch)) {
-        newmidivalue = 1;
-    }
+    } else if (options.testFlag(MidiOption::Button)) {
+        return newmidivalue != 0;
+    } else if (options.testFlag(MidiOption::Switch)) {
+        return 1;
+    } else if (options.testFlag(MidiOption::Spread64)) {
+        // Distance away from centre point (aka "relative CC")
+        newmidivalue = newmidivalue - 64.;
 
-    if (options.testFlag(MidiOption::Spread64)) {
-        //qDebug() << "MIDI_OPT_SPREAD64";
-        // BJW: Spread64: Distance away from centre point (aka "relative CC")
-        // Uses a similar non-linear scaling formula as ControlTTRotary::getValueFromWidget()
+        // FIXME
+        // Use a similar non-linear scaling formula as ControlTTRotary::getValueFromWidget()
         // but with added sensitivity adjustment. This formula is still experimental.
-
-        newmidivalue = newmidivalue - 64.;
-        //FIXME
-        //double distance = _newmidivalue - 64.;
+        // double distance = _newmidivalue - 64.;
         // _newmidivalue = distance * distance * sensitivity / 50000.;
-        //if (distance < 0.)
+        // if (distance < 0.)
         //    _newmidivalue = -newmidivalue;
-
-        //qDebug() << "Spread64: in " << distance << "  out " << newmidivalue;
-    }
-
-    if (options.testFlag(MidiOption::HercJog)) {
+    } else if (options.testFlag(MidiOption::HercJog)) {
         if (newmidivalue > 64.) {
             newmidivalue -= 128.;
         }
         newmidivalue += prevmidivalue;
-        //if (_prevmidivalue != 0.0) { qDebug() << "AAAAAAAAAAAA" << prevmidivalue; }
-    }
-
-    if (options.testFlag(MidiOption::HercJogFast)) {
+    } else if (options.testFlag(MidiOption::HercJogFast)) {
         if (newmidivalue > 64.) {
             newmidivalue -= 128.;
         }

src/controllers/midi/midimessage.h

@@ -105,11 +105,14 @@ QDebug operator<<(QDebug debug, MidiOpCode midiOpCode);
 qhash_seed_t qHash(MidiOpCode key, qhash_seed_t seed);
 
 enum class MidiOption : uint16_t {
+    /// Normal: return input value
     None = 0x0000,
+    /// Return 127 - input value
     Invert = 0x0001,
     Rot64 = 0x0002,
     Rot64Invert = 0x0004,
     Rot64Fast = 0x0008,
+    /// Returns difference to previous value
     Diff = 0x0010,
     /// Button Down (!=00) and Button Up (00) events happen together
     Button = 0x0020,
@@ -125,9 +128,9 @@ enum class MidiOption : uint16_t {
     SoftTakeover = 0x0400,
     /// Maps a MIDI control to a custom JavaScript function
     Script = 0x0800,
-    /// Nessage supplies the LSB of a 14-bit message
+    /// Message supplies the LSB of a 14-bit message
     FourteenBitLSB = 0x1000,
-    /// Nessage supplies the MSB of a 14-bit message
+    /// Message supplies the MSB of a 14-bit message
     FourteenBitMSB = 0x2000,
     /// Generic Hercules Range Correction (0x01 -> +5; 0x7f -> -5)
     HercJogFast = 0x4000,
@@ -136,6 +139,22 @@ Q_DECLARE_FLAGS(MidiOptions, MidiOption);
 Q_DECLARE_OPERATORS_FOR_FLAGS(MidiOptions);
 Q_DECLARE_METATYPE(MidiOptions);
 
+inline bool midiOptionIsModifier(MidiOption option) {
+    // Actually the same as !midiOptionIsExclusive() but kept separate
+    // for now to allow extension
+    return option == MidiOption::Invert || option == MidiOption::SoftTakeover;
+}
+
+inline bool midiOptionIsExclusive(MidiOption option) {
+    switch (option) {
+    case MidiOption::Invert:
+    case MidiOption::SoftTakeover:
+        return false;
+    default:
+        return true;
+    }
+}
+
 struct MidiOutput {
     MidiOutput()
             : message(0) {