Support usage of user-provided SMuFL fonts #25773

RootCubed · 2024-12-08T13:06:03Z

Relevant issues: #24761 and #16189

Based on the work of @cbjeukendrup in https://github.com/cbjeukendrup/MuseScore/tree/custom-music-fonts.
This PR introduces functionality that allows users to provide custom SMuFL fonts in MuseScore.
It adds a new folder selection in Preferences -> Folders -> MusicFonts (Default: MuseScore4[Development]/MusicFonts).
It searches through this folder for directories with SMuFL files - <font name>.ttf/.otf files and a matching metadata json file (<font name>.json, metadata.json or <font name>_metadata.json).

It also supports the standard installation directories as specified by the SMuFL docs.

So, if you wanted to add the Sebastian font, you would create a directory in MusicFonts called Sebastian and place the files Sebastian.otf, SebastianText.otf and metadata.json (renamed from Sebastian.json) inside it.

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)

oktophonie · 2024-12-08T14:18:28Z

Nice!!

However: the SMuFL specification describes how installation should work; might it not be better follow that instead? Users may already have fonts installed in this manner if they are using them for other software.

Specifically: the fonts are installed in the usual way (according to the operating system) and then the metadata json files are placed in the correct locations as described on this page.

RootCubed · 2024-12-08T14:25:36Z

Oh, good point - I was not aware that the specification also describes the installation process, thanks for the hint.

I'll try to implement that then 👍

Jojo-Schmitz · 2024-12-08T14:33:37Z

I've done this in Mu3 in a 3-level way, system-wide, user-wide, both as per that SMuFL standard, and MuseScore only (the 'private' way, as per SMuFL, basically the way you did it.

See also https://github.com/Jojo-Schmitz/MuseScore/wiki/Musescore-3-Evolution-Features#using-external-smufl-fonts

RootCubed · 2024-12-08T14:58:19Z

Is there any specific reason why the privately installed fonts should use metadata.json instead of <Font name>.json? MuseScore internally uses metadata.json, but if we support the SMuFL spec we might as well have it consistent for all external fonts, right?

oktophonie · 2024-12-08T15:35:46Z

The correct canonical form seems to be fontname.json and, yes, it would be good to be consistent.

I think metadata.json exists for mysterious legacy reasons (also if you look in the repositories for Bravura - and indeed for Leland which follows it - the filename is fontname_metadata.json, even though the installed filename should be just fontname.json).

It might be nice to recognise all those variants unless it'll cause conflicts. (The metadata should be inside a folder which follows the font name, anyway.)

Jojo-Schmitz · 2024-12-08T15:39:42Z

so fontname.json, fontname_metadata.json and metadata.json, right? In what order, what precendence, should more that one of those exist?

RootCubed · 2024-12-08T16:01:29Z

Probably <fontname>.json first, then metadata.json, then <fontname>_metadata.json. It might also make sense to change the aliases in src/engraving/data/fonts/<fontname>.qrc so that they use the <fontname>.json naming convention.

BTW, I think I will actually do fontname.toLower() + "_metadata.json" because for the SMuFL fonts I found using this naming convention it was always e.g.

Bravura.otf -> bravura_metadata.json
Petaluma.otf -> petaluma_metadata.json

Jojo-Schmitz · 2024-12-08T16:24:34Z

Allegedly there are issues with spaces in the name of fonts and/or their directories under Linux

Also what to do if there's a Musical Symbols Font and a Musical Text font, with a separate metadata.json, like e.g. Finale Ash, which consist of Finale Ash Text,json, Finale Ash Text.otf, Finale Ash.json, Finale Ash.otf

RootCubed · 2024-12-08T16:34:54Z

I believe MuseScore's engraving system does not support SMuFL metadata for musical text fonts because they seem to be handled as "normal" fonts, so that would not be supported for now (but the text font would be detected properly, just the metadata file discarded).

src/notation/internal/engravingfontscontroller.cpp

See musescore#25773 (review)

RootCubed · 2024-12-08T19:42:11Z

I have only confirmed correct functionality for Windows - would be great if someone could test on MacOS and Linux since the font search will look through different directories depending on the OS.

See musescore#25773 (review)

src/notation/internal/engravingfontscontroller.cpp

cbjeukendrup · 2024-12-09T00:08:11Z

@RootCubed Nice work!
To be honest, when I saw this pull request, I was a bit disappointed, because I would have loved to finish this project myself, when the right moment is there. I would certainly have appreciated if you had asked me, before taking my branch.
Anyway, this PR seems to be going in a good direction, so let's make the most out of this collaboration.

It would be great if you could add the following to the commit message of the first commit, by way of attribution:

Based on:
https://github.com/cbjeukendrup/MuseScore/commit/1883ee5430af4a66ac940690543efb30273b643c
https://github.com/cbjeukendrup/MuseScore/commit/4ff079ac9618b29c903dbd8aae34e1085c9ef87b
https://github.com/cbjeukendrup/MuseScore/commit/5ad19c861f7a2343ba25476134df97ab94393fd9

Co-authored-by: Casper Jeukendrup <[email protected]>

With that out of the way, I'll take a look at the code and leave some comments!

src/engraving/internal/engravingfontsprovider.cpp

src/framework/ui/internal/uiconfiguration.h

src/notation/inotationconfiguration.h

src/notation/internal/engravingfontscontroller.cpp

cbjeukendrup · 2024-12-09T01:29:27Z

src/notation/internal/engravingfontscontroller.cpp

+
+        muse::io::path_t symbolFontPath;
+        muse::io::path_t textFontPath;
+        QDirIterator fontFiles(iterator.filePath(), { "*.otf", "*.ttf" }, QDir::Files);


I'm afraid it's going to be a bit more complicated:

Currently, you're looking for font files in (subdirs of) the given path, which works well for fonts from configuration()->userMusicFontPath() (because the design with that is that the user puts both the otf/ttf and the metadata file side-by-side in a subdirectory there).

But for fonts from systemFontsPaths, only the metadata file is stored in that path; the otf/ttf is installed in the default system directory for fonts.

Scanning all system fonts directories for fonts, and then looking for corresponding metadata files, seems not very efficient. The other way around works potentially better, i.e. scan for metadata files and then check if there is a corresponding font in the font database (note that in this case, you probably don't have to load the font by reading the otf/ttf, because it is already in the font database).

So you'll probably end up with two scanning functions: one for fonts from configuration()->userMusicFontPath(), and one for "installed" fonts, in either the global or user-local system font directories.

Yeah, I just noticed that when trying to install some SMuFL fonts. It seems I actually misread the spec there as I thought that the fonts should be installed in the same place as the metadata file.

But that's actually fine, no more guessing metadata naming conventions 😅

One question about this, though: should we open the metadata file and look at the fontName field, or just assume from the folder name?

Hm... I'm not sure to be honest. Little bit annoying that the spec is so unclear about how exactly it is supposed to work.
Perhaps for now do what's easiest and then come back to it later if it ever turns out that the other way was better?

This is the last thing I need to get working, and it's proving to be quite an annoyance...

It seems that you can't get the file path to a system font via QFont, which is needed to add a font to IFontsDatabase... any ideas on how to deal with this?

Well, I'm just manually searching through QStandardPaths::standardLocations(QStandardPaths::FontsLocation) for now. (With an exception for Windows, since it doesn't include %LOCALAPPDATA%/Microsoft/Windows/Fonts 🙃 )

src/notation/internal/engravingfontscontroller.h

diedeno · 2024-12-09T09:52:47Z

This is great news!
Note that there is a problem with the Valerio font, at least on Linux.

diedeno · 2024-12-09T10:02:28Z

Trying the Ouverture fonts AloisenU and AloisenGrooveU results in a crash (does not start up)

Dec 9 10:56:41 L-SYN kernel: [10771.244468] main[18602]: segfault at 18 ip 00005567fb167d8b sp 00007ffceecdb5e0 error 4 in mscore4portabledev[5567fa9af000+2234000]
Dec 9 10:56:41 L-SYN kernel: [10771.244479] Code: 41 54 55 53 48 83 ec 28 64 48 8b 04 25 28 00 00 00 48 89 44 24 18 31 c0 48 8b 06 0f 11 07 48 c7 47 10 00 00 00 00 48 8b 58 08 <48> 8b 6b 18 48 83 c3 08 48 39 dd 0f 84 9f 00 00 00 48 8d 4c 24 10
Dec 9 10:56:41 L-SYN systemd[1]: tmp-.mount_MuseSc2X5KLp.mount: Deactivated successfully.

But these fonts not useful anyway.

Jojo-Schmitz · 2024-12-09T10:08:38Z

This is great news! Note that there is a problem with the Valerio font, at leass on Linux. (the opposite of what we had with MS3.7)

I've been trying to fix that with #25050 (and Jojo-Schmitz#672 for Mu3), to fix #25088. Some discussion about that in https://musescore.org/en/node/369769#comment-1261591 ff.

RootCubed · 2024-12-09T11:22:21Z

@cbjeukendrup Thanks for taking the time to look at the PR! I apologize for not asking before starting work on this, in hindsight it would have obviously been the right thing to do. I'll make sure to add the attribution after I apply the code review. (Would you like it squashed all into one commit in the end?)

I'll try to get through the review later today.

cbjeukendrup · 2024-12-09T11:38:23Z

@RootCubed Thanks! Squashing all commits is not necessary; but perhaps it's nice to squash those "Fix CI" commits into their respective parent commits.

rpatters1 · 2024-12-09T11:56:24Z

Is there a reason to support a musescore-specific location in addition to the standard locations? I am currently working on a utility that emits .mss style files. It will basically be impossible for the utility reliably to find any SMuFL fonts located there. This is just one small use case, but it seems like a custom location opens a can of worms.

rpatters1 · 2024-12-09T12:06:13Z

I don't see any support for the userFontsPath specified in the SMuFL spec. I only see support for System and MuseScore-Custom locations.

Here is the code I wrote for Finale Lua plugins, that finds the SMuFL font path. (Because Finale was only Mac and Win, it does not have Linux, but adding it would be straightforward.)

local calc_smufl_directory = function(for_user)
    local is_on_windows = finenv.UI():IsOnWindows()
    local do_getenv = function(win_var, mac_var)
        if finenv.UI():IsOnWindows() then
            return win_var and os.getenv(win_var) or ""
        else
            return mac_var and os.getenv(mac_var) or ""
        end
    end
    local smufl_directory = for_user and do_getenv("LOCALAPPDATA", "HOME") or do_getenv("COMMONPROGRAMFILES")
    if not is_on_windows then
        smufl_directory = smufl_directory .. "/Library/Application Support"
    end
    smufl_directory = smufl_directory .. "/SMuFL/Fonts/"
    return smufl_directory
end

Jojo-Schmitz · 2024-12-09T12:09:24Z

The SMuFL spec does mention System-wide location, User-specific location and Private fonts

rpatters1 · 2024-12-09T12:12:26Z

Right. I think we need to support the user-specific-location, and currently I don't see where it is supported.

As for the private location, I am questioning whether that should be a user-facing feature in MuseScore. Is there a need for it?

rpatters1 · 2024-12-09T12:18:30Z

BTW, on macOS it is extremely important not to rely on ~ to get to the user home directory. One should use the "HOME" environment variable instead. The ~ only works in shell scripts.

I realize Qt no doubt provides a standard path that bypasses these concerns. But I thought I would mention it, since it is a common mistake on the macOS platform, and OP mentioned a lack of knowledge about macOS. (Also, the SMuFL standard erroneously mentions ~.)

Jojo-Schmitz · 2024-12-09T12:19:40Z

Right. I think we need to support the user-specific-location, and currently I don't see where it is supported.

It is, here:

    QStringList systemFontsPaths = QStandardPaths::standardLocations(QStandardPaths::GenericDataLocation).first(2);

using the first 2 entries of the list, user and system

rpatters1 · 2024-12-09T12:28:20Z

QStringList systemFontsPaths = ...

Might I suggest renaming this to standardFontPaths in that case? Using "system" to include both system and user is very confusing.

See musescore#25773 (review)

rpatters1 · 2024-12-09T12:39:32Z

I still would like to know the use-case for a user-facing private location. For one, looking at the changes, I am not certain the folder itself is actually user facing. It seems as though the intent is to remove the hard-coded list and instead store them there. If so, I get that use case. But if there a reason for the user to put fonts there, I don't understand it.

See musescore#25773 (review)

RootCubed · 2024-12-09T13:53:53Z

I still would like to know the use-case for a user-facing private location. For one, looking at the changes, I am not certain the folder itself is actually user facing. It seems as though the intent is to remove the hard-coded list and instead store them there. If so, I get that use case. But if there a reason for the user to put fonts there, I don't understand it.

TBH, I just did it because the MU3 implementations did it that way. I think the motivation is similar to how VSTs also have the option for custom storage locations - Fonts that install in non-standard-compliant locations, ability to install fonts on a separate drive, etc.

rpatters1 · 2024-12-09T14:03:01Z

I think the motivation is similar to how VSTs also have the option for custom storage locations - Fonts that install in non-standard-compliant locations, ability to install fonts on a separate drive, etc.

The more users are encouraged to do that, the less use external utilities like mine will have. But admittedly, if users are storing fonts in the private folder, no other programs will be able to access them. Perhaps that is incentive enough not to put them there.

src/notation/internal/engravingfontscontroller.cpp

cbjeukendrup · 2024-12-09T15:26:51Z

About the question whether we should support MuseScore-specific user-specified folders: we've been discussing this internally, and couldn't come up with a convincing use case for it. Perhaps, in order to encourage standardisation, we should not do it.
The reason that I originally did it that way, was that the information about standard paths was at that time either not available yet, or not known to me; or perhaps it was, but I did it this way because it was mostly intended as a quick-and-dirty prototype still.

It's a bit difficult to make a decision, because it feels weird to build something that has only questionable use cases, but at the same time it feels weird to discard it while 90% of it has already been built. Perhaps we can archive that part for now, and bring it back if and when we decide we still want it.

Jojo-Schmitz · 2024-12-09T15:38:49Z

Alone for testing new version of e.g. Leland private fonts would be useful

diedeno · 2024-12-09T16:23:45Z

Alone for testing new version of e.g. Leland private fonts would be useful

I agree. Some fonts are being actively developed. e.g. Sebastian https://github.com/fkretlow/sebastian/releases and this provides a risk free way to test.

rpatters1 · 2024-12-09T16:36:47Z

A private location for testing makes a lot of sense. I just am questioning whether it should be configurable thru the UI.

diedeno · 2024-12-10T18:09:33Z

Latest Linux build:
When using user-specific location, the directory name has to be exactly like the fontname in the metadata file.
so:
~/.local/share/SMuFL/Fonts/Sebastian/metadata.json = OK, but
~/.local/share/SMuFL/Fonts/sebastian/metadata.json does not work.
Also,
~/.local/share/SMuFL/Fonts/Finale Ash = OK but not FinaleAsh
This did work using the musescore-specific location in the previous build.
This is not a big problem of course.

Jojo-Schmitz · 2024-12-10T18:23:48Z

src/notation/internal/engravingfontscontroller.cpp

+
+        while (fontFiles.hasNext()) {
+            fontFiles.next();
+            metadataPath = fontFiles.filePath();


So the last .json file wins?, No order of precendencd, of Fontname.json, fontname_metadate.json, metadata.json?

The spec clearly states that the only standard filename is fontname.json. Is it really necessary to support non-standard installations? Also, the standard is very clear that the precedence is user-then-system.

(I realize that many on this thread already know this. I'm just explicitly bringing it out.)

I would argue that there is no reason to not support other, non-standard-compliant metadata file names, especially since this is a reality already (see e.g. Bravura, Leland, many others...)

In any case, there should always only be one .json file in that directory, so I went with this approach.

The precedence should be handled correctly - see scanAllDirectories.

Jojo-Schmitz · 2024-12-10T18:34:24Z

src/notation/view/widgets/editstyle.cpp

-    // It's also stored in score file as the musicalTextFont
-    musicalTextFont->addItem("Leland Text", "Leland Text");
-    musicalTextFont->addItem("Bravura Text", "Bravura Text");
-    musicalTextFont->addItem("Emmentaler Text", "MScore Text");


Note that this is the odd one out

This behaviour is kept by using fontFamilyName in line 832 of the same file.

Jojo-Schmitz · 2024-12-10T18:34:35Z

src/notation/view/widgets/editstyle.cpp

-    musicalTextFont->addItem("Leland Text", "Leland Text");
-    musicalTextFont->addItem("Bravura Text", "Bravura Text");
-    musicalTextFont->addItem("Emmentaler Text", "MScore Text");
-    musicalTextFont->addItem("Gonville Text", "Gootville Text");


Hmm, this one too

diedeno · 2024-12-10T19:06:25Z

These fonts are installed as fontname_regular.otf by the OS:
~/.local/share/fonts/Unknown Vendor/OpenType/Valerio$ ls
Valerio_Regular.otf

The other 2: Finale_Engraver_Regular.otf and Gootville_Regular.otf

For Valerio: the original filename is Valerio.otf, but installed Valerio_Regular.otf

How should this be handled in the SmuFL/Fonts directories?

RootCubed · 2024-12-10T19:15:03Z

Linux:

_19:48:25.023 | ERROR | main_thread | EngravingFontsController::scanDirectory | Music font not found for Gootville

19:48:25.027 | ERROR | main_thread | EngravingFontsController::scanDirectory | Music font not found for Finale Engraver

19:48:25.029 | ERROR | main_thread | EngravingFontsController::scanDirectory | Music font not found for Valerio__

These fonts are installed as fontname_regular.otf by the OS:

~/.local/share/fonts/Unknown Vendor/OpenType/Valerio$ ls

Valerio_Regular.otf

The other 2: Finale_Engraver_Regular.otf and Gootville_Regular.otf

For Valerio: the original filename is Valerio.otf, but installed Valerio_Regular.otf

How should this be handled in the SmuFL/Fonts directories?

Ugh yep, this is where it would have been handy to be able to use QFont for finding fonts... Unfortunately, I'm not sure what the best solution is here. (Ideally, we want to search for family names, not for font file names, but going through all ttf/otf files to look at the family name seems like a rather terrible idea)

Support usage of user-provided SMuFL fonts

c2ddac8

oktophonie requested review from oktophonie and cbjeukendrup December 8, 2024 14:28

Fix CI failure

a6903ec

Support standard SMuFL font locations

5fdade2

Jojo-Schmitz reviewed Dec 8, 2024

View reviewed changes

src/notation/internal/engravingfontscontroller.cpp Outdated Show resolved Hide resolved

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 8, 2024

Fix path on Windows where SMuFL expects system fonts to be

389b2c0

See musescore#25773 (review)

Jojo-Schmitz mentioned this pull request Dec 8, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be Jojo-Schmitz/MuseScore#720

Open

Fix SMuFL paths on Windows and Linux

98b2314

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 8, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be

734ecfd

See musescore#25773 (review)

Fix CI failure

ecd5f20

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 8, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be

546b684

See musescore#25773 (review)

Jojo-Schmitz reviewed Dec 8, 2024

View reviewed changes

src/notation/internal/engravingfontscontroller.cpp Outdated Show resolved Hide resolved

cbjeukendrup reviewed Dec 9, 2024

View reviewed changes

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 9, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be

bece4ec

See musescore#25773 (review)

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 9, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be

edc70e5

See musescore#25773 (review)

Jojo-Schmitz added a commit to Jojo-Schmitz/MuseScore that referenced this pull request Dec 9, 2024

Fix path on Windows and Linux where SMuFL expects system fonts to be

d315c0a

See musescore#25773 (review)

RootCubed commented Dec 9, 2024

View reviewed changes

src/notation/internal/engravingfontscontroller.cpp Outdated Show resolved Hide resolved

This comment was marked as resolved.

Sign in to view

RootCubed added 5 commits December 10, 2024 14:08

Apply code review, rework EngravingFontsController

74d7942

Remove IEngravingFontsProvider injection in uiconfiguration.h

c7ebe8a

Fix Linux compile error

2ca574a

Fix compiler warning (member initialization order)

5e29aa5

Fix Linux compile error

569d5dd

Jojo-Schmitz reviewed Dec 10, 2024

View reviewed changes

Support usage of user-provided SMuFL fonts #25773

Are you sure you want to change the base?

Support usage of user-provided SMuFL fonts #25773

Conversation

RootCubed commented Dec 8, 2024 • edited Loading

oktophonie commented Dec 8, 2024

RootCubed commented Dec 8, 2024

Jojo-Schmitz commented Dec 8, 2024 • edited Loading

RootCubed commented Dec 8, 2024

oktophonie commented Dec 8, 2024 • edited Loading

Jojo-Schmitz commented Dec 8, 2024 • edited Loading

RootCubed commented Dec 8, 2024

Jojo-Schmitz commented Dec 8, 2024 • edited Loading

RootCubed commented Dec 8, 2024

RootCubed commented Dec 8, 2024

cbjeukendrup commented Dec 9, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diedeno commented Dec 9, 2024 • edited Loading

diedeno commented Dec 9, 2024 • edited Loading

Jojo-Schmitz commented Dec 9, 2024 • edited Loading

RootCubed commented Dec 9, 2024

cbjeukendrup commented Dec 9, 2024

rpatters1 commented Dec 9, 2024

rpatters1 commented Dec 9, 2024

Jojo-Schmitz commented Dec 9, 2024

rpatters1 commented Dec 9, 2024 • edited Loading

rpatters1 commented Dec 9, 2024 • edited Loading

Jojo-Schmitz commented Dec 9, 2024 • edited Loading

rpatters1 commented Dec 9, 2024

rpatters1 commented Dec 9, 2024

RootCubed commented Dec 9, 2024

rpatters1 commented Dec 9, 2024

cbjeukendrup commented Dec 9, 2024

Jojo-Schmitz commented Dec 9, 2024

diedeno commented Dec 9, 2024

rpatters1 commented Dec 9, 2024

This comment was marked as resolved.

diedeno commented Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

diedeno commented Dec 10, 2024

RootCubed commented Dec 10, 2024 • edited Loading

RootCubed commented Dec 8, 2024 •

edited

Loading

Jojo-Schmitz commented Dec 8, 2024 •

edited

Loading

oktophonie commented Dec 8, 2024 •

edited

Loading

Jojo-Schmitz commented Dec 8, 2024 •

edited

Loading

Jojo-Schmitz commented Dec 8, 2024 •

edited

Loading

diedeno commented Dec 9, 2024 •

edited

Loading

diedeno commented Dec 9, 2024 •

edited

Loading

Jojo-Schmitz commented Dec 9, 2024 •

edited

Loading

rpatters1 commented Dec 9, 2024 •

edited

Loading

rpatters1 commented Dec 9, 2024 •

edited

Loading

Jojo-Schmitz commented Dec 9, 2024 •

edited

Loading

diedeno commented Dec 10, 2024 •

edited

Loading

RootCubed commented Dec 10, 2024 •

edited

Loading