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 #43906: Support 256th, 512th, and 1024th Notes [WIP] #2677

Closed
wants to merge 6 commits into from

Conversation

ajyoon
Copy link
Contributor

@ajyoon ajyoon commented Jun 20, 2016

  • Add glyphs for flags and rests in Mscore and Gootville fonts (@Gootector is it okay to do this as a temporary hack to get support across the different fonts before you do this yourself if you're planning to?) Bravura already includes the required glyphs.
  • Partial support for shadow notes. (Bravura and Gootville flags display incorrectly in shadow notes, strangely enough not in Mscore, which is the one I had to touch the most)
  • Rework duration validations in various entry points to these durations
  • Allow tuplets with these note values
  • Allow these durations to be accessed by dots
  • Add importmidi support for these values
  • Update importmidi preferences options to allow smallest note value of 1024
  • Prevent a fatal assert in MidiOperations::QuantValue fractionToQuantValue with a default value
  • Misc. code clean-up in the vicinity

Potentially problematic change needing review and feedback: Change MScore::division from 480 to 512 65536. Current handling of ticks relied on rounding errors when translating DurationType's to ticks, causing bars missing tiny amounts (usually a 1024th value) when pasting sections and similar operations involving the new values. Changing MScore::division is also causing most tests to fail currently because of a diff between the embedded division value in the test references and certain tick values diverging. Is this a good way to proceed? If not, are there some better solutions?

TODO's (maybe more appropriate for a separate PR?):

  • Add pad icons for new values (currently can be reached by pressing Q repeatedly on a note)
  • Correct shadow note display for Bravura and Gootville

@ajyoon
Copy link
Contributor Author

ajyoon commented Jun 20, 2016

mscore_font
gootville_example

@ajyoon ajyoon force-pushed the support-256-512-1024-durs branch 4 times, most recently from 90e90c3 to eec1c05 Compare June 20, 2016 04:48
@@ -81,6 +81,7 @@ QMap<QString, QString> nmap {
{ "rests.5", "rest32nd" },
{ "rests.6", "rest64th" },
{ "rests.7", "rest128th" },
{ "rests.8", "rest256th" },
Copy link
Contributor

Choose a reason for hiding this comment

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

nothing for 512th and 1024th?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point - my understanding is this code is intended to create links between Lilypond's Feta/Emmentaler font and the SMuFL compliant Mscore font. Glyphs for rests.9 and rests.10 are not defined in Feta. Actually now that I double check http://lilypond.org/doc/v2.18/Documentation/notation/the-feta-font#rest-glyphs it looks like rests.8 isn't defined either, so this might be leftover from my testing and I should just cut that line too. Or am I misunderstanding the purpose of this code?

Copy link
Contributor

Choose a reason for hiding this comment

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

No idea, I was just wondering...

@Jojo-Schmitz
Copy link
Contributor

The mtest failures are all around division and ticks. I wonder whether we really need to adjust all the mtest files or whethr and how to have some backward compatibility to avoid all these failures, which might mean that we can't add these new shorter notes to 2.0 files, only to 3.0 ones?

@ajyoon
Copy link
Contributor Author

ajyoon commented Jun 20, 2016

I feel like there is a way to do this as a non-breaking change... The whole issue is coming from the fact that ticks have any place at all in the score object model. If all positions were represented as compound fractions this would be a non-issue, and it would also fix a lot of the bugs in tuplets right now as well (and let us fully support arbitrarily nested tuplets too!). But maybe that is outside of the scope of 2.x

@Jojo-Schmitz
Copy link
Contributor

I think this change it too big to make it into 2.0.4, so it's be for 3.0 only?

@lasconic
Copy link
Contributor

I didn't take a close look yet but two things.

  1. If we increase MScore::division, I would increase it enough to accomodate the smaller possible note, including weird tuplets, 512 cannot accommodate a simple 8th note triple (it's not possible to divise it by 3...). Ideally, ticks should be come fractions... but it's out of the score of this PR.
  2. The division is saved in the MuseScore file, in the tag. So there should be a way to change the internal MScore::division and use the to do a proper conversion.

@ajyoon
Copy link
Contributor Author

ajyoon commented Jun 23, 2016

It was actually working before for most common tuples, but certain values (1024th 9-tuplets, etc.) broke things. Increased division by some arbitrarily high amount and all my testing is working, but this still feels like a band-aid to me.

It seems like there is already some built in way to convert the embedded division value in the scores which might handle the compatibility issue for us. Maybe it's here:

int fileDivision(int t) const { return ((qint64)t * MScore::division + _fileDivision/2) / _fileDivision; }
but I'm having trouble making sense of the purpose of the math so I'm not sure.

I also added a sanity check to ScoreView::cmdCreateTuplet to help catch these weird errors sooner.

Seems to me that there won't be a way around making this change without editing mtests...

case 7:
opers.quantValue.setDefaultValue(QuantValue::Q_512, false);
break;
case 8:
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation issue with those case statements

@Gootector
Copy link
Contributor

1024th, 512th and 256th notes will be added by me in the next version of the Gootville font - 1.4. But... Who are using these notes?

@ajyoon
Copy link
Contributor Author

ajyoon commented Jun 26, 2016

@Gootector Is it ok to use my slightly hacky glyphs in the meantime before you finish 1.4 just so we can have everything supported in the meantime?

These values appear in Beethoven even - https://en.wikipedia.org/wiki/Two_hundred_fifty-sixth_note - as well as lots of contemporary music. The aim is to make this environment more permissive for composers so we don't decide for them what they should or shouldn't notate (to be honest, personally I would never write these values, but many of my peers do). It's an easy enough feature to implement with all the scaffolding already in place.

@Gootector
Copy link
Contributor

@ajyoon I'll add 256th-1024th noteflags and rests to the font files - as a new glyphs. No problem for me. If it's necessary...
OK. Let's assume that 256th note is necessary. In MuseScore 4.0: 2048th and 4096th? I've a problem with reading 256th note in scores... 1024th - this note is unreadable for me and should exist in theory only. It's my opinion, of course.

@Jojo-Schmitz
Copy link
Contributor

Bravura supports up to 1024th, so should any other SMuFL font. More shouldn't be needed

@lasconic
Copy link
Contributor

Increasing the division to 65536 will limit the length of score to MAX_INTEGER/65536= 32768 quarters notes, = 8192 measures in 4/4. (assuming MAX_INTEGER = 2^31)
@wschweer thinks it's a bit low. Can we find a middle ground?

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 27, 2016

how about 26880, it divides by 256, 3, 5 and 7, so we should be fine up until at least 10-plets (only x-plets with x being a prime number >= 11 won't work) of 1024th and allow more than double the lenght of a score than with 65536 (which still won't allow triplets, 5-plets or 7-plets of 1024th)?
Or is my math and/or logic flawed?

@mgavioli
Copy link
Contributor

I was about to comment along @Jojo-Schmitz line: 65536 is only divisible by 2 (several times) and does not accommodate any tuplet at all.

I was going to suggest 9 * 7 * 5 = 315 * a convenient amount of powers of 2; for instance, 315 * 32 = 10080 or 315 * 64 = 20160. 9 is required to allow 9-plets.

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jun 27, 2016

Ah, yes, I forgot 9-plets.

@Gootector
Copy link
Contributor

Note values in commercial scorewriters:
Finale - max 128th
Sibelius - max 512th

@lasconic
Copy link
Contributor

Extreme of conventional notation http://homes.soic.indiana.edu/donbyrd/CMNExtremesBody.htm#duration

For 1024th and 3,5,6,9 tuplets, we would need 256*315=80640...

@mgavioli
Copy link
Contributor

Indeed! But it is so important to support tuplets of 1024th?

315*64 = 20160 would support tuplets of 256th (1/4 / 1/256 = 64), which is already quite esoteric...

@lasconic
Copy link
Contributor

Mmm, we still need to be able to represent 1024th, so the number needs to be divisible by 256 right?

@mgavioli
Copy link
Contributor

Ops, yes!

@ajyoon
Copy link
Contributor Author

ajyoon commented Jun 27, 2016

To be honest I don't understand the mechanics behind how tuplets interract with ticks, since my tests with these values (even triplets with the previously tested division of 512, which does not divide evenly) were working. It seems the tick division doesn't need to be perfectly divisible by the note fraction to work. Some kind of sneaky rounding voodoo is suspected.

@MarcSabatella
Copy link
Contributor

There is indeed "voodoo" scattered throughout the code - various fudge factors worked in to tick calculations. But they only work up to a point, so it would be nice to not have to rely on these so much.

@ajyoon
Copy link
Contributor Author

ajyoon commented Jul 3, 2016

Thanks everyone for the feedback! I've reduced the division value to 4096 per @lasconic and my testing with creating tuplets safely and all is all still working. Of course mtests are still failing. Does anyone have some idea how to update mtests safely here? It seems we'd just need to update the saved division tag and update tick values.

@ajyoon
Copy link
Contributor Author

ajyoon commented Aug 14, 2016

Apologies for the delay on this, I spent the night taking another crack at this but after rebase the tests are failing even worse than before. I suspect b85cad9 may be involved, but I'm not entirely sure. I tried rebuilding the tests using this branch with a script that uses the build to open and save every .mscx file in mtest except for those in compat which aren't -ref files, which brought the failure rate to more like 50% but even still it seems not to be a very safe way to go about it. Is this the right idea for how tests should be rebuilt? If not, what might be a good way to go about this?

Would it be helpful or just more noise for me to push my test tinkering?

@ajyoon
Copy link
Contributor Author

ajyoon commented Aug 20, 2016

If this doesn't seem worth pursuing for the compatibility risk for such a trivial feature, please feel free to close and I'll work on something more useful

@@ -81,6 +81,9 @@ QMap<QString, QString> nmap {
{ "rests.5", "rest32nd" },
{ "rests.6", "rest64th" },
{ "rests.7", "rest128th" },
// { "rests.8", "rest256th" }, // Glyphs currently don't exist in Emmentaler
Copy link
Contributor

Choose a reason for hiding this comment

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

so these need to get added

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm wondering if genft is actually doing anything at all at this point. It seems like it was used previously when converting the original Emmantaler to the MuseScore font, but since the two have forked I don't really understand what this code does anymore. Do we still need it at all?

@Jojo-Schmitz
Copy link
Contributor

Rebase needed. And thanks to the new fallback, the added glyphs for MScore and Gootville are not vital anymore (but would still be appreciated)

@ajyoon
Copy link
Contributor Author

ajyoon commented Sep 27, 2016

Rebased with new smufl mapping accounted for. Also rolled back all changes on libmscore/sym.cpp per discussion about it not being necessary because those mappings were set up for older font handling systems.

@Jojo-Schmitz
Copy link
Contributor

See also #3102 and #3107

@smoge
Copy link

smoge commented Mar 27, 2017

A typesetting software should allow the publisher or composer to make those decisions and do their work. The developers, on the other hand, should work to provide the framework for them to decide this. In the repertoire, this notation has been used since the 17th century...

The shortest note value to have ever been used in a published work is the  1⁄1024 note (notated incorrectly as a  1⁄2048) in Anthony Philip Heinrich's Toccata Grande Cromatica from The Sylviad, Set 2, written around 1825;  1⁄256 notes occur frequently in this piece, and some  1⁄512 notes also appear.[2] For comparison, the shortest notated duration supported by Finale is a  1⁄4096 note,[7] while LilyPond can write notes with up to 27 beams, as short as a  1⁄1073741824 (= 2−30) note

https://en.wikipedia.org/wiki/Two_hundred_fifty-sixth_note

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented Jul 13, 2018

Rebase needed.
I wonder whether in the light of #3812 the 'correct' setting tor MScore::division becomes less of an issue.

@anatoly-os
Copy link
Contributor

The shortest notes are fully supported now thanks to the switching to Fractions.

@anatoly-os anatoly-os closed this May 21, 2019
@ajyoon
Copy link
Contributor Author

ajyoon commented May 22, 2019

Awesome, thanks!

@Jojo-Schmitz
Copy link
Contributor

Jojo-Schmitz commented May 22, 2019

Still we don't have notes that short, still 128th is the shortest note we can enter (via toolbar or shortcuts), except via entering a 128th and than using Q to half its duration

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

8 participants