Skip to content

Update fonts#632

Closed
k4b7 wants to merge 2 commits intomasterfrom
update_fonts
Closed

Update fonts#632
k4b7 wants to merge 2 commits intomasterfrom
update_fonts

Conversation

@k4b7
Copy link
Member

@k4b7 k4b7 commented Jan 17, 2017

The updated fonts include the last few commits to https://github.com/Khan/MathJax-dev/ as well as changes in Khan/MathJax-dev#3.

Left/right single quotes work with the typewriter font.

The fonts also support latin ligatures, e.g. æ, œ, Æ, and Œ.

@k4b7
Copy link
Member Author

k4b7 commented Jan 17, 2017

screen shot 2017-01-16 at 11 17 10 pm

screen shot 2017-01-16 at 11 17 04 pm

@gagern
Copy link
Collaborator

gagern commented Jan 19, 2017

I ran this again, and couldn't reproduce the font files byte for byte. Looking at the output from ttfdump or ttx, It seems as though the only relevant change is the modification of the created and modified entries in the head table. Pretty annoying, since this means there is no simple way to see whether re-running the tool introduces any relevant changes, or merely updates the timestamps with no other modification. So what do we want to do?

  1. Accept this as a fact of life, trust that all changes will be irrelevant or beneficial, and move on.
  2. Manually inspect all differences using ttfdump or ttx and diff.
  3. Write some script to ttx the files, check whether the timestamps and checksums are the only things that changed, and revert to the base version against which the comparison was performed (e.g. HEAD) if there are no other changes.
  4. Store the ttx files in our repository, and only generate the other formats locally. This means developers will need more tools, and more time. But it also means we see each and every modification in the corresponding source file using the regular GitHub diffs. We could even do merge commits with conflict resolution on these files.
  5. Do both: have ttx files for readable versions, and other file formats for use by the browsers. Use a Travis check to ensure that these stay in sync: whoever modifies one file must modify the other as well, in order to keep Travis happy.
  6. Set up textconv settings in .gitattributes in such a way that git diff shows the diff on the ttx file. This will be very helpful for local reviews, but not much use with the GitHub GUI. On the other hand, we want to test each PR locally before accepting, right?

To help us decide, I actually created a branch that contains TTX files. It's not using them yet, and it does have the TTF, too. But it does show the effect of your pull request here in a human readable fasion: gagern@feefc7c

That diff shows that pretty much all of your font file modifications are spurious. The only relevant ones are

These are changes that should actually be reviewed, but all the other modifications should in my opinion not be part of this pull request here. Can you drop them please?

I'm also a bit surprised tha Katex_Main-BoldItalic is not affected by this PR here. Is there a reason why the newly added glyphs are available only in three of our four styles?

@gagern
Copy link
Collaborator

gagern commented Jan 19, 2017

I've had a stab at approach 6 in #635. So far it looks as though we can't fully automate the process, though: users still have to enter some settings in one of their config files. Which allows them to choose different tools, or tools installed in locations outside their $PATH, or whatever. Still somewhat inconvenient. Currently it can only handle ttf and woff. woff2 requires additional dependencies, and eot doesn't seem to be supported at all.

@gagern gagern mentioned this pull request Jan 20, 2017
19 tasks
@k4b7 k4b7 self-assigned this Jan 22, 2017
@k4b7
Copy link
Member Author

k4b7 commented Jan 23, 2017

I think 6 is definitely better than what we were doing before which was 1. 3 sounds nice, but it's also more work, but it might be worth doing that work if we think we're going to change the fonts more often which seems entirely likely considering issues around missing spaces, accent characters, Euro symbol, etc.

@gagern
Copy link
Collaborator

gagern commented Jan 23, 2017

Do we want to hold this merge here till we can get the things from #638 in as well? That would help keep the repo size down, but would delay #614. I guess we shouldn't delay too much if it can be avoided.

@k4b7
Copy link
Member Author

k4b7 commented Jan 23, 2017

@gagern are you concerned that too many updates to the font files will balloon the repo size because the binary diffs are big?

@gagern
Copy link
Collaborator

gagern commented Jan 23, 2017

Yes. The WOFF fonts in particular are compressed, so a slight change in one part of the font file will likely lead to large changes throughout the file, and therefore bad delta compression of repository data. At 2.2 M the fonts directory is the largest portion of our repository. We should not lightly add 2.2 M a couple of times if we can avoid it.

In the long run, it might make sense to only deploy the TTF files, and generate all the others locally or when rolling a release. Since the TTFs are afaik less compressed, they should delta-compress much better. If I find the time I could investigate whether we can get the machinery for WOFF, WOFF2 and EOT compiled to JavaScript using Emscripten, but that won't happen in the next couple of weeks.

@edemaine
Copy link
Member

Any way the font metrics data could be updated to include horizontal metrics? Relevant to #587.

@k4b7
Copy link
Member Author

k4b7 commented Mar 15, 2017

@edemaine we just have to run make metrics_extended and commit the output, see https://github.com/Khan/KaTeX/blob/master/Makefile#L112-L113. I added a --width option to format_json.py a while back that includes the glyph widths in the output.

@edemaine
Copy link
Member

Ah, cool. So that is already part of this PR. I'm not sure what the optimal timing is, but it sounds like #587 could already use this info. If we want to continue to hold on this PR, I guess we could make and commit new metrics for the old fonts, before switching to new fonts.

@edemaine
Copy link
Member

I do think it'd be good to get horizontal metrics in, so that we can start playing with box sizes (though I'm not really sure when I can start on that). Would it make sense to use a separate repo for fonts, so that we don't worry as much about increasing the size of this one? Or switch to fewer font formats as suggested by @gagern?

@k4b7
Copy link
Member Author

k4b7 commented Apr 29, 2017

If we had the fonts in the a separate repo, we could set things up to do a shallow checkout of that repo... then it wouldn't matter if the size that repo ballooned.

@edemaine
Copy link
Member

FWIW, I just read this Atlassian article about how to deal with large binary files in a Git repo. I learned about lots of options, though I'm not sure I have a specific proposal. Submodules could be a way to do multiple repos, but also increase use complexity...

@k4b7
Copy link
Member Author

k4b7 commented Aug 20, 2017

If I find the time I could investigate whether we can get the machinery for WOFF, WOFF2 and EOT compiled to JavaScript using Emscripten, but that won't happen in the next couple of weeks.

Another option might be to mount static/fonts within the docker at the same place where the ttf folder would exist. We should be able to run make -C fonts/OTF/TeX eot woff woff2 to build the other font files.

@edemaine
Copy link
Member

Are you suggesting using a docker to build eot, woff, woff2 files? I think that's fine as a way for people to update the fonts when the fonts change, but I don't think it should be required for users not playing with the fonts. I.e. doesn't fix the "font changes influencing repo size" issue. But maybe that wasn't your point.

Back to the repo size issue, I feel like the shallow checkout of another repo dedicated to fonts seems reasonable. Will this affect npm, bower, CDN?

@k4b7 k4b7 mentioned this pull request Aug 20, 2017
@k4b7 k4b7 added the on hold label Aug 22, 2017
@k4b7
Copy link
Member Author

k4b7 commented Aug 22, 2017

I'm putting this on hold until we can move our fonts to a separate repo.

@k4b7 k4b7 mentioned this pull request Aug 22, 2017
2 tasks
@k4b7
Copy link
Member Author

k4b7 commented Aug 22, 2017

Blocked on #816.

@k4b7
Copy link
Member Author

k4b7 commented Nov 28, 2017

Now that #891 has landed, this is unblocked.

@k4b7 k4b7 removed the blocked label Nov 28, 2017
@k4b7
Copy link
Member Author

k4b7 commented Nov 28, 2017

These changes will need to be made to https://github.com/KaTeX/katex-fonts.

@k4b7
Copy link
Member Author

k4b7 commented Dec 22, 2017

Now that the fonts are in a separate repo, all we need are the changes in symbols.js and mappings.pl. I'm going to open a new PR b/c I think rebasing this will be more trouble than it's worth. I'll also add commands for the ligatures, e.g. \ae, \oe, etc.

@k4b7 k4b7 closed this Dec 22, 2017
@k4b7 k4b7 deleted the update_fonts branch December 22, 2017 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants