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

Invalid type flags in the Bungee fonts #5724

Open
IvanUkhov opened this issue Dec 19, 2022 · 22 comments
Open

Invalid type flags in the Bungee fonts #5724

IvanUkhov opened this issue Dec 19, 2022 · 22 comments
Assignees
Labels
-- Needs Upstream Resolution Upstream fix required before moving forward I Font Bug
Milestone

Comments

@IvanUkhov
Copy link

Describe the bug
The fsType field in the OS/2 and Windows table is set to one in the following files:

  • ofl/bungeecolor/BungeeColor-Regular.ttf and
  • ofl/bungeespice/BungeeSpice-Regular.ttf.

This is not valid according to the specification:

Valid fonts must set at most one of bits 1, 2 or 3; bit 0 is permanently reserved and must be zero.

To Reproduce
Check bit 0 in OS/2. For instance, using fonttools:

from fontTools.ttLib import TTFont

font = TTFont("ofl/bungeespice/BungeeSpice-Regular.ttf")
font["OS/2"].fsType
# 1

Expected behavior
Bit 0 should not be set.

@anthrotype
Copy link
Member

thanks. It looks like bit 0 of OS/2.fsType field was incorrectly set directly in the source files, see https://github.com/djrrb/Bungee/blob/fc391285f3a5eb0968f0171a61d09112bc83d0c8/sources/2-build/Bungee_Color/Bungee_Color-Regular.ufo/fontinfo.plist#L100-L103

    <key>openTypeOS2Type</key>
    <array>
      <integer>0</integer>
    </array>

I believe the intention was to define the font as "installable embdedding" which correspond to value 0 (not bit 0) with all the bits unset.

We could maybe file the same issue in the upstream repository so it gets fixed in the sources.

@chrissimpkins
Copy link
Collaborator

Assigned @vv-monsalve to address this at the source level

@vv-monsalve
Copy link
Collaborator

@anthrotype Inspecting Recursive as a reference and opening the .ufo file in Glyphs app, everything looks as expected in the source file.

Screen Shot 2023-07-27 at 19 38 27

@anthrotype
Copy link
Member

anthrotype commented Jul 28, 2023

The bug was about Bungee, not Recursive. As the OP said, the bit 0 is reserved and must be left unset. If the intention is to have "installable" embedding then all the bits must be left unset.
So the openTypeOS2Type array in the UFO fontinfo.plist must be left empty, without that 0. The array contains the indices of the bits that should be set.

@vv-monsalve
Copy link
Collaborator

vv-monsalve commented Jul 28, 2023

Recursive as a reference

It is clear this issue is about Bungee fonts indeed.

What was not clear (since I'm not deeply familiarized .ufo sources), is how it should be set up in .ufo files. So I did two things when trying to validate how the 0 value must be set to have the "installable" configuration:

  • Inspected another .ufo source to see how it was done (so I used Recursive as a reference). When doing so, I saw it is settled the same way (as in the above link)
  • Opened the Bungee_Color-Regular.ufo file in Glyphs, and it is showing the "installable" option (that is the picture attached above).

After doing that, everything seemed to be as expected. This may not be conclusive, of course, but I was reporting it here to evaluate the situation.

I tried to use fonmatke with that Bungee_Color-Regular.ufo to confirm how the binary would end up, but it's reporting an error regarding an open contour, so it was not possible. The repository doesn't include build instructions, so no further build trials can be performed if there is a specific process for it.

So the openTypeOS2Type array in the UFO fontinfo.plist must be left empty, without that 0. The array contains the indices of the bits that should be set.

Now, what I understand from what you are saying here, is that it should be set up in one of the below options (could you confirm which one it should be?)

a.

    <key>openTypeOS2Type</key>
    <array>
    </array>

b

    <key>openTypeOS2Type</key>
    <array>
      <integer></integer>
    </array>

@anthrotype
Copy link
Member

a

@davelab6
Copy link
Member

Great work investigating this @vv-monsalve!

@justvanrossum do you have any suggestions how the sources got this way? :)

@vv-monsalve
Copy link
Collaborator

PR with a fix was made to Bungee's repo.

@vv-monsalve
Copy link
Collaborator

This issue was reported as already being addressed as part of a big update for the fonts.

@vv-monsalve
Copy link
Collaborator

Just addressed this in the sources at this commit of the Upstream repo

@vv-monsalve vv-monsalve added the -- Needs Upstream Resolution Upstream fix required before moving forward label Sep 5, 2023
@chrissimpkins
Copy link
Collaborator

I spoke with @justvanrossum and there are plans to cut a new release in the next several weeks. These changes will be included in the next release.

@vv-monsalve
Copy link
Collaborator

@justvanrossum, do you have an estimated date for the next release?

@justvanrossum
Copy link
Contributor

I just finished the last work and I will run a preview release later. Will post an update here.

@justvanrossum
Copy link
Contributor

Here is a preview:

The fonts for GF are in Bungee_Basic and Bungee_Color.

We ran fontbakery on individual fonts, as it isn’t really a RIBBI family. Here is how we invoke it:

@vv-monsalve
Copy link
Collaborator

Excellent! Thank you.

The fonts for GF are in Bungee_Basic and Bungee_Color.

I'll start assessing them in preparation for the update.

We ran fontbakery on individual fonts, as it isn’t really a RIBBI family. Here is how we invoke it:

I gues you are saying and using this to avoid some Fails reported for the fonts, so to better understand this I'll run the standard FB tests for GF and let you know from there. We'll need to be aware of which Fails are them.

@vv-monsalve
Copy link
Collaborator

I've confirmed the elided checks are valid and opened a new Issue with diffenator proofs for you to review/confirm on a few spotted issues.
I'll add more images for all the fonts that will be updated.

@vv-monsalve vv-monsalve modified the milestones: 2023 Q4, 2024 Q1 Dec 15, 2023
@chrissimpkins chrissimpkins modified the milestones: 2024 Q1, 2024 Q2 Jan 10, 2024
@davelab6
Copy link
Member

@justvanrossum please could you give us a status update on this, as we are planning @vv-monsalve 's time for Q1 and would like to know if you think you'll be ready to hand off to her in this period :)

@justvanrossum
Copy link
Contributor

Apologies, this somehow fell under my radar. Working on it now, should be completed soon.

@chrissimpkins
Copy link
Collaborator

Any updates here Just?

@justvanrossum
Copy link
Contributor

This has been long resolved. I received a follow-up issue:

To which I proposed a fix:

For which I haven't gotten any feedback.

@vv-monsalve
Copy link
Collaborator

Apologies. Bungee is on my project list, but I have yet to have the opportunity to return to it.

I'll update you as soon as I resume work on it. Confirming: Would you prefer I merge that PR as part of that process? If not, please feel free to merge it, and I'll work from the latest files on the master branch.

@justvanrossum
Copy link
Contributor

Would you prefer I merge that PR as part of that process? If not, please feel free to merge it, and I'll work from the latest files on the master branch.

I would prefer if you could checkout the PR branch and confirm whether it fixes djrrb/Bungee#108 in a satisfactory way.

But if it's easier for you if I merge now, and we amend later with a new PR if new issues are found, we can do that, too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-- Needs Upstream Resolution Upstream fix required before moving forward I Font Bug
Projects
Status: In Progress
Development

No branches or pull requests

8 participants