add "a Tempo" in the Tempo palette

[rtbo]

Aims to provide "a Tempo" text mark to reset the tempo after a gradual change. At the moment it is a simple tempo text that defaults to 120 BPM and can be edited to whatever the user wants.

It is already better than present situation, but I'd prefer the item to automatically pick-up the tempo prior to previous gradual change. Don't really know how to achieve this though.

• 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)

[randoguyname]

In regards to the comment of automatically picking up the tempo; to the developer who works on this: keep in mind that users will expect that to update automatically by default as well, but also to be able to manually select a tempo. Perhaps by default a checkbox titled "Detect tempo automatically" should be ticked by default, which then updates with changes to the score, and users can uncheck that and do whatever they want.

[rtbo]

I added TEMPO_RESET_PREVIOUS property on TempoText and added corresponding check box in the inspector panel.
Whenever it is checked, the tempo spinbox is disabled and previous explicit (or default) tempo is restored.
When unchecked, the item behaves just like any other TempoText item.

The checkbox will be there for every TempoText so I chose generic text description ("reset previous tempo"). I'm open to suggestion for better name/description.

TODO:

• add a tooltip on the checkbox
• add a test
• french translation

[rtbo]

I changed terminology to "restore previous tempo".
Here is with tooltip

[rtbo]

I've run tests manually of all functions, but it's not obvious what I could do as automated test.
Is it possible to script playback start on a test score and may be check the tempomap?
Or is it OK to not include automated test for this?

[rtbo]

Regarding UX, maybe the additional Restore previous tempo check-box isn't very valuable.
Isn't it better if the Follow written tempo do what's right when the text is a tempo (case insensitive)?
The fact that a tempo is proposed in the tempo palette self-documents the feature in a much better way IMO.

@CombinedAtom @randoguyname @Tantacrul opinions?

[Jojo-Schmitz]

The French translation doesn't belong into this PR.
Those are done on Transifex, once the new strings got pushed there.

[Tantacrul]

Regarding UX, maybe the additional Restore previous tempo check-box isn't very valuable. Isn't it better if the Follow written tempo do what's right when the text is a tempo (case insensitive)? The fact that a tempo is proposed in the tempo palette self-documents the feature in a much better way IMO.

@CombinedAtom @randoguyname @Tantacrul opinions?

Just tested this. Nice work!

If you mean that the functionality should read the characters 'a tempo' and only then apply the correct logic, I would advise against that. We have generally agreed to avoid trying to deduce intent from written text due to how unreliable it is - and how it needs to localise if a different language is chosen. I think having a checkbox (enabled by default) that you can disable makes more sense. Sorry if I've misunderstood your question.

I actually like 'Restore previous tempo' setting and I think we should get rid of 'follow written tempo'.

• 'Restore previous tempo' already does what we need
• Turning it off allows the user to specify an exact tempo anyway, so all needs are covered

A few points which I think will help the whole thing feel a little easier to understand:

1. I'd place the 'a tempo' palette in the main section, at the bottom of the list. Don't keep it in 'More'. It's a primary option, I feel.
2. Change the text from 'override written tempo' to simply 'set tempo'.

Then I think we're good! Thanks very much.

I'll review again when you're done.

[cbjeukendrup]

@Tantacrul We can't simply remove the "follow written tempo" functionality, because that also has implications for normal tempo markings (because the new "a tempo" item is in fact just a tempo marking, and therefore it shares its properties widget with normal tempo markings).

So we need to design a widget that would work well with both "normal" tempo markings and "a tempo" markings.

How about something like this?

(OBVIOUSLY, the "metric modulation" option should not be there yet, but I've added it to convey the point of this design)

[Tantacrul]

Can we not just have different options for either thing, since the user won't care about what goes on under the hood? I'd rather leave any discussion of other kinds of tempo markings (other than 'a tempo') out of this.

For example, I definitely do not think an 'a tempo' marking should be as complex as this. If the user wants to set something complex as a metric modulation, they shouldn't be using 'a tempo' to do it, no?

However, a fully-realised tempo marking probably should have these kinds of options.

My point is that we should show or hide certain options based on the outward purpose of the marking, rather than the internal similarity.

[cbjeukendrup]

I'd rather leave any discussion of tempo markings out of this.

Of course I understand that, but unfortunately it isn't that easy.

The problem is that MuseScore has no way of distinguishing an "a tempo" marking from any other tempo marking (with this implementation).

Comparing the text is no option, because who says that the user will call this item "a tempo"? They might also write "Tempo I". So anything about comparing text is not more than a fragile heuristic.
The rigorous solution would be to implement the "a tempo" item as a separate class with a separate item type, but I'm not sure whether it is a good idea to create yet another new class in Libmscore.
Another option would be to add just a bool isATempo to the Tempo class, but that's not great either.
(Conclusion: from a technical POV it's not a very nice thing to implement.)

Beside that, from a user point of view, I wonder whether "a tempo" should get a different treatment at all. If it did, then a strange situation would occur when the user changes their mind and changes the text from "a tempo" to an explicit metronome marking. MuseScore would still think that it is an "a tempo" item, because it was initially created as such, and thus it displays the "a tempo" options, while the user thinks that it is just a tempo item, and wonders why the Inspector displays a different set of options than for any other tempo item. Instead of just changing the text, the user would need to remove the "a tempo" item and add a new normal tempo marking.

So I guess the main question is: would a user intuitively expect that "a tempo" is a different kind of item than any other tempo marking and therefore you can't change one into the other without deleting and re-adding?
My personal answer is no, I would expect them to be the same kind of thing, only the effect on the playback tempo is slightly different. But that's just my answer.

[Tantacrul]

I think the case of someone changing 'A tempo' to a metric marking is an unlikely thing to happen - and there are ways we could avoid that confusion (by giving an 'A tempo' object an explicit 'A tempo' name in Properties, for example)

However, let me narrow my issue with UX in this implementation:

• First, for an 'a tempo' marking we have a 'follow written tempo' checkbox that doesn't make much sense, given the context
• Second (and much worse), this solution forces us to place an illogical 'Restore previous tempo' option in all other tempo markings, which I think is a non-starter. This is a classic MS3 solution I want us to avoid in 4. In the image below, 'Restore previous tempo' makes no sense. It serves no purpose in this context.

The illogical extra option is not worth the benefit of an 'a tempo' marking.

I think they have to be separate objects.

[Tantacrul]

Actually, to be a little clearer: I don't necessarily think they need to be different objects. Just that they need different options to appear in the Properties panel. This implementation requires that the comprehensibility of tempo markings take too much of a hit.

As it stands, I think this solution can't be used, unfortunately.

I do appreciate the explanation though @cbjeukendrup.

[cbjeukendrup]

Okay. @rtbo I suggest to implement this in the following way:

• Change the new TEMPO_RESET_PREVIOUS property to determine whether the tempo marking is an "a tempo" marking (rather than whether the marking does indeed use the previous tempo or that the user has customised the playback tempo)
• Instead, use the TEMPO_FOLLOW_TEXT property to determine whether the tempo marking does indeed use the previous tempo or that the user has customised the playback tempo

So:

	TEMPO_RESET_PREVIOUS == false	TEMPO_RESET_PREVIOUS == true
TEMPO_FOLLOW_TEXT == false	This is a normal tempo marking where the user has overridden the playback tempo	This is an "a tempo" marking where the user has overridden the playback tempo
TEMPO_FOLLOW_TEXT == true	This is a normal tempo marking where the playback tempo is determined automatically based on what's written in the text of the marking	This is an "a tempo" marking where the playback tempo is determined automatically by looking at the previous tempo marking

Then, you'll need to create separate Inspector views for "a tempo" and normal markings. To see an example of how you can create different views/models based on the properties of an item, you can start by looking at HairpinLineSettingsModel (and the places where it's used), which differentiates based on whether the selected hairpin is a crescendo or decrescendo.

[rtbo]

I'm not sure to get the UX right.

What options are to be proposed for the "a tempo" mark?

Either the user can override the tempo to a specific one, in such case the same options as regular tempo apply:

• "follow text" check box
• "override tempo" spin button (enabled if "follow text" is unchecked)

Or the user don't have this choice, in which case there is no option to display at all. (FOLLOW_TEXT and RESTORE_PREVIOUS properties are true and can't be changed on such object)

In either case, I would change the "Tempo" section name by "Restore previous tempo (a tempo)" for clarity for the user.

[Tantacrul]

I would have one option

• A checkbox that says 'Set specific tempo' (rather than 'override tempo', which is less clear).
• If you check the box, then it enables a text field where the specific tempo can be set.
• If the box is unchecked, then the 'A tempo' mark just behaves as it should

[rtbo]

Here is what I have now

Ideally I'd like to have the checkbox where the greyed out title is.

[Tantacrul]

OK, given that you have the word 'specific' twice. I think we can simplify this.

I'd change the second title from 'specific tempo' to just 'tempo'.

I get what you are saying about the checkbox but it's no biggy. We never created a checkbox / title component like that before.

[rtbo]

I've rebased the PR on master.
Is there anything left preventing the merge?

[cbjeukendrup]

This looks very good! A few minor comments.

[cbjeukendrup]

A rebase is needed again...

[Jojo-Schmitz]

It is a completely painless and conflict-free rebase, I guess you could rebase and merge in one step.
It does build cleanly on Windows/MSVC too.

I'll submit a new PR (even if only for the tests to run), see #18017

[bkunda]

@cbjeukendrup @Tantacrul I've just quickly tested this (macOS only) and it seems to be working well.
Really liking the "Tempo primo" option too.
UX seems to align with what was discussed above.
Happy with this one! Thanks so much @rtbo :-)

[bkunda]

Ah sorry! One more thing we might need to reconsider:

(This always seems to be afterthought!)

"Restore previous tempo" is a very long title for these buttons, and is likely to not localize well.

We could also think of a slightly modified icon (this can be done separately – no need to block the present PR).

But the label itself reflects the section header, and probably should be decided on here.

[Jojo-Schmitz]

Just drop the "Restore" (both actually)? "Previous tempo" or even "Prev. tempo" might be enough, as should be "Initial tempo"

[bkunda]

Just drop the "Restore" (both actually)? "Previous tempo" or even "Prev. tempo" might be enough, as should be "Initial tempo"

My concern with this is that the use of the verb in the Properties panel is currently clarifying the UX by making it clear what the action does.

Alternatively, how about we simply using the actual object name – the function of which should be clear enough to the user who wishes to use it?

[Jojo-Schmitz]

Ah, so "A tempo" and "Tempo primo". Better indeed.

[bkunda]

@rtbo if you're able to make those brief section heading changes I suggested above, I think we'll be able to get this one past the post for the next release.

@cbjeukendrup @Tantacrul FYI.

[rtbo]

I should be able to work on the new requests over the weekend.

For my information, where is this "notation" panel in the UI ? Never seen that before.

[cbjeukendrup]

For my information, where is this "notation" panel in the UI ? Never seen that before.

It's what you see in the Properties panel when multiple elements of different types are selected in the score.

[bkunda]

I've given this a final test on macOS. All good to go from my end.
Thank you so much @rtbo! People are going to love this!