-
Notifications
You must be signed in to change notification settings - Fork 71
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
Flip misnamed is_bitmap_embedding_allowed
function, increase lenience of embed permissions for older OS/2 versions
#137
Conversation
// Flag introduced in version 2 | ||
true | ||
} else { | ||
let n = Stream::read_at::<u16>(self.data, TYPE_OFFSET).unwrap_or(0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't we always return true for old versions because of unwrap_or(0)
? I agree that your code is more readable, just clarifying the logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old versions (specifically versions 0 and 1) neglected to specify that unused bits are zeroed, so they could have any value: "Applications must ignore bits 4 to 15 when reading a version 0 or version 1 table."
While unlikely any fonts in the wild actually did write non-zeroes here, might as well be strict with the implementation x3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right, my bad. I thought we're slicing the exact length, therefore read out of bound would always return 0.
I personally have no idea how it should work. And it's hard to test. Will see how CoreText handles it. |
I have checked FreeType, fontTools, and FontForge for their behaviour - forgive me if I am missing obvious ones, I am not a usual visitor to the realm of font parsing!
In general, it seems that the support of these flags is a bit hit-or-miss in terms of adherence to the Microsoft spec I posted above! |
I took a closer look and indeed As for other apps, I couldn't find a similar API in CoreGraphics/CoreText. Font Book on macOS and Explorer on Windows do show font's permissions, but I don't have a font with the bitmap bit set. Do you have any?
Well, then welcome to a barely documented mess. You might be overwhelmed by how much Microsoft spec documents, but it's just a tip of an iceberg. |
I have made up some barebones test fonts with the relevant flags set here. (I figured it would be legally dubious to set these flags on an actual font i just grabbed somewhere :P ) If rather you meant had I ever encountered these fonts in the wild, I can't say I have - I am just in the process of writing a software which may need to perform font embedding and I would like to respect the licensing rules of any font the user may import. On my current system, all of my fonts are FOSS, so certainly none there have such restriction for me to test! |
Interesting. Seems like Apple recognizes this flag. Here is what Font Book.app shows:
and just in case, here is Apple Color Emoji for the reference:
And its
Yeah, finding a real-world file that actually uses a specific feature is one of the challenges. |
I managed to get FontForge to export with OS/2 version 1 after much fiddling (not sure what i did to make it happen, the version dropdown doesn't actually work!) Please give it a test :3 Notably, I set the flags even when they're invalid to set on this version. FontForge reads them regardless, as does current |
So the same as |
So I guess the question now is whether behavior should match the spec to the letter, or follow what the rest of the ecosystem does - it seems most others parse and use the bits regardless of OS/2 version, as current master does. My purpose for following the spec was to achieve maximum leniency - allow embedding wherever possible, and sealing up those little edge-cases to make it more likely that embedding was allowed while still following the letter of the law :3 |
I think we can keep checking OS/2 version. It doesn't add much complexity and not many fonts still use old OS/2 versions. |
After referencing this Microsoft
fsType
specification, I noticed an inconsistency in the functions{Face, os2::Table}::is_bitmap_embedding_allowed
which seemed to have an inverted interpretation of the flag's meaning. The checked flag, if set, really means "is embed required to be bitmap". Since that name isn't very clear either, I inverted the name tois_outline_embedding_allowed
while keeping the logical return value unflipped.Further, the specification allows much leniency for earlier versions of the OS/2 table, this PR adjusts the logic of
os2::Table::{permissions, is_subsetting_allowed, is_outline_embedding_allowed}
to take advantage of this to be as lenient as allowable for embeds.After these changes, I am not sure the default values for when the table isn't found are correct. I assumed that, in a version prior to the flags' introduction, both subsetting and outline embedding are allowed due to the negative naming of the flags (eg. "no subsetting" implies subsetting is the default) but I was unable to find a resource that states this outright.