-
Notifications
You must be signed in to change notification settings - Fork 10.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Replace most Foxit fonts with Liberation fonts #14843
Comments
Would this reduce the size of the built-in PDF.js viewer? |
Difficult to tell without trying, and it might also be possible to compact the Liberation fonts. |
OK, thanks. It seems worth investigating since on GeckoView size is important. |
#16363 is going to do part of this. |
I need to make some tests to be sure that everything is fine, but my plan is to convert the different Liberations fonts we need (except LiberationSans because we want to be able to rescale it on the fly for XFA) into WOFF2 format. |
Please keep the Node.js, or browsers with |
@Snuffleupagus, do you have any idea on how
At least in the Firefox context could you imagine a case where this option is really useful ? |
As previously mentioned, that option is mostly used in Node.js environments. Hence if you remove that you've essentially made PDF.js significantly less usable in Node.js, which is bound to lead to a never ending stream of user complaints (and it'd probably be an api-major change).
Please note that all of this requires the font-data to be parse-able by the code in If the WOFF2 format is sufficiently compatible with TrueType and/or OpenType, you might be able to "extract" usable parts from it such that our existing font-parsing works; see e.g. how the TrueTypeCollection-support is implemented. |
Afaik WOFF2 format is different of the usual formats (at least because data are compressed with brotli), so we won't be able to pass a woff2 file to any parsers we've. |
Interesting, I need to think about that. |
As mentioned that simply won't work in either Node.js environments or when we fallback to Basically, if you want either of those cases to still work then the standard font-data files need to still be parse-able by the "regular" PDF.js font-code as far as I can tell. |
Another idea/question: Could we perhaps convert the Liberation fonts to CFF instead? That should be more compact than TrueType, while still retaining the ability to pass them to the regular PDF.js font-code; please also see #12726 (comment). |
There are few tools we can play with to convert the font:
For example with the LiberationSans-Regular.ttf we've in the repo:
Just doing that reduces the size from 139K to126K... it isn't exciting and it's because of hinting code. If now I run ttfautohint without -dehint (to keep some hinting informations), then run the subsetter and finally compress into woff2 I get a 36K files (and we've hinting data). So about node.js we can keep ttf fonts (and strip out whatever we want). I'll ask tuesday (monday is a bank holiday for a lot of people) if we have some telemetry about disabled web-fonts. |
If I were to guess, I'd say that users probably disable web-fonts for security reasons (and I cannot imagine it being common). There's obviously a question if the fonts embedded in PDF documents should actually be considered "web-fonts" as such, and it might indeed be fine to just allow the built-in Firefox PDF Viewer to always use PDF fonts (regardless of user-settings). |
As seen in #12726 (comment) the Foxit fonts are not "complete" enough, which leads to broken rendering in some PDF documents.
(This is one reason that we're not unconditionally always loading the standard font data-files, another being that doing so affects performance according to the discussion in that PR.)
Furthermore, as suggested and agreed upon in #12726 (comment), #12726 (comment), and #12726 (comment), the idea was to replace the Foxit fonts (except
FoxitSymbol.pfb
andFoxitDingbats.pfb
) with the more complete Liberation fonts instead.Note that we already ship some Liberation fonts, for use with XFA-documents, and that we thus currently have some duplication in the standard font files. Hence we should replace the
FoxitFixed...
andFoxitSerif...
fonts with their Liberation equivalents./cc @calixteman
The text was updated successfully, but these errors were encountered: