search

harfbuzz / boring-expansion-spec

Better-Engineered Font Formats; Part 1. Boring Expansion
80 stars 9 forks source link

[`CFF2`] Conflicting text regarding the default vsindex when processing CharStrings #157

Open behdad opened 1 month ago

behdad commented 1 month ago

I'm surprised at what I think I'm seeing. Please correct me if I'm wrong.

Part of the CFF2 spec says:

By default, the first ItemVariationData structure (index 0) will be used. The vsindex operator can be used for glyphs that require a different list of regions.

But then there's text that says:

The vsindex key may be used in a PrivateDICT to select a different list of regions for the group of glyphs associated with that PrivateDICT. vsindex may also be used in a CharString to select the list of regions for that glyph. When used in a CharString, it overrides a setting in the associated PrivateDICT.

And in another place:

When used within a PrivateDICT, it has effect not only for variation of values specified by PrivateDICT keys but also for variation in all CharStrings associated with that PrivateDICT. However, a vsindex operator can also be used within a CharString, taking precedence over the vsindex specified in the PrivateDICT.

Humm. So, which is it. If a CharString doesn't have a vsindex operator, is the first ItemVariationData structure (index 0) used, or the vsindex specified in the PrivateDICT is used? Looks like the intention is the latter. I find the first quotation above misleading and suggest changing it to say that it defaults to 0 in the PrivateDICT case only.

I think I have unfortunately written some code, either in FontTools or HarfBuzz, that assumes that vsindex defaults to 0 in CharString processing.

skef commented 1 month ago

These specifications don't really conflict, they just don't pull the whole definition into one place. I believe its:

if vsindex operator in charstring:
   vsindex = (charstring vsindex operand)
else if vsindex operator in glyph's privatedict:
   vsindex = (privatedict vsindex operand)
else
   vsindex = 0
behdad commented 1 month ago

What you sketch is correct indeed. But I have in the past read "By default, the first ItemVariationData structure (index 0) will be used. The vsindex operator can be used for glyphs that require a different list of regions." and assumed the glyphs default to vsindex 0 if none is specified. I'm still auditing FontTools & HB code for that assumption I might have put into code.

skef commented 1 month ago

So we just need to tune up the language -- I'm fine with that.