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

[MU3] Fix #277279: Lack of Capabilities in the Custom Time Signature Creator #6550

Closed

Conversation

elerouxx
Copy link
Contributor

@elerouxx elerouxx commented Sep 11, 2020

Resolves: https://musescore.org/en/node/277279
Lack of Capabilities in the Custom Time Signature Creator
https://musescore.org/en/node/279745
Allow periods in custom time signatures
https://musescore.org/en/node/277665
Cannot use a decimal dot or vulgar fractures like ½ for time signatures

See other requests:
https://musescore.org/en/node/312343 Time SIgnature Palette: Parenthesis
https://musescore.org/en/node/312343 3/4 Time Signature with Secondary 6/8 in parenthesis
https://musescore.org/en/node/305902 Dual Time Signatures

This PR replaces the appearance numerator/denominator fields with a single parsed text that allows more capabilites to the appearance of time signatures. These changes were made in the Time Signature creator (timedialog), Time Signature properties, and additionally added to the time signature Inspector directly, which is the faster way to enter text and change the appearance of a time signature (can be done locally if a single t/s is selected pressing CTRL).

This single field is properly parsed to identify "blocks" of time signatures (numerator/denominator) as well as complementary glyphs such as Operators +=-x and Parentheses ( ). In the absence of a denominator, the glyphs are drawn vertically centered, as current, and the Plus and Parentheses glyphs are replaced by the large versions.

Examples:
"2/4"
"3/4(6/8)"
"3+2/8"
"2/8+5/8"
"C"
"4/4=C"

image

(NOTE: The 'Large Time Signature' feature from the inspector and at the end of the video below was removed from this PR, for a future work)

In action:
parser_ts_inspector2

Bracketed special characters:

  • To enter fractional glyphs, use brackets: ex. 3[1/2]/4

  • Note value glyphs like quarter, dotted quarter, can be entered in denominators (Carl Off style time signatures) by using numbers and dots within brackets: [2] [4] [8] and [16] where [8] is an eight note, [4.] is a dotted quarter note. Ex: 3/[8.]

  • Other (unrecognized) texts between brackets will be discarded.

  • Other glyphs include Open Penderecki time signature ( type '~' ) Open X time signature (type uppercase 'X') Comma ',' and period or augmentation dot '.'

  • Spaces can be added to improve visual and will force separation of 'blocks'.

Additional notes:

  • All Time Signature 'official' SMuFL glyphs have been enabled (see https://w3c.github.io/smufl/gitbook/tables/time-signatures.html)
  • Big parentheses ( ) (SymId::timeSigParensLeft / Right) are not well implemented in Emmentaler and Gonville fonts. Emmentaler has bad bboxes/metrics, and in Gonville these glyphs are identical to the smaller SymId::timeSigParensLeftSmall. (both cases might need special handling or fontworks)
  • The 'Narrow' glyphs from SMuFL have not been activated because they are just the common font time signatures being condensed horizontally, which can be achieved via the time signature Scaling in the inspector.
  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [NA] I created the test (mtest, vtest, script test) to verify the changes I made

@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch 3 times, most recently from 92a60f6 to 15d7e57 Compare September 13, 2020 16:43
@elerouxx elerouxx changed the title [WIP] Fix #277279: Lack of Capabilities in the Custom Time Signature Creator [WIP] Fix #277279: Lack of Capabilities in the Custom Time Signature Creator +collect_artifacts Sep 13, 2020
@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from 49b3d68 to ff58f41 Compare September 13, 2020 18:05
@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from ff58f41 to 875eeb1 Compare September 17, 2020 14:45
@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Oct 29, 2020

Any chance to get this ready for 3.6?

BTW: that collect_artifacts is no longer needed, those are now always generated

@shoogle
Copy link
Contributor

shoogle commented Oct 29, 2020

I think this is the best solution that could make it into 3.6 rather than having to wait for 4.0.

I recommend:

  1. Remove the separate input fields for numerator and denominator (i.e. just keep the parser text field).
  2. Add some funky examples to the Time Signatures palette in the Advanced workspace. This should help alleviate the discoverability problem.

@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from 875eeb1 to df68a7d Compare November 23, 2020 16:27
@elerouxx elerouxx changed the title [WIP] Fix #277279: Lack of Capabilities in the Custom Time Signature Creator +collect_artifacts Fix #277279: Lack of Capabilities in the Custom Time Signature Creator Nov 23, 2020
@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch 2 times, most recently from d2ddd26 to 554ea6c Compare November 23, 2020 18:28
@ecstrema
Copy link
Contributor

Just tested the PR, and it works like a charm.

It also comes with a bonus: It's fairly common in jazz Charts to show a closing bracked after the time sig, like in this example:
image

This was possible already with Musejazz, that provides a closing parenthesis by default in its common and cut time sigs, but can now be done with any. More comments in this thread.

@ecstrema
Copy link
Contributor

ecstrema commented Nov 23, 2020

haha tumbao 3-3-2 in a time sig:
image

However, here's the text I used to make that work: [1]/[[4.]+[4.]+[4]
As you can see, there's a closing bracket missing. It doesn't work if I add it. Not a merge blocker though.

@ecstrema
Copy link
Contributor

looking at the vtest diff, it looks like this pr solves the mordent issues?
image

@elerouxx elerouxx marked this pull request as ready for review November 23, 2020 21:19
@ecstrema ecstrema mentioned this pull request Nov 23, 2020
8 tasks
@elerouxx
Copy link
Contributor Author

looking at the vtest diff, it looks like this pr solves the mordent issues?

Hey! Thanks for your comments. I was just updating the PR and preparing it for merge. Nice that you are having fun :D

Yes, this includes a small fix with the 'ADVANCE' value not being properly added to some glyphs when fallback is needed (for example the plus sign on a 3+2/4 time signature in gootville font in 3.5.2) and the side effect is that the ornaments were fixed.

@elerouxx
Copy link
Contributor Author

haha tumbao 3-3-2 in a time sig:
image

However, here's the text I used to make that work: [1]/[[4.]+[4.]+[4]
As you can see, there's a closing bracket missing. It doesn't work if I add it. Not a merge blocker though.

Actually, plus signs aren't intended to work on a denominator, but you hacked it :D - but if you close that bracket, everything inside becomes a code and is not recognized.

@Tantacrul
Copy link
Contributor

Tantacrul commented Nov 24, 2020

I have a few comments.

First, when you click on a time signature and select 'large time signature' the result is a smaller, squashed time signature. I know what's happening here, but we can't allow UX like that in this app. You'd need to have a good understanding of SMuFL standards and quite a bit of development insight to understand what is going on. I don't think that option should exist unless it produces a desirable result by default. The user should not have to go messing around with scale settings. Most users will just consider it to be a new broken feature.

Select time signature:
image

Select 'Large Time Signature Font'
image

Result:
image

Second: The scaling settings are quite frustrating.

  • There should be a way to 'link' the X and the Y, so you don't have to keep clicking the scale up/down button for both
  • The user should also be able to 'unlink' them too
  • The up/down buttons should work in increments of 1.0, not 0.1. If the user wants to enter a precise value after the decimal point, I feel they would choose to do that with text.

Third, the note values look extremely strange when 'Large Time Signature Font' is selected. Pointing downwards and quite massive:

Result:
image

Settings:
image

Fourth, I do not believe users will understand how this works. For example, the only reason I understand that you can get a note value by typing [4] or [2] is because I watched the accompanying GIF. This is a very raw implementation.

For 3.6, I would suggest removing the 'Large Time signature font' option, since it simply produces too many UX woes. I would implement the scaling UX improvements and definitely prepare documentation to explain how the time signature text box is supposed to work.

Ultimately, I think this solution needs design work, so that it is understandable without documentation (which is currently a must-have requirement). I know pretty much how I'd do this from a design point of view - but obviously - since 3.6 is about to be released, I don't think we can get it done in time.

@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from 554ea6c to 1c2bd7c Compare November 24, 2020 13:23
@Jojo-Schmitz
Copy link
Contributor

looking at the vtest diff, it looks like this pr solves the mordent issues?
image

So those vtest diffs are a Good Thing (TM)

@ecstrema
Copy link
Contributor

@Tantacrul Dorico has a lot of these latex inputs, and even though it requires some documentation reading, in the end it is super fast to type and powerful.

These custom time sigs are for advanced users. I wouldn't see someone inputting 4/8=[4.]+[8] without a deep knowledge of what he is doing anyway.

@Tantacrul
Copy link
Contributor

Tantacrul commented Nov 24, 2020

@Tantacrul Dorico has a lot of these latex inputs, and even though it requires some documentation reading, in the end it is super fast to type and powerful.

These custom time sigs are for advanced users. I wouldn't see someone inputting 4/8=[4.]+[8] without a deep knowledge of what he is doing anyway.

How do these advanced users figure this out? I'm an advanced user and I would never have guessed.

As a general rule, do not put options in front of people that can not be deduced. This is a fairly common design principle.

Another point I'd make is that it is confusing too. It could easily be misconstrued by a less advanced user as something that allows you to actually change your time signature. I wouldn't be surprised to hear feedback from people trying this out by typing '3/8' into that box and thinking the result is broken.

The difference between this and the Dorico solution is that Dorico provide proper time signature options in a place people can find them, with the Latex stuff being a fast method of achieving the same thing. Here, we're putting a far less useful thing up font - assuming people will figure it out - and hoping it won't be misconstrued as the type of time signature settings most people will be looking for.

@elerouxx
Copy link
Contributor Author

Just added a bunch of links to issues and discussions about time signature needs I tried to keep in mind. Basically, people having to hide time signatures to work around graphic needs, which are common engraving practices.

The visual needs of time signatures can be complex and very diverse (as they are in real case examples) even if the real values are simpler. Dorico has a nice implementation but even so there will be limitations by design, from what I see.

I'd like to see working real Numerator/Denominator fields above the Appearance Text field but that was not possible with current limitations, both mine and from the current code. Maybe a better label would help clarify this is meant for appearance only? ( Alternate Text, Appearance Override?)

I opted for the smart text among other ideas that were discussed, because it doesn't block any future development: ultimately this property of the timeSig will most probably be saved on a string anyway, even if the interface is changed i.e. to a graphic draggable interface or whatever else.

As for the bracket 'codes', most users won't never use them, as they cover really specific needs. So that user will probably have to find out 'how to' in the documentation for the first time he needs fractions, or the very rare Carl Orff signatures (the note values in denominators). Other than that, the use of the text field is pretty straightforward, and it reflects instantly on the selected time signature.
Other options were discussed (like using 'q' or 'w', or '8th' and '4th' instead of bracketed codes, and some non practical ideas for fractional symbols) but none of the alternatives seemed better.

@shoogle
Copy link
Contributor

shoogle commented Nov 24, 2020

How do these advanced users figure this out?

I suggested adding some funky examples to the Time Signatures palette in the Advanced workspace. I don't think it has been done yet, but if it was then it would enable users to add one of those examples and right click on it to see how it was constructed.

It could easily be misconstrued by a less advanced user as something that allows you to actually change your time signature.

The existing dialog had that problem already, so I don't think that should count against this PR.

Ultimately I think this PR is a great improvement over what was available before, with no downsides that were not already present (except the Large Time Signature issue that is going to be removed/resolved anyway). Clearly a better interface is desirable at some point, but I see no reason to make advanced users wait for it when this is ready now. Even when that interface comes along, we will in all likelihood end up storing the digits in the MSCX file using the same LaTeX-style code as is implemented here.

@elerouxx
Copy link
Contributor Author

I think it's important to understand that the note value symbols, as well as the vulgar fractions, are here because of specific, uncommon but known styles of time signatures several people requested, and whoever needs them is because they know what to do with them. It's not like they are common available 'choices' for common use, and that users will have to figure out how to arbitrarily use in their music or otherwise will have problems creating time signatures. It's an advanced feature, just that.

@elerouxx
Copy link
Contributor Author

Since I removed the 'Large' checkbox, I will not link or otherwise modify the Scale X and Y fields - Although I agree with the suggestion, that feature was implemented long time ago (not by me), it is not related to this PR anymore, and most importantly, I have no clue how to do that.

@Jojo-Schmitz
Copy link
Contributor

rebase needed

@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from 1c2bd7c to 1a529ed Compare January 24, 2021 12:54
@elerouxx elerouxx marked this pull request as draft January 24, 2021 12:57
@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from 1a529ed to d4f95d0 Compare January 24, 2021 13:33
@elerouxx elerouxx marked this pull request as ready for review January 24, 2021 14:15
@elerouxx elerouxx force-pushed the timeSig-alternate-text-3.x(ts-style) branch from d4f95d0 to 0181bf5 Compare January 24, 2021 16:04
@elerouxx
Copy link
Contributor Author

rebase needed

Rebased. Also fixed 'Other' radio button and combo not working with the new single string implementation.
TODO: This PR fixes refreshing of the notegroups in timeSigProperties, but this refresh only works with the text field. Other appearance buttons (Cut, other...) don't refresh the notegroups in properties dialog.

@igorkorsukov igorkorsukov changed the title Fix #277279: Lack of Capabilities in the Custom Time Signature Creator [MU3] Fix #277279: Lack of Capabilities in the Custom Time Signature Creator Feb 5, 2021
@vpereverzev
Copy link
Member

3.x merges are closed now, I suggest to create a new one after the discussion with Tantacrul about the UX part of this feature

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.

None yet

6 participants