Hhea.pm update() bug

[ClintGoss]

I believe that Hhea->update() needs the following modification. This is based on the convention of taking the "extent" for empty glyphs to be the LSB rather than the ADW. I've done some (but not extensive) testing on this ...

 for ($i = 0; $i < $num; $i++)
    {
        $aw = $hmtx->{'advance'}[$i];
        $lsb = $hmtx->{'lsb'}[$i];
        if (defined $glyphs->[$i])
        { $ext = $lsb + $glyphs->[$i]->read->{'xMax'} - $glyphs->[$i]{'xMin'}; }
        else
	# CG 5/26/2020 Extent of an empty glyph should be LSB + 0 glyph width.
	# Cannot find the definition of this anywhere, but this agrees with how FontCreator sets hhea.xMaxExtent
        # { $ext = $aw; }
	{ $ext = $lsb; }

        $maw = $aw if ($aw > $maw);
        $mlsb = $lsb if ($lsb < $mlsb or $i == 0);
        $mrsb = $aw - $ext if ($aw - $ext < $mrsb or $i == 0);
        $mext = $ext if ($ext > $mext);
    }

[bobh0303]

Actually, the OT spec says:

The values in the minRightSidebearing, minLeftSideBearing and xMaxExtent should be computed using only glyphs that have contours. Glyphs with no contours should be ignored for the purposes of these calculations. All reserved areas must be set to 0.

Certainly this means, in the least, that whitespace glyphs contribute to advanceWidthMax but not to any of the other three fields in question, so I suspect neither the current nor proposed code can be right.

But, imo, this actually opens a small can of worms. What exactly is meant by "glyphs that have contours" ? Does it mean that composite glyphs don't count since they don't actually have contours? I wouldn't think so, in which case the wording is poor.

Assuming the test of (defined $glyphs->[$i]) is adequate, a clearer organization of the code might be:

    for ($i = 0; $i < $num; $i++)
    {
        $aw = $hmtx->{'advance'}[$i];
        $maw = $aw if ($aw > $maw);
        if (defined $glyphs->[$i])  # actual condition to be determined
        { 
            $lsb = $hmtx->{'lsb'}[$i];
            $ext = $lsb + $glyphs->[$i]->read->{'xMax'} - $glyphs->[$i]{'xMin'}; }
            $mlsb = $lsb if ($lsb < $mlsb or $i == 0);
            $mrsb = $aw - $ext if ($aw - $ext < $mrsb or $i == 0);
            $mext = $ext if ($ext > $mext);
        }
    }

[ClintGoss]

What exactly is meant by "glyphs that have contours" ?

Hangs on the meaning of have ... bordering on an existential question ...

I'm thinking that if GID 0 does not pass the test

      if (defined $glyphs->[$i])  # actual condition to be determined

... then the reversed if tests might be a problem (even without use warnings) since $mlsb will not be set until a negative $lsb is encountered:

           $mlsb = $lsb if ($lsb < $mlsb or $i == 0);
           $mrsb = $aw - $ext if ($aw - $ext < $mrsb or $i == 0);

... I'm just imagining how this would work - I've done no testing on this.

[bobh0303]

I'm thinking that if GID 0 does not pass the test

 if (defined $glyphs->[$i])  # actual condition to be determined

... then the reversed if tests might be a problem (even without use warnings) since $mlsb will not be set until a negative $lsb is encountered...

Is this better?

    for ($i = 0; $i < $num; $i++)
    {
        $aw = $hmtx->{'advance'}[$i];
        $maw = $aw if ($aw > $maw);
        if (defined $glyphs->[$i])
        { 
            $lsb = $hmtx->{'lsb'}[$i];
            $ext = $lsb + $glyphs->[$i]->read->{'xMax'} - $glyphs->[$i]{'xMin'}; 
            $mlsb = $lsb if (!defined($mlsb) or $lsb < $mlsb);
            $mrsb = $aw - $ext if (!defined($mrsb) or $aw - $ext < $mrsb);
            $mext = $ext if ($ext > $mext);
        }
    }

[ClintGoss]

I made a small modification ...

        $maw = $aw if (!defined ($maw) or $aw > $maw);

... to avoid the situation of $maw never being set in the case of a (pathological?) font with only zero ADWs (and thus never being greater than the undefined $maw typecast to zero):

    for ($i = 0; $i < $num; $i++)
    {
        $aw = $hmtx->{'advance'}[$i];
        $maw = $aw if (!defined ($maw) or $aw > $maw);
        if (defined $glyphs->[$i])
        { 
            $lsb = $hmtx->{'lsb'}[$i];
            $ext = $lsb + $glyphs->[$i]->read->{'xMax'} - $glyphs->[$i]{'xMin'}; 
            $mlsb = $lsb if (!defined($mlsb) or $lsb < $mlsb);
            $mrsb = $aw - $ext if (!defined($mrsb) or $aw - $ext < $mrsb);
            $mext = $ext if ($ext > $mext);
        }
    }

I ran my largest pipeline process with this code installed in Hhea.pm::update(). The pipeline runs this code to produce 614 .ttf fonts. The results pass validation checks on the 'hhea' values set by this code (which had been failing because of incorrect MaxExtent values), so I think we're set.

Thanks for the assistance!!