Skip to content

pianoroll: nudge/snap while dragging notes #5848

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

Merged
IanCaio merged 12 commits into from
Mar 5, 2021
Merged

pianoroll: nudge/snap while dragging notes #5848

Show file tree
Hide file tree
Changes from 9 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 @@ -210,6 +210,8 @@ protected slots:
void clearGhostPattern();
void glueNotes();

void changeSnapMode();


signals:
void currentPatternChanged();
Expand Down Expand Up @@ -253,6 +255,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 @@ -294,7 +303,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 @@ -316,6 +325,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 @@ -338,6 +348,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 @@ -513,6 +524,7 @@ private slots:
ComboBox * m_keyComboBox;
ComboBox * m_scaleComboBox;
ComboBox * m_chordComboBox;
ComboBox* m_snapComboBox;
QPushButton * m_clearGhostButton;

};
Expand Down
117 changes: 99 additions & 18 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 @@ -432,6 +432,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 @@ -1182,10 +1189,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

@Veratil Veratil Feb 6, 2021

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

@Spekular Spekular Feb 14, 2021

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

@Veratil Veratil Feb 14, 2021

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

@Spekular Spekular Feb 15, 2021

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

@IanCaio IanCaio Feb 15, 2021

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

@Veratil Veratil Feb 18, 2021

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 @@ -1238,10 +1248,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 @@ -2195,10 +2208,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 @@ -2535,7 +2551,7 @@ void PianoRoll::mouseMoveEvent( 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 @@ -2550,10 +2566,9 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )


// if they're not holding alt, quantize the offset
if( ! alt )
if (!alt)
{
off_ticks = floor( off_ticks / quantization() )
* quantization();
off_ticks = floor(off_ticks / quantization()) * quantization();
}

// make sure notes won't go outside boundary conditions
Expand Down Expand Up @@ -2595,6 +2610,38 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
else
{
// moving note
if (m_gridMode == gridSnap && quantization() > 1)
{
// Get the relative X position of mouse inside PianoRoll viewport
int viewportMouseX = x - m_whiteKeyWidth;
// Convert it to a TimePos
TimePos mousePos(viewportMouseX * TimePos::ticksPerBar() / m_ppb + m_currentPosition);
// Calculate the initial offset when we clicked on the note in Ticks
int initialOffset = (m_moveStartX - m_whiteKeyWidth) * TimePos::ticksPerBar() / m_ppb
+ m_mouseDownTick - m_currentNote->oldPos().getTicks();
// Remove that initial offset to the mouse X position for a more
// fluid movement
mousePos -= initialOffset;

// 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);

// Overwrite the offset we had in ticks with the distance between
// the note we are moving and the calculated position from the mouse
off_ticks = snapEnd
? mousePosEndQ.getTicks() - m_currentNote->oldPos().getTicks() - m_currentNote->oldLength().getTicks()
: mousePosQ.getTicks() - m_currentNote->oldPos().getTicks();
}

int pos_ticks = note->oldPos().getTicks() + off_ticks;
int key_num = note->oldKey() + off_key;

Expand All @@ -2604,8 +2651,8 @@ void PianoRoll::dragNotes( int x, int y, bool alt, bool shift, bool ctrl )
key_num = qMax(0, key_num);
key_num = qMin(key_num, NumKeys);

note->setPos( TimePos( pos_ticks ) );
note->setKey( key_num );
note->setPos(TimePos(pos_ticks));
note->setKey(key_num);
}
}
}
Expand Down Expand Up @@ -2704,6 +2751,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 @@ -4360,8 +4417,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 @@ -4510,6 +4573,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 @@ -4577,6 +4649,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