Embedded Arimo font widths and used that when fonts are missing #11191
Conversation
|
I can't really review this implementation because I'm not too familiar with the font conversion code so I'll leave that for others, but I think in general the idea of falling back to a default font is better than not having any font at all. However, I thought that when a font is not embedded, PDF.js will substitute it. Does this patch perhaps address the case where no suitable fallback can be found? |
There was a problem hiding this comment.
I've tried to do an initial review pass, please see the inline comments.
Please note that the single most important point here is that, for reasons of both readability and maintainable of the code, you need to create a valid font dictionary such that all of the special cases (in the various font parsing functions) can be removed.
|
The way I see it when some font is not embedded in the pdf, the pdf.js always logs a warning and returns the ErrorFont and doesn't display anything in the pdf. At this point, I just want to embed/construct my own font and use it to display something anyway. |
I took a different approach here by generating a dictionary and using it for a fallback font. So most of the old "special cases" code was removed and left with only a few lines of code to achieve the same result.
|
Closing since the pull request above built upon the changes in this pull request and was merged. Thank you for contributing this! |
This fixes this bug: #11162
And answers my question: #11175
I embedded Arimo font widths and used them to fall back to a default font when the fonts are missing.
It passes lint tests but doesn't pass 8-10 others.
When comparing screenshots of failed tests the only differences are a few pixels that are not visible by the naked eye.
Also in the attached file, you don't normally see the number 3 in "Page 13" but with this fix, you see it which causes the test to fail.
pr4922.pdf