diff --git a/src/base/Instrument.cpp b/src/base/Instrument.cpp index b58f4a2ad..0c6ff98ca 100644 --- a/src/base/Instrument.cpp +++ b/src/base/Instrument.cpp @@ -222,7 +222,7 @@ Instrument::isProgramValid() const oBankIter != validBanks.end(); ++oBankIter) { - if (oBankIter->partialCompare(m_program.getBank())) { + if (oBankIter->compareKey(m_program.getBank())) { bankValid = true; break; } diff --git a/src/base/MidiDevice.cpp b/src/base/MidiDevice.cpp index 93dc7157d..95152b0b9 100644 --- a/src/base/MidiDevice.cpp +++ b/src/base/MidiDevice.cpp @@ -463,7 +463,7 @@ MidiDevice::getPrograms(const MidiBank &bank) const for (ProgramList::const_iterator it = m_programList.begin(); it != m_programList.end(); ++it) { - if (it->getBank().partialCompare(bank)) programs.push_back(*it); + if (it->getBank().compareKey(bank)) programs.push_back(*it); } return programs; @@ -497,7 +497,7 @@ MidiDevice::getBankName(const MidiBank &bank) const { for (BankList::const_iterator it = m_bankList.begin(); it != m_bankList.end(); ++it) { - if ((*it).partialCompare(bank)) return it->getName(); + if ((*it).compareKey(bank)) return it->getName(); } return ""; } @@ -606,7 +606,7 @@ MidiDevice::toXmlString() const // for (pt = m_programList.begin(); pt != m_programList.end(); ++pt) { - if (pt->getBank().partialCompare(*it)) + if (pt->getBank().compareKey(*it)) { midiDevice << " getProgram() << "\" " @@ -740,7 +740,7 @@ MidiDevice::mergeBankList(const BankList &bankList) { for (oIt = m_bankList.begin(); oIt != m_bankList.end(); ++oIt) { - if ((*it).partialCompare(*oIt)) + if ((*it).compareKey(*oIt)) { clash = true; break; diff --git a/src/base/MidiProgram.cpp b/src/base/MidiProgram.cpp index acc47da8f..72fabd4f7 100644 --- a/src/base/MidiProgram.cpp +++ b/src/base/MidiProgram.cpp @@ -42,7 +42,7 @@ MidiBank::operator==(const MidiBank &rhs) const } bool -MidiBank::partialCompare(const MidiBank &rhs) const +MidiBank::compareKey(const MidiBank &rhs) const { return (m_percussion == rhs.m_percussion && m_msb == rhs.m_msb && @@ -95,7 +95,7 @@ MidiProgram::MidiProgram(const MidiBank &bank, MidiByte program, const std::stri bool MidiProgram::partialCompare(const MidiProgram &rhs) const { - return m_bank.partialCompare(rhs.m_bank) && m_program == rhs.m_program; + return m_bank.compareKey(rhs.m_bank) && m_program == rhs.m_program; } bool diff --git a/src/base/MidiProgram.h b/src/base/MidiProgram.h index e959bdead..75c620083 100644 --- a/src/base/MidiProgram.h +++ b/src/base/MidiProgram.h @@ -53,21 +53,45 @@ class MidiBank /// A full comparison of all fields. /** - * This probably isn't what you want. See partialCompare(). + * MIDIInstrumentParameterPanel::updateBankComboBox() uses this to + * detect changes that might require a repopulation of the bank combobox. + * + * See MidiBank::compareKey(). */ bool operator==(const MidiBank &rhs) const; bool operator!=(const MidiBank &rhs) const { return !operator==(rhs); } - /// Compare all fields except name. + /// Compare MSB:LSB:Percussion. /** * Since MidiProgram stores a partial MidiBank object (without name), * a partial comparison such as this is frequently needed. + * + * ??? Should this really compare percussion? I think that's wrong + * right now. See if we can cause bugs with it. Then fix it. */ - bool partialCompare(const MidiBank &rhs) const; + bool compareKey(const MidiBank &rhs) const; + + // Only compares (percussion), msb, and lsb. + // Most useful for sorting and searching. + // This is specifically NOT operator<() because it does not compare + // all fields. + bool lessKey(const MidiBank &rhs) const + { + // ??? We'll need percussion soon. + //if (m_percussion == rhs.m_percussion) { + if (m_msb == rhs.m_msb) + return (m_lsb < rhs.m_lsb); + return (m_msb < rhs.m_msb); + //} + //return (m_percussion < rhs.m_percussion); + } private: - bool m_percussion; + // Key fields. + bool m_percussion; // Not a key field yet. But soon. MidiByte m_msb; MidiByte m_lsb; + + // Data fields. std::string m_name; }; @@ -96,13 +120,30 @@ class MidiProgram // m_keyMapping. bool partialCompareWithName(const MidiProgram &rhs) const; + // This only compares bank and program. + // Most useful for sorting and searching. + // Currently this is only used by MidiProgramsEditor for sorting. + bool lessKey(const MidiProgram &rhs) const + { + if (m_bank.compareKey(rhs.m_bank)) + return (m_program < rhs.m_program); + return m_bank.lessKey(rhs.m_bank); + } + private: + // Key fields. MidiBank m_bank; MidiByte m_program; + + // Data fields. std::string m_name; std::string m_keyMapping; }; +// ??? std::vector? This means all throughout rg we have to do linear +// searches. Wouldn't a std::set<> indexed by MSB:LSB:Percussion:PC +// make a *lot* more sense in the long run? Should reduce CPU usage +// and complexity significantly. typedef std::vector ProgramList; inline bool diff --git a/src/gui/editors/parameters/MIDIInstrumentParameterPanel.cpp b/src/gui/editors/parameters/MIDIInstrumentParameterPanel.cpp index c5a90a825..045e88780 100644 --- a/src/gui/editors/parameters/MIDIInstrumentParameterPanel.cpp +++ b/src/gui/editors/parameters/MIDIInstrumentParameterPanel.cpp @@ -566,7 +566,8 @@ MIDIInstrumentParameterPanel::updateBankComboBox() // Find the selected bank in the MIDI Device's bank list. for (unsigned int i = 0; i < banks.size(); ++i) { - if (getSelectedInstrument()->getProgram().getBank().partialCompare(banks[i])) { + if (getSelectedInstrument()->getProgram().getBank().compareKey( + banks[i])) { currentBank = i; break; } diff --git a/src/gui/studio/BankEditorDialog.cpp b/src/gui/studio/BankEditorDialog.cpp index 4e2a2022c..22d4bc1ac 100644 --- a/src/gui/studio/BankEditorDialog.cpp +++ b/src/gui/studio/BankEditorDialog.cpp @@ -1172,10 +1172,10 @@ BankEditorDialog::slotDelete() it != m_programList.end(); ++it) { // If this program isn't in the bank that is being deleted, - // add it to the new program list. We use partialCompare() + // add it to the new program list. We use compareKey() // because the MidiBank objects in the program list do not // have their name fields filled in. - if (!it->getBank().partialCompare(bank)) + if (!it->getBank().compareKey(bank)) newProgramList.push_back(*it); } @@ -1691,7 +1691,7 @@ BankEditorDialog::slotEditPaste() // Remove programs that will be overwritten // for (it = m_programList.begin(); it != m_programList.end(); ++it) { - if (!(it->getBank().partialCompare(m_lastBank))) + if (!(it->getBank().compareKey(m_lastBank))) tempProg.push_back(*it); } m_programList = tempProg; @@ -1704,7 +1704,7 @@ BankEditorDialog::slotEditPaste() // Add the new programs // for (it = tempProg.begin(); it != tempProg.end(); ++it) { - if (it->getBank().partialCompare(sourceBank)) { + if (it->getBank().compareKey(sourceBank)) { // Insert with new MSB and LSB // MidiProgram copyProgram(m_lastBank, @@ -1895,7 +1895,7 @@ bool BankEditorDialog::tracksUsingBank(const MidiBank& bank, const MidiProgram& program = instrument->getProgram(); const MidiBank& ibank = program.getBank(); - if (bank.partialCompare(ibank)) { + if (bank.compareKey(ibank)) { // Found a Track using this bank trackPositions.push_back(track->getPosition()); } diff --git a/src/gui/studio/MidiKeyMappingEditor.cpp b/src/gui/studio/MidiKeyMappingEditor.cpp index 53dbb433b..ba21884c8 100644 --- a/src/gui/studio/MidiKeyMappingEditor.cpp +++ b/src/gui/studio/MidiKeyMappingEditor.cpp @@ -16,6 +16,7 @@ */ #define RG_MODULE_STRING "[MidiKeyMappingEditor]" +#define RG_NO_DEBUG_PRINT #include "MidiKeyMappingEditor.h" #include "NameSetEditor.h" @@ -69,8 +70,6 @@ MidiKeyMappingEditor::makeAdditionalWidget(QWidget */* parent */) void MidiKeyMappingEditor::clearAll() { - blockAllSignals(true); - for (size_t i = 0; i < m_names.size(); ++i) m_names[i]->clear(); @@ -79,9 +78,6 @@ MidiKeyMappingEditor::clearAll() m_librarian->clear(); m_librarianEmail->clear(); setEnabled(false); - - blockAllSignals(false); - } void @@ -126,8 +122,6 @@ MidiKeyMappingEditor::reset() m_mapping = *m; - blockAllSignals(true); - // Librarian details // m_librarian->setText(strtoqstr(m_device->getLibrarianName())); @@ -145,14 +139,11 @@ MidiKeyMappingEditor::reset() if ( (int)i == index) { QString name = strtoqstr(it->second); - m_completions << name; m_names[i]->setText(name); m_names[i]->setCursorPosition(0); } } } - - blockAllSignals(false); } void @@ -177,17 +168,8 @@ MidiKeyMappingEditor::slotNameChanged(const QString &name) void MidiKeyMappingEditor::slotKeyMapButtonPressed() -{} - -void MidiKeyMappingEditor::blockAllSignals(bool block) { - QList allChildren = - findChildren((QRegularExpression)"[0-9]+"); - QList::iterator it; - - for (it = allChildren.begin(); it != allChildren.end(); ++it) { - (*it)->blockSignals(block); - } } + } diff --git a/src/gui/studio/MidiKeyMappingEditor.h b/src/gui/studio/MidiKeyMappingEditor.h index 734396557..9a595d314 100644 --- a/src/gui/studio/MidiKeyMappingEditor.h +++ b/src/gui/studio/MidiKeyMappingEditor.h @@ -54,7 +54,6 @@ public slots: protected: virtual QWidget *makeAdditionalWidget(QWidget *parent); - void blockAllSignals(bool block); //--------------- Data members --------------------------------- diff --git a/src/gui/studio/MidiProgramsEditor.cpp b/src/gui/studio/MidiProgramsEditor.cpp index c53158206..6491f2e0e 100644 --- a/src/gui/studio/MidiProgramsEditor.cpp +++ b/src/gui/studio/MidiProgramsEditor.cpp @@ -15,10 +15,11 @@ COPYING included with this distribution for more information. */ - #define RG_MODULE_STRING "[MidiProgramsEditor]" +#define RG_NO_DEBUG_PRINT #include "MidiProgramsEditor.h" + #include "MidiBankTreeWidgetItem.h" #include "NameSetEditor.h" #include "BankEditorDialog.h" @@ -33,433 +34,446 @@ #include #include -#include #include +#include #include -#include -#include -#include #include #include #include #include -#include #include #include -#include #include -#include -#include #include -#include -#include +#include // std::sort +#include namespace Rosegarden { -MidiProgramsEditor::MidiProgramsEditor(BankEditorDialog* bankEditor, - QWidget* parent) : - NameSetEditor(bankEditor, - tr("Bank and Program details"), // title - parent, - true), // showKeyMapButtons - m_device(nullptr), - m_bankList(bankEditor->getBankList()), - m_programList(bankEditor->getProgramList()), - m_oldBank(false, 0, 0) + +// Load once for performance. +static const QIcon &getNoKeyMapIcon() { - QWidget *additionalWidget = makeAdditionalWidget(m_topFrame); - if (additionalWidget) { - m_topLayout->addWidget(additionalWidget, 0, 0, 3, 3); - } + // Use a white icon to indicate there is no keymap for this program. + static const QIcon noKeyMapIcon(IconLoader::loadPixmap("key-white")); + + return noKeyMapIcon; } -QWidget * -MidiProgramsEditor::makeAdditionalWidget(QWidget *parent) +// Load once for performance. +static const QIcon &getKeyMapIcon() { - QFrame *frame = new QFrame(parent); + // Use a green icon to indicate there *is* a keymap for this program. + static const QIcon keyMapIcon(IconLoader::loadPixmap("key-green")); + + return keyMapIcon; +} - m_percussion = new QCheckBox(frame); - m_msb = new QSpinBox(frame); - m_lsb = new QSpinBox(frame); +MidiProgramsEditor::MidiProgramsEditor(BankEditorDialog *bankEditor, + QWidget *parent) : + NameSetEditor(bankEditor, + tr("Bank and Program details"), // title + parent, + true), // showKeyMapButtons + m_bankList(bankEditor->getBankList()), + m_programList(bankEditor->getProgramList()) +{ + QFrame *frame = new QFrame(m_topFrame); frame->setContentsMargins(0, 0, 0, 0); + QGridLayout *gridLayout = new QGridLayout(frame); gridLayout->setSpacing(0); + // Percussion gridLayout->addWidget(new QLabel(tr("Percussion"), frame), 0, 0, Qt::AlignLeft); - gridLayout->addWidget(m_percussion, 0, 1, Qt::AlignLeft); + m_percussion = new QCheckBox(frame); connect(m_percussion, &QAbstractButton::clicked, - this, &MidiProgramsEditor::slotNewPercussion); + this, &MidiProgramsEditor::slotPercussionClicked); + gridLayout->addWidget(m_percussion, 0, 1, Qt::AlignLeft); + // MSB Value gridLayout->addWidget(new QLabel(tr("MSB Value"), frame), 1, 0, Qt::AlignLeft); + m_msb = new QSpinBox(frame); + m_msb->setToolTip(tr("Selects a MSB controller Bank number (MSB/LSB pairs are always unique for any Device)")); m_msb->setMinimum(0); m_msb->setMaximum(127); + connect(m_msb, + //QOverload::of(&QSpinBox::valueChanged), // Qt5.7+ + static_cast(&QSpinBox::valueChanged), + this, &MidiProgramsEditor::slotNewMSB); gridLayout->addWidget(m_msb, 1, 1, Qt::AlignLeft); - m_msb->setToolTip(tr("Selects a MSB controller Bank number (MSB/LSB pairs are always unique for any Device)")); - - m_lsb->setToolTip(tr("Selects a LSB controller Bank number (MSB/LSB pairs are always unique for any Device)")); - - connect(m_msb, SIGNAL(valueChanged(int)), - this, SLOT(slotNewMSB(int))); - + // LSB Value gridLayout->addWidget(new QLabel(tr("LSB Value"), frame), 2, 0, Qt::AlignLeft); + m_lsb = new QSpinBox(frame); + m_lsb->setToolTip(tr("Selects a LSB controller Bank number (MSB/LSB pairs are always unique for any Device)")); m_lsb->setMinimum(0); m_lsb->setMaximum(127); + connect(m_lsb, + //QOverload::of(&QSpinBox::valueChanged), // Qt5.7+ + static_cast(&QSpinBox::valueChanged), + this, &MidiProgramsEditor::slotNewLSB); gridLayout->addWidget(m_lsb, 2, 1, Qt::AlignLeft); - connect(m_lsb, SIGNAL(valueChanged(int)), - this, SLOT(slotNewLSB(int))); - - return frame; + m_topLayout->addWidget(frame, 0, 0, 3, 3); } ProgramList MidiProgramsEditor::getBankSubset(const MidiBank &bank) { - ProgramList program; - ProgramList::iterator it; + ProgramList programList; - for (it = m_programList.begin(); it != m_programList.end(); ++it) { - if (it->getBank().partialCompare(bank)) - program.push_back(*it); + // For each program, copy the ones for the requested bank to programList. + for (const MidiProgram &program : m_programList) { + if (program.getBank().compareKey(bank)) + programList.push_back(program); } - return program; -} - -MidiBank* -MidiProgramsEditor::getCurrentBank() -{ - return m_currentBank; + return programList; } void -MidiProgramsEditor::modifyCurrentPrograms(const MidiBank &oldBank, - const MidiBank &newBank) +MidiProgramsEditor::modifyCurrentPrograms( + const MidiBank &oldBank, const MidiBank &newBank) { - ProgramList::iterator it; - - for (it = m_programList.begin(); it != m_programList.end(); ++it) { - if (it->getBank().partialCompare(oldBank)) { - *it = MidiProgram(newBank, it->getProgram(), it->getName()); - } + // For each program in m_programList... + for (MidiProgram &program : m_programList) { + // If this one is in the old bank, update it to the new. + if (program.getBank().compareKey(oldBank)) + program = MidiProgram( + newBank, program.getProgram(), program.getName()); } } void MidiProgramsEditor::clearAll() { - blockAllSignals(true); - - for (size_t i = 0; i < m_names.size(); ++i) - m_names[i]->clear(); - setTitle(tr("Bank and Program details")); m_percussion->setChecked(false); m_msb->setValue(0); m_lsb->setValue(0); + m_currentBank = nullptr; + m_librarian->clear(); m_librarianEmail->clear(); - m_currentBank = nullptr; - setEnabled(false); - blockAllSignals(false); + for (size_t i = 0; i < m_names.size(); ++i) + m_names[i]->clear(); + + setEnabled(false); } void -MidiProgramsEditor::populate(QTreeWidgetItem* item) +MidiProgramsEditor::populate(QTreeWidgetItem *item) { - RG_DEBUG << "MidiProgramsEditor::populate\n"; + RG_DEBUG << "populate()"; - MidiBankTreeWidgetItem* bankItem = dynamic_cast(item); + MidiBankTreeWidgetItem *bankItem = + dynamic_cast(item); if (!bankItem) { - RG_DEBUG << "MidiProgramsEditor::populate : not a bank item - returning\n"; - return ; + RG_DEBUG << "populate(): not a bank item - returning"; + return; } + RG_DEBUG << "populate() : bankItem->getBank = " << bankItem->getBank(); + + //m_currentBank = m_device->getBankByIndex(bankItem->getBank()); + m_currentBank = &(m_bankList[bankItem->getBank()]); + m_oldBank = *m_currentBank; + DeviceId deviceId = bankItem->getDeviceId(); m_device = m_bankEditor->getMidiDevice(deviceId); if (!m_device) - return ; + return; setEnabled(true); - setBankName(item->text(0)); - - RG_DEBUG << "MidiProgramsEditor::populate : bankItem->getBank = " - << bankItem->getBank(); - - m_currentBank = &(m_bankList[bankItem->getBank()]); // m_device->getBankByIndex(bankItem->getBank()); - - blockAllSignals(true); + setTitle(bankItem->text(0)); - // set the bank values + // Percussion m_percussion->setChecked(m_currentBank->isPercussion()); + + // MSB Value m_msb->setValue(m_currentBank->getMSB()); - m_lsb->setValue(m_currentBank->getLSB()); - m_oldBank = *m_currentBank; + // LSB Value + m_lsb->setValue(m_currentBank->getLSB()); - // Librarian details - // + // Provided By m_librarian->setText(strtoqstr(m_device->getLibrarianName())); m_librarianEmail->setText(strtoqstr(m_device->getLibrarianEmail())); - ProgramList programSubset = getBankSubset(*m_currentBank); - ProgramList::iterator it; + // Program List - QPixmap noKeyPixmap, keyPixmap; + // Get the programs for the current bank. + ProgramList programSubset = getBankSubset(*m_currentBank); - noKeyPixmap = IconLoader::loadPixmap("key-white"); - keyPixmap = IconLoader::loadPixmap("key-green"); + const bool haveKeyMappings = (m_device->getKeyMappings().size() > 0); - bool haveKeyMappings = m_device->getKeyMappings().size() > 0; + // For each name field (LineEdit) on the UI... + // programIndex is also the program change number. + for (size_t programIndex = 0; programIndex < m_names.size(); ++programIndex) { - for (unsigned int i = 0; i < (unsigned int)m_names.size(); i++) { + QToolButton *keyMapButton = getKeyMapButton(programIndex); + keyMapButton->setMaximumHeight(12); + keyMapButton->setEnabled(haveKeyMappings); - m_names[i]->clear(); - getKeyMapButton(i)->setEnabled(haveKeyMappings); - getKeyMapButton(i)->setIcon(QIcon(noKeyPixmap)); - // QToolTip::remove - // ( getKeyMapButton(i) ); - getKeyMapButton(i)->setToolTip(QString("")); //@@@ Usefull ? - getKeyMapButton(i)->setMaximumHeight( 12 ); - - for (it = programSubset.begin(); it != programSubset.end(); ++it) { - if (it->getProgram() == i) { - - // zero in on "Harpsichord" vs. "Coupled Harpsichord to cut down - // on noise (0-based) -// if (i == 6) std::cout << "it->getName(): " << it->getName() << std::endl; - QString programName = strtoqstr(it->getName()); - m_completions << programName; - m_names[i]->setText(programName); - - if (m_device->getKeyMappingForProgram(*it)) { - getKeyMapButton(i)->setIcon(QIcon(keyPixmap)); - getKeyMapButton(i)->setToolTip - (tr("Key Mapping: %1") - .arg(strtoqstr(m_device->getKeyMappingForProgram(*it)->getName()))); - } + bool found = false; + MidiProgram foundProgram; + // Find the program in programSubset. + for (const MidiProgram &midiProgram : programSubset) { + // Found? We're done. + if (midiProgram.getProgram() == programIndex) { + found = true; + foundProgram = midiProgram; break; } } - // show start of label - m_names[i]->setCursorPosition(0); - } + // If not found, clear and continue. + if (!found) { + m_names[programIndex]->clear(); + keyMapButton->setIcon(getNoKeyMapIcon()); + keyMapButton->setToolTip(""); + continue; + } - blockAllSignals(false); + // Found it. + + // Name + + QString programName = strtoqstr(foundProgram.getName()); + m_names[programIndex]->setText(programName); + // Show start of label. + m_names[programIndex]->setCursorPosition(0); + + // Icon and ToolTip + + const MidiKeyMapping *midiKeyMapping = + m_device->getKeyMappingForProgram(foundProgram); + if (midiKeyMapping) { + // Indicate that this program has a keymap. + keyMapButton->setIcon(getKeyMapIcon()); + // Put the name in the tool tip. + keyMapButton->setToolTip(tr("Key Mapping: %1").arg( + strtoqstr(midiKeyMapping->getName()))); + } else { // No key mapping. + // Indicate that this program has no keymap. + keyMapButton->setIcon(getNoKeyMapIcon()); + keyMapButton->setToolTip(""); + } + } } void MidiProgramsEditor::reset() { - m_percussion->blockSignals(true); - m_msb->blockSignals(true); - m_lsb->blockSignals(true); + // Go back to m_oldBank's MSB/LSB and percussion setting. m_percussion->setChecked(m_oldBank.isPercussion()); m_msb->setValue(m_oldBank.getMSB()); m_lsb->setValue(m_oldBank.getLSB()); + // Make sure all the programs in m_programList are set back to the m_oldBank + // MSB/LSB. if (m_currentBank) { modifyCurrentPrograms(*m_currentBank, m_oldBank); *m_currentBank = m_oldBank; } - - m_percussion->blockSignals(false); - m_msb->blockSignals(false); - m_lsb->blockSignals(false); } void -MidiProgramsEditor::slotNewPercussion() +MidiProgramsEditor::slotPercussionClicked() { - RG_DEBUG << "MidiProgramsEditor::slotNewPercussion"; - bool percussion = false; // Doesn't matter - MidiBank *newBank; - if (banklistContains(MidiBank(percussion, m_msb->value(), m_lsb->value()))) { - RG_DEBUG << "MidiProgramsEditor::slotNewPercussion: calling setChecked(" << !percussion << ")"; - newBank = new MidiBank(m_percussion->isChecked(), - m_msb->value(), - m_lsb->value(), getCurrentBank()->getName()); - } else { - newBank = new MidiBank(true, - m_msb->value(), - m_lsb->value()); - } - modifyCurrentPrograms(*getCurrentBank(), *newBank); - *getCurrentBank() = *newBank; - m_bankEditor->slotApply(); + RG_DEBUG << "slotPercussionClicked()"; - // Hack to force the percussion icons to switch state if needed. - // code stole from populate. - if (m_device) { - bool haveKeyMappings = m_device->getKeyMappings().size() > 0; + MidiBank newBank( + m_percussion->isChecked(), + m_msb->value(), + m_lsb->value(), + m_currentBank->getName()); - for (unsigned int i = 0; i < (unsigned int)m_names.size(); i++) { - getKeyMapButton(i)->setEnabled(haveKeyMappings); - } - } + // Make sure the programs in m_programList have the new percussion setting. + modifyCurrentPrograms(*m_currentBank, newBank); + + // Update the current bank. + *m_currentBank = newBank; + + // Refresh the tree so that it shows "Percussion" or "Bank" as appropriate. + m_bankEditor->slotApply(); } void MidiProgramsEditor::slotNewMSB(int value) { - RG_DEBUG << "MidiProgramsEditor::slotNewMSB(" << value << ")\n"; + RG_DEBUG << "slotNewMSB(" << value << ")"; - m_msb->blockSignals(true); + // ??? Not sure we should clean this up since we are getting rid of it. int msb; try { - msb = ensureUniqueMSB(value, value > getCurrentBank()->getMSB()); + msb = ensureUniqueMSB(value, value > m_currentBank->getMSB()); } catch (bool) { - msb = getCurrentBank()->getMSB(); + msb = m_currentBank->getMSB(); } MidiBank newBank(m_percussion->isChecked(), msb, - m_lsb->value(), getCurrentBank()->getName()); + m_lsb->value(), + m_currentBank->getName()); - modifyCurrentPrograms(*getCurrentBank(), newBank); + modifyCurrentPrograms(*m_currentBank, newBank); m_msb->setValue(msb); - *getCurrentBank() = newBank; - m_msb->blockSignals(false); + *m_currentBank = newBank; + // Refresh the tree so that it shows the new MSB. m_bankEditor->slotApply(); } void MidiProgramsEditor::slotNewLSB(int value) { - RG_DEBUG << "MidiProgramsEditor::slotNewLSB(" << value << ")\n"; + RG_DEBUG << "slotNewLSB(" << value << ")"; - m_lsb->blockSignals(true); + // ??? Not sure we should clean this up since we are getting rid of it. int lsb; try { - lsb = ensureUniqueLSB(value, value > getCurrentBank()->getLSB()); + lsb = ensureUniqueLSB(value, value > m_currentBank->getLSB()); } catch (bool) { - lsb = getCurrentBank()->getLSB(); + lsb = m_currentBank->getLSB(); } MidiBank newBank(m_percussion->isChecked(), m_msb->value(), - lsb, getCurrentBank()->getName()); + lsb, + m_currentBank->getName()); - modifyCurrentPrograms(*getCurrentBank(), newBank); + modifyCurrentPrograms(*m_currentBank, newBank); m_lsb->setValue(lsb); - *getCurrentBank() = newBank; - m_lsb->blockSignals(false); + *m_currentBank = newBank; + // Refresh the tree so that it shows the new MSB. m_bankEditor->slotApply(); } -struct ProgramCmp -{ - bool operator()(const Rosegarden::MidiProgram &p1, - const Rosegarden::MidiProgram &p2) const - { - if (p1.getProgram() == p2.getProgram()) { - const Rosegarden::MidiBank &b1(p1.getBank()); - const Rosegarden::MidiBank &b2(p2.getBank()); - if (b1.getMSB() == b2.getMSB()) - if (b1.getLSB() == b2.getLSB()) - return ((b1.isPercussion() ? 1 : 0) < (b2.isPercussion() ? 1 : 0)); - else return (b1.getLSB() < b2.getLSB()); - else return (b1.getMSB() < b2.getMSB()); - } else return (p1.getProgram() < p2.getProgram()); - } -}; - void -MidiProgramsEditor::slotNameChanged(const QString& programName) +MidiProgramsEditor::slotNameChanged(const QString &programName) { - const LineEdit *lineEdit = dynamic_cast(sender()); + //RG_DEBUG << "slotNameChanged(" << programName << ")"; - if (!lineEdit) { - RG_WARNING << "slotNameChanged(): WARNING: Signal sender is not a LineEdit."; - return; - } + // This is called for every single change to the edit box. E.g. + // If the user types "hi", this slot is called twice. Once with + // "h" and again with "hi". - const unsigned id = lineEdit->property("index").toUInt(); + // ??? Can we be more efficient? E.g. only make the change when + // the cursor leaves the edit box, or "Ok" is clicked? That + // would help with the command history spam issue. - //RG_DEBUG << "slotNameChanged(" << programName << ") : id = " << id; + if (!m_currentBank) { + RG_WARNING << "slotNameChanged(): WARNING: m_currentBank is nullptr."; + return; + } - MidiBank *currBank = getCurrentBank(); + const LineEdit *lineEdit = dynamic_cast(sender()); - if (!currBank) { - RG_WARNING << "slotNameChanged(): WARNING: currBank is nullptr."; + if (!lineEdit) { + RG_WARNING << "slotNameChanged(): WARNING: Signal sender is not a LineEdit."; return; } - //RG_DEBUG << "slotNameChanged(): currBank: " << currBank; + // Zero-based program number. This is the same as the MIDI program change. + const unsigned programNumber = lineEdit->property("index").toUInt(); - //RG_DEBUG << "slotNameChanged(): current bank name: " << currBank->getName(); + //RG_DEBUG << "slotNameChanged(" << programName << ") : id = " << programNumber; - MidiProgram *program = getProgram(*currBank, id); + // Get the MidiProgram that needs to be changed from m_programList. + ProgramList::iterator programIter = + getProgramIter(*m_currentBank, programNumber); - // If the MidiProgram doesn't exist - if (!program) { - // Do nothing if program name is empty + // If the MidiProgram doesn't exist in m_programList, add it. + if (programIter == m_programList.end()) { + // If the program name is empty, do nothing. if (programName.isEmpty()) return; // Create a new MidiProgram and add it to m_programList. - MidiProgram newProgram(*getCurrentBank(), id); + MidiProgram newProgram(*m_currentBank, programNumber); m_programList.push_back(newProgram); - // Sort by program number. - std::sort(m_programList.begin(), m_programList.end(), ProgramCmp()); - - // Now, get the MidiProgram from the m_programList. - program = getProgram(*getCurrentBank(), id); - - } else { - // If we've found a program and the label is now empty, - // remove it from the program list. + // Sort m_programList. + // We need to sort this for the MIPP. It just needs the PCs in + // order. That's how it displays them. + // If we do not sort this, the .rg file is also mixed up. But + // that's not a serious issue since no one looks at that. + // ??? Is there a better place to sort this? What if we take + // the first steps toward moving to std::set? Make it a + // std::set, but continue using it like a vector. That + // way it is always sorted. + std::sort(m_programList.begin(), + m_programList.end(), + [](const MidiProgram &lhs, const MidiProgram &rhs){ return lhs.lessKey(rhs); }); + + // Get the new MidiProgram from m_programList. + programIter = getProgramIter(*m_currentBank, programNumber); + + } else { // The MidiProgram already exists in m_programList. + + // If the label is now empty... if (programName.isEmpty()) { - for (ProgramList::iterator it = m_programList.begin(); - it != m_programList.end(); - ++it) { - if (static_cast(it->getProgram()) == id) { - m_programList.erase(it); - m_bankEditor->slotApply(); + //RG_DEBUG << "slotNameChanged(): deleting empty program (" << programNumber << ")"; + m_programList.erase(programIter); + // Call the parent's slotApply() to make the change to the + // document. + // ??? Command History SPAM. + // ??? This creates a command and adds it to the history. + // Are we seeing a ton of things appear in the undo history? + // Yes! This spams the undo history until it is useless. + // And there's no way to undo from within the editor. So + // adding to the command history seems pointless. + // ??? We should only call this on switching banks or + // closing the BankEditorDialog. We need to review + // BankEditorDialog to make sure this is the best solution. + // ??? Other parts of the editor call this for an update. But + // program name changes do not appear on the tree. So an + // update of any kind has no value. + m_bankEditor->slotApply(); - //RG_DEBUG << "slotNameChanged(): deleting empty program (" << id << ")"; - - return; - } - } + return; } } - if (!program) { - RG_WARNING << "slotNameChanged(): WARNING: program is nullptr."; + if (programIter == m_programList.end()) { + RG_WARNING << "slotNameChanged(): WARNING: programIter is end()."; return; } //RG_DEBUG << "slotNameChanged(): program: " << program; // If the name has actually changed - if (qstrtostr(programName) != program->getName()) { - program->setName(qstrtostr(programName)); + if (qstrtostr(programName) != programIter->getName()) { + programIter->setName(qstrtostr(programName)); + // Call the parent's slotApply() to make the change to the + // document. + // ??? Command History SPAM. See comments above. + // ??? We should only call this on switching banks or + // closing the BankEditorDialog. We need to review + // BankEditorDialog to make sure this is the best solution. m_bankEditor->slotApply(); } } @@ -467,125 +481,134 @@ MidiProgramsEditor::slotNameChanged(const QString& programName) void MidiProgramsEditor::slotKeyMapButtonPressed() { - QToolButton *button = dynamic_cast(sender()); - - if (!button) { - RG_WARNING << "slotKeyMapButtonPressed() : WARNING: Sender is not a QPushButton."; - return; - } - if (!m_device) return; - const KeyMappingList &kml = m_device->getKeyMappings(); - if (kml.empty()) + QToolButton *button = dynamic_cast(sender()); + if (!button) { + RG_WARNING << "slotKeyMapButtonPressed(): WARNING: Sender is not a QPushButton."; return; + } const unsigned id = button->property("index").toUInt(); - MidiProgram *program = getProgram(*getCurrentBank(), id); + MidiProgram *program = getProgram(*m_currentBank, id); if (!program) return; - m_currentMenuProgram = id; + const KeyMappingList &keyMappingList = m_device->getKeyMappings(); + if (keyMappingList.empty()) + return; + + // Save the program number we are editing for slotKeyMapMenuItemSelected(). + m_keyMapProgramNumber = id; + + // Create a pop-up menu filled with the available key mappings. - // Create a new popup menu. QMenu *menu = new QMenu(button); const MidiKeyMapping *currentMapping = - m_device->getKeyMappingForProgram(*program); + m_device->getKeyMappingForProgram(*program); + // Keep track of the current key map selection for + // popup menu positioning. int currentKeyMap = 0; - QAction *a = menu->addAction(tr("")); - a->setObjectName("0"); + // Add the initial "". + QAction *action = menu->addAction(tr("")); + action->setObjectName("0"); - for (size_t i = 0; i < kml.size(); ++i) { - a = menu->addAction(strtoqstr(kml[i].getName())); - a->setObjectName(QString("%1").arg(i+1)); + // For each key mapping... + for (size_t i = 0; i < keyMappingList.size(); ++i) { + // Add the mapping to the menu. + action = menu->addAction(strtoqstr(keyMappingList[i].getName())); + action->setObjectName(QString("%1").arg(i+1)); - if (currentMapping && (kml[i] == *currentMapping)) + // If the current keymap for this program is found, keep track of it. + if (currentMapping && (keyMappingList[i] == *currentMapping)) currentKeyMap = static_cast(i + 1); } - connect(menu, SIGNAL(triggered(QAction *)), - this, SLOT(slotKeyMapMenuItemSelected(QAction *))); + connect(menu, &QMenu::triggered, + this, &MidiProgramsEditor::slotKeyMapMenuItemSelected); // Compute the position for the pop-up menu. - // QMenu::popup() can do this for us, but it doesn't place the - // cursor over top of the current selection. + // Make sure the menu will be positioned such that the mouse pointer + // is over the currently selected item. - // Get the QRect for the current entry. QRect actionRect = menu->actionGeometry(menu->actions().value(currentKeyMap)); - - QPoint pos = QCursor::pos(); - pos.rx() -= 10; - pos.ry() -= actionRect.top() + actionRect.height() / 2; + QPoint menuPos = QCursor::pos(); + // Adjust position so that the mouse will end up on top of + // the current selection. + menuPos.rx() -= 10; + menuPos.ry() -= actionRect.top() + actionRect.height() / 2; // Display the menu. - menu->popup(pos); -} + menu->popup(menuPos); -void -MidiProgramsEditor::slotKeyMapMenuItemSelected(QAction *a) -{ - slotKeyMapMenuItemSelected(a->objectName().toInt()); + // slotKeyMapMenuItemSelected() is the next step in this process. } void -MidiProgramsEditor::slotKeyMapMenuItemSelected(int i) +MidiProgramsEditor::slotKeyMapMenuItemSelected(QAction *action) { + // The user has selected an item from the menu presented + // by slotKeyMapButtonPressed(). + if (!m_device) - return ; + return; - const KeyMappingList &kml = m_device->getKeyMappings(); - if (kml.empty()) - return ; + const KeyMappingList &keyMappingList = m_device->getKeyMappings(); + if (keyMappingList.empty()) + return; - MidiProgram *program = getProgram(*getCurrentBank(), m_currentMenuProgram); + MidiProgram *program = getProgram(*m_currentBank, m_keyMapProgramNumber); if (!program) - return ; + return; + + // Extract the key map number from the object name. + // Subtract one to convert from 1-based key map number to 0-based. + // Simplifies keyMappingList[] vector access. + const int keyMapNumber = action->objectName().toInt() - 1; std::string newMapping; - if (i == 0) { // no key mapping + // No key mapping? + if (keyMapNumber <= -1) { newMapping = ""; } else { - --i; - if (i < (int)kml.size()) { - newMapping = kml[i].getName(); - } + if (keyMapNumber < static_cast(keyMappingList.size())) + newMapping = keyMappingList[keyMapNumber].getName(); } + // Set the key mapping. + // ??? Only the name is used? Then we need to disallow key mappings + // with empty names. BankEditorDialog currently allows this. m_device->setKeyMappingForProgram(*program, newMapping); -// QString pixmapDir = KGlobal::dirs()->findResource("appdata", "pixmaps/"); - QIcon icon; - bool haveKeyMappings = (m_device->getKeyMappings().size() > 0); //@@@ JAS restored from before port/ - QToolButton *btn = getKeyMapButton(m_currentMenuProgram); + // Update the key mapping icon. - if (newMapping.empty()) { - icon = IconLoader::load( "key-white" ); - if( ! icon.isNull() ) { - btn->setIcon( icon ); - } - // QToolTip::remove(btn); - btn->setToolTip(QString("")); //@@@ Usefull ? + bool haveKeyMappings = (m_device->getKeyMappings().size() > 0); + QToolButton *keyMapButton = getKeyMapButton(m_keyMapProgramNumber); + + // selected? + if (keyMapNumber == -1) { + keyMapButton->setIcon(getNoKeyMapIcon()); + keyMapButton->setToolTip(""); } else { - icon = IconLoader::load( "key-green" ); - if( ! icon.isNull() ){ - btn->setIcon( icon ); - } - btn->setToolTip(tr("Key Mapping: %1").arg(strtoqstr(newMapping))); + keyMapButton->setIcon(getKeyMapIcon()); + keyMapButton->setToolTip(tr("Key Mapping: %1").arg(strtoqstr(newMapping))); } - btn->setEnabled(haveKeyMappings); + keyMapButton->setEnabled(haveKeyMappings); } int MidiProgramsEditor::ensureUniqueMSB(int msb, bool ascending) { + // ??? Not sure we should clean this up since we are getting rid of it. + bool percussion = false; // Doesn't matter int newMSB = msb; while (banklistContains(MidiBank(percussion, @@ -606,6 +629,8 @@ MidiProgramsEditor::ensureUniqueMSB(int msb, bool ascending) int MidiProgramsEditor::ensureUniqueLSB(int lsb, bool ascending) { + // ??? Not sure we should clean this up since we are getting rid of it. + bool percussion = false; // Doesn't matter int newLSB = lsb; while (banklistContains(MidiBank(percussion, @@ -627,54 +652,48 @@ bool MidiProgramsEditor::banklistContains(const MidiBank &bank) { // For each bank - for (BankList::iterator it = m_bankList.begin(); - it != m_bankList.end(); - ++it) + for (const MidiBank ¤tBank : m_bankList) { // Just compare the MSB/LSB. - if (it->getMSB() == bank.getMSB() && it->getLSB() == bank.getLSB()) + if (currentBank.getMSB() == bank.getMSB() && + currentBank.getLSB() == bank.getLSB()) return true; } return false; } -MidiProgram* +MidiProgram * MidiProgramsEditor::getProgram(const MidiBank &bank, int programNo) { - ProgramList::iterator it = m_programList.begin(); - - for (; it != m_programList.end(); ++it) { - if (it->getBank().partialCompare(bank) && - it->getProgram() == programNo) { - - //Only show hits to avoid overflow of console. - RG_DEBUG << "it->getBank() " << "== bank"; - return &(*it); + for (MidiProgram &midiProgram : m_programList) { + // Match? + if (midiProgram.getBank().getMSB() == bank.getMSB() && + midiProgram.getBank().getLSB() == bank.getLSB() && + midiProgram.getProgram() == programNo) { + return &midiProgram; } } return nullptr; } -void -MidiProgramsEditor::setBankName(const QString& s) -{ - setTitle(s); -} - -void MidiProgramsEditor::blockAllSignals(bool block) +ProgramList::iterator +MidiProgramsEditor::getProgramIter(const MidiBank &bank, int programNo) { - QList allChildren = - findChildren((QRegularExpression)"[0-9]+"); - QList::iterator it; - - for (it = allChildren.begin(); it != allChildren.end(); ++it) { - (*it)->blockSignals(block); + // For each program in m_programList... + for (ProgramList::iterator programIter = m_programList.begin(); + programIter != m_programList.end(); + ++programIter) { + // Match? + if (programIter->getBank().getMSB() == bank.getMSB() && + programIter->getBank().getLSB() == bank.getLSB() && + programIter->getProgram() == programNo) + return programIter; } - m_msb->blockSignals(block); - m_lsb->blockSignals(block); + return m_programList.end(); } + } diff --git a/src/gui/studio/MidiProgramsEditor.h b/src/gui/studio/MidiProgramsEditor.h index db8501bd0..d0d1fe852 100644 --- a/src/gui/studio/MidiProgramsEditor.h +++ b/src/gui/studio/MidiProgramsEditor.h @@ -1,4 +1,3 @@ - /* -*- c-basic-offset: 4 indent-tabs-mode: nil -*- vi:set ts=8 sts=4 sw=4: */ /* @@ -19,22 +18,20 @@ #ifndef RG_MIDIPROGRAMSEDITOR_H #define RG_MIDIPROGRAMSEDITOR_H -#include "base/MidiProgram.h" +#include "base/MidiProgram.h" // BankList, ProgramList #include "NameSetEditor.h" - class QWidget; class QString; class QSpinBox; class QTreeWidgetItem; class QCheckBox; -class BankList; namespace Rosegarden { -class MidiProgram; + class MidiDevice; class BankEditorDialog; @@ -42,70 +39,93 @@ class BankEditorDialog; class MidiProgramsEditor : public NameSetEditor { Q_OBJECT + public: + MidiProgramsEditor(BankEditorDialog *bankEditor, QWidget *parent); + /// Switch to the cleared and disabled state. + /** + * Called at the end of populateDeviceEditors() if no valid + * bank or program is selected in the tree. + */ void clearAll(); - void populate(QTreeWidgetItem*); + + /// Show the programs for the selected bank. + void populate(QTreeWidgetItem *); + + /// Limited undo in response to the Reset button in the lower left. + /** + * This appears to be a very limited undo for just the MSB/LSB, + * and the percussion setting. + */ void reset(); -public slots: +private slots: - // Check that any new MSB/LSB combination is unique for this device - // + /// Check that any new MSB/LSB combination is unique for this device. void slotNewMSB(int value); void slotNewLSB(int value); - void slotNewPercussion(); // gets value from checkbox + void slotPercussionClicked(); + /// One of the program names was changed by the user. void slotNameChanged(const QString &) override; void slotKeyMapButtonPressed() override; - void slotKeyMapMenuItemSelected(QAction *); - void slotKeyMapMenuItemSelected(int); + void slotKeyMapMenuItemSelected(QAction *action); -protected: +private: - MidiBank* getCurrentBank(); + MidiDevice *m_device{nullptr}; - int ensureUniqueMSB(int msb, bool ascending); - int ensureUniqueLSB(int lsb, bool ascending); + // Widgets - // Does the banklist contain this combination already? + QCheckBox *m_percussion; + QSpinBox *m_msb; + QSpinBox *m_lsb; + + // Banks + + /// The BankList we are editing. + /** + * We use this to check for dupes and make changes to the banks. + */ + BankList &m_bankList; + // Does m_bankList contain this combination already? // Disregard percussion bool, we care only about msb / lsb // in these situations. - // bool banklistContains(const MidiBank &); + int ensureUniqueMSB(int msb, bool ascending); + int ensureUniqueLSB(int lsb, bool ascending); - ProgramList getBankSubset(const MidiBank &); + /// The bank we are editing right now. + MidiBank *m_currentBank{nullptr}; + /// Used by reset() to restore the original MSB/LSB/Percussion. + MidiBank m_oldBank{false, 0, 0}; + + // Programs + /// Programs for the device being edited. + ProgramList &m_programList; + /// Get a pointer into the program list. + MidiProgram *getProgram(const MidiBank &bank, int programNo); + /// Get an iterator which is more flexible. + ProgramList::iterator getProgramIter(const MidiBank &bank, int programNo); + /// Get a ProgramList containing only the programs in a bank. + ProgramList getBankSubset(const MidiBank &); /// Set the currently loaded programs to new MSB and LSB void modifyCurrentPrograms(const MidiBank &oldBank, const MidiBank &newBank); - // Get a program (pointer into program list) for modification - // - MidiProgram* getProgram(const MidiBank &bank, int programNo); - - void setBankName(const QString& s); - - virtual QWidget *makeAdditionalWidget(QWidget *parent); - - void blockAllSignals(bool block); - - //--------------- Data members --------------------------------- - QCheckBox *m_percussion; - QSpinBox *m_msb; - QSpinBox *m_lsb; - - MidiDevice *m_device; - - MidiBank *m_currentBank; - BankList &m_bankList; - ProgramList &m_programList; - - MidiBank m_oldBank; - - unsigned int m_currentMenuProgram; + /// For assigning keymaps to programs. + /** + * Holds the index of the keymap button that was pressed. Set by + * slotKeyMapButtonPressed(). + * + * Used by slotKeyMapMenuItemSelected() to make sure the keymap + * selection ends up associated with the right program. + */ + unsigned int m_keyMapProgramNumber; }; diff --git a/src/gui/studio/NameSetEditor.cpp b/src/gui/studio/NameSetEditor.cpp index fa6346829..6585a1b19 100644 --- a/src/gui/studio/NameSetEditor.cpp +++ b/src/gui/studio/NameSetEditor.cpp @@ -51,7 +51,6 @@ NameSetEditor::NameSetEditor(BankEditorDialog *bankEditor, m_librarian(nullptr), m_librarianEmail(nullptr), m_names(), - m_completions(), m_numberingBaseButton(nullptr), m_numberingBase(1), m_labels(), @@ -155,11 +154,10 @@ NameSetEditor::NameSetEditor(BankEditorDialog *bankEditor, // sure why. lineEdit->setObjectName(QString("Line Edit %1").arg(index)); lineEdit->setProperty("index", index); - lineEdit->setCompleter(new QCompleter(m_completions)); m_names.push_back(lineEdit); connect(m_names[index], - &QLineEdit::textChanged, + &QLineEdit::textEdited, this, &NameSetEditor::slotNameChanged); rowLayout->addWidget(lineEdit, 1); diff --git a/src/gui/studio/NameSetEditor.h b/src/gui/studio/NameSetEditor.h index 3c508cffc..caa4253c6 100644 --- a/src/gui/studio/NameSetEditor.h +++ b/src/gui/studio/NameSetEditor.h @@ -85,6 +85,8 @@ public slots: QWidget *parent, bool showKeyMapButtons = false); + // ??? Key mappings are only used by one deriver (MidiProgramsEditor). + // Push this down to there. QToolButton *getKeyMapButton(int n) { return m_keyMapButtons[n]; } const QToolButton *getKeyMapButton(int n) const { return m_keyMapButtons[n]; } @@ -111,7 +113,6 @@ public slots: QLabel *m_librarianEmail; std::vector m_names; - QStringList m_completions; private: QPushButton *m_numberingBaseButton;