vexflow icon indicating copy to clipboard operation
vexflow copied to clipboard

Gonville glyphs generated from remap only SMuFL

Open rvilarl opened this issue 2 years ago • 7 comments

@ronyeh I made a SMuFL version only remapping the glyphs to the SMuFL locations.

Mission impossible to get no visual differences, the glyphs in tools/fonts/other/gonville.ts differ to the ones in Gonville-18_20200703.otf.

I suggest to accept this differences otherwise we will not manage to get to use a SMuFL version.

rvilarl avatar Dec 06 '22 20:12 rvilarl

The problem is other projects rely on Gonville (OSMD and maybe others). So if we introduce a big visual change it'll "break" others' projects.

I agree that we should try to convert Gonville to SMuFL. Thanks for doing the hard work.

However, we should try as much as possible to minimize the visual differences. I remember when looking for the Gonville OTF, I couldn't find the original Gonville font file that was extracted into our glyphs/metrics data. So yes, it's likely that we can not reach 0 visual differences.

However, we should try our best to document what has changed, and possibly add patches to avoid the bigger changes. For example, I think the smaller treble clef was not a good change, since it is obviously different from the Gonville designer's intention (the example on his Gonville landing page has the bigger treble clef, and I think it's normal that the clef's spiral touches the E and B staff lines and is centered on the G line).

ronyeh avatar Dec 06 '22 20:12 ronyeh

@ronyeh there are not bigger changes just many changes. This has happened already in the past with other PRs.

rvilarl avatar Dec 06 '22 20:12 rvilarl

Thanks for the info. Let me review the visual changes before merging.

In the meantime, can you document (on the wiki) how you created this SMuFL version of Gonville? It might be helpful for us in the future, in case we need to convert / modernize some other legacy music font. What tools did you use? Any important gotchas or surprises?

Thanks!

ronyeh avatar Dec 06 '22 20:12 ronyeh

@ronyeh rebased to include the last merge in master

rvilarl avatar Dec 07 '22 23:12 rvilarl

I'm comparing the new images to the reference images, and it seems like the new ones are consistently bigger. Is there a way we can scale it down just a touch so that it is closer to the existing Gonville?

ronyeh avatar Dec 08 '22 00:12 ronyeh

Some example of the size changes. (references above and current below) Annotation Bottom_Annotation Gonville_reference Annotation Bottom_Annotation Gonville_current Annotation Harmonics Gonville_reference Annotation Harmonics Gonville_current Accidental Accidental_Arrangement_Special_Cases Gonville_reference Accidental Accidental_Arrangement_Special_Cases Gonville_current Formatter Proportional_Formatting___No_Tuning Gonville_reference Formatter Proportional_Formatting___No_Tuning Gonville_current

rvilarl avatar Dec 08 '22 05:12 rvilarl

I'm comparing the new images to the reference images, and it seems like the new ones are consistently bigger. Is there a way we can scale it down just a touch so that it is closer to the existing Gonville?

This is possible with two different approaches:

  • scale in the python script. This scritpt https://github.com/rvilarl/gonville/blob/main/gonville2smufl.py now includes 5 elements: original code, target code, x translation, y translation and rotation. I could add a 6th element scale.
  • modify the point elements in the metrics.

Looking at one example. Clef default point is 40 in Gonville and 32 for all the other SMuFL fonts. I think that the right approach would be to adjust the points in the metrics files. But is it worth the effort looking at the differences above?

Scaling the font is contra productive to reach a common metric file for all the fonts.

@ronyeh what do you think?

rvilarl avatar Dec 08 '22 05:12 rvilarl

@AaronDavidNewman @mscuthbert @sschmidTU are you fine with the minor size changes above?

rvilarl avatar Dec 10 '22 08:12 rvilarl

I think it looks fine - I had to zoom in to 250% before i could see the difference, even though the diagrams had pixel width on them.

I'm just curious, do we know the source of the difference? I don't know that there is a lot of science behind all the scale values, tbh.

In a number of the modifiers, I see a hard-coded font size of 38, but there is also NOTATION_FONT_SCALE which is 39. For some files, they are formatted with one value but then drawn with the other.

AaronDavidNewman avatar Dec 10 '22 20:12 AaronDavidNewman

I'm just curious, do we know the source of the difference? I don't know that there is a lot of science behind all the scale values, tbh.

The glyphs in tools/fonts/other/gonville.ts differ to the ones in Gonville-18_20200703.otf. Why? I do not know.

rvilarl avatar Dec 10 '22 22:12 rvilarl

@mscuthbert @sschmidTU what do you think? Are the differences acceptable?

rvilarl avatar Dec 12 '22 18:12 rvilarl

Good idea to ping the music experts. :-) I am not an expert in music engraving typography, so my main concern is that the new version looks different enough to "look worse" in subtle ways.

I think long term, we should have some "gold standard" engraved pieces that we should have VexFlow strive for. I like the Bach demo that we have, and I know that you have rendered more complicated pieces in VexFlow. Perhaps we can add those pieces into our test suite?

Once I am done with the image comparison tool, I can make a better recommendation. But my guess is that we should 1) merge the comparison tool, then 2) merge this PR assuming there are no big errors, and then 3) fix any small errors as they are reported.

ronyeh avatar Dec 12 '22 21:12 ronyeh

Once I am done with the image comparison tool, I can make a better recommendation. But my guess is that we should 1) merge the comparison tool, then 2) merge this PR assuming there are no big errors, and then 3) fix any small errors as they are reported.

This is a good approach, is the tool showing the overlap now?

rvilarl avatar Dec 12 '22 21:12 rvilarl

I would prefer no size or other changes, but if that's difficult, I'm fine with subtle changes, especially seeing that when adopting Vexflow >4 the visual diffs compared to our current OSMD/vexflow 1.2.93 version will probably be all over the whole sheet in a way that each sheet needs a manual examination of the diffs anyways.

sschmidTU avatar Dec 13 '22 17:12 sschmidTU

I would prefer no size or other changes, but if that's difficult, I'm fine with subtle changes, especially seeing that when adopting Vexflow >4 the visual diffs compared to our current OSMD/vexflow 1.2.93 version will probably be all over the whole sheet in a way that each sheet needs a manual examination of the diffs anyways.

Thanks! This is helpful.

Do you have a good example that you use for OSMD that we can add to the VexFlow tests (like our Bach Demo)? This example can include the most common symbols and ornaments that you'd encounter in a score. It doesn't have to be a complete piece or even a single piece (a couple measures from different pieces smashed together into one test score is fine by me).

ronyeh avatar Dec 13 '22 17:12 ronyeh

@ronyeh Would a MusicXML file as a test even be useful for you? We mostly only have tests like that. We have the "OSMD Function Test - All", which is a nice little collection of elements, though it's by now not half as complete as I'd like it to, maybe we'll expand it. But for what it has for now, it's a nice little test. image

OSMD_function_test_all.zip

sschmidTU avatar Dec 13 '22 18:12 sschmidTU

musicxml is great. I know Rodrigo has some expertise in converting it to VexFlow, haha. But perhaps it is time we considered adding a mxml parser into VexFlow itself.... Maybe for v5?

ronyeh avatar Dec 13 '22 18:12 ronyeh

@ronyeh That's basically what OSMD is, and we've been working on it for years, so... good luck ;) though I know Rodrigo is working on something like that as well.

sschmidTU avatar Dec 13 '22 21:12 sschmidTU

musicxml is great. I know Rodrigo has some expertise in converting it to VexFlow, haha. But perhaps it is time we considered adding a mxml parser into VexFlow itself.... Maybe for v5?

Hi @ronyeh I would leave Vexflow as a representation layer only. MusicXML is very complex.

@ronyeh That's basically what OSMD is, and we've been working on it for years, so... good luck ;) though I know Rodrigo is working on something like that as well.

Hi @sschmidTU the aim of VexMl is to test Vexflow using the Unofficial MusicXML Test Suite. I would have tried to do it with OSMD but merging Vexflow 4.0 seemed to be mission impossible. I have not given up though. :)

rvilarl avatar Dec 13 '22 23:12 rvilarl

Hi all --

I don't see a reason to prefer the old to the new or vice-versa. The changes are subtle enough that they're just a matter of taste. I do think though that this will be a significant change for users (it will require a lot of work to rebless all of the test images on my projects) so I hope that it's only done as part of a semantic-version change (5.0) and that the benefits justify the work for end users who care about pixel-consistent formatting.

Please, please, please don't include a MusicXML parser or any parser within Vexflow. Make it a separate project. (I was against including EasyScore as well). There are very different use cases for Vexflow and for MusicXML parsing. A MusicXML parser for Vexflow would be hugely welcomed, but this library should be for rendering objects passed to it, not for converting other formats to Vexflow objects. music21j has its own MusicXML parser and vexflow output engine. I'd hate to need to load in another parser that I won't be using.

(Oh, and just so I'm not thought of as not believing in the importance of MusicXML: I'll be taking over from Michael Good the position editor of the MusicXML standard starting in January 2023. So I definitely think it's amazing; but it is too complex to add on to this project, and coupling the two too tightly will hinder developing either)

mscuthbert avatar Dec 14 '22 00:12 mscuthbert

Thanks all for your input.

@mscuthbert so if we are able to get it to look the "same" as today, and of course with backwards compatible API, you are OK if we version it VexFlow 4.2.0? By "same" I mean not-very-perceptible differences that would pass our previous visual diff script threshold.

If we are unable to achieve the "same" Gonville output, you are OK with moving forward if we just bump our major revision to VexFlow 5.0?

ronyeh avatar Dec 14 '22 00:12 ronyeh

I don't see a reason to prefer the old to the new or vice-versa. The changes are subtle enough that they're just a matter of taste. I do think though that this will be a significant change for users (it will require a lot of work to rebless all of the test images on my projects) so I hope that it's only done as part of a semantic-version change (5.0) and that the benefits justify the work for end users who care about pixel-consistent formatting.

@mscuthbert Same, I don't mind the changes on principle, it's just that it's a subtle change that requires re-blessing a lot of images and verifying visual diffs, and some users might not like it. But as I said, it should be fine, especially with our Vexflow 4+ adoption requiring re-blessing and lots of visual diffs anyways.

And yes, I think MusicXML parsing in Vexflow directly is pretty much off the table after recent comments. (Maybe better as an external library as is)

By the way, congrats on the MusicXML standard role, sounds great! I think they'll be lucky to have you.

sschmidTU avatar Dec 14 '22 00:12 sschmidTU

I think my ugghh is that we're just in the process today of finishing the reblessing of thousands of images for Vexflow 4.0, so the prospect of doing it all again so soon is a bit agonizing to me.

Thanks @sschmidTU and @ronyeh and @rvilarl for all the great work!

mscuthbert avatar Dec 14 '22 00:12 mscuthbert

I think my ugghh is that we're just in the process today of finishing the reblessing of thousands of images for Vexflow 4.0, so the prospect of doing it all again so soon is a bit agonizing to me.

@mscuthbert first of all congratulations for the MusicXML role! Then I have two questions:

  • what font do you use?
  • which method do you use for comparing images? something similar to the threshold used in visual_regression.sh?

rvilarl avatar Dec 14 '22 07:12 rvilarl

There has to be a good way for us to scale down the Gonville SMuFL glyphs a little. They are consistently larger than the 4.1.0 release. In addition to being subtly different, it also changes formatting in some larger examples (e.g., Bach demo).

Here's one small case where I think 4.1.0 is better: gonville_time_sig

If you look at the time signatures in all our test images, the new version ("current" in the gif above) is no longer vertically centered. The bigger number glyphs mean that the time signature touches the top staff line, but just barely misses the bottom staff line.

Once again, I'm no music engraving expert. If someone has an official guide to music score typography, we could follow that. :-) But given that the previous version was vertically centered and this one isn't, I'd tend to lean toward vertically centered as the "more correct" output.

ronyeh avatar Dec 15 '22 09:12 ronyeh

Bach Demo. The new version has larger glyphs, so we are pushing everything to the right. This affects the formatting. "Reference" is 4.1.0. The dots are also larger/heavier in the new version.

gonville_bach

ronyeh avatar Dec 15 '22 09:12 ronyeh

As an aside, I just did some research on music engraving. :-)

https://www.youtube.com/watch?v=BvyoKdW-Big

The video is fascinating. The precursor to VexFlow, haha.

https://www.kazu-classicalguitar.co.uk/essays/music-engraving-legible-sheet-music

ronyeh avatar Dec 15 '22 09:12 ronyeh

@ronyeh please have a look at the images, I have scaled the font to 96%. The differences are really minor.

rvilarl avatar Dec 15 '22 17:12 rvilarl

@rvilarl Can you check this output file:

pptr-Articulation.Articulation___Staccato_Staccatissimo.Gonville.svg

The arrow symbol seems to have flipped directions. Is this a bug, or a fixed bug? arrows

ronyeh avatar Dec 16 '22 09:12 ronyeh

The new tool under vexflow/tools/compare/index.html can be used to review the changes to the Gonville font.

I use these search terms:

gonville !png

This narrows the list down to only SVG gonville files, which speeds up my review.

ronyeh avatar Dec 16 '22 09:12 ronyeh