-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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.
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
Add Playhead and Timeline to Pattern Editor #7794
base: master
Are you sure you want to change the base?
Conversation
Edit: Fixed. I decided to add a new signal to
|
m_currentPosition, Song::PlayMode::Pattern, this | ||
); | ||
connect(m_timeLine, &TimeLineWidget::positionChanged, this, &PatternEditor::updatePosition); | ||
static_cast<QVBoxLayout*>( layout() )->insertWidget( 0, m_timeLine ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure why we need the static_cast
here?
static_cast<QVBoxLayout*>( layout() )->insertWidget( 0, m_timeLine ); | |
layout()->insertWidget(0, m_timeLine); |
src/gui/editors/PatternEditor.cpp
Outdated
// Currently there is no way to get the clipviews corresponding to the current pattern index, so | ||
// all of them get updated at once. It's not ideal but it works. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just add a new function to do that? I agree that what you have right now probably works though, but just wondering why.
Also, classes like TrackView
s should have a virtual
member function that does any updating if necessary, and then this function can simply call that function. This pattern of checking if something is a certain instrument track, sample track, midi clip, etc, and then doing some specific thing doesn't seem right because it indicates a lack of abstraction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not just add a new function to do that?
How would that function work? The pattern editor has access to the tracks which have access to their vector of Clip
s, and the index of each clip corresponds to the pattern it belongs to. You can get a Clip
from a ClipView
since it stores a pointer to it, but you can't get the ClipView
from Clip
, since I think we want the core and gui to be separate(?)
One idea I had was to store the ClipViews
in a separate vector in each TrackView
. Then, hopefully if they are added in the same order as the Clip
s, you can use the same pattern index to get the right ClipView
s. But, there is a problem because the order of the clips in their vector can change when the user drags the pattern tracks around. It might be possible to somehow get the clip view vector to also update when that happens, but it would probably be complicated.
Also, classes like TrackViews should have a virtual member function that does any updating if necessary, and then this function can simply call that function.
Yeah that's probably a good idea.
Co-authored-by: Sotonye Atemie <satemiej@gmail.com>
This PR adds a visual indicator to show the pattern editor playing, along with a timeline widget to allow the user to skip forward/backward in a pattern.
Addresses #1195
2025-03-18.17-00-04.mp4
I noticed that the timeline arrow position moves smoothly while the step highlighting moves in steps, which kind of looks weird. Are you guys okay with that, or do you have any ideas for a better solution?
Changes
TimeLineWidget
toPatternEditor
, and set it up in the constructor, along with some helper variables (the track head width, which was copied from the Song Editor, and the max number of steps in the clips to calculate the pixels per bar, which has its own function to update)step_btn_highlight
, to overlay the steps which are currently being played.TrackView::getClipViews()
to allow the Pattern Editor to access the midi clip views to update them when the playhead moves. Previously, afaik, once the clip views are created, they leave the scope and cannot be accessed again, which is a problem.MidiClip::steps()
to allow the pattern editor to know how long the clips are, and update the pixelsPerBar for the timeline accordingly (since we don't have scrolling yet; maybe this will change in the future). On second, this could probably be achieved viaMidiClip::length()
, but it probably doesn't hurt to handle it with steps. I can rework this if anyone wants.Note: the current signal/slot system for the pattern editor is very odd;
updatePosition
doesn't seem to be called when the playback position changes, which means that I had to make it be triggered by something else. I have decided to connect it toTimeLineWidget::positionChanged
, which works, but this may have to be reworked when #7454 is completed.