adobe-type-tools / afdko

Adobe Font Development Kit for OpenType
https://adobe-type-tools.github.io/afdko/
Other
1.06k stars 167 forks source link

[hotconvert] BUG?/clarification on how HOT_ADD_STUB_DSIG is supposed to work. #1647

Open NSGod opened 1 year ago

NSGod commented 1 year ago

I've encountered what I believe might be a bug, though first I'd thought I'd get clarification on how the HOT_ADD_STUB_DSIG flag is supposed to be used. I don't know enough about how makeotfexe leverages hotconvert to know if it's possible that it would ever encounter this problem or not.

I have an app that's leveraging the hotconvert library as a framework to do the conversion of fonts to OTF data. I'm creating a single hotCtx and using it to convert more than one font. I'd like for each converted font to have a stub 'DSIG' table. It appears the convert flags are reset after

The following is an excerpt from a sample project I set up at NSGod/afdko/md-dsig-stub-bug-demo to demonstrate the problem I'm encountering (apologies but it's written in Objective-C):

static hotCtx                _hotCtx;
static hotCallbacks     _hotCallbacks;
static mdHotCallbacks { ....};

int main(int argc, const char * argv[]) {
   @autoreleasepool {
   srcDataHandle = [[FFMutableDataHandle alloc] init];
   CFFDataHandle = [[FFMutableDataHandle alloc] init];
   OTFDataHandle = [[FFMutableDataHandle alloc] init];

   _hotCallbacks = mdHotCallbacks;
   _hotCallbacks.ctx = NULL;
   _hotCtx = hotNew(&_hotCallbacks);
   hotSetConvertFlags(_hotCtx, HOT_ADD_STUB_DSIG);

   NSURL *tempDirURL = [[NSURL fileURLWithPath:NSTemporaryDirectory()]
                                                  URLByAppendingPathComponent:@"md-dsig-stub-fix-078j"];
   NSURL *pfaURL = [tempDirURL URLByAppendingPathComponent:@"type1.pfa"];

   for (NSUInteger i = 0; i < 4; i++) {
      srcDataHandle.data = [NSData dataWithContentsOfURL:pfaURL];

      int psInfo = 0;
      hotReadFontOverrides fontOverrides = {0};

      char *fontName = hotReadFont(_hotCtx, HOT_SUPRESS_HINT_WARNINGS |
                                                            HOT_NO_OLD_OPS, &psInfo, &fontOverrides);
      (void)fontName;

      /* If we don't set the HOT_ADD_STUB_DSIG flag each time through, only the first font
       produced will have a 'DSIG' table. Despite setting this flag each time, though, only
      the 'DSIG' table in the first font will be properly written; in subsequent fonts, the
      'DSIG' table will have zero data length.
       */
      hotSetConvertFlags(_hotCtx, HOT_ADD_STUB_DSIG);

      if (hotAddName(_hotCtx, HOT_NAME_MS_PLATFORM, HOT_NAME_MS_UGL,
                                   HOT_NAME_MS_ENGLISH, HOT_NAME_FAMILY, "Source Sans Pro")) {
      }
      if (hotAddName(_hotCtx, HOT_NAME_MAC_PLATFORM, HOT_NAME_MAC_ROMAN,
                                   HOT_NAME_MAC_ENGLISH, HOT_NAME_FAMILY, "Source Sans Pro")) {
      }

      hotCommonData common = {0};
      hotWinData win = {0};
      hotMacData mac = {0};

      win.Family = WINDOWS_ROMAN;   /* This is not currently used by the hot lib; */
                                  /* it currently always sets OS2.sFamily to "undefined". */
      win.CharSet = WIN_NONSYMBOLCHARSET;
      win.DefaultChar = WIN_SPACE; /* We don't have any fonts that use the bullet as the .notdef. */
      win.BreakChar = WIN_SPACE;

      common.flags = HOT_MAC;
      common.os2Version = 4;
      common.fsSelectionMask_on = USE_TYPO_METRICS;
      hotAddMiscData(_hotCtx, &common, &win, &mac);

      hotConvert(_hotCtx);
      // write out file here....
   }

As mentioned in the code comments, I need to call hotSetConvertFlags(_hotCtx, HOT_ADD_STUB_DSIG) each time through the loop otherwise only the first font will have a 'DSIG' table. I'm realizing now that this is likely the intended behavior. That said, though, subsequently-produced fonts will have a 'DSIG' entry, but the table will have a zero data length. (If you need to build this project, you'll need to do it on a Mac, and read the createXcodeProjectReadMe.txt for guidelines).

For example, running ttx on the subsequent fonts shows the following XML in the .ttx file:

  <DSIG ERROR="decompilation error" raw="True">
    <!-- An error occurred during the decompilation of this table -->
    <!-- Traceback (most recent call last):
           File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/fontTools/ttLib/ttFont.py", line 470, in _readTable
             table.decompile(data, self)
           File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/fontTools/ttLib/tables/D_S_I_G_.py", line 43, in decompile
             dummy, newData = sstruct.unpack2(DSIG_HeaderFormat, data, self)
           File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/fontTools/misc/sstruct.py", line 106, in unpack2
             return unpack(fmt, data[:length], obj), data[length:]
           File "/opt/local/Library/Frameworks/Python.framework/Versions/3.10/lib/python3.10/site-packages/fontTools/misc/sstruct.py", line 88, in unpack
             elements = struct.unpack(formatstring, data)
         struct.error: unpack requires a buffer of 8 bytes
          -->
    <hexdata>
    </hexdata>
  </DSIG>

I don't yet know enough about the makeotfexe/Python interface/Python testing side of things to know whether it'd be possible to contrive an example that would demonstrate the same problem using only the existing tools.

So my question is, is this correct, that I should call hotSetConvertFlags(_hotCtx, HOT_ADD_STUB_DSIG) each time through the loop (for each font to convert)?

In any case, I have what I believe to be a fix for this issue which I'll create a pull request for.