From ebc606aaf2d3cf1483189d1d56e2354396cf92f4 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 13:09:23 -0700 Subject: [PATCH 01/12] Move vol/pan s_textFloat handling code into separate functions; this avoids some code duplication & makes the mouseevent/wheelevent functions slightly smaller --- include/PianoRoll.h | 5 ++ src/gui/editors/PianoRoll.cpp | 87 +++++++++++++++++++++-------------- 2 files changed, 57 insertions(+), 35 deletions(-) diff --git a/include/PianoRoll.h b/include/PianoRoll.h index 15b2e4ffc20..eb70976fa26 100644 --- a/include/PianoRoll.h +++ b/include/PianoRoll.h @@ -69,6 +69,11 @@ class PianoRoll : public QWidget /*! \brief Resets settings to default when e.g. creating a new project */ void reset(); + // functions to display the hover-text labeling a note's volume/panning + void showTextFloat(const QString &text, const QPoint &pos, int timeout=-1); + void showVolTextFloat(volume_t vol, const QPoint &pos, int timeout=-1); + void showPanTextFloat(panning_t pan, const QPoint &pos, int timeout=-1); + void setCurrentPattern( Pattern* newPattern ); inline void stopRecording() diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 9496f358169..caff5c801a5 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -431,6 +431,48 @@ void PianoRoll::reset() m_lastNotePanning = DefaultPanning; } +void PianoRoll::showTextFloat(const QString &text, const QPoint &pos, int timeout) +{ + s_textFloat->setText( text ); + // show the float, offset slightly so as to not obscure anything + s_textFloat->moveGlobal( this, pos + QPoint(4, 16) ); + if (timeout == -1) + { + s_textFloat->show(); + } + else + { + s_textFloat->setVisibilityTimeOut( timeout ); + } +} + + +void PianoRoll::showVolTextFloat(volume_t vol, const QPoint &pos, int timeout) +{ + //! \todo display velocity for MIDI-based instruments + // possibly dBV values too? not sure if it makes sense for note volumes... + showTextFloat( tr("Volume: %1%").arg( vol ), pos, timeout ); +} + + +void PianoRoll::showPanTextFloat(panning_t pan, const QPoint &pos, int timeout) +{ + QString text; + if( pan < 0 ) + { + text = tr("Panning: %1% left").arg( qAbs( pan ) ); + } + else if( pan > 0 ) + { + text = tr("Panning: %1% right").arg( qAbs( pan ) ); + } + else + { + text = tr("Panning: center"); + } + showTextFloat( text, pos, timeout ); +} + void PianoRoll::changeNoteEditMode( int i ) @@ -2060,25 +2102,12 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) if( m_noteEditMode == NoteEditVolume ) { m_lastNoteVolume = vol; - //! \todo display velocity for MIDI-based instruments - // possibly dBV values too? not sure if it makes sense for note volumes... - s_textFloat->setText( tr("Volume: %1%").arg( vol ) ); + showVolTextFloat( vol, me->pos() ); } else if( m_noteEditMode == NoteEditPanning ) { m_lastNotePanning = pan; - if( pan < 0 ) - { - s_textFloat->setText( tr("Panning: %1% left").arg( qAbs( pan ) ) ); - } - else if( pan > 0 ) - { - s_textFloat->setText( tr("Panning: %1% right").arg( qAbs( pan ) ) ); - } - else - { - s_textFloat->setText( tr("Panning: center") ); - } + showPanTextFloat( pan, me->pos() ); } // When alt is pressed we only edit the note under the cursor @@ -2127,9 +2156,6 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) // Emit pattern has changed m_pattern->dataChanged(); - // Show the new volume value - s_textFloat->moveGlobal( this, QPoint( me->x() + 4, me->y() + 16 ) ); - s_textFloat->show(); } else if( me->buttons() == Qt::NoButton && m_editMode == ModeDraw ) @@ -3246,7 +3272,11 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) volume_t vol = tLimit( n->getVolume() + step, MinVolume, MaxVolume ); n->setVolume( vol ); } - s_textFloat->setText( tr("Volume: %1%").arg( nv[0]->getVolume() ) ); + if (nv.size() == 1) + { + // show the volume hover-text only if editing a single note + showVolTextFloat( nv[0]->getVolume(), we->pos(), 1000 ); + } } else if( m_noteEditMode == NoteEditPanning ) { @@ -3255,24 +3285,11 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) panning_t pan = tLimit( n->getPanning() + step, PanningLeft, PanningRight ); n->setPanning( pan ); } - panning_t pan = nv[0]->getPanning(); - if( pan < 0 ) + if (nv.size() == 1) { - s_textFloat->setText( tr("Panning: %1% left").arg( qAbs( pan ) ) ); + // show the pan hover-text only if editing a single note + showPanTextFloat( nv[0]->getPanning(), we->pos(), 1000 ); } - else if( pan > 0 ) - { - s_textFloat->setText( tr("Panning: %1% right").arg( qAbs( pan ) ) ); - } - else - { - s_textFloat->setText( tr("Panning: center") ); - } - } - if( nv.size() == 1 ) - { - s_textFloat->moveGlobal( this, QPoint( we->x() + 4, we->y() + 16 ) ); - s_textFloat->setVisibilityTimeOut( 1000 ); } update(); } From a8fa9b75a1ee4d8ac173a8a6ad70233d3fac981e Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 13:12:08 -0700 Subject: [PATCH 02/12] Simplify conditional by initializing vol/pan to defaults --- src/gui/editors/PianoRoll.cpp | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index caff5c801a5..af7e35161dc 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -2077,8 +2077,8 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) // determine what volume/panning to set note to // if middle-click, set to defaults - volume_t vol; - panning_t pan; + volume_t vol = DefaultVolume; + panning_t pan = DefaultPanning; if( me->buttons() & Qt::LeftButton ) { @@ -2093,11 +2093,6 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) ( (float)( PanningRight - PanningLeft ) ), PanningLeft, PanningRight); } - else - { - vol = DefaultVolume; - pan = DefaultPanning; - } if( m_noteEditMode == NoteEditVolume ) { From f568f7b4a4d920202a01c747d40e27cc3607fd54 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 13:14:38 -0700 Subject: [PATCH 03/12] Formatting/tabbing fixes --- src/gui/editors/PianoRoll.cpp | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index af7e35161dc..6a7c75d2903 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -2200,45 +2200,42 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) { if( QApplication::overrideCursor() ) { - if( QApplication::overrideCursor()->shape() != Qt::SizeHorCursor ) + if( QApplication::overrideCursor()->shape() != Qt::SizeHorCursor ) { - while( QApplication::overrideCursor() != NULL ) - { - QApplication::restoreOverrideCursor(); - } + while( QApplication::overrideCursor() != NULL ) + { + QApplication::restoreOverrideCursor(); + } - QCursor c( Qt::SizeHorCursor ); - QApplication::setOverrideCursor( c ); + QCursor c( Qt::SizeHorCursor ); + QApplication::setOverrideCursor( c ); } } else { QCursor c( Qt::SizeHorCursor ); - QApplication::setOverrideCursor( - c ); + QApplication::setOverrideCursor( c ); } } else { if( QApplication::overrideCursor() ) { - if( QApplication::overrideCursor()->shape() != Qt::SizeAllCursor ) + if( QApplication::overrideCursor()->shape() != Qt::SizeAllCursor ) { - while( QApplication::overrideCursor() != NULL ) - { - QApplication::restoreOverrideCursor(); - } + while( QApplication::overrideCursor() != NULL ) + { + QApplication::restoreOverrideCursor(); + } - QCursor c( Qt::SizeAllCursor ); - QApplication::setOverrideCursor( - c ); + QCursor c( Qt::SizeAllCursor ); + QApplication::setOverrideCursor( c ); } } else { QCursor c( Qt::SizeAllCursor ); - QApplication::setOverrideCursor( - c ); + QApplication::setOverrideCursor( c ); } } } From a32368cc3ad0aa617b2f07c924cc8d3df8a8a7d4 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 13:27:32 -0700 Subject: [PATCH 04/12] Display pan/volume on wheel event if all edited notes have same pan/volume; previous behavior was to display the value only if one note was being edited --- src/gui/editors/PianoRoll.cpp | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 6a7c75d2903..5767012fe04 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -3264,9 +3264,15 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) volume_t vol = tLimit( n->getVolume() + step, MinVolume, MaxVolume ); n->setVolume( vol ); } - if (nv.size() == 1) + bool allVolumesEqual = std::all_of( nv.begin(), nv.end(), + [nv](const Note *note) + { + return note->getVolume() == nv[0]->getVolume(); + }); + if ( allVolumesEqual ) { - // show the volume hover-text only if editing a single note + // show the volume hover-text only if all notes have the + // same volume showVolTextFloat( nv[0]->getVolume(), we->pos(), 1000 ); } } @@ -3277,9 +3283,15 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) panning_t pan = tLimit( n->getPanning() + step, PanningLeft, PanningRight ); n->setPanning( pan ); } - if (nv.size() == 1) + bool allPansEqual = std::all_of( nv.begin(), nv.end(), + [nv](const Note *note) + { + return note->getPanning() == nv[0]->getPanning(); + }); + if ( allPansEqual ) { - // show the pan hover-text only if editing a single note + // show the pan hover-text only if all notes have the same + // panning showPanTextFloat( nv[0]->getPanning(), we->pos(), 1000 ); } } From 8504633103f665cb69d6732b110d5c8cb40a6c6c Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 13:54:53 -0700 Subject: [PATCH 05/12] Remove duplicated scrolling code from keyPressEvent() --- src/gui/editors/PianoRoll.cpp | 193 +++++++++++----------------------- 1 file changed, 62 insertions(+), 131 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 5767012fe04..86593606805 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -966,155 +966,86 @@ void PianoRoll::keyPressEvent(QKeyEvent* ke ) switch( ke->key() ) { case Qt::Key_Up: - if( ( ke->modifiers() & Qt::ControlModifier ) && m_action == ActionNone ) - { - // shift selection up an octave - // if nothing selected, shift _everything_ - shiftSemiTone( +12 ); - } - else if((ke->modifiers() & Qt::ShiftModifier) && m_action == ActionNone) - { - // Move selected notes up by one semitone - shiftSemiTone( 1 ); - } - else - { - // scroll - m_topBottomScroll->setValue( - m_topBottomScroll->value() - - cm_scrollAmtVert ); - - // if they are moving notes around or resizing, - // recalculate the note/resize position - if( m_action == ActionMoveNote || - m_action == ActionResizeNote ) - { - dragNotes( m_lastMouseX, m_lastMouseY, - ke->modifiers() & Qt::AltModifier, - ke->modifiers() & Qt::ShiftModifier, - ke->modifiers() & Qt::ControlModifier ); - } - } - ke->accept(); - break; - case Qt::Key_Down: - if( ke->modifiers() & Qt::ControlModifier && m_action == ActionNone ) - { - // shift selection down an octave - // if nothing selected, shift _everything_ - shiftSemiTone( -12 ); - } - else if((ke->modifiers() & Qt::ShiftModifier) && m_action == ActionNone) { - // Move selected notes down by one semitone - shiftSemiTone( -1 ); - } - else - { - // scroll - m_topBottomScroll->setValue( - m_topBottomScroll->value() + - cm_scrollAmtVert ); - - // if they are moving notes around or resizing, - // recalculate the note/resize position - if( m_action == ActionMoveNote || - m_action == ActionResizeNote ) + int direction = (ke->key() == Qt::Key_Up ? +1 : -1); + if( ( ke->modifiers() & Qt::ControlModifier ) && m_action == ActionNone ) { - dragNotes( m_lastMouseX, m_lastMouseY, - ke->modifiers() & Qt::AltModifier, - ke->modifiers() & Qt::ShiftModifier, - ke->modifiers() & Qt::ControlModifier ); + // shift selection up an octave + // if nothing selected, shift _everything_ + shiftSemiTone( 12 * direction ); } - } - ke->accept(); - break; - - case Qt::Key_Left: - if( ke->modifiers() & Qt::ControlModifier && m_action == ActionNone ) - { - // Move selected notes by one bar to the left - shiftPos( - MidiTime::ticksPerTact() ); - } - else if( ke->modifiers() & Qt::ShiftModifier && m_action == ActionNone) - { - // move notes - bool quantized = ! ( ke->modifiers() & Qt::AltModifier ); - int amt = quantized ? quantization() : 1; - shiftPos( -amt ); - } - else if( ke->modifiers() & Qt::AltModifier) - { - Pattern * p = m_pattern->previousPattern(); - if(p != NULL) + else if((ke->modifiers() & Qt::ShiftModifier) && m_action == ActionNone) { - setCurrentPattern(p); + // Move selected notes up by one semitone + shiftSemiTone( 1 * direction ); } - } - else - { - // scroll - m_leftRightScroll->setValue( - m_leftRightScroll->value() - - cm_scrollAmtHoriz ); - - // if they are moving notes around or resizing, - // recalculate the note/resize position - if( m_action == ActionMoveNote || - m_action == ActionResizeNote ) + else { - dragNotes( m_lastMouseX, m_lastMouseY, - ke->modifiers() & Qt::AltModifier, - ke->modifiers() & Qt::ShiftModifier, - ke->modifiers() & Qt::ControlModifier ); + // scroll + m_topBottomScroll->setValue( m_topBottomScroll->value() - + cm_scrollAmtVert * direction ); + + // if they are moving notes around or resizing, + // recalculate the note/resize position + if( m_action == ActionMoveNote || + m_action == ActionResizeNote ) + { + dragNotes( m_lastMouseX, m_lastMouseY, + ke->modifiers() & Qt::AltModifier, + ke->modifiers() & Qt::ShiftModifier, + ke->modifiers() & Qt::ControlModifier ); + } } - + ke->accept(); + break; } - ke->accept(); - break; case Qt::Key_Right: - if( ke->modifiers() & Qt::ControlModifier && m_action == ActionNone) - { - // Move selected notes by one bar to the right - shiftPos( MidiTime::ticksPerTact() ); - } - else if( ke->modifiers() & Qt::ShiftModifier && m_action == ActionNone) + case Qt::Key_Left: { - // move notes - bool quantized = !( ke->modifiers() & Qt::AltModifier ); - int amt = quantized ? quantization() : 1; - shiftPos( +amt ); - } - else if( ke->modifiers() & Qt::AltModifier) { - Pattern * p = m_pattern->nextPattern(); - if(p != NULL) + int direction = (ke->key() == Qt::Key_Right ? +1 : -1); + if( ke->modifiers() & Qt::ControlModifier && m_action == ActionNone ) { - setCurrentPattern(p); + // Move selected notes by one bar to the left + shiftPos( direction * MidiTime::ticksPerTact() ); } - } - else - { - // scroll - m_leftRightScroll->setValue( - m_leftRightScroll->value() + - cm_scrollAmtHoriz ); - - // if they are moving notes around or resizing, - // recalculate the note/resize position - if( m_action == ActionMoveNote || - m_action == ActionResizeNote ) + else if( ke->modifiers() & Qt::ShiftModifier && m_action == ActionNone) + { + // move notes + bool quantized = ! ( ke->modifiers() & Qt::AltModifier ); + int amt = quantized ? quantization() : 1; + shiftPos( direction * amt ); + } + else if( ke->modifiers() & Qt::AltModifier) { - dragNotes( m_lastMouseX, m_lastMouseY, - ke->modifiers() & Qt::AltModifier, - ke->modifiers() & Qt::ShiftModifier, - ke->modifiers() & Qt::ControlModifier ); + Pattern * p = m_pattern->previousPattern(); + if(p != NULL) + { + setCurrentPattern(p); + } } + else + { + // scroll + m_leftRightScroll->setValue( m_leftRightScroll->value() + + direction * cm_scrollAmtHoriz ); + + // if they are moving notes around or resizing, + // recalculate the note/resize position + if( m_action == ActionMoveNote || + m_action == ActionResizeNote ) + { + dragNotes( m_lastMouseX, m_lastMouseY, + ke->modifiers() & Qt::AltModifier, + ke->modifiers() & Qt::ShiftModifier, + ke->modifiers() & Qt::ControlModifier ); + } + } + ke->accept(); + break; } - ke->accept(); - break; case Qt::Key_A: if( ke->modifiers() & Qt::ControlModifier ) From 68da3ad64fe1849c0fb03f2b9095fafbf9df0668 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 14:40:36 -0700 Subject: [PATCH 06/12] Remove duplicated code in note move/resize cursor calculation; also breaks apart a lengthy conditional into calculationss that are easier to understand. --- src/gui/editors/PianoRoll.cpp | 52 +++++++++-------------------------- 1 file changed, 13 insertions(+), 39 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 86593606805..867e79c345e 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -2120,54 +2120,28 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me ) if( it != notes.begin()-1 ) { Note *note = *it; + // x coordinate of the right edge of the note + int noteRightX = ( note->pos() + note->length() - + m_currentPosition) * m_ppt/MidiTime::ticksPerTact(); // cursor at the "tail" of the note? - if( note->length() > 0 && - pos_ticks*m_ppt / - MidiTime::ticksPerTact() > - ( note->pos() + - note->length() )*m_ppt/ - MidiTime::ticksPerTact()- - RESIZE_AREA_WIDTH ) + bool atTail = note->length() > 0 && x > noteRightX - + RESIZE_AREA_WIDTH; + Qt::CursorShape cursorShape = atTail ? Qt::SizeHorCursor : + Qt::SizeAllCursor; + if( QApplication::overrideCursor() ) { - if( QApplication::overrideCursor() ) + if( QApplication::overrideCursor()->shape() != cursorShape ) { - if( QApplication::overrideCursor()->shape() != Qt::SizeHorCursor ) + while( QApplication::overrideCursor() != NULL ) { - while( QApplication::overrideCursor() != NULL ) - { - QApplication::restoreOverrideCursor(); - } - - QCursor c( Qt::SizeHorCursor ); - QApplication::setOverrideCursor( c ); + QApplication::restoreOverrideCursor(); } - } - else - { - QCursor c( Qt::SizeHorCursor ); - QApplication::setOverrideCursor( c ); + QApplication::setOverrideCursor(QCursor(cursorShape)); } } else { - if( QApplication::overrideCursor() ) - { - if( QApplication::overrideCursor()->shape() != Qt::SizeAllCursor ) - { - while( QApplication::overrideCursor() != NULL ) - { - QApplication::restoreOverrideCursor(); - } - - QCursor c( Qt::SizeAllCursor ); - QApplication::setOverrideCursor( c ); - } - } - else - { - QCursor c( Qt::SizeAllCursor ); - QApplication::setOverrideCursor( c ); - } + QApplication::setOverrideCursor(QCursor(cursorShape)); } } else From 6a9e105c90a229dfdfad61deb8790f7fce069df9 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:04:46 -0700 Subject: [PATCH 07/12] Const-correctness fixes --- include/PianoRoll.h | 4 ++-- src/gui/editors/PianoRoll.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/PianoRoll.h b/include/PianoRoll.h index eb70976fa26..f874ec2f55c 100644 --- a/include/PianoRoll.h +++ b/include/PianoRoll.h @@ -126,7 +126,7 @@ class PianoRoll : public QWidget int getKey( int y ) const; static inline void drawNoteRect( QPainter & p, int x, int y, - int width, Note * n, const QColor & noteCol ); + int width, const Note * n, const QColor & noteCol ); void removeSelection(); void selectAll(); void getSelectedNotes( NoteVector & selected_notes ); @@ -326,7 +326,7 @@ protected slots: void copy_to_clipboard(const NoteVector & notes ) const; - void drawDetuningInfo( QPainter & _p, Note * _n, int _x, int _y ); + void drawDetuningInfo( QPainter & _p, const Note * _n, int _x, int _y ) const; bool mouseOverNote(); Note * noteUnderMouse(); diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 867e79c345e..9816a730541 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -702,7 +702,7 @@ void PianoRoll::setBarColor( const QColor & c ) inline void PianoRoll::drawNoteRect(QPainter & p, int x, int y, - int width, Note * n, const QColor & noteCol ) + int width, const Note * n, const QColor & noteCol ) { ++x; ++y; @@ -770,8 +770,8 @@ inline void PianoRoll::drawNoteRect(QPainter & p, int x, int y, -inline void PianoRoll::drawDetuningInfo( QPainter & _p, Note * _n, int _x, - int _y ) +inline void PianoRoll::drawDetuningInfo( QPainter & _p, const Note * _n, int _x, + int _y ) const { int middle_y = _y + KEY_LINE_HEIGHT / 2; _p.setPen( noteColor() ); From 2e111129bd7582ad56d0c980d3b0b10f5cf40b30 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:11:25 -0700 Subject: [PATCH 08/12] Remove unnecessary 'inline' attributes; 'inline' is only needed when a function is defined in the header, as a way to avoid multiple definitions error when linking. It is otherwise useless --- include/PianoRoll.h | 14 +++++++------- src/gui/editors/PianoRoll.cpp | 16 ++++++++-------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/PianoRoll.h b/include/PianoRoll.h index f874ec2f55c..e3a5fa69620 100644 --- a/include/PianoRoll.h +++ b/include/PianoRoll.h @@ -125,7 +125,7 @@ class PianoRoll : public QWidget virtual void focusOutEvent( QFocusEvent * ); int getKey( int y ) const; - static inline void drawNoteRect( QPainter & p, int x, int y, + static void drawNoteRect( QPainter & p, int x, int y, int width, const Note * n, const QColor & noteCol ); void removeSelection(); void selectAll(); @@ -230,12 +230,12 @@ protected slots: void testPlayKey( int _key, int _vol, int _pan ); void pauseTestNotes(bool pause = true ); - inline int noteEditTop() const; - inline int keyAreaBottom() const; - inline int noteEditBottom() const; - inline int keyAreaTop() const; - inline int noteEditRight() const; - inline int noteEditLeft() const; + int noteEditTop() const; + int keyAreaBottom() const; + int noteEditBottom() const; + int keyAreaTop() const; + int noteEditRight() const; + int noteEditLeft() const; void dragNotes( int x, int y, bool alt, bool shift, bool ctrl ); diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 9816a730541..3cdef8f74e8 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -701,7 +701,7 @@ void PianoRoll::setBarColor( const QColor & c ) -inline void PianoRoll::drawNoteRect(QPainter & p, int x, int y, +void PianoRoll::drawNoteRect(QPainter & p, int x, int y, int width, const Note * n, const QColor & noteCol ) { ++x; @@ -770,7 +770,7 @@ inline void PianoRoll::drawNoteRect(QPainter & p, int x, int y, -inline void PianoRoll::drawDetuningInfo( QPainter & _p, const Note * _n, int _x, +void PianoRoll::drawDetuningInfo( QPainter & _p, const Note * _n, int _x, int _y ) const { int middle_y = _y + KEY_LINE_HEIGHT / 2; @@ -1179,7 +1179,7 @@ void PianoRoll::leaveEvent(QEvent * e ) -inline int PianoRoll::noteEditTop() const +int PianoRoll::noteEditTop() const { return height() - PR_BOTTOM_MARGIN - m_notesEditHeight + NOTE_EDIT_RESIZE_BAR; @@ -1188,7 +1188,7 @@ inline int PianoRoll::noteEditTop() const -inline int PianoRoll::noteEditBottom() const +int PianoRoll::noteEditBottom() const { return height() - PR_BOTTOM_MARGIN; } @@ -1196,7 +1196,7 @@ inline int PianoRoll::noteEditBottom() const -inline int PianoRoll::noteEditRight() const +int PianoRoll::noteEditRight() const { return width() - PR_RIGHT_MARGIN; } @@ -1204,7 +1204,7 @@ inline int PianoRoll::noteEditRight() const -inline int PianoRoll::noteEditLeft() const +int PianoRoll::noteEditLeft() const { return WHITE_KEY_WIDTH; } @@ -1212,7 +1212,7 @@ inline int PianoRoll::noteEditLeft() const -inline int PianoRoll::keyAreaTop() const +int PianoRoll::keyAreaTop() const { return PR_TOP_MARGIN; } @@ -1220,7 +1220,7 @@ inline int PianoRoll::keyAreaTop() const -inline int PianoRoll::keyAreaBottom() const +int PianoRoll::keyAreaBottom() const { return height() - PR_BOTTOM_MARGIN - m_notesEditHeight; } From a17d915ccfeda17993ec19afa4bd6f97f34146e2 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:21:47 -0700 Subject: [PATCH 09/12] Prefer C++11 range-based for loops over explicit iterators or Qt's foreach; note: Qt's foreach actually duplicates the container before iterating, as well (not hugely problematic for performance since Qt containers are copy-on-write, but is still semantically misleading) --- src/gui/editors/PianoRoll.cpp | 130 +++++++++------------------------- 1 file changed, 33 insertions(+), 97 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 3cdef8f74e8..96cd55f3f91 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -834,13 +834,8 @@ void PianoRoll::clearSelectedNotes() { if( m_pattern != NULL ) { - // get note-vector of current pattern - const NoteVector & notes = m_pattern->notes(); - - // will be our iterator in the following loop - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) { - Note *note = *it; + for( Note *note : m_pattern->notes() ) + { note->setSelected( false ); } } @@ -852,11 +847,8 @@ void PianoRoll::clearSelectedNotes() void PianoRoll::shiftSemiTone( int amount ) // shift notes by amount semitones { bool useAllNotes = ! isSelection(); - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) + for( Note *note : m_pattern->notes() ) { - Note *note = *it; // if none are selected, move all notes, otherwise // only move selected notes if( useAllNotes || note->selected() ) @@ -876,13 +868,10 @@ void PianoRoll::shiftSemiTone( int amount ) // shift notes by amount semitones void PianoRoll::shiftPos( int amount ) //shift notes pos by amount { bool useAllNotes = ! isSelection(); - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; bool first = true; - for( it = notes.begin(); it != notes.end(); ++it ) + for( Note *note : m_pattern->notes() ) { - Note *note = *it; // if none are selected, move all notes, otherwise // only move selected notes if( note->selected() || (useAllNotes && note->length() > 0) ) @@ -914,11 +903,8 @@ void PianoRoll::shiftPos( int amount ) //shift notes pos by amount bool PianoRoll::isSelection() const // are any notes selected? { - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) + for( const Note *note : m_pattern->notes() ) { - Note *note = *it; if( note->selected() ) { return true; @@ -934,11 +920,8 @@ int PianoRoll::selectionCount() const // how many notes are selected? { int sum = 0; - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) + for( const Note *note : m_pattern->notes() ) { - Note *note = *it; if( note->selected() ) { ++sum; @@ -1634,14 +1617,10 @@ void PianoRoll::mouseDoubleClickEvent(QMouseEvent * me ) MidiTime::ticksPerTact() / m_ppt + m_currentPosition; const int ticks_middle = x * MidiTime::ticksPerTact() / m_ppt + m_currentPosition; - // get note-vector of current pattern - NoteVector notes; - notes += m_pattern->notes(); - // go through notes to figure out which one we want to change bool altPressed = me->modifiers() & Qt::AltModifier; NoteVector nv; - foreach( Note * i, notes ) + for ( Note * i : m_pattern->notes() ) { if( i->withinRange( ticks_start, ticks_end ) || ( i->selected() && !altPressed ) ) { @@ -1651,12 +1630,12 @@ void PianoRoll::mouseDoubleClickEvent(QMouseEvent * me ) // make sure we're on a note if( nv.size() > 0 ) { - Note * closest = NULL; + const Note * closest = NULL; int closest_dist = 9999999; // if we caught multiple notes, find the closest... if( nv.size() > 1 ) { - foreach( Note * i, nv ) + for ( const Note * i : nv ) { const int dist = qAbs( i->pos().getTicks() - ticks_middle ); if( dist < closest_dist ) { closest = i; closest_dist = dist; } @@ -1665,7 +1644,7 @@ void PianoRoll::mouseDoubleClickEvent(QMouseEvent * me ) NoteVector::Iterator it = nv.begin(); while( it != nv.end() ) { - Note *note = *it; + const Note *note = *it; if( note->pos().getTicks() != closest->pos().getTicks() ) { it = nv.erase( it ); @@ -1709,11 +1688,8 @@ void PianoRoll::testPlayNote( Note * n ) void PianoRoll::pauseTestNotes( bool pause ) { - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it = notes.begin(); - while( it != notes.end() ) + for (Note *note : m_pattern->notes()) { - Note *note = *it; if( note->isPlaying() ) { if( pause ) @@ -1728,8 +1704,6 @@ void PianoRoll::pauseTestNotes( bool pause ) testPlayNote( note ); } } - - ++it; } } @@ -1780,12 +1754,8 @@ void PianoRoll::computeSelectedNotes(bool shift) //int y_base = noteEditTop() - 1; if( hasValidPattern() ) { - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; - - for( it = notes.begin(); it != notes.end(); ++it ) + for( Note *note : m_pattern->notes() ) { - Note *note = *it; // make a new selection unless they're holding shift if( ! shift ) { @@ -1874,20 +1844,14 @@ void PianoRoll::mouseReleaseEvent( QMouseEvent * me ) if( hasValidPattern() ) { // turn off all notes that are playing - const NoteVector & notes = m_pattern->notes(); - - NoteVector::ConstIterator it = notes.begin(); - while( it != notes.end() ) + for ( Note *note : m_pattern->notes() ) { - Note *note = *it; if( note->isPlaying() ) { m_pattern->instrumentTrack()->pianoModel()-> handleKeyRelease( note->key() ); note->setIsPlaying( false ); } - - ++it; } // stop playing keys that we let go of @@ -2372,9 +2336,8 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl ) if (m_action == ActionMoveNote) { - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + for (Note *note : notes) { - Note *note = *it; if( note->selected() ) { if( ! ( shift && ! m_startedWithShift ) ) @@ -2422,19 +2385,17 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl ) // This factor is such that the endpoint of the note whose handle is being dragged should lie under the cursor. // first, determine the start-point of the left-most selected note: int stretchStartTick = -1; - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + for (const Note *note : notes) { - Note *note = *it; if (note->selected() && (stretchStartTick < 0 || note->oldPos().getTicks() < stretchStartTick)) { stretchStartTick = note->oldPos().getTicks(); } } // determine the ending tick of the right-most selected note - Note *posteriorNote = nullptr; - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + const Note *posteriorNote = nullptr; + for (const Note *note : notes) { - Note *note = *it; if (note->selected() && (posteriorNote == nullptr || note->oldPos().getTicks() + note->oldLength().getTicks() > posteriorNote->oldPos().getTicks() + posteriorNote->oldLength().getTicks())) @@ -2451,9 +2412,8 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl ) // process all selected notes & determine how much the endpoint of the right-most note was shifted int posteriorDeltaThisFrame = 0; - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + for (Note *note : notes) { - Note *note = *it; if(note->selected()) { // scale relative start and end positions by scaleFactor @@ -2490,9 +2450,8 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl ) if (ctrl || selectionCount() == 1) { // if holding ctrl or only one note is selected, reposition posterior notes - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + for (Note *note : notes) { - Note *note = *it; if (!note->selected() && note->pos().getTicks() >= posteriorEndTick) { int newStart = note->pos().getTicks() + posteriorDeltaThisFrame; @@ -2504,9 +2463,8 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl ) else { // shift is not pressed; stretch length of selected notes but not their position - for (NoteVector::ConstIterator it = notes.begin(); it != notes.end(); ++it ) + for (Note *note : notes) { - Note *note = *it; if (note->selected()) { int newLength = note->oldLength() + off_ticks; @@ -2904,17 +2862,13 @@ void PianoRoll::paintEvent(QPaintEvent * pe ) width() - WHITE_KEY_WIDTH, height() - PR_TOP_MARGIN ); - const NoteVector & notes = m_pattern->notes(); - const int visible_keys = ( keyAreaBottom()-keyAreaTop() ) / KEY_LINE_HEIGHT + 2; QPolygon editHandles; - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) + for( const Note *note : m_pattern->notes() ) { - Note *note = *it; int len_ticks = note->length(); if( len_ticks == 0 ) @@ -2994,7 +2948,7 @@ void PianoRoll::paintEvent(QPaintEvent * pe ) if( note->hasDetuningInfo() ) { - drawDetuningInfo( p, *it, + drawDetuningInfo( p, note, x + WHITE_KEY_WIDTH, y_base - key * KEY_LINE_HEIGHT ); } @@ -3144,15 +3098,11 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) int ticks_end = ( x + pixel_range / 2 ) * MidiTime::ticksPerTact() / m_ppt + m_currentPosition; - // get note-vector of current pattern - NoteVector notes; - notes += m_pattern->notes(); - // When alt is pressed we only edit the note under the cursor bool altPressed = we->modifiers() & Qt::AltModifier; // go through notes to figure out which one we want to change NoteVector nv; - foreach( Note * i, notes ) + for ( Note * i : m_pattern->notes() ) { if( i->withinRange( ticks_start, ticks_end ) || ( i->selected() && !altPressed ) ) { @@ -3164,7 +3114,7 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) const int step = we->delta() > 0 ? 1.0 : -1.0; if( m_noteEditMode == NoteEditVolume ) { - foreach( Note * n, nv ) + for ( Note * n : nv ) { volume_t vol = tLimit( n->getVolume() + step, MinVolume, MaxVolume ); n->setVolume( vol ); @@ -3183,7 +3133,7 @@ void PianoRoll::wheelEvent(QWheelEvent * we ) } else if( m_noteEditMode == NoteEditPanning ) { - foreach( Note * n, nv ) + for ( Note * n : nv ) { panning_t pan = tLimit( n->getPanning() + step, PanningLeft, PanningRight ); n->setPanning( pan ); @@ -3485,15 +3435,11 @@ void PianoRoll::selectAll() return; } - const NoteVector & notes = m_pattern->notes(); - // if first_time = true, we HAVE to set the vars for select bool first_time = true; - NoteVector::ConstIterator it; - for( it = notes.begin(); it != notes.end(); ++it ) + for( const Note *note : m_pattern->notes() ) { - Note *note = *it; int len_ticks = note->length(); if( len_ticks > 0 ) @@ -3545,12 +3491,8 @@ void PianoRoll::getSelectedNotes(NoteVector & selected_notes ) return; } - const NoteVector & notes = m_pattern->notes(); - NoteVector::ConstIterator it; - - for( it = notes.begin(); it != notes.end(); ++it ) + for( Note *note : m_pattern->notes() ) { - Note *note = *it; if( note->selected() ) { selected_notes.push_back( note ); @@ -3574,7 +3516,7 @@ void PianoRoll::enterValue( NoteVector* nv ) if( ok ) { - foreach( Note * n, *nv ) + for ( Note * n : *nv ) { n->setVolume( new_val ); } @@ -3593,7 +3535,7 @@ void PianoRoll::enterValue( NoteVector* nv ) if( ok ) { - foreach( Note * n, *nv ) + for ( Note * n : *nv ) { n->setPanning( new_val ); } @@ -3611,10 +3553,9 @@ void PianoRoll::copy_to_clipboard( const NoteVector & notes ) const dataFile.content().appendChild( note_list ); MidiTime start_pos( notes.front()->pos().getTact(), 0 ); - for( NoteVector::ConstIterator it = notes.begin(); it != notes.end(); - ++it ) + for( const Note *note : notes ) { - Note clip_note( **it ); + Note clip_note( *note ); clip_note.setPos( clip_note.pos( start_pos ) ); clip_note.saveState( dataFile, note_list ); } @@ -3657,11 +3598,9 @@ void PianoRoll::cutSelectedNotes() copy_to_clipboard( selected_notes ); Engine::getSong()->setModified(); - NoteVector::Iterator it; - for( it = selected_notes.begin(); it != selected_notes.end(); ++it ) + for( const Note *note : selected_notes ) { - Note *note = *it; // note (the memory of it) is also deleted by // pattern::removeNote(...) so we don't have to do that m_pattern->removeNote( note ); @@ -3910,9 +3849,6 @@ Note * PianoRoll::noteUnderMouse() { QPoint pos = mapFromGlobal( QCursor::pos() ); - // get note-vector of current pattern - const NoteVector & notes = m_pattern->notes(); - if( pos.x() <= WHITE_KEY_WIDTH || pos.x() > width() - SCROLLBAR_SIZE || pos.y() < PR_TOP_MARGIN @@ -3926,7 +3862,7 @@ Note * PianoRoll::noteUnderMouse() MidiTime::ticksPerTact() / m_ppt + m_currentPosition; // loop through whole note-vector... - for( Note* const& note : notes ) + for( Note* const& note : m_pattern->notes() ) { // and check whether the cursor is over an // existing note From ca7028933afc7cfadeff083e717e6318285b7fe3 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:24:38 -0700 Subject: [PATCH 10/12] Remove extraneous conditional --- src/gui/editors/PianoRoll.cpp | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 96cd55f3f91..9588978a1e1 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -584,27 +584,23 @@ void PianoRoll::setCurrentPattern( Pattern* newPattern ) m_leftRightScroll->setValue( 0 ); - const NoteVector & notes = m_pattern->notes(); + // determine the central key so that we can scroll to it int central_key = 0; - if( ! notes.empty() ) + int total_notes = 0; + for( const Note *note : m_pattern->notes() ) { - // determine the central key so that we can scroll to it - int total_notes = 0; - for( const Note* const& note : notes ) + if( note->length() > 0 ) { - if( note->length() > 0 ) - { - central_key += note->key(); - ++total_notes; - } + central_key += note->key(); + ++total_notes; } + } - if( total_notes > 0 ) - { - central_key = central_key / total_notes - - ( KeysPerOctave * NumOctaves - m_totalKeysToScroll ) / 2; - m_startKey = tLimit( central_key, 0, NumOctaves * KeysPerOctave ); - } + if( total_notes > 0 ) + { + central_key = central_key / total_notes - + ( KeysPerOctave * NumOctaves - m_totalKeysToScroll ) / 2; + m_startKey = tLimit( central_key, 0, NumOctaves * KeysPerOctave ); } // resizeEvent() does the rest for us (scrolling, range-checking From ff94f8b4ce220a04898dd129adb999565ed62178 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:27:27 -0700 Subject: [PATCH 11/12] Rename copy_to_clipboard -> copyToClipboard --- include/PianoRoll.h | 2 +- src/gui/editors/PianoRoll.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/include/PianoRoll.h b/include/PianoRoll.h index e3a5fa69620..eaff75fb84b 100644 --- a/include/PianoRoll.h +++ b/include/PianoRoll.h @@ -324,7 +324,7 @@ protected slots: TimeLineWidget * m_timeLine; bool m_scrollBack; - void copy_to_clipboard(const NoteVector & notes ) const; + void copyToClipboard(const NoteVector & notes ) const; void drawDetuningInfo( QPainter & _p, const Note * _n, int _x, int _y ) const; bool mouseOverNote(); diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 9588978a1e1..104bbc841b4 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -3542,7 +3542,7 @@ void PianoRoll::enterValue( NoteVector* nv ) } -void PianoRoll::copy_to_clipboard( const NoteVector & notes ) const +void PianoRoll::copyToClipboard( const NoteVector & notes ) const { DataFile dataFile( DataFile::ClipboardData ); QDomElement note_list = dataFile.createElement( "note-list" ); @@ -3572,7 +3572,7 @@ void PianoRoll::copySelectedNotes() if( ! selected_notes.empty() ) { - copy_to_clipboard( selected_notes ); + copyToClipboard( selected_notes ); } } @@ -3591,7 +3591,7 @@ void PianoRoll::cutSelectedNotes() if( ! selected_notes.empty() ) { - copy_to_clipboard( selected_notes ); + copyToClipboard( selected_notes ); Engine::getSong()->setModified(); From db18fa61fe3483191249e115ac58eba44950a916 Mon Sep 17 00:00:00 2001 From: Colin Wallace Date: Sun, 6 Sep 2015 15:33:13 -0700 Subject: [PATCH 12/12] have getSelectedNotes() return the selected notes, rather than appending them to a write-back parameter --- include/PianoRoll.h | 2 +- src/gui/editors/PianoRoll.cpp | 23 +++++++++++------------ 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/include/PianoRoll.h b/include/PianoRoll.h index eaff75fb84b..e4821ab01d2 100644 --- a/include/PianoRoll.h +++ b/include/PianoRoll.h @@ -129,7 +129,7 @@ class PianoRoll : public QWidget int width, const Note * n, const QColor & noteCol ); void removeSelection(); void selectAll(); - void getSelectedNotes( NoteVector & selected_notes ); + NoteVector getSelectedNotes(); // for entering values with dblclick in the vol/pan bars void enterValue( NoteVector* nv ); diff --git a/src/gui/editors/PianoRoll.cpp b/src/gui/editors/PianoRoll.cpp index 104bbc841b4..e74e3c60255 100644 --- a/src/gui/editors/PianoRoll.cpp +++ b/src/gui/editors/PianoRoll.cpp @@ -3480,20 +3480,21 @@ void PianoRoll::selectAll() // returns vector with pointers to all selected notes -void PianoRoll::getSelectedNotes(NoteVector & selected_notes ) +NoteVector PianoRoll::getSelectedNotes() { - if( ! hasValidPattern() ) - { - return; - } + NoteVector selectedNotes; - for( Note *note : m_pattern->notes() ) + if (hasValidPattern()) { - if( note->selected() ) + for( Note *note : m_pattern->notes() ) { - selected_notes.push_back( note ); + if( note->selected() ) + { + selectedNotes.push_back( note ); + } } } + return selectedNotes; } @@ -3567,8 +3568,7 @@ void PianoRoll::copyToClipboard( const NoteVector & notes ) const void PianoRoll::copySelectedNotes() { - NoteVector selected_notes; - getSelectedNotes( selected_notes ); + NoteVector selected_notes = getSelectedNotes(); if( ! selected_notes.empty() ) { @@ -3586,8 +3586,7 @@ void PianoRoll::cutSelectedNotes() return; } - NoteVector selected_notes; - getSelectedNotes( selected_notes ); + NoteVector selected_notes = getSelectedNotes(); if( ! selected_notes.empty() ) {