gregorio-project / gregorio

The Gregorio Project
http://gregorio-project.github.io
Other
164 stars 43 forks source link

Name font calibration distances #1526

Closed rpspringuel closed 3 years ago

rpspringuel commented 3 years ago

There were several "magic distances" still in the code. All of these distances are related to calibrating things to the fonts and so should not be user editable. However, having those numbers scattered through the codebase is harder to maintain (if you change one, you have to figure out which of the others also needs to change, by how much, and where they all are), so based on relationships between the numbers, I've reduced them to 6 named dimensions which are then used as needed in the various calculations.

Fixes #1521.

rpspringuel commented 3 years ago

One question on this remains:

Right now we have noteadditionalspacelinestext which is the amount of space added per level of really low note and is approximately ((\gre@dimen@interstafflinedistancebase + \gre@dimen@stafflinethicknessbase) / 2 ) (the vertical distance between notes a half-step apart). Should we replace this distance with the formula or is there a reason why when adding space between the notes and the lyrics, one would want to add something other than the distance actually required by the low note position?

rpspringuel commented 3 years ago

It should be noted that currently in the code ((\gre@dimen@interstafflinedistancebase + \gre@dimen@stafflinethicknessbase) / 2 ) is used for high notes above the staff. For consistencies sake, if noteadditionalspacelinestext it to be retained, then we should do something similar for the above staff notes.

rpspringuel commented 3 years ago

What about the issue with noteadditionalspacelinestext? Is it more reasonable to have the amount of space added (both above and below the staff) be based how much outside the staff the high/low notes are, or allow the user to customize the amount of space added per step outside the staff?

henryso commented 3 years ago

If it works, then go for it. I know there's some weirdness in spaces above and below the staff.

rpspringuel commented 3 years ago

I've pushed the changes to eliminate noteadditionalspacelinestext in favor of the automatic calculation. Tests with extremely low notes are altered slightly, as the default value for that distance was not exactly what the calculation would have predicted. If everything looks good, then this is ready for merge.

henryso commented 3 years ago

Is there a way to set things up so that the spacing of those low notes are not changed? I suspect that some people (like me, for instance) will have scores already typeset and want to retain the old spacing, even if it's wonky.

rpspringuel commented 3 years ago

I could probably toggle the behavior with a conditional. Let me look at that after Mass.

Sent with GitHawk

rpspringuel commented 3 years ago

Okay, I think I've got that worked out. Please look it over and let me know if you see any problems.

rpspringuel commented 3 years ago

Note that the associated test PR has changed due to some of the monkey business I did to extract the old test results for the backwards compatibility tests. That process was much harder than it really should have been.

henryso commented 3 years ago

I don't think this is deprecation. I see this as an alternative that doesn't really go away, like the old vs new style oriscus orientation algorithms.

rpspringuel commented 3 years ago

Hmmm... That’s going to require a slightly different approach.

Now, currently the additional space added for high notes is calculated while the additional space for low notes is manually set. If we’re going to preserve the ability to manually set the space for low notes, should we add the ability to do so for high notes? If so, should the user be able to independently decide whether to use the manual or automatic setting for low and high notes (requiring two switches with two positions each), or does it suffice to have there be a single manual vs automatic setting?

Sent with GitHawk

henryso commented 3 years ago

I think that there should be a "legacy" switch only for the lower notes as the upper notes have been working as they've been working thus far and no one has been asking for a way to make the high note spacing work like the low note spacing. We'd only need this for the people who have things typeset and laid out and for whom such a change would cause a big mess and a need to re-layout the work.

rpspringuel commented 3 years ago

Okay. I can work on that tomorrow.

Sent with GitHawk

rpspringuel commented 3 years ago

Converted. I'll be offline on Ash Wednesday so if you spot anything I'll deal with it on Thursday.

henryso commented 3 years ago

Looks OK to me.