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

#5339, #5340: Fix pasting notes from bb editor #5502

Closed
wants to merge 5 commits into from
Closed

#5339, #5340: Fix pasting notes from bb editor #5502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
913fe52
Fix pasting notes from the BB Editors' Piano Roll to different Piano …
thmueller64 Jul 18, 2020
434012e
Remove range adjustment when removing notes
thmueller64 Jul 18, 2020
6a62afb
Update src/tracks/Pattern.cpp
thmueller64 Aug 3, 2020
d4de43d
Update src/gui/editors/PianoRoll.cpp
thmueller64 Aug 3, 2020
cef300d
Revert changes to default Pattern as requested.
thmueller64 Aug 3, 2020
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
5 changes: 5 additions & 0 deletions include/InstrumentTrack.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,11 @@ class LMMS_EXPORT InstrumentTrack : public Track, public MidiEventProcessor
return m_instrument;
}

f_cnt_t envFrames()
{
return m_soundShaping.envFrames();
}

void deleteNotePluginData( NotePlayHandle * _n );

// name-stuff
Expand Down
1 change: 1 addition & 0 deletions include/Pattern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ protected slots:

void setType( PatternTypes _new_pattern_type );
void checkType();
void adjustSteps();

void resizeToFirstTrack();

Expand Down
9 changes: 9 additions & 0 deletions src/gui/editors/PianoRoll.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4126,6 +4126,15 @@ void PianoRoll::pasteNotes()
cur_note.restoreState( list.item( i ).toElement() );
cur_note.setPos( cur_note.pos() + Note::quantized( m_timeLine->pos(), quantization() ) );

if ((cur_note.length().getTicks() == -DefaultTicksPerBar) && (m_pattern->type() != Pattern::BeatPattern))
{
// the length of copied notes is set to the length of the instruments envelope
f_cnt_t envFrames = m_pattern->instrumentTrack()->envFrames();
// if notes have zero length due to rounding set the length to a small value
tick_t noteLength = static_cast<tick_t>(qMax(1.0f, ceil(envFrames / Engine::framesPerTick())));
cur_note.setLength(MidiTime(noteLength));
}

// select it
cur_note.setSelected( true );

Expand Down
8 changes: 7 additions & 1 deletion src/tracks/Pattern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ Note * Pattern::addNote( const Note & _new_note, const bool _quant_pos )
m_notes.insert(std::upper_bound(m_notes.begin(), m_notes.end(), new_note, Note::lessThan), new_note);
instrumentTrack()->unlock();

adjustSteps();
Copy link
Member

Choose a reason for hiding this comment

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

Could you explain why you update m_steps only in this case, not whenever the length changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When a user clicks on "Add steps" and creates a pattern on the added steps, if he then deselects all of the steps in the extended range for a moment, the range is shrunk immediately. I thought this would be a bit annoying, though it looks nicer when steps are moved inside the piano roll or removed and the beat pattern adapts. This would be a one line change essentialy.

Copy link
Contributor

Choose a reason for hiding this comment

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

I could be wrong, that's my first glance on the PR. But from what I read, on master m_steps is only changed by methods related to the BBTrack. When you have an MelodyPattern (adding notes through the piano roll), it seems that the length of the pattern changes but m_steps remains the same value. So when you delete the notes and it goes back to being a BeatPattern, the number of steps is the one you set earlier on the BBEditor. I didn't test yet but I think with this line you'd end up updating the number of steps as you add notes and once you go back to a BeatPattern you'd have the last value you had, not the one you set on the BBEditor. Changing that behavior is not really necessary and IMHO it makes sense to keep BeatPattern number of steps separated from the MelodyPattern length.

checkType();
updateLength();

Expand All @@ -244,14 +245,19 @@ void Pattern::removeNote( Note * _note_to_del )
++it;
}
instrumentTrack()->unlock();

checkType();
updateLength();

emit dataChanged();
}


void Pattern::adjustSteps()
{
m_steps = qMax(1, MidiTime(m_notes.last()->pos()).nextFullBar()) * MidiTime::stepsPerBar();
}


// returns a pointer to the note at specified step, or NULL if note doesn't exist

Note * Pattern::noteAtStep( int _step )
Expand Down