Skip to content

Add Playhead and Timeline to Pattern Editor #7794

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

Merged
merged 14 commits into from
Jul 2, 2025

Conversation

regulus79
Copy link
Contributor

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

  • Added a TimeLineWidget to PatternEditor, 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)
  • Added a new icon, step_btn_highlight, to overlay the steps which are currently being played.
  • Added a getter function, 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.
  • I was not able to neatly solve the issue of reordering the clip views when the clips get reordered (when pattern tracks are moved around in the song editor), so currently I loop over all of them to update them. It's not great, but it works.
  • Added a getter function 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 via MidiClip::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 to TimeLineWidget::positionChanged, which works, but this may have to be reworked when #7454 is completed.

@regulus79
Copy link
Contributor Author

regulus79 commented Mar 19, 2025

Edit: Fixed. I decided to add a new signal to PatternStore which is triggered by the midi clips when the time signature changes, which in turn is connected to update the pattern editor timeline stuff.

Okay--I'm having a bit of an issue updating the timeline length when the time signature changes, since the midi clips respond to the Song::timeSignatureChanged signal after the pattern editor, so I can't get the updated clip lengths. I could potentially recompute them right there, or I could retrigger MidiClip::changeTimeSignature(), or perhaps something else. I may have to think about this.

@Rossmaxx Rossmaxx linked an issue Mar 19, 2025 that may be closed by this pull request
@regulus79
Copy link
Contributor Author

I have decided to make the code simpler by using the clip length instead of the clip steps. This means I do not need to edit MidiClip.h

@regulus79 regulus79 mentioned this pull request Mar 23, 2025
@regulus79
Copy link
Contributor Author

Do you guys want me to add some kind of timeline playhead for normal midi clips/sample clips/automation clips in the pattern editor?

@regulus79
Copy link
Contributor Author

regulus79 commented Jun 1, 2025

Do you guys want me to add the stop behavior and loop buttons?

@bratpeki
Copy link
Member

bratpeki commented Jun 3, 2025

Just noticed an issue with BB clips where you can make them too big by duplicating the contents, leading to freezing. Opening an appropriate issue. The PR seems great, though! No looping, keep it behaving like how it behaves now, to not make it too large.

@bratpeki
Copy link
Member

bratpeki commented Jun 3, 2025

I'd love some more eyes on this, maybe @szeli1 has time?

@bratpeki bratpeki requested a review from szeli1 June 3, 2025 21:45
@messmerd messmerd self-requested a review June 4, 2025 05:12
@bratpeki bratpeki self-assigned this Jun 4, 2025
@bratpeki
Copy link
Member

This is still great.


We should address the loop points issue, though.

Getting rid of the playhead is not an option, as it removes the ability to arbitrarily choose where to place the playhead, which is also a great addition.

The other option is removing the looping behavior, which introduces a special case to the playhead behavior in the code. Yikes.


There's also how the cells light up but don't play what's inside if your playhead is halfway through the cell:

2025-06-27.00-13-13.mp4

This is standard LMMS behavior, it happens for notes as well, so there's no point in sweating over it, it's actually preferred as it keeps consistency. I just wanted to point it out.


In any case, follow up PR material. LGTM and merge.

regulus79 and others added 2 commits July 2, 2025 15:51
Co-authored-by: Dalton Messmer <messmer.dalton@gmail.com>
Copy link
Member

@messmerd messmerd left a comment

Choose a reason for hiding this comment

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

Haven't tested it, but it looks fine to me code-wise

@regulus79 regulus79 merged commit bbdfeff into LMMS:master Jul 2, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Playhead for the b&b editor
5 participants