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

Figure out why our Oswald has significantly different glyphs than fontmake's #250

Closed
rsheeter opened this issue Apr 10, 2023 · 26 comments
Closed
Assignees

Comments

@rsheeter
Copy link
Contributor

rsheeter commented Apr 10, 2023

So significantly so it makes me wonder if we are using the wrong master as default or something of that nature. Also the point order seems ... rotated?

image

The diff is huge; it will probably be helpful to start with a version of the source where most of the glyphs are deleted.

@anthrotype
Copy link
Member

anthrotype commented Apr 11, 2023

The difference in glyph names is because in python fontmake the glyph names are renamed to final, "production" name (following AGLFN conventions), this is done in the ufo2ft postProcessor based on a number of conditions (e.g. presence of public.postscriptNames, if I remember the name of the lib key correctly, typing from memory) and options. It is done as a post-processing step because the feature file is supposed to contain source glyph names.

The difference in point order is because fontmake-py reverses the contour direction by default when generating TrueType outlines, because it assumes that inputs are postscript cubic outlines and as such in counterclockwise direction as PS recommends, so the result of the direction flipping is that TT outlines get clockwise direction as recommended by the respective spec.
#174

@anthrotype
Copy link
Member

I like how the spell checker corrects fontmake as "don't make"

@anthrotype
Copy link
Member

For better comparisons you can disable glyph renaming with --no-production-names, and also --keep-direction to not reverse contour direction.

@rsheeter
Copy link
Contributor Author

The flags are very handy short-term, ty. Should we ultimatley just implement the renaming and direction shifting?

@rsheeter
Copy link
Contributor Author

fontmake

  <avar>
    <segment axis="wght">
      <mapping from="-1.0" to="-1.0"/>
      <mapping from="-0.5" to="-0.625"/>
      <mapping from="0.0" to="0.0"/>
      <mapping from="0.3333" to="0.5161"/>
      <mapping from="0.6667" to="0.7871"/>
      <mapping from="1.0" to="1.0"/>
    </segment>
  </avar>

  <fvar>

    <!-- Weight -->
    <Axis>
      <AxisTag>wght</AxisTag>
      <Flags>0x0</Flags>
      <MinValue>200.0</MinValue>
      <DefaultValue>400.0</DefaultValue>
      <MaxValue>700.0</MaxValue>
      <AxisNameID>256</AxisNameID>
    </Axis>

    <!-- ExtraLight -->
    <NamedInstance flags="0x0" subfamilyNameID="257">
      <coord axis="wght" value="200.0"/>
    </NamedInstance>

    <!-- Light -->
    <NamedInstance flags="0x0" subfamilyNameID="258">
      <coord axis="wght" value="300.0"/>
    </NamedInstance>

    <!-- Regular -->
    <NamedInstance flags="0x0" subfamilyNameID="259">
      <coord axis="wght" value="400.0"/>
    </NamedInstance>

    <!-- Medium -->
    <NamedInstance flags="0x0" subfamilyNameID="260">
      <coord axis="wght" value="500.0"/>
    </NamedInstance>

    <!-- SemiBold -->
    <NamedInstance flags="0x0" subfamilyNameID="261">
      <coord axis="wght" value="600.0"/>
    </NamedInstance>

    <!-- Bold -->
    <NamedInstance flags="0x0" subfamilyNameID="262">
      <coord axis="wght" value="700.0"/>
    </NamedInstance>
  </fvar>

fontmake-rs

  <fvar>

    <!-- Weight -->
    <Axis>
      <AxisTag>wght</AxisTag>
      <Flags>0x0</Flags>
      <MinValue>200.0</MinValue>
      <DefaultValue>200.0</DefaultValue>
      <MaxValue>710.0</MaxValue>
      <AxisNameID>256</AxisNameID>
    </Axis>
  </fvar>

The different choice of default likely explains why all our glyph coordinates are different.

@rsheeter
Copy link
Contributor Author

rsheeter commented Apr 14, 2023

Much better after #252. A few major sources of difference remain:

  1. We don't remove impliied on-curves. For example, this on-curve could have been omitted:
         <pt x="179" y="-8" on="0"/>
         <pt x="214" y="11" on="1"/> <!-- I'm unnecessary! -->
         <pt x="250" y="30" on="0"/>
  2. We convert things to contours that fontmake does not (Only convert to contours if basis vectors vary #266)
    <!-- python -->
     <TTGlyph name="uacute" xMin="50" yMin="-8" xMax="385" yMax="824">
       <component glyphName="u" x="0" y="0" flags="0x204"/>
       <component glyphName="acutecomb" x="220" y="0" flags="0x4"/>
     </TTGlyph>
    
    <!-- Rust -->
     <TTGlyph name="uacute" xMin="50" yMin="-8" xMax="385" yMax="824">
       <contour>
         <pt x="137" y="-8" on="1"/>
         <pt x="179" y="-8" on="0"/>
         <pt x="214" y="11" on="1"/>
         <pt x="250" y="30" on="0"/>
         <pt x="278" y="57" on="1"/>
         <pt x="278" y="0" on="1"/>
         <pt x="385" y="0" on="1"/>
         <pt x="385" y="578" on="1"/>
         <pt x="278" y="578" on="1"/>
         <pt x="278" y="112" on="1"/>
         <pt x="260" y="97" on="0"/>
         <pt x="241" y="87" on="1"/>
         <pt x="221" y="76" on="0"/>
         <pt x="203" y="76" on="1"/>
         <pt x="175" y="76" on="0"/>
         <pt x="166" y="94" on="1"/>
         <pt x="157" y="111" on="0"/>
         <pt x="157" y="142" on="1"/>
         <pt x="157" y="578" on="1"/>
         <pt x="50" y="578" on="1"/>
         <pt x="50" y="110" on="1"/>
         <pt x="50" y="82" on="0"/>
         <pt x="57" y="55" on="1"/>
         <pt x="65" y="28" on="0"/>
         <pt x="84" y="10" on="1"/>
         <pt x="103" y="-8" on="0"/>
       </contour>
       <contour>
         <pt x="192" y="626" on="1"/>
         <pt x="246" y="626" on="1"/>
         <pt x="358" y="824" on="1"/>
         <pt x="251" y="824" on="1"/>
       </contour>
       <instructions/>
     </TTGlyph>
  3. We emit different component flags (Set USE_MY_METRICS similar to how ufo2ft does it #267)
    <!-- python -->
     <TTGlyph name="u-cy" xMin="12" yMin="-145" xMax="381" yMax="578">
       <component glyphName="y" x="0" y="0" flags="0x204"/>
     </TTGlyph>
    
    <!-- Rust -->
    <TTGlyph name="u-cy" xMin="12" yMin="-145" xMax="381" yMax="578">
       <component glyphName="y" x="0" y="0" flags="0x4"/>
     </TTGlyph>

@anthrotype
Copy link
Member

I have WIP code that I didn't push yet for removing implied oncurves, will do when I get back next week.
For the component flags, I think we are not setting the USE_MY_METRICS flag, whereas fontmake (ufo2ft) is. You set that to the first component whose base glyph has the same advance width as the composite glyph itself (honestly I don't remember wether/why that is useful at all, could be some ancient optimisation).
I don't know why the composite glyph in your ttx dump above was decomposed to simple contours.

@rsheeter
Copy link
Contributor Author

rsheeter commented Apr 14, 2023

I don't know why the composite glyph in your ttx dump above was decomposed to simple contours.

Safe bet that I screwed something up :D

QQ, I thought we said fontmake only went one level deep with components by default but when I build Oswald with fontmake and inspect maxp it seems to be saying otherwise?

  <maxp>
    ...
    <maxComponentElements value="4"/>
    <maxComponentDepth value="3"/> <!-- isn't this supposed to be 0 or 1? -->
  </maxp>

@rsheeter rsheeter reopened this Apr 14, 2023
@rsheeter
Copy link
Contributor Author

(apparently if you press enough keys you can accidentally close an issue)

@anthrotype
Copy link
Member

No, what we said is fontmake keeps nested composite glyphs by default, but ufo2ft has an optional filter that flattens them to depth 1, and gftools builder activates that by default

@rsheeter
Copy link
Contributor Author

Ah yes, ty, I forgot gftools runs fontmake with unique arguments. Amusingly I just ran into the same thing with Roboto Serif: gftools effectively runs fontmake -m roboto-serif/sources/RobotoSerif.designspace -o variable --filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter

@rsheeter
Copy link
Contributor Author

rsheeter commented Apr 14, 2023

Updated the instructions on the milestone to suggest building with --filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter. Once we have implicit on-curves and USE_MY_METRICS glyf will be pretty similar. Still seeing some cases where fontc is more enthused about conversion to contours though.

@anthrotype
Copy link
Member

Still seeing some cases where fontc is more enthused about conversion to contours though

I wonder if it's because you're traversing the component tree in breadth-first order (you pop from the front/left of the frontier and extend at the back, rather than at the front), whereas ufo2ft deepCopyContours function does a (recursive) deep-first traversal:
https://github.com/googlefonts/ufo2ft/blob/0c0a570b84d1351ab704ba1fa5ae03aeef51179f/Lib/ufo2ft/util.py#L166

@anthrotype
Copy link
Member

hm.. I tried to follow the instructions from the https://github.com/googlefonts/fontmake-rs/milestone/4 but when I try to build Oswald.glyphs (checked out at latest commit from upstream Oswald repo), I get this error from fontc:

$ rm -rf build && cargo run -p fontc -- --source ../OswaldFont/sources/Oswald.glyphs
    Finished dev [unoptimized + debuginfo] target(s) in 0.26s
     Running `target/debug/fontc --source ../OswaldFont/sources/Oswald.glyphs`
2023-04-18T16:26:21.459033Z: ThreadId(1): ERROR: Fe(Glyph(commaturnedmod)) failed Fe(NoMasterForGlyph { master: "3FB85DAF-C7EF-4F81-BE96-1E190958879E", glyph: commaturnedmod })
Error: TasksFailed([(Fe(Glyph(commaturnedmod)), "No master 3FB85DAF-C7EF-4F81-BE96-1E190958879E exists. Referenced by glyph commaturnedmod.")])

@rsheeter
Copy link
Contributor Author

In case anyone else looks here, use https://github.com/googlefonts/OswaldFont at 5298dbd8f4478518940aa047b1f498225a87b9ed. The latest (at time of writing) commit, 2570e33cf0e5fbadd4781108cbbf98f57e7faec3, introduced variation incompatible glyphs that fontc chokes on. #263 tracks adding a flag to ignore such problems.

@rsheeter
Copy link
Contributor Author

rsheeter commented Apr 18, 2023

A fourth source of difference, fontmake seems to avoid glyph components that are themselves components. For example:

<!-- both --> 
    <TTGlyph name="A">...simple glyph...</TTGlyph>
    <TTGlyph name="A-cy" xMin="19" yMin="0" xMax="473" yMax="810">
      <component glyphName="A" x="0" y="0" flags="0x204"/>
    </TTGlyph>

<!-- fontmake (python) directly references A -->
    <TTGlyph name="Abreve-cy" xMin="19" yMin="0" xMax="473" yMax="1009">
      <component glyphName="A" x="0" y="0" flags="0x204"/>
      <component glyphName="brevecomb-cy" x="-53" y="232" flags="0x4"/>
    </TTGlyph>

<!-- fontc indirectly references A by way of A-cy (which is A with a 0 offset) -->
    <TTGlyph name="Abreve-cy" xMin="19" yMin="0" xMax="473" yMax="1009">
      <component glyphName="A-cy" x="0" y="0" flags="0x204"/>
      <component glyphName="brevecomb-cy" x="-53" y="232" flags="0x4"/>
    </TTGlyph>

Guessing this is the work of one of the filters (--filter FlattenComponentsFilter --filter DecomposeTransformedComponentsFilter, see https://github.com/googlefonts/fontmake-rs/milestone/4)

@rsheeter
Copy link
Contributor Author

A fifth source of difference, there are glyphs where the bbox (xMin/Max, yMin/Max) fontc produces doesn't match that of fontmake. At a glance I think it's when an off-curve point contains the extrema; perhaps I managed to only included on-curve when computing (?)

@anthrotype
Copy link
Member

fontmake seems to avoid glyph components that are themselves components

yeah, that's the result of --filter FlattenComponentsFilter, which un-nests components; it is not the default fontmake behavior and I am not 100% convinced it should be the default for the new fontmake-rs. I'm a bit wary to make the latter too opinionated and GF-specific. A font containing a component of a component (of a component, etc.) is perfectly valid anad up to spec. Only very ancient PS printers are reported to have issues with nested components (/cc I remember @moyogo knew more details), and gftools probalby likes to err on the side of caution.

@moyogo
Copy link

moyogo commented Apr 19, 2023

@anthrotype I don’t think this only affects "ancient" PS printers, but I have no data to prove nor disprove that.
Turn it off and we’ll find out 😉

@anthrotype
Copy link
Member

yeah, that "ancient" was my wishful thinking perhaps.

BTW, this is the discussion in FontBakery that made us enable that filter by default in gftools:
fonttools/fontbakery#2961

@anthrotype
Copy link
Member

Turn it off and we’ll find out

worth noting that vanilla fontmake does not enable this by default and I don't remember having received reports that this should be turned on by default from non gftools.builder users of fontmake.

@moyogo
Copy link

moyogo commented Apr 19, 2023

@anthrotype
https://github.com/google/fonts/search?q=nested+components&type=issues (specifically google/fonts#5120 google/fonts#2848)

@anthrotype
Copy link
Member

glyphs where the bbox (xMin/Max, yMin/Max) fontc produces doesn't match that of fontmake

perhaps related to faulty bbox, I also noticed a sixth difference, that some of the left sidebearings (seems to be from the composite glyphs) are incorrectly set to 0 in the font built by fontc

   <hmtx>
     <mtx name=".notdef" width="682" lsb="90"/>
     <mtx name="A" width="492" lsb="19"/>
-    <mtx name="A-cy" width="492" lsb="19"/>
+    <mtx name="A-cy" width="492" lsb="0"/>
     <mtx name="AE" width="635" lsb="-51"/>
     <mtx name="AEacute" width="635" lsb="-51"/>
     <mtx name="Aacute" width="492" lsb="19"/>
@@ -1047,9 +1047,9 @@
     <mtx name="Ccircumflex" width="515" lsb="48"/>
     <mtx name="Cdotaccent" width="515" lsb="48"/>
     <mtx name="Che-cy" width="551" lsb="50"/>
-    <mtx name="Cheabkhasian-cy" width="695" lsb="15"/>
+    <mtx name="Cheabkhasian-cy" width="695" lsb="17"/>
     <mtx name="Chedescender-cy" width="574" lsb="49"/>
-    <mtx name="Chedescenderabkhasian-cy" width="685" lsb="15"/>
+    <mtx name="Chedescenderabkhasian-cy" width="685" lsb="17"/>

@anthrotype
Copy link
Member

the question is, do we want to keep repeating ourselves for the next thirty years that "some printer drivers do not handle nested components, or transformed components properly", or do we finally get to the bottom of this and give names and surnames to this buggy pieces of software and demand they get fixed finally? fontmake-rs is designed to be VF first, and VFs are relatively new technology, so we should be able to allow ourselves to break backward compatibility with pre-VF software. And one can always do the un-nesting and component decomposition when generating static instances that are meant to be used for non-VF-aware software like these unspecified "PS printer drivers"...

@anthrotype
Copy link
Member

anthrotype commented Apr 19, 2023

Regarding the --filter DecomposeTransformedComponentsFilter which gftools.builder sets true by default, while fontmake leaves it optional; there are two reasons why that can be enabled:

  1. once again, the infamous old PS printer drivers;
  2. because in VF only component translation can vary so if different masters use different 2x2 transforms the component can't variate.

Now, regarding the 2), unfortunately fontmake-py still does not do the right thing of decomposing components when an incompatible 2x2 transform occur in some masters -- it has been suggested several times but never implemenetedm cf. googlefonts/fontmake#595 (fontmake-rs correctly does that as of , whereas fontmake-rs does that now as of #266); however, since the optional ufo2ft DecomposeTransformedComponentsFilter was made mostly to tackle issue 1) (see googlefonts/ufo2ft#399), people who want to accomplish 2) also end up wanting to use that. But this gets rid not only of components whose 2x2 transform is different across masters, but of all transformed components everywhere, which is in my opinion too greedy and unwarranted, and I would like to insist that fontmake-rs does not do that by default.

@moyogo
Copy link

moyogo commented Apr 20, 2023

One issue with DecomposeTransformedComponentsFilter is that it filters per master instead of across all masters. So if one master doesn’t have transformed component and another does, an incompatibilty is introduced.
Both Filters and FeatureWriters need to be VariableFilters and VariableFeatureWriters that work across all masters before they write.

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

No branches or pull requests

3 participants