Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pianoroll: nudge/snap while dragging notes #5848

Merged
merged 12 commits into from
Mar 5, 2021
+134 −46
Merged

pianoroll: nudge/snap while dragging notes #5848

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
0a782b7
pianoroll: nudge/snap while dragging notes
rghvdberg Dec 13, 2020
740197f
snap note start or note end to nearest grid line
rghvdberg Dec 14, 2020
627add5
fix member variable name
rghvdberg Dec 20, 2020
f194f19
change default width of pianoroll
rghvdberg Jan 10, 2021
e826493
remove trailing white space
rghvdberg Jan 10, 2021
d946222
use mouse position instead of m_draggednote
rghvdberg Jan 31, 2021
d27d047
Uses enum instead of text for GridMode selection
IanCaio Feb 4, 2021
b09da77
Fixes last code style issues
IanCaio Feb 4, 2021
e17cce8
Removes forgotten comma on enum
IanCaio Feb 4, 2021
c0e3453
Rewrites some of the grid logic
IanCaio Feb 5, 2021
8c3c5fc
Make noteOffset an int instead of TimePos
IanCaio Feb 5, 2021
9cc8eb5
Merge branch 'master' into master
IanCaio Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Binary file added data/themes/default/gridmode.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
14 changes: 13 additions & 1 deletion include/PianoRoll.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,8 @@ protected slots:
void fitNoteLengths(bool fill);
void constrainNoteLengths(bool constrainMax);

void changeSnapMode();


signals:
void currentPatternChanged();
Expand Down Expand Up @@ -258,6 +260,13 @@ protected slots:
PR_BLACK_KEY
};

enum GridMode
{
gridNudge,
gridSnap
// gridFree
};

PositionLine * m_positionLine;

QVector<QString> m_nemStr; // gui names of each edit mode
Expand Down Expand Up @@ -302,7 +311,7 @@ protected slots:
int noteEditRight() const;
int noteEditLeft() const;

void dragNotes( int x, int y, bool alt, bool shift, bool ctrl );
void dragNotes(int x, int y, bool alt, bool shift, bool ctrl);

static const int cm_scrollAmtHoriz = 10;
static const int cm_scrollAmtVert = 1;
Expand All @@ -325,6 +334,7 @@ protected slots:
ComboBoxModel m_keyModel;
ComboBoxModel m_scaleModel;
ComboBoxModel m_chordModel;
ComboBoxModel m_snapModel;

static const QVector<double> m_zoomLevels;
static const QVector<double> m_zoomYLevels;
Expand All @@ -347,6 +357,7 @@ protected slots:
Note * m_currentNote;
Actions m_action;
NoteEditMode m_noteEditMode;
GridMode m_gridMode;

int m_selectStartTick;
int m_selectedTick;
Expand Down Expand Up @@ -528,6 +539,7 @@ private slots:
ComboBox * m_keyComboBox;
ComboBox * m_scaleComboBox;
ComboBox * m_chordComboBox;
ComboBox* m_snapComboBox;
QPushButton * m_clearGhostButton;

};
Expand Down
166 changes: 121 additions & 45 deletions src/gui/editors/PianoRoll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ typedef AutomationPattern::timeMap timeMap;


// some constants...
const int INITIAL_PIANOROLL_WIDTH = 860;
const int INITIAL_PIANOROLL_WIDTH = 970;
const int INITIAL_PIANOROLL_HEIGHT = 485;

const int SCROLLBAR_SIZE = 12;
Expand Down Expand Up @@ -438,6 +438,13 @@ PianoRoll::PianoRoll() :
connect( m_timeLine, SIGNAL( regionSelectedFromPixels( int, int ) ),
this, SLOT( selectRegionFromPixels( int, int ) ) );

// Set up snap model
m_snapModel.addItem(tr("Nudge"));
m_snapModel.addItem(tr("Snap"));
m_snapModel.setValue(0);
connect(&m_snapModel, SIGNAL(dataChanged()),
this, SLOT(changeSnapMode()));

m_stepRecorder.initialize();
}

Expand Down Expand Up @@ -1300,10 +1307,13 @@ void PianoRoll::keyPressEvent(QKeyEvent* ke)
if( m_action == ActionMoveNote ||
m_action == ActionResizeNote )
{
dragNotes( m_lastMouseX, m_lastMouseY,
ke->modifiers() & Qt::AltModifier,
ke->modifiers() & Qt::ShiftModifier,
ke->modifiers() & Qt::ControlModifier );
dragNotes(
m_lastMouseX,
m_lastMouseY,
ke->modifiers() & Qt::AltModifier,
ke->modifiers() & Qt::ShiftModifier,
ke->modifiers() & Qt::ControlModifier
Comment on lines +1313 to +1315
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we repeat this function call a few times, can we at least fix this? Just pass the event or modifiers in and test in the function for what we need.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not strongly for or against this change, but wouldn't that couple dragNotes more closely to (1) the Qt UI and (2) these specific modifiers? If for example the argument "alt" were renamed to "unquantized" (or inverted and renamed to "quantized") then the dragNotes function is independent of which shortcut is used to perform unquantized actions.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dragging notes is inherently a UI thing, not a core thing.

I do like the idea of changing the names from ctrl alt and shift, and I would rather just pass in a modifiers value to the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dragging notes is inherently a UI thing, not a core thing.

Sure, but at the moment this function contains a lot of logic that doesn't rely on the UI. The same quantization strategies are relevant regardless of how the move is input.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the renaming of the arguments and possibly compressing them in a single one seems to be the middle ground: We keep the method decoupled from the Qt event and make the modifiers more generic (allowing us to change them later).

Can I suggest that we do it separately though? As I mentioned on a previous comment, this method needs a bit of an overhaul. Just renaming the arguments would also require some thought because some modifiers are used for multiple actions depending on context. I think both the method refactoring and changing of arguments could be done in a single separate PR, they would both benefit on each other.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I suggest that we do it separately though?

Sure. 👍

);
}
}
ke->accept();
Expand Down Expand Up @@ -1356,10 +1366,13 @@ void PianoRoll::keyPressEvent(QKeyEvent* ke)
if( m_action == ActionMoveNote ||
m_action == ActionResizeNote )
{
dragNotes( m_lastMouseX, m_lastMouseY,
ke->modifiers() & Qt::AltModifier,
ke->modifiers() & Qt::ShiftModifier,
ke->modifiers() & Qt::ControlModifier );
dragNotes(
m_lastMouseX,
m_lastMouseY,
ke->modifiers() & Qt::AltModifier,
ke->modifiers() & Qt::ShiftModifier,
ke->modifiers() & Qt::ControlModifier
);
}

}
Expand Down Expand Up @@ -2387,10 +2400,13 @@ void PianoRoll::mouseMoveEvent( QMouseEvent * me )
pauseTestNotes();
}

dragNotes( me->x(), me->y(),
dragNotes(
me->x(),
me->y(),
me->modifiers() & Qt::AltModifier,
me->modifiers() & Qt::ShiftModifier,
me->modifiers() & Qt::ControlModifier );
me->modifiers() & Qt::ControlModifier
);

if( replay_note && m_action == ActionMoveNote && ! ( ( me->modifiers() & Qt::ShiftModifier ) && ! m_startedWithShift ) )
{
Expand Down Expand Up @@ -2745,7 +2761,7 @@ void PianoRoll::updateKnifePos(QMouseEvent* me)



void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
void PianoRoll::dragNotes(int x, int y, bool alt, bool shift, bool ctrl)
{
// dragging one or more notes around

Expand All @@ -2758,43 +2774,67 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
off_ticks -= m_mouseDownTick - m_currentPosition;
off_key -= m_mouseDownKey - m_startKey;

// get note-vector of current pattern
const NoteVector & notes = m_pattern->notes();

// if they're not holding alt, quantize the offset
if( ! alt )
if (m_action == ActionMoveNote)
{
off_ticks = floor( off_ticks / quantization() )
* quantization();
}
// Calculate the offset for either Nudge or Snap modes
int noteOffset = off_ticks;
if (m_gridMode == gridSnap && quantization () > 1)
{
// Get the mouse timeline absolute position
TimePos mousePos(m_currentNote->oldPos().getTicks() + off_ticks);

// make sure notes won't go outside boundary conditions
if( m_action == ActionMoveNote && ! ( shift && ! m_startedWithShift ) )
{
if( m_moveBoundaryLeft + off_ticks < 0 )
// We create a mousePos that is relative to the end of the note instead
// of the beginning. That's to see if we will snap the beginning or end
// of the note
TimePos mousePosEnd(mousePos);
mousePosEnd += m_currentNote->oldLength();

// Now we quantize the mouse position to snap it to the grid
TimePos mousePosQ = mousePos.quantize(static_cast<float>(quantization()) / DefaultTicksPerBar);
TimePos mousePosEndQ = mousePosEnd.quantize(static_cast<float>(quantization()) / DefaultTicksPerBar);

bool snapEnd = abs(mousePosEndQ - mousePosEnd) < abs(mousePosQ - mousePos);

// Set the offset
noteOffset = snapEnd
? mousePosEndQ.getTicks() - m_currentNote->oldPos().getTicks() - m_currentNote->oldLength().getTicks()
: mousePosQ.getTicks() - m_currentNote->oldPos().getTicks();
}
else if (m_gridMode == gridNudge)
{
off_ticks -= (off_ticks + m_moveBoundaryLeft);
// if they're not holding alt, quantize the offset
if (!alt)
{
noteOffset = floor(off_ticks / quantization()) * quantization();
}
}
if( m_moveBoundaryTop + off_key > NumKeys )

// Make sure notes won't go outside boundary conditions
if (m_moveBoundaryLeft + noteOffset < 0)
{
off_key -= NumKeys - (m_moveBoundaryTop + off_key);
noteOffset = -m_moveBoundaryLeft;
}
if( m_moveBoundaryBottom + off_key < 0 )
if (m_moveBoundaryTop + off_key >= NumKeys)
{
off_key -= (m_moveBoundaryBottom + off_key);
off_key = -m_moveBoundaryTop + NumKeys - 1;
}
if (m_moveBoundaryBottom + off_key < 0)
{
off_key = -m_moveBoundaryBottom;
}
}


// get note-vector of current pattern
const NoteVector & notes = m_pattern->notes();

if (m_action == ActionMoveNote)
{
// Apply offset to all selected notes
for (Note *note : getSelectedNotes())
{
if (shift && ! m_startedWithShift)
// Quick resize is only enabled on Nudge mode, since resizing the note
// while in Snap mode breaks the calculation of the note offset
if (shift && ! m_startedWithShift && m_gridMode == gridNudge)
{
// quick resize, toggled by holding shift after starting a note move, but not before
int ticks_new = note->oldLength().getTicks() + off_ticks;
int ticks_new = note->oldLength().getTicks() + noteOffset;
if( ticks_new <= 0 )
{
ticks_new = 1;
Expand All @@ -2805,17 +2845,13 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
else
{
// moving note
int pos_ticks = note->oldPos().getTicks() + off_ticks;
int key_num = note->oldKey() + off_key;

// ticks can't be negative
pos_ticks = qMax(0, pos_ticks);
// upper/lower bound checks on key_num
key_num = qMax(0, key_num);
key_num = qMin(key_num, NumKeys);
// Final position of the note
TimePos posTicks(note->oldPos().getTicks() + noteOffset);
int key_num = note->oldKey() + off_key;

note->setPos( TimePos( pos_ticks ) );
note->setKey( key_num );
note->setPos(posTicks);
note->setKey(key_num);
}
}
}
Expand All @@ -2827,6 +2863,12 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
// If shift + ctrl then we also rearrange all posterior notes (sticky)
// If shift is pressed but only one note is selected, apply sticky

// Quantize the resizing if alt is not pressed
if (!alt)
{
off_ticks = floor(off_ticks / quantization()) * quantization();
}

auto selectedNotes = getSelectedNotes();

if (shift)
Expand Down Expand Up @@ -2914,6 +2956,16 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
// shift is not pressed; stretch length of selected notes but not their position
int minLength = alt ? 1 : m_minResizeLen.getTicks();

if (m_gridMode == gridSnap)
{
// Calculate the end point of the note being dragged
TimePos oldEndPoint = m_currentNote->oldPos() + m_currentNote->oldLength();
// Quantize that position
TimePos quantizedEndPoint = Note::quantized(oldEndPoint, quantization());
// Add that difference to the offset from the resize
off_ticks += quantizedEndPoint - oldEndPoint;
}

for (Note *note : selectedNotes)
{
int newLength = qMax(minLength, note->oldLength() + off_ticks);
Expand Down Expand Up @@ -4611,8 +4663,14 @@ Note * PianoRoll::noteUnderMouse()
return NULL;
}

void PianoRoll::changeSnapMode()
{
// gridNudge,
// gridSnap,
// gridFree - to be implemented


m_gridMode = static_cast<GridMode>(m_snapModel.value());
}

PianoRollWindow::PianoRollWindow() :
Editor(true, true),
Expand Down Expand Up @@ -4783,6 +4841,15 @@ PianoRollWindow::PianoRollWindow() :
m_chordComboBox->setFixedSize( 105, ComboBox::DEFAULT_HEIGHT );
m_chordComboBox->setToolTip( tr( "Chord" ) );

// setup snap-stuff
QLabel* snapLbl = new QLabel(m_toolBar);
snapLbl->setPixmap(embed::getIconPixmap("gridmode"));

m_snapComboBox = new ComboBox(m_toolBar);
m_snapComboBox->setModel(&m_editor->m_snapModel);
m_snapComboBox->setFixedSize(105, ComboBox::DEFAULT_HEIGHT);
m_snapComboBox->setToolTip(tr("Snap mode"));

// -- Clear ghost pattern button
m_clearGhostButton = new QPushButton( m_toolBar );
m_clearGhostButton->setIcon( embed::getIconPixmap( "clear_ghost_note" ) );
Expand Down Expand Up @@ -4850,6 +4917,15 @@ PianoRollWindow::PianoRollWindow() :
zoomAndNotesToolBar->addSeparator();
zoomAndNotesToolBar->addWidget( m_clearGhostButton );

QWidget* snapWidget = new QWidget();
QHBoxLayout* snapHbox = new QHBoxLayout();
snapHbox->setContentsMargins(0, 0, 0, 0);
snapHbox->addWidget(snapLbl);
snapHbox->addWidget(m_snapComboBox);
snapWidget->setLayout(snapHbox);
zoomAndNotesToolBar->addSeparator();
zoomAndNotesToolBar->addWidget(snapWidget);

// setup our actual window
setFocusPolicy( Qt::StrongFocus );
setFocus();
Expand Down