Fixes #88 - created and modified are not parsed/encoded correctly

[petergoldstein]

The created and modified values in the TTF header is a custom type called a long date time. A definition can be found here:

https://developer.apple.com/fonts/TrueType-Reference-Manual/RM06/Chap6.html

The bytes are big endian encoded, and the basis is offset from standard Epoch. To make these values at all useful in Ruby, they need to be properly decoded from bytes and then offset corrected to get a Time. Similarly, we need to do the reverse transition when encoding in the header.

Fixes #88

@gettalong and @pointlessone , a technical review would be helpful if you've got the time. Thanks.

[gettalong]

Why not use Time.new(1904, 1, 1) and use that for calculations?

[petergoldstein]

Sure, that'd be fine and probably clearer.

[petergoldstein]

Need to include all fields plus the TZ to get it right, but I've made that change.

[gettalong]

Looks good to me!

[pointlessone]

1. I think it's more complicated than it needs to be. We never use these values for anything so for a long time we were fine with them being deserialized incorrectly: endiannes would be OK when we incorrectly serialized them back. We surely can fix the endiannes but neither Prawn, nor TTFunk care for what this value is exactly.
2. These attributes are a public API. Since the dawn of time they were integers. We probably should be a little cautious changing types.

[petergoldstein]

@pointlessone So honestly, those comments don't make sense to me. At a minimum they are contradictory, and more generally they seem to ignore the fact that this is a Ruby library.

We can't at the same time say "we don't care about what these values are" and "we shouldn't change them because they're in a public API". It's one or the other. As the values have simply never been even remotely correct, I don't see why there's an issue with changing them. No one could have possibly ever used the values for anything.

Moreover, as this is a Ruby library, it should conform to Ruby's extremely well-established conventions for time. Ruby uses Epoch time - a zero of January 1, 1970 00:00:00 UTC - a convention it inherited from UNIX. Using an alternate zero - in this case January 1, 1904 00:00:00 UTC - is going to be confusing to everyone.

If the created value is a long, then the expectation of a Ruby developer is that the value is seconds since Epoch and that this code works:

 f = TTFunk::File.open(File.join(File.expand_path('spec/fonts', __dir__), 'DejaVuSans.ttf''))
created_as_time = Time.at(f.header.created)

It doesn't. created_as_time winds up being offset 66 years into the future from the correct value.

In my view there's no point having incorrect, confusing values in a public API. If the values are incorrect, you fix them, you don't just say "eh, doing this right is just too complex". Especially since we're talking about addition here.

[pointlessone]

We can't at the same time say "we don't care about what these values are" and "we shouldn't change them because they're in a public API". It's one or the other.

No, it's both.

• "We don't care what these values are". Here "we" refers to Prawn. No gem in Prawn uses these values for anything. From the projects point of view it's an opaque value. It could be a raw bytes string and it wouldn't matter to Prawn.
• "We shouldn't change them because they're in a public API". This is a statement about introduction of breaking changes to the API. We're trying to follow SemVer. We have to be concious about breaking changes in TTFunk because it's both past 1.0 and is actually used outside of Prawn.

I agree that having them as a Time is better. I recognize that these particular fields are probably not that important as they're basically arbitrarily set in font files.

Yet, I want us to take a little bit more serious approach towards breaking changes.

[jenskutilek]

I care about those values. I’m building my own fonts and reading this value when the fonts are analyzed for inclusion in our web shop.

I’ve been working around this issue like this, using the same approach as the established Python FontTools:

  def date_from_sfnt_timestamp(value)
    # Convert the head created or modified timestamp to a readable value.
    return Time.zone.now unless value

    # FIXME: Work around endianness bug in ttfunk
    value = [value].pack("q").unpack1("q>")

    # Timestamp is out of range; ignore top bytes
    value &= 0xFFFFFFFF if value > 0xFFFFFFFF

    # Timestamp seems very low; regard as unix timestamp
    value += 0x7C259DC0 if value < 0x7C259DC0 # January 1, 1970 00:00:00

    Time.at([0, value + Time.new(1904, 1, 1, 0, 0, 0, 0).to_i].max)
  end

ttf = TTFunk::File.open(...)
date_from_sfnt_timestamp(ttf.header.modified)

probably not that important as they're basically arbitrarily set in font files.

Most font editors set those fields to a reasonable value by default nowadays. I agree that for the average font user, it is probably not very important.

[pointlessone]

@petergoldstein I took a look a this again and I think we should keep created and modified as integers. Yes, let's fix endianness but let's keep the types. Let's keep the helper methods for parsing into Time, too.

We might also add created_time (or created_at if you prefer the Rails Way) and modified_time that would return parsed Time. But this might get complicated in terms of keeping both versions in sync.

Will you be able to work on this any time soon?

[pointlessone]

@petergoldstein I merged parts of this PR outside of GitHub. Thank you for your contribution.