-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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 #105936 and move tempo selection to timesig page of New Wizard #3671
fix #105936 and move tempo selection to timesig page of New Wizard #3671
Conversation
The code looks good, but I didn't test, yet. |
ead1d01
to
501d36a
Compare
@lasconic That was NOT intentional; good catch. That was actually the result of a bigger problem; I wasn't following the form with which the other time elements had been constructed. For some reason the key signature page followed a different form than this page. I just amended my commit to address this. |
501d36a
to
e7d328c
Compare
mscore/newwizard.cpp
Outdated
setAccessibleName(title()); | ||
setAccessibleDescription(subTitle()); | ||
|
||
w = new TimesigWizard; | ||
QGridLayout* grid = new QGridLayout; | ||
grid->addWidget(w, 0, 0); | ||
|
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.
Also, please remove this code if it's not needed.
mscore/file.cpp
Outdated
@@ -725,10 +725,52 @@ MasterScore* MuseScore::getNewFile() | |||
delete nvb; | |||
} | |||
|
|||
if (newWizard->createTempo()) { | |||
double tempo = newWizard->tempo(); | |||
double tempo = 80; |
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.
Shouldn't this be 120?
<number>120</number> | ||
</property> | ||
</widget> | ||
</item> |
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.
Probably need to add a horizontal space here
<item>
<spacer name="horizontalSpacer">
<property name="orientation">
<enum>Qt::Horizontal</enum>
</property>
<property name="sizeHint" stdset="0">
<size>
<width>40</width>
<height>20</height>
</size>
</property>
</spacer>
</item>
e7d328c
to
6712db1
Compare
OK, the UI looks good now. Thorough MuseScore, BPM means quarter per minute (and not beat per minute), probably inherited from MIDI. |
@lasconic I agree that this PR does change the logic. I do think it's for the best. Your opinion matters more than mine, but I shall state it anyways: On one hand, it is good to be consistent with what Musescore defines as BPM. How about if I add an indication to the tempo selection in the wizard that shows "quarter note equals" or "dotted quarter equals" depending on the time signature? Would that be good? Edit: to clarify, for the proposed indication I mean the quarter note symbol and equals sign, not the literal text "quarter note equals" |
I'd rather have 120 quarter notes per minute staying the default, so recalculate the number for display? |
How about if I add an indication to the tempo selection in the wizard that shows "quarter note equals" or "dotted quarter equals" depending on the time signature? Would that be good? Unlike Jojo, I wouldn't mind if we keep 120 unit per minute. @Jojo-Schmitz why do you think it's better to keep 120 QPM ? |
I'd rather keep the established default of 120 quarter notes per minute, which for non x/4 meters might mean to have to show the recalculated values, so quarter equals 120 but dotted quarter equals 80, half note equals 60 etc. for it to show what the default is if tempo text is not enabled. I'd like to see the same in the tempo text palette |
Yes, I got that but why ? :) |
Because I'd like it to show the default, at least as long as it is disabled |
What is exactly is my job here? Have we decided on a course of action? |
Why? |
Resurrected in PR #5957 |
Fixes https://musescore.org/en/node/105936
And, as suggested in the issue, moves the tempo field in the New Wizard to the more logical page dealing with time signatures, as opposed to the page dealing with key signatures as it had been before.