react-pdf icon indicating copy to clipboard operation
react-pdf copied to clipboard

feat: Add Font Fallback

Open ahmed-novalabs opened this issue 3 years ago • 11 comments

  • Updated and Optimized #508 for version 2.0.
  • Added some basic tests for the font package

Should fix #1771, #499. Is part of #933, #856.

ahmed-novalabs avatar May 28 '22 16:05 ahmed-novalabs

Thanks @ahmed-novalabs for working on this! Can you provide a test plan for this PR?

If I understood correctly, user will need to provide a unicodeRange to each font definition in the form of a regex. I find this a bit odd, in the sense that if feels like it's exposing something to the user that should be possible to compute inside. Isn't it? Most people doesn't know what unicode range the font they are using covers, so by exposing this API I feel many will be confused. Thoughts?

diegomura avatar May 30 '22 04:05 diegomura

@diegomura I think the main benefit is optimization by only loading the used fonts based on the text and unicodeRange. The behavior should be to load all the used fonts that don't have a unicodeRange and only load fonts with unicodeRange if they are used and the regex matches at least one char in the text.

There is a similar CSS property. I am not sure if there is a better way to achieve this.

For the testing plan, I don't have a lot of experience with testing, so I am not sure how to make or approach that. If you can provide an example or a guide of what you expect, I can work on it.

ahmed-novalabs avatar May 30 '22 06:05 ahmed-novalabs

What's in this PR

  • Added unicodeRange to the font object to support loading only needed fonts
  • Added support for multiple fonts per <Text/> by modifying the font substitution stage to split the font by , and going through the list of fonts one by one till a font that supports the current glyph is found.

Things to test:

  • Pass multiple fonts in the form of Font1, Font2 with characters that aren't supported by the first font and see if the character is rendered in Font2 correctly. (Also with multiple languages)
  • Set a limited unicodeRange for a font and try rendering the document once with at least one character that matches the range and another time with no characters that match the range. The font should be loaded the first time, but not the 2nd time.
  • Try rendering text with one font, and then adding more text that needs another font to be rendered and update using the update function in React and see if it works correctly (Causes a weird bug).
  • Test in node.js

ahmed-novalabs avatar Jun 10 '22 23:06 ahmed-novalabs

Any update on this?

@diegomura @ahmed-novalabs

ghost avatar Aug 17 '22 06:08 ghost

@ahmed-novalabs Is there specific reason to go with new parameter fontStack than allowing fontFamily to be a string or array?

ghost avatar Aug 22 '22 08:08 ghost

Any updates on this PR?

chasovskiy avatar Sep 07 '22 14:09 chasovskiy

I lost access to the ahmed-novalabs account.

@chathu-novade I am not sure I understand the question, fontStack is an internal variable that holds the font stack, it's not for use by the user. The user can just pass multiple fonts using the fontFamily attribute just like HTML. e.g.:

fontFamily: "MyFont1, MyFont2"

ahmedwalid05 avatar Dec 08 '22 04:12 ahmedwalid05

@diegomura the unicode-range is a standard CSS feature https://developer.mozilla.org/en-US/docs/Web/CSS/@font-face/unicode-range. For example in one project we use it to make sure all numbers are rendered with the monospaced version of the font for better readability.

That said it already would be very helpful to allow to support multiple fonts to support various glyphs e.g. arabic, cyrillic. Most fonts offer dedicated fonts for arabic and so on, because apparently there is a limit of 64K glyphs for a TrueType file (https://github.com/notofonts/noto-fonts/issues/13#issuecomment-110099560). So you can get Noto Sans and Noto Sans Arabic. Combining multiple fonts is one solution, but at some point you hit the 64K limit again.

The easiest going forward afaik would be to allow defining multiple fonts e.g. fontFamily: "NotoSans, NotoSansArabic" or fontFamily: ["NotoSans", "NotoSansArabic"]. If an arabic glyph is not found in NotoSans it would fall back to NotoSansArabic.

If you think this is a good idea I would be happy to extract this part from the PR and create a new one.

nikgraf avatar Feb 07 '24 19:02 nikgraf

@nikgraf i think it is!

diegomura avatar Feb 10 '24 16:02 diegomura

great, will work on it in the coming days

nikgraf avatar Feb 13 '24 11:02 nikgraf

PR is ready: https://github.com/diegomura/react-pdf/pull/2640

nikgraf avatar Feb 14 '24 16:02 nikgraf

@diegomura wanted to ask if you could review the PR 🔝

nikgraf avatar Mar 15 '24 07:03 nikgraf