Skip to content

The GSUB lookup list is walked twice, where rebasing the offsets would do #122

Description

@jakejackson1

Follow-up to #112.

TTFontFile::_getGSUBtables() walks the GSUB LookupList twice. The old code did too - the
second walk was introduced by a comment reading // Now repeats as original to get Substitution rules - and #117 kept the shape while giving both walks the same reader:

$GSLookup = $this->readLookupList($lookupListOffset, $gsubOffset, 7);   // TTFontFile.php:1274
$Lookup = $this->readGSUBsubstitutions($lookupListOffset);              // TTFontFile.php:1275

and inside that second one (TTFontFile.php:1334):

foreach ($this->readLookupList($lookupListOffset, 0, 7) as $i => $lookup) {

The only difference between the two calls is the third argument: the first asks for subtable
offsets relative to the start of GSUB, because the shaper reads a cached copy of GSUB alone
and has no idea where in the file it came from; the second asks for them relative to the
start of the file, because these passes are reading the file itself.

That difference is arithmetic, not parsing. The second walk re-reads every lookup header and
re-resolves every Type 7 Extension indirection - a seek and three reads per extension
subtable - to arrive at the same numbers plus $gsubOffset.

The fix is already written, in the subclass

OtlDump::absoluteSubtables() (OtlDump.php:1930) does exactly this rebasing in six lines,
for the GPOS side:

private function absoluteSubtables(array $lookups, $tableOffset)
{
    foreach ($lookups as $i => $lookup) {
        foreach ($lookup['Subtables'] as $c => $offset) {
            $lookups[$i]['Subtables'][$c] = $tableOffset + $offset;
        }
    }

    return $lookups;
}

Moving it onto TTFontFile and having readGSUBsubstitutions() take the $GSLookup and
$gsubOffset that _getGSUBtables() already holds removes the second walk. The two
structures are keyed differently - ['Subtables'][$c] against
['Subtable'][$c]['Offset'] - so the mapping has to be written, which is the only reason
this is an issue and not a one-line change.

Worth how much

Once per font-cache build, over the largest lookup list in the font: 273 lookups for Noto
Sans, more for a full CJK face. Not a hot path, and this is filed for tidiness rather than
speed - the comment at TTFontFile.php:1325-1330 currently explains the second walk as
inherent when the subclass three files away shows it is not.

Safety

tests/data/fontcache/*.json pins everything _getGSUBtables() produces for every font in
tests/data/ttf, and tests/data/otldump/*.txt pins what the dump makes of the same
lookups.

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