Ease timeline zoom level restriction#1962
Merged
sapphire-arches merged 7 commits intoMay 20, 2024
Merged
Conversation
Contributor
m-m-adams
reviewed
May 19, 2024
m-m-adams
left a comment
Collaborator
There was a problem hiding this comment.
Small nit but lgtm. As mentioned on discord I'm good to merge as is (in community features) or to add a "catch" at the old max zoom level and just have the feature always on
trappar
commented
May 19, 2024
fd6a73a to
a91dcf8
Compare
a91dcf8 to
0d49459
Compare
sapphire-arches
left a comment
Collaborator
There was a problem hiding this comment.
2 minor nitpicks and this should get an entry in CHANGELOG.md, then lgtm
tastycode
pushed a commit
to tastycode/DelugeFirmware
that referenced
this pull request
Jul 3, 2024
* Add runtime option for unrestricting zoom out * Update documentation * Remove community feature in favor of short delay when crossing clip length zoom level * Implement changes based on PR feedback * Refine variable naming * Update changelog * Update comments for doxygen
|
This feature doesn't seem present in the 1.1 firmware. Is it gated behind a setting? |
Contributor
Author
No, this will be in 1.2. You can try nightly if you want to use it now. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the feature I suggested here: #314
What
The maximum zoom level has been increased. Now, the maximum zoom is the point the point where the entire sequence is represented by a single grid cell.
Why
Personally I never found this limitation to make sense. Why stop me from zooming out beyond my clip? If zoom only affected visibility, it would make sense... but zoom also affects entered note length. By allowing more freedom to zoom, you can access instant entry of note lengths that were previously not possible.
For example, this makes it super quick to enter basic chord progressions.
Given a brand new project, to create a 4 chord progression where each chord lasts for 1-bar:
Prevously the fastest way I know to do this would be:
Total inputs to make a 4-chord tri-tone chord progression with both methods:
Notes
As a community feature?
I don't think this needs to be a community feature for the following two reasonsL
What even is "max zoom" anymore?
This PR degrades the ability to understand what clip's
getMaxZoommeans. The clip's max zoom isn't actually the timeline's max zoom anymore.Ideally I would have liked to have updated the actual behavior of
Clip::getMaxZoom()insrc/deluge/model/clip/clip.cpp, but that function is used in many more places than the timeline.