afdko icon indicating copy to clipboard operation
afdko copied to clipboard

[tx] (t1w) glyph type doesn't match font type

Open takaakifuji opened this issue 5 years ago • 10 comments

$ tx -t1 -o font.pfa font.ufo
tx: --- font.ufo
tx: (t1w) glyph type doesn't match font type

tx started to fail when font.ufo contains glyph names in the cid00000 format. It seems that the change in fb27836 causes this failure.

takaakifuji avatar Oct 26 '20 04:10 takaakifuji

I guess the UFO contain a mix of glyphs with cid00000 format names and non CID names? The assumption was that all glyphs in a CID-keyed font would be named with the cid00000 pattern and otherwise wouldn't be named like that, but I see now it's not the best assumption. We will adjust it to account for this.

punchcutter avatar Oct 26 '20 05:10 punchcutter

As far as I observe the issue with the 3.5.1 release, tx fails when at least one glyph is named with cid00000 format in the given UFO.

Talking of tx, I suppose the output of $ tx -t1 should always be name-keyed anyway. My concern is that such behavioral change of tx could potentially break the existing build pipelines for CJK fonts.

takaakifuji avatar Oct 26 '20 06:10 takaakifuji

This fix was meant only for UFO because tx has never correctly handled UFO as a CID-keyed source. The change shouldn't break existing build pipelines because that part never worked in the first place. It should only be a problem right now because of the mistaken logic in reading cidxxxxx names.

punchcutter avatar Oct 26 '20 06:10 punchcutter

After I posted to #1247, I managed to have a quick look at the codebase.

t1write.c ensures that the CIDFontName and the ROS are not empty: https://github.com/adobe-type-tools/afdko/blob/6b810de99b31e6e9c85331bb6a006b5603cd246e/c/public/lib/source/t1write/t1write.c#L1899-L1903

My take is to evaluate the same condition in prior to testing the name of each glyph in uforead.c:

if (h->top.cid.Registry.ptr != ABF_UNSET_PTR &&
    h->top.cid.Ordering.ptr != ABF_UNSET_PTR &&
    h->top.cid.Supplement != ABF_UNSET_INT &&
    h->top.cid.CIDFontName.ptr != ABF_UNSET_PTR) { ... }

https://github.com/adobe-type-tools/afdko/blob/6b810de99b31e6e9c85331bb6a006b5603cd246e/c/public/lib/source/uforead/uforead.c#L3455-L3466

tx complains if:

  1. ABF_CID_FONT has empty CIDFontName or ROS
  2. ABF_CID_FONT contains glyphs without ABF_GLYPH_CID flags

Thus, by this fix, we can let tx gracefully fail in case if 1. is false and 2. is true, which is seemingly not a valid input. I think it will eventually make the behaviour of tx consistent to the 3.5.0 version unless the newly-introduced keys appear in lib.plist. It doesn't look to me it's a good idea to use such glyph name convention to determine if the given UFO is CID-keyed or not, and it seems we already have the dedicated lib.plist keys for such purpose.

I'm willing to prepare any patch or test cases if you need.

takaakifuji avatar Oct 28 '20 03:10 takaakifuji

For a long time I had been using the following workflow to modify an existing glyph in a font:

  1. Extract the glyph to be modified, e.g. tx -t1 -decid -g 4321 cidfont.ps.OTC.HC glyphs.pfa
  2. Convert glyphs.pfa to ufo format and work with it. I choose .ufo as the working format because of the number of tools available.
  3. Convert the file back to pfa for merging, e.g. tx -t1 -g cid4321 glyphs.ufo glyphs_new.pfa.

In this case, the .ufo file stands in the middle of the workflow. It is not the source format, nor is it the final version before building the otf. Unlike the followed number, the cid prefix isn't important to me, just that they are named this way by the tx command with the -decid option, and there is no way to choose otherwise. But after updating to the latest version of AFDKO the command in step 3 fails with the error message (t1w) glyph type doesn't match font type because of the special handling of CID00000 names.

tamcy avatar Apr 10 '21 09:04 tamcy

I apologize for the inconvenience. This is entirely my fault for merging before the work was 100% done. This is fixed for an upcoming release where we are implementing support for CID-keyed UFO as a source. By default this will convert a CID-keyed file like cidfont.ps.OTC.HC into a CID-keyed UFO which can be edited and then converted back to a CID-keyed Type 1 font. Your old workflow will still work the same as before.

punchcutter avatar Apr 10 '21 11:04 punchcutter

There is no need to apologize, glad to know that the cid00000 naming can still be used (it's a pain to rename the glyph names). Thanks for the work!

tamcy avatar Apr 10 '21 14:04 tamcy

Do you currently have a plan to compile a brief documentation on the recent support for the CID-keyed UFO? The changes in #1353 sounds super promising to me, and I want to learn more about the design/usage/scenarios of the improvement when it gets ready. I also expect the patch would fix the regression issue mentioned here.

takaakifuji avatar May 17 '21 08:05 takaakifuji

Yes, once we fix up a couple things I plan to write some documentation.

punchcutter avatar May 17 '21 12:05 punchcutter

Thanks, that would definately help us a lot. Keep up the good work!

takaakifuji avatar May 18 '21 00:05 takaakifuji

This took longer than I expected, but proper CID-keyed UFOs are now possible. The latest release has everything and a guide is https://github.com/adobe-type-tools/afdko/blob/develop/docs/CIDKeyedUFOGuide.md

punchcutter avatar Apr 10 '23 12:04 punchcutter

@takaakifuji I'll close this issue now that 3.9.4 has a lot of UFO and CID-keyed UFO improvements. Please open a new issue if you run into anything else.

punchcutter avatar Apr 11 '23 05:04 punchcutter

@punchcutter Thank you for the reminder! I've just tested it for a limited amount of time, but all of the commands in the example worked as advertised. I'll report if we find any issues with this CID-keyed UFO thing.

takaakifuji avatar Apr 12 '23 06:04 takaakifuji