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.

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

Lock pattern editor #1482

Conversation

theGreatWhiteShark
Copy link
Contributor

Implements the highly anticipated (#1409, #1458, #575, #717) locking of the PatternEditor to always show the current pattern playing while in song mode. In case there are multiple patterns in one column, it will show the one newly recorded notes will be inserted to.

At first I was a quite skeptical to deselect the pattern (no pattern shown at all) whenever the transport position passes an empty column. But it grew on my very quickly. Now, recording in Hydrogen is much more intuitive. There is no pattern in a column? It's quite evident that played notes won't be recorded now. Same thing in case of multiple patterns being located in the same row and the bottom-most is not the largest one.

I also introduced a border highlighting to the pattern list in the SongEditor when right-clicked. Then I wanted to have the same behavior in the Instrument list of the PatternEditor, was puzzled why the alternateRowColors were not picked up in both the Song and PatternEditor... well, one thing lead to another, things look a little bit different now.

songEditor_tweak

@cme what do you think about the enlarged grid cells in the SongEditor? I found them to fit quite natural when introducing the alternate row color (which can be turned off via the preferences). But I have no strong opinion regarding this and it's easy to revert to the former squares.

P.S. still a draft since it requires #1435 to keep the PatternEditor always in sync.
P.P.S. it requires a refreshing/importing of the default color theme to look like above

Full changes:

  • a new button for locking the PatternEditor to show the Pattern recorded notes will be inserted into while in Song mode was introduced
  • the draw and select buttons have been fused into a single one
  • some wrapper functions in the Hydrogen class have been introduced guard against segfaults in case a button is pressed while no song is set yet. Well, just in case.
  • if in song mode and the patternEditor locked button was checked in the SongEditorPanel, the PatternEditor does now follow the pattern new notes will be introduced into. (Note that this is not perfectly working right now. After merging the reworked AudioEngine it will be so much easier and cleaner to implement and thus patched later)
  • the GUI was made more resilent against no pattern being selected (happens whenever there is an empty column in the SongEditor while the PatternEditor is locked)
  • buttons of SongEditorPanel have been resized. Those for moving patterns up and down are now located on top of each other which allowed to enlarge the all others in order to make their icons more pronounced.
  • right-clicking a row in the SongEditorPatternList does now highlight it with a border like other widgets. This highlight persists until the associated dialog window is closed.
  • highlighting a list item when right-clicking is now supported and behaves the same in both the pattern list of the SongEditor and the instrument list of the PatternEditor
  • both pattern list and instrument list are now drawn within Qt instead of rendering a shipped .png file. In addition, they give optical feedback when hovered
  • background color, alternate row color, and selected row color are now properly supported in both PatternEditor and SongEditor. It felt quite natural to rework the design of the pattern grid in the SongEditor when having alternating colors. But it is a quick fix to change it back to the old design
  • reworking the default colors for Song and PatternEditor
  • button can now be toggled in both song and pattern mode (in pattern mode there is no change in behavior depending on its state. So, it does no harm)
  • in pattern mode the locked button is unchecked while in song mode it is checked (grey background vs. blue one). This hints that the locking only takes effect in song mode
  • when left clicking on a pattern name while the PatternEditor is locked, the lock button flashes red (changes its background for a short amount of time). This should hint why nothing did happened when clicking.
  • highlight the selected instrument in PatternEditor if no pattern is selected. The grid will remain disabled. This way it is more obvious we deal with a no pattern.
  • display transport position in PatternEditor even if no pattern is selected in case the PatternEditor is locked
  • fix boundary colors and mark fill area in PatternEditorRuler stretching further out than the current pattern in a more greyish color (there was a different color used previously too. But it was so similar to the actual one I never took notice of this)
  • instrument name label in InstrumentEditor, Timeline grid lines, SongEditorPositionRuler grid lines and numbers are now highlighted when hovered. (I know that there is a "too much" in highlighting and I might already crossed the line with the SongEditorPositionRuler. I try to get a feel while using it. But for some elements in Hydrogen I did not expect to find them interactive at the beginning.)

- a new button for locking the PatternEditor to show the Pattern recorded notes will be inserted into while in Song mode was introduced
- the draw and select buttons have been fused into a single one
- the buttons to move a pattern up and down and to add a new one in the SongEditorPanel have been shrinked a couple of pixels to allow for the three ones to the right to be bigger and more pronounced
- some wrapper functions in the Hydrogen class have been introduced guard against segfaults in case a button is pressed while no song is set yet. Well, just in case.
- if in song mode and the patternEditor locked button was checked in the SongEditorPanel, the PatternEditor does now follow the pattern new notes will be introduced into. (Note that this is not perfectly working right now. After merging the reworked AudioEngine it will be so much easier and cleaner to implement and thus patched later)
- the GUI was made more resilent against no pattern being selected (happens whenever there is an empty column in the SongEditor while the PatternEditor is locked)
- buttons of SongEditorPanel have been resized. Those for moving patterns up and down are now located on top of each other which allowed to enlarge the all others in order to make their icons more pronounced.
- right-clicking a row in the SongEditorPatternList does now highlight it with a border like other widgets. This highlight persists until the associated dialog window is closed.
- highlighting a list item when right-clicking is now supported and behaves the same in both the pattern list of the SongEditor and the instrument list of the PatternEditor
- both pattern list and instrument list are now drawn within Qt instead of rendering a shipped .png file. In addition, they give optical feedback when hovered
- background color, alternate row color, and selected row color are now properly supported in both PatternEditor and SongEditor. It felt quite natural to rework the design of the pattern grid in the SongEditor when having alternating colors. But it is a quick fix to change it back to the old design
- reworking the default colors for Song and PatternEditor
- button can now be toggled in both song and pattern mode (in pattern mode there is no change in behavior depending on its state. So, it does no harm)
- in pattern mode the locked button is unchecked while in song mode it is checked (grey background vs. blue one). This hints that the locking only takes effect in song mode
- when left clicking on a pattern name while the PatternEditor is locked, the lock button flashes red (changes its background for a short amount of time). This should hint why nothing did happened when clicking.
- highlight the selected instrument in PatternEditor if no pattern is selected. The grid will remain disabled. This way it is more obvious we deal with a no pattern.
- display transport position in PatternEditor even if no pattern is selected in case the PatternEditor is locked
- fix boundary colors and mark fill area in PatternEditorRuler stretching further out than the current pattern in a more greyish color (there was a different color used previously too. But it was so similar to the actual one I never took notice of this)
- instrument name label in InstrumentEditor, Timeline grid lines, SongEditorPositionRuler grid lines and numbers are now highlighted when hovered. (I know that there is a "too much" in highlighting and I might already crossed the line with the SongEditorPositionRuler. I try to get a feel while using it. But for some elements in Hydrogen I did not expect to find them interactive at the beginning.)
@theGreatWhiteShark theGreatWhiteShark marked this pull request as draft February 11, 2022 21:09
@cme
Copy link
Contributor

cme commented Feb 12, 2022

I like the look of the alternating row colours. But... just checking, you saw this right?

(I can't tell if there's interaction or anything because it's a bit hard to see from the code due to the starting branch included in the PR)

@theGreatWhiteShark
Copy link
Contributor Author

you saw #1437 right?

Yes, but no. I missed the part of the option added to the preferences and thought it's about an automated scrolling in case of multiple patterns being selected in one row and some of them are not in view anymore. 😬

Okay. I like your approach of just switching the selected pattern once the playback is started more. I was a little bit too restrictive when forbidding to change patterns at all once the new option was enabled. That leaves a couple of features to trade off where our implementations differ

  1. If multiple patterns are present in one row, which one to pick? The one requiring the least amount of scrolling or the bottom-most one? I like the latter more since it helps a lot during recording.
  2. Should the pattern be unset (set to nullptr in PatternEditor) if there is no pattern selected in a grid? I slightly tend to yes since better indicates that no notes will be recorded. On the other hand, this feels more like something "for the sake of completeness" than a handy feature.
  3. Should the option be toggled from within the Preferences or the SongEditorPanel? No idea. I can't tell yet how often I would toggle it. Have to use it first. Intuitively I would say I just turn it on once and the preferences are a better place to store it than the individual song.

(I can't tell if there's interaction or anything because it's a bit hard to see from the code due to the starting branch included in the PR)

Yeah. I'm starting to get sloppy and mix different things in PRs lately. But I plan to stop tweaking the GUI once v1.2.0-beta is released.

@cme
Copy link
Contributor

cme commented Feb 14, 2022

Sorry we ended up duplicating work :(

My thought was mostly to try and make it do the "right thing", or at least something unsurprising and useful. I realise now though that the 'right thing' might be different between Recording and playback modes for questions 1 and 2. Your preferred answers feel right for recording!

@theGreatWhiteShark
Copy link
Contributor Author

My thought was mostly to try and make it do the "right thing", or at least something unsurprising and useful. I realise now though that the 'right thing' might be different between Recording and playback modes for questions 1 and 2. Your preferred answers feel right for recording!

Yeah. But I would suggest we just pick one way to handle it and do not provide an additional choice for the user to switch betwenn playback or recording behavior or doing so automatically. That would probably unnecessary complex.

@theGreatWhiteShark theGreatWhiteShark merged commit c768ac0 into hydrogen-music:master Mar 28, 2022
@theGreatWhiteShark
Copy link
Contributor Author

Hmm? When got this one merged? Seems like I messed up some git stuff because I wanted to pick just all the GUI-related changes in #1544. I'll fix it in a new PR

@theGreatWhiteShark theGreatWhiteShark deleted the phil-lock-pattern-editor branch April 11, 2022 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants