-
-
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
Changed bar lines to follow snap size #7034
Conversation
Good job. Nice to have this feature implemented but it would help if you could change the defaults too acc to #6420. IMO, 1/16 should be the default snapping with a bit of zoom in. That's what most daws follow these days. |
Alright, I'll do that. |
The coarse grid width, fine grid width, emboss width, and horizontal line width are all configurable now as QSS properties, as well as the offset of the emboss. The default themes have been updated accordingly. This does break backwards compatibility with old 1.2 themes, but I assumed that that was going to happen sooner or later with 1.3. |
Requesting review from @michaelgregorius |
Hi @BoredGuy1, thanks for your PR! I really like that the Song Editor now defaults to a more zoomed in view! I'd like to propose to use a default quantization of 1/4 for the Song Editor though because it makes it look less "busy". It would also coincide with the tempo which is BPM, i.e. 1/4 notes per minute. Heres a visual comparison of the two options: Using 1/4 is also the default in REAPER to compare against another DAW: |
Hey @michaelgregorius, thanks for the review. And sorry for the late response (been busy). I agree, default 1/4 definitely looks cleaner than 1/16. That should be an easy change. I think I'll be able to add that (along with all the requested changes) in the coming week. |
OK, everything should be good now. |
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.
Just one comment on coding convention.
Ready for review again. |
Still looks good to me. @DomClark, @zonkmachine and @Rossmaxx: Is it also good for you? There only seem to be resolved comments now. |
Yeah it's fine with me. Tbh 1/16 was too cluttered to begin with. Thanks to Michael to suggest the 1/4 snapping. I didn't know that 1/4 was supposed to be better. LGTM |
Pull request #7271 fixes some performance problems that got introduced by this pull request due to missing initialization of some variables. |
* Added lines in between bars * Changed bar lines to follow snap size * Changed default zoom and quantization value * Added constants for line widths * Added QSS configuration for new grid line colors * Tied line widths to QSS properties * Changed default quantization to 1/4 * Removed clear() from destructor model * Removed destructor in ComboBoxModel.h * Changed member set/get functions to pass by value * Updated signal connection with newer syntax
Overview
Vertical lines now appear or disappear depending on quantization value.
Vertical lines between bars are 1 pixel wide with no emboss. Vertical lines at the end of bars are now 2 pixels wide with 1 pixel emboss to make them stand out. (MIN_PIXELS_PER_BAR has been increased from 2 to 4 to compensate.)
To account for small quantization values, vertical lines between bars will automatically disappear as the user zooms out. (Vertical lines at the end of bars will not disappear.)
Related Threads
#5111, #6420
Screenshots
2 bar quantization (big lines appear every other bar)
1/2 bar quantization (big lines appear every bar, one small line appears within each bar)
1/16 bar quantization (big lines appear every bar, 15 small lines appear within each bar)