Skip to content

getHMTX and getCMAP4 write their results onto the parser instead of returning them #120

Description

@jakejackson1

Follow-up to #91.

#91 named one open design call:

getCMAP4 and getHMTX being shared is the one real design call: either the subsetter
keeps the parser for them, or they move to it.

#117 took the first option: FontSubsetter holds a TTFontFile, opens the file through it,
and asks it for seek_table, get_table_pos, getCMAP4 and getHMTX. That is the smaller
change and it works, but it only half-settles the question, because those two readers do not
return what they read - they write it onto the parser:

reader writes
TTFontFile::getHMTX(), TTFontFile.php:3456 $this->charWidths, $this->defaultWidth
TTFontFile::getCMAP4(), TTFontFile.php:3545 $this->maxUniChar

Both already take the two maps they build by reference, so returning the other three is the
only change the signature needs.

What it costs as it stands

  • FontSubsetter::defaultWidth() and maxUniChar() exist only to reach back through the
    composition for results the call should have handed over.
  • After a makeSubset(), the TTFontFile's charWidths, defaultWidth and maxUniChar
    describe the subsetting run rather than any getMetrics() run, so the object's fields no
    longer answer "what did this parser read".
  • Read each OTL table once, split the parser from the subsetter, split applyOTL, and clear the follow-ups (#90, #91, #92, #112, #118-#122) #117 shipped a bug of exactly this kind and caught it in review: FontSubsetter had
    declared its own $maxUniChar, which shadowed the parser's - and the parser's is the one
    getHMTX sizes the width table from. Only a font with a format 12 cmap would have reached
    it, and no font in tests/data/ttf has one, so no fixture would ever have said so.

That last point is the argument for doing this: the ownership is unstated, so getting it
wrong is silent.

What a fix looks like

getHMTX() returns [$charWidths, $defaultWidth] and getCMAP4() returns $maxUniChar.
Ten call sites across TTFontFile, OtlDump and FontSubsetter. extractInfo() is the
one that then has to assign charWidths itself, which is the point - it is the caller that
wants it.

With that done, FontSubsetter uses $this->font only for the table directory, which is
the borrowing its class docblock already claims.

Safety

tests/data/fontcache/*.json pins everything extractInfo produces and
tests/data/subset/*.json pins every byte of all 138 font programs, including the
defaultWidth and maxUniChar the writer hands back. Both cover every font in
tests/data/ttf.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions