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

fix #23609: Add max fermata time stretch up to 10000%. #24050

Merged

Conversation

chilkaditya
Copy link
Contributor

@chilkaditya chilkaditya commented Aug 15, 2024

Resolves: #23609

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

@cbjeukendrup
Copy link
Contributor

  • I signed the CLA

What is your username on MuseScore.org?

@chilkaditya
Copy link
Contributor Author

@cbjeukendrup "chilkasidehus" this is my username.

@@ -53,7 +53,7 @@ ExpandableBlank {

step: 1
decimals: 0
maxValue: 400
maxValue: 10000
minValue: 0
Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Aug 15, 2024

Choose a reason for hiding this comment

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

While at it we may want to reduce minValue to 1, as that's the lowest value that has an effect.
See #23609 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please correct me if I am wrong , so 0% has the same effect as 100% and for 1% to 99 % is kind of first forward or skip the note, so if we set min value to 100% then, Is this solution is feasible?

Copy link
Contributor

@Jojo-Schmitz Jojo-Schmitz Aug 15, 2024

Choose a reason for hiding this comment

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

1-99 shortens the duration, 0 and 100 leave the normal duration, >100 lengthens the duration

Copy link
Contributor

Choose a reason for hiding this comment

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

and IMHO 0 doing nothing (or rather the same as 100) seems a bug, disallowing 0 is one way to fix it
Then again 0 might be seen as disabling the fermata effect, if so it isn't a bug.
@cbjeukendrup

Copy link
Contributor

Choose a reason for hiding this comment

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

It might make the most sense to set the min to 100 (which does nothing), I'm not sure what the use-case of a fermata producing faster playback is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also an approach... But I learned that the duration of a fermata is defined by the community conductor and by the conductor alone, and as such might be shorter

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should i set min value to 100% as of now?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes please!

@zacjansheski
Copy link
Contributor

Tested on MacOS 14, Windows 11, Ubuntu 22.04.3. Approved
#23609 FIXED

@cbjeukendrup cbjeukendrup merged commit 37366e1 into musescore:master Aug 15, 2024
11 checks passed
@chilkaditya chilkaditya deleted the feat/add-max-fermata-time-stretch branch August 16, 2024 01:25
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.

Allow fermata time stretch greater than 400%
4 participants