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

Investigate whether other music fonts work #29

Closed
ajyoon opened this issue May 11, 2022 · 11 comments
Closed

Investigate whether other music fonts work #29

ajyoon opened this issue May 11, 2022 · 11 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@ajyoon
Copy link
Collaborator

ajyoon commented May 11, 2022

In theory we should support any SMuFL-compliant music font, but we've only ever worked with and tested against Bravura. We should try out some other fonts and see what breaks.

@ajyoon ajyoon added enhancement New feature or request help wanted Extra attention is needed labels May 11, 2022
@craigvear
Copy link
Contributor

https://github.com/fkretlow/sebastian sebastian (Bach) looks like a good one to test.

@ajyoon
Copy link
Collaborator Author

ajyoon commented May 13, 2022

music fonts can be registered with neoscore.register_music_font

@jgarte
Copy link
Contributor

jgarte commented Jun 20, 2022

@ajyoon

Should tests be written for each SMuFL-compliant music font?

@ajyoon
Copy link
Collaborator Author

ajyoon commented Jun 21, 2022

I don't think so, unless we want to bundle the new font in the repo. I'm open to it, but only if user demand is enough to warrant increasing the package size. This ticket is more about manually testing out examples with other SMuFL-compliant fonts and seeing if anything breaks or looks wrong. I expect bugs would surface and we'd want to spin off separate issues for this.

That said, I think @craigvear has been chipping away at this, so if you want to take a crack at it we should confirm with him first. Craig, what do you think?

@craigvear
Copy link
Contributor

craigvear commented Jun 21, 2022

Hi both. I did have a little trial with Sebastian font, but then got distracted with other priorities. @jgarte please do have a play with it.

And thanks for all you help with this @jgarte truly appreciated.

@jgarte
Copy link
Contributor

jgarte commented Jun 22, 2022

@craigvear @ajyoon

  1. What are the steps involved in properly testing other music fonts?
  2. How many do we want to test in total in order to close this issue?

@ajyoon
Copy link
Collaborator Author

ajyoon commented Jun 23, 2022

I think the basic process would be something like..

  • Download one or two SMuFL-compliant fonts (maybe Sebastian and Petaluma would be good?)
  • Try modifying some of the provided repo examples to register the new music font and then update the examples to use the new fonts. Music fonts are automatically inherited from objects' ancestor chains with the HasMusicFont mixin, so updating the font requires working out where the font is being inherited from when not explicitly provided. In staff contexts, all fonts are usually inherited from the staff, whose font family defaults to Bravura. Staff font families can be changed simply with the music_font_family arg in the constructor.
  • Take notes of anything that breaks, anything confusing (for documentation improvements), anything awkward about the API (for potential library improvements), and report back here.

Once we know this functionality works, we'll update the documentation to describe the process, but that will be a separate ticket.

@ajyoon
Copy link
Collaborator Author

ajyoon commented Jun 23, 2022

We might want to consider adding a way to globally change the default music font (maybe the global default text font too), but that would be a separate ticket down the line too.

@Xavman42
Copy link
Contributor

Xavman42 commented Jan 1, 2023

Just noticed that there are some clipping issues with the Bravura font when used in a Text object:
image

Here's the code used to generate this image:

from neoscore.common import *

neoscore.setup()

lowercase_alphabet = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j", "k", "l", "m",
                      "n", "o", "p", "q", "r", "s", "t", "u", "v", "w", "x", "y", "z"]
uppercase_alphabet = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M",
                      "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"]
numbers = ["1", "2", "3", "4", "5", "6", "7", "8", "9", "0"]
other = ["`", "~", "!", "@", "#", "$", "%", "^", "&", "*", "(", ")", "-", "_", "=", "+", "[", "{", "]", "}", ";", ":",
         "'", "\"", ",", "<", ".", ">", "/", "?", "\\", "|"]

for index, text in enumerate(lowercase_alphabet):
    Text((Unit(0), Unit(index*10)), None, text, Font("Bravura", (index % 7)+5))

for index, text in enumerate(uppercase_alphabet):
    Text((Unit(20), Unit(index*10)), None, text, Font("Bravura", (index % 7)+5))

for index, text in enumerate(numbers):
    Text((Unit(40), Unit(index*10)), None, text, Font("Bravura", (index % 7)+5))

for index, text in enumerate(other):
    Text((Unit(60), Unit(index*10)), None, text, Font("Bravura", (index % 7)+5))

neoscore.set_viewport_scale(8)
neoscore.show()

@ajyoon
Copy link
Collaborator Author

ajyoon commented Jan 2, 2023

Thanks for reporting that, I've noticed similar issues with other fonts and plaint Text objects before too. If I remember right, the issue affects different environments differently since it relies on low-level text layout systems. I'll try to take a look this week.

@ajyoon
Copy link
Collaborator Author

ajyoon commented Feb 14, 2023

I built a little example demonstrating this capability and tested it locally. I found one small bug and fixed it in the above commit.

Sebastian:
sebastian

Bravura:
bravura
(don't mind the broken trill appearance, that's because I used the wrong glyph for it)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants