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

Support/use musical font files' x-offset for stems #672

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Jojo-Schmitz
Copy link
Owner

@Jojo-Schmitz Jojo-Schmitz commented Oct 5, 2024

Resolves: musescore#25088
Resolves: https://musescore.org/en/node/369769#comment-1261591

Needs special treatment for Bravura, Emmentaler (particularly bad metadata apparenty), Gonville and Petaluma, these fonts' metadata seem to disargee with the stems being offset by half a stem width to the left for upstem notes/chords.

@Jojo-Schmitz Jojo-Schmitz force-pushed the 3.x-musical-font-stem-x-offset branch 3 times, most recently from b56ed2c to db96eb7 Compare October 5, 2024 14:54
@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 5, 2024

This PR seems to prove that the CI action for vtests is basically useless :-(
It should have reported Emmentaler to fail as miserable as this
chord-layout-3-1 diff
So now the 'special' offset is only applied to Emmentaler quarter notes

But more exceptions are needed, a holy mess...

@Jojo-Schmitz Jojo-Schmitz force-pushed the 3.x-musical-font-stem-x-offset branch 2 times, most recently from 26aafcb to 0222fd4 Compare October 5, 2024 15:10
@worldwideweary
Copy link

Here's to hoping you get this resolved...
even though it's not a pressing issue, it sounds like a big enough task to get all of it squared away.
That old school Valerio font would be kind of fun to have working well

@Jojo-Schmitz
Copy link
Owner Author

I'm quite happy with it now, was able to reduce the exceptions to Bravura, Emmentaler, Gonville and Petaluma, where for Emmentaler and Gonvilla we might be able to instead fix their metadata.
Remaining issue with Valerio is beams:
stems-3 7-Valerio

@worldwideweary
Copy link

worldwideweary commented Oct 6, 2024

Oh yeah that looks better. Two things I noticed: Check out the braces for the staves... on a grand staff. Its y-offset is imbalanced, and not sure the cause:

image

Also, those stems for the whole notes look like ever so slightly could be benefited from having the y-starting positions be some how reduced (nit pick) so that the width of the stem doesn't show up inside the "empty" part of the note head and blends instead with the head's sort of "stemmy" portions. But whatever, it's not a big deal:

image

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 6, 2024

I wouldn't like to code Valerio exceptions, I'd rather restrict exeptions to builtin fonts that need them
But let's see whether y-offestting the stems for long notes by half a spatium.
Actually that seems to be the case, so I'll add it.

No good idea what's going wrong that that brace, it is only Valerio and only this brace, not braceSmall, braceLarge or braceLarger (which it'd fallback to be using Bravura's), Removing that brace's metadata doesn't seem to make any difference.
Special casing Valerio in bracket.cpp, line 133 helps:

diff --git a/libmscore/bracket.cpp b/libmscore/bracket.cpp
index 26f572fa48..aefd3ca0d7 100644
--- a/libmscore/bracket.cpp
+++ b/libmscore/bracket.cpp
@@ -130,7 +130,7 @@ void Bracket::setStaffSpan(int a, int b)

             if (v == 1)
                   _braceSymbol = SymId::braceSmall;
-            else if (v <= 2)
+            else if (v <= 2 && score()->styleSt(Sid::musicalSymbolFont) != "Valerio")
                   _braceSymbol = SymId::brace;
             else if (v <= 3)
                   _braceSymbol = SymId::braceLarge;

Or treat Valerio the same as Emmentaler and Gonville, i.e. draw the brace 'manually' rather than taking the font's glyph.
It'd then use braceLarge, and from Bravura, but I'm not sure I like that idea, see above. I'd rather have the font's Author to correct this in the font. I've asked in the Steinberg forum, https://forums.steinberg.net/t/valerio-a-16th-century-music-font/941064/13?u=jojo-schmitz

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 6, 2024

The Valerio brace issue is resolved, in version 1.001 of that font

@Jojo-Schmitz
Copy link
Owner Author

There are more issues, but without working vtests I can only judge from the vtests of the port of this PR to upstream master, see musescore#25050 (comment)

@worldwideweary
Copy link

Are the vtests in 3.7 not as robust as the 4.x series? I wonder "whadda they got that I ain't got?" as related to this repo

@Jojo-Schmitz
Copy link
Owner Author

They apparently don't do anything usefull at all. on upstream master the method has changed quite drastical, here not at all or no much vs. the former working ones upstream 3.x

@worldwideweary
Copy link

Damn, and since layout is different on 4, can't just easily backport them...

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 7, 2024

OK, taking a different approach now for the special treatments (changing the x-offsets for the 'broken' fonts), only for the affected glyphs (basically noteheadBlack and noteheadHalf) rather than the affected durations, either in code (for Bravura and Pertaluma) or via the fonts' metadata.json (for Emmentaler and Gonville). Similar for the defaults for noteheadDoubleHole (where all fonts seem to agree, except Legato)
Let's see whether that improves the failing vtests on master

@Jojo-Schmitz
Copy link
Owner Author

Jojo-Schmitz commented Oct 7, 2024

Fixed for Legato and also for AloisenU and AloisenGrooveU.

2 remaining issues (at least), in addition to the beaming issue with Valerio there's an issue with slurs, in all fonts (here shown with Valerio), and upstem as well as partly downstem:
image

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.

2 participants