googlefonts / fontc

Where in we pursue oxidizing (context: https://github.com/googlefonts/oxidize) fontmake.
Apache License 2.0
83 stars 14 forks source link

GS build with fontc raises OTS error + warning #582

Closed chrissimpkins closed 12 months ago

chrissimpkins commented 1 year ago

fontc source commit GS source commit

Fonts compiled in the default GH Actions Linux runner environment with the current stable Rust toolchain

ERROR: avar: Axis count mismatch
ERROR: Table discarded
WARNING: STAT: Unexpected non-zero offsetToAxisValueOffsets
File sanitized successfully!
anthrotype commented 1 year ago

Thanks Chris. The avar axis miscount is more worrying, we should look into it. The STAT is kind of a known issue in that we don't populate any axis values but only build a stub, just like varLib does by default, without full support for DSv5 axis labels (norad I think still doesn't parse those so fontc doesn't read them yet).

We have in the CI a simple script that runs OTS on a simple font but maybe it's better to run it on a full blown font like GS to catch more real world errors

rsheeter commented 1 year ago

+1 to OTS the GS we build

chrissimpkins commented 12 months ago

Added OTS tests on the Roman and Italic files in https://github.com/googlefonts/fontc/pull/589

chrissimpkins commented 12 months ago

The current error and warning do not yield a non-zero exit status code so CI will pass. The tests show the same ERROR/WARNING report that is shown in the OP here.

anthrotype commented 12 months ago

reopening to investigate if we can get rid of the STAT warning as well

anthrotype commented 12 months ago

apparently OTS is complaining that we have a non-zero offsetToAxisValueOffsets despite the axisValueCount is 0:

  if (this->axisValueCount == 0) {
    if (this->offsetToAxisValueOffsets != 0) {
      Warning("Unexpected non-zero offsetToAxisValueOffsets");
      this->offsetToAxisValueOffsets = 0;
    }

https://github.com/khaledhosny/ots/blob/be9445820c57ee12e738bd45b56f65648509682d/src/stat.cc#L91-L95

the generated_stat.rs looks like this?

    /// Offset in bytes from the beginning of the STAT table to the
    /// start of the design axes value offsets array. If axisValueCount
    /// is zero, set to zero; if axisValueCount is greater than zero,
    /// must be greater than zero.
    pub offset_to_axis_values: OffsetMarker<Vec<OffsetMarker<AxisValue>>, WIDTH_32>,

https://github.com/googlefonts/fontations/blob/dec4e1b073ce28c82df32f7c132517f46781dcd5/write-fonts/generated/generated_stat.rs#L19-L23

Basically if the axis_values array is empty, we want the offset to be 0. Do we need to use a NullableOffsetMarker perhaps?