Skip to content
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

registerFont does not allow for two or more fonts with the same (preferred) family name, style, and weight #1572

Closed
ihuzum opened this issue May 5, 2020 · 29 comments · Fixed by #1987

Comments

@ihuzum
Copy link

ihuzum commented May 5, 2020

Issue or Feature

Cannot use two or more fonts that have the same preferredFamily, and the same style and weight, even though they are different fonts (they do have different family name, different postScriptName).

Example fonts:

  1. CopalStd-Solid
  1. CopalStd-Decorated

The above properties can be inspected by uploading the font(s) here: https://opentype.js.org/font-inspector.html and looking at the Naming table.

Steps to Reproduce

const Canvas = require('canvas')
const fs = require('fs');

Canvas.registerFont(__dirname + '/CopalStd-Decorated.otf', { family: 'Copal Std Decorated'});
Canvas.registerFont(__dirname + '/CopalStd-Solid.otf', { family: 'Copal Std Solid'});

const width = 800, height = 300;
const canvas = Canvas.createCanvas(width, height);
const ctx = canvas.getContext('2d');

ctx.fillStyle = '#FFFFFF';
ctx.fillRect(0, 0, width, height);

ctx.fillStyle = '#FF0000';
ctx.font = '50px Copal Std Solid'
ctx.fillText('Copal Std Solid', 50, 100)
ctx.font = '50px Copal Std Decorated'
ctx.fillText('Copal Std Decorated', 50, 200)

const out = fs.createWriteStream(__dirname + '/test.jpeg')
const stream = canvas.createJPEGStream()
stream.pipe(out)
out.on('finish', () =>  console.log('The JPEG file was created.'))
// etc.

The result of this sample code is that both texts are rendered Copal Std Decorated (the font that was registered first), and it's impossible to render a text with Copal Std Solid (font that has the same preferredFamily, even though it is a different font).

test

Expected:
test2

Your Environment

  • Version of node-canvas: 2.6.1
  • Environment: node 10.15.3 on Linux
@chearon
Copy link
Collaborator

chearon commented May 6, 2020

At first I thought we did something wrong with the preferred family too, but then I noticed the decorated font actually has a different font weight, meaning our internal descriptors should be unique for each font.

I tried it on Linux and couldn't repro. Tried it on macOS too. Do you know if you're using prebuilds or not?

@ihuzum
Copy link
Author

ihuzum commented May 7, 2020

@chearon thank you so much for looking into this!

I had referenced older versions of the example fonts, which indeed, have different weights, and thus the issue could not be reproduced.
In my local example, I was actually using newer versions of the fonts (from Adobe Fonts, which is not freely available for download). But I have also found newer versions (revisions) of the fonts, available here:

Copal Std Solid: https://www.download-free-fonts.com/details/72446/copal-std-solid
Copal Std Decorated: https://www.download-free-fonts.com/details/73488/copal-std-decorated

which do have the same weight (900), for both fonts; thus the internal descriptors are not unique anymore.

In the project that we're using node-canvas, we (potentially) render all fonts from Adobe Fonts, and found quite a few examples, besides Copal Std, that were hitting the same issue. So the issue is not isolated to this particular set of fonts, Copal Std, but reproduces for many more.

And yes, we are using prebuilds.

@chearon
Copy link
Collaborator

chearon commented May 8, 2020

Ah that makes more sense!

I took a look at FontConfig for reference and it seems equally fooled by these fonts. They're recognized as having the same weight and while it does read the preferred family and family name, if preferred matches it stops there:

Screen Shot 2020-05-08 at 5 48 27 PM

So we won't be able to get unique descriptors by searching for "preferredFamily","family", unfortunately... and swapping those would break fonts that use preferred family correctly.

Probably this is not what you wanted to hear but I think for now I think the only solution is to patch the fonts. You'd think addFont("file.ttf", {family: "X"}) and then selecting it with "X" would always work, but we don't have good enough system APIs to use the specific TTF. Some day I'll find the time to bring font selection totally into node-canvas so we don't keep having these edge cases...

@ihuzum
Copy link
Author

ihuzum commented May 8, 2020

@chearon Thank you for the quick reply, much much appreciated. 🙇‍♀️

Yes, we did try patching the fonts themselves, by assigning unique identifiers to preferredFamily, family, and that worked with the current version of node-canvas. But, of course, patching the fonts is a very slow and costly process.

I understand that doing font selection entirely in node-canvas is a larger undertaking.

But what do you think about including subfamily and preferredSubfamily respectively, if they exist?
FontConfig does seem to do a better match if those are included in the pattern matching.
So searching for "preferredFamily preferredSubfamily", "family subfamily":

Screen Shot 2020-05-08 at 4 49 25 PM

I've also seen other projects that use pango directly overcoming this same issue, by using https://developer.gnome.org/pango/stable/pango-FreeType-Fonts-and-Rendering.html#pango-ft2-font-map-set-default-substitute. In the PangoFT2SubstituteFunc, they remove FC_FAMILY from the FcPattern passed it, and adds a FC_POSTSCRIPT_NAME using what was in POSTSCRIPT_NAME. I'm not necessarily suggesting doing the same in node-canvas (especially if the above solution with using both (preferred)Family and (preferred)Subfamily works!), just giving examples of how other projects managed to do it.

Again, thank you SO much for looking into this!

@chearon
Copy link
Collaborator

chearon commented May 9, 2020

I've also seen other projects that use pango directly overcoming this same issue, by using https://developer.gnome.org/pango/stable/pango-FreeType-Fonts-and-Rendering.html#pango-ft2-font-map-set-default-substitute.

I had no idea about this function! Is the postscript name usually unique? I've also had the idea that setting the FC_FILE on the pattern would guarantee picking the file you want. That should be possible with this pango_ft2_font_map_set_default_substitute, though I haven't tested if FC_FILE actually works for selection. I only think it would work because I've read some of the FontConfig matching code.

The one roadblock I see is that pango_ft2_font_map_set_default_substitute doesn't accept a PangoFontDescription, so I'm not sure how we would know what FC_FILE or FC_POSTSCRIPT_NAME to assign. I'll try to look more into this soon though, seems promising!

But what do you think about including subfamily and preferredSubfamily respectively, if they exist?

This is a good idea but I'm not sure if it will break selection on macOS/Windows since Pango won't use FontConfig if not on Linux (edit: I just realized we could obviously use an #ifdef __linux__ for this). Might be worth testing out though.

@ihuzum
Copy link
Author

ihuzum commented May 12, 2020

@chearon Yes, the postscript name is unique. I haven't found any case in which it was not.
And yes, setting FC_FILE on the pattern should also work.

The code that is using pango_ft2_font_map_set_default_substitute to overcome pango+fontconfig limitations, is doing a little bit of a workaround. When they're constructing the PangoFontDescription, they're doing a pango_font_description_set_family(desc, @PostScriptName) - the postscript name prefixed with the actual '@' character. So, though semantically not correct, they are working around the uniqueness problem.

Even though pango_ft2_font_map_set_default_substitute doesn't accept a PangoFontDescription directly, it does accept a PangoFT2SubstituteFunc that we can define, and that will modify the FcPattern being used to match the font. And that is the function that looks at the FcPattern it's passed (fontConfigPattern), and if the family it detects (family = Fc.PatternGetString(fontConfigPattern, Fc.FAMILY, 0)) starts with a '@', then it removes family from the pattern: Fc.PatternRemove(fontConfigPattern, Fc.FAMILY, 0), and adds it as a postscript name (without the '@'): Fc.PatternAddString(fontConfigPattern, Fc.POSTSCRIPT_NAME, family).

Hope this helps. I'd be thrilled to see this work in node-canvas. 😀

@ihuzum
Copy link
Author

ihuzum commented May 18, 2020

@chearon Just following up on this. Do you think using pango_ft2_font_map_set_default_substitute would be a good fix for this issue? Any chance/plan to make the fix in node-canvas?

Much appreciated!

@chearon
Copy link
Collaborator

chearon commented May 19, 2020

Yes definitely, I hope to start implementing soon.

function that looks at the FcPattern it's passed (fontConfigPattern), and if the family it detects (family = Fc.PatternGetString(fontConfigPattern, Fc.FAMILY, 0)) starts with a '@', then it removes family from the pattern: Fc.PatternRemove(fontConfigPattern, Fc.FAMILY, 0), and adds it as a postscript name (without the '@'): Fc.PatternAddString(fontConfigPattern, Fc.POSTSCRIPT_NAME, family).

Brilliant. If PostScript name lookups work on macOS, this could solve an even larger problem space on that platform, where weight lookups are sort of unpredictable. (edit: there's nothing like pango_ft2_font_map_set_default_substitute for macOS or Windows 😞... but it might not be hard to convince Pango to accept a PR)

Even if we find out that it doesn't work on certain platforms we can always put some #ifdefs around the code. Thank you for the insight!

@ihuzum
Copy link
Author

ihuzum commented May 19, 2020

@chearon Hearing this will be fixed in node-canvas is the best news of the week! Maybe even month! Doing a dance party to celebrate this 💃 . Thank you so much.

@chearon
Copy link
Collaborator

chearon commented May 25, 2020

@ihuzum unfortunately I've run into an issue with the way Pango's class hierarchy is setup. pango_ft2_font_map_set_default_substitute, the function we want to use, is for an instance of PangoFT2FontMap, but in node-canvas we're working with a PangoCairoFCFontMap. The full hierarchies are:

Want: PangoFT2FontMap -> PangoFCFontMap -> PangoFontMap
Using: PangoCairoFCFontMap (implements PangoCairoFontMap) -> PangoFCFontMap -> PangoFontMap

So the functionality was added to something that doesn't benefit Cairo users. I don't understand what PangoFT2FontMap is for. Maybe if you are just creating Pango layouts but not rendering to Cairo.

Anyways, the patch would be super easy and reasonable since 2/3 classes that inherit fromPangoFcFontMap (PangoFT2FontMap and PangoXFTFontMap) have a _set_default_substitute. So the next step will be to start badgering them and maybe make a patch.

@chearon
Copy link
Collaborator

chearon commented May 27, 2020

I wrote a patch to add pango_fc_font_map_set_default_substitute, then added the feature to node-canvas. It worked, I could pick both fonts with different custom names:

Screen Shot 2020-05-26 at 11 23 29 PM

Then I went to go bother the Pango team and found that there had been mention of adding the very same function 13 years ago! Hopefully we can get this function 🤞I really want this because it would give us perfect matching on Linux, where most people have node-canvas running in prod.

@ihuzum
Copy link
Author

ihuzum commented May 27, 2020

@chearon This is SUCH good & amazing news! It's awesome that you could write the patch for pango too and proved that it worked. Looking forward to trying it out, when this becomes available in node-canvas, whatever form (let me know). It's one of the last puzzle pieces in our project, so seeing it coming together is really exciting.

@chearon
Copy link
Collaborator

chearon commented Jun 9, 2020

@ihuzum it's amazing for me too, thanks for pointing me in the right direction! I have been wanting a more reliable way to select fonts for a while but I never thought of that.

Yesterday I finally got around to making a merge request with Pango. Behdad himself responded to the original issue in support of it, so things are looking peachy. We'll have to wait for Pango 1.46 to justify a PR in node-canvas, but when we get there things should move quickly and node-canvas will get the upgrade.

@ihuzum
Copy link
Author

ihuzum commented Jun 18, 2020

This is remarkable, and a great win for everyone! I can't wait to use this when it will finally get merged/published in node-canvas.

@chearon
Copy link
Collaborator

chearon commented Sep 21, 2020

My Pango PR is now merged, and will be in version 1.47. Looking at their release, history, that shouldn't be too long 🤞

@cmjunior
Copy link

cmjunior commented Oct 8, 2020

@chearon this version of Pango, 1.47, is already available for download. How can I upgrade this on my distro? I´m using an CentOS AWS´s distro.

@chearon
Copy link
Collaborator

chearon commented Oct 8, 2020

Thanks, I hadn't noticed! If you want the better font selection, we have to make changes in node-canvas first. I'll have that PR this week. If your distro doesn't have it, you'll have to compile Pango yourself. Linux from scratch is a great guide.

@david-sabata
Copy link

Hi @chearon and thank you for your contribution to Pango! Is there any news regarding leveraging that change in node-canvas?

@chearon
Copy link
Collaborator

chearon commented Aug 7, 2021

Yes, I've actually written the code already but I got hung up on something. I'll try to revive it now

chearon added a commit to chearon/node-canvas that referenced this issue Aug 7, 2021
greatly improves font matching accuracy

Fixes Automattic#1572
chearon added a commit to chearon/node-canvas that referenced this issue Aug 7, 2021
greatly improves font matching accuracy

Fixes Automattic#1572
@mjgerace
Copy link

@chearon any updates on this?

@avra-m3
Copy link

avra-m3 commented Nov 11, 2021

Second this, any updates?

@windschaser
Copy link

any updates? @chearon Will there be prebuilts for new version that fix the bug?

@Modsofthenation
Copy link

Any further updates on this change? @chearon

@brosenquist
Copy link

@chearon I wondering about the status of the code changes you made in node canvas. We were really looking forward to this fix. If you don't have time to make a PR, is there a branch with your changes that someone else might be able to pick up and push forward?

@chearon
Copy link
Collaborator

chearon commented Feb 9, 2022

Sorry guys. I'm trying to finish it today actually. I ran into a ton of problems with a cleanup commit, and problems with updating the Pango prebuild dependency that spawned more problems 😤. I will finish this week though.

chearon added a commit to chearon/node-canvas that referenced this issue Feb 9, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
chearon added a commit to chearon/node-canvas that referenced this issue Feb 9, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
@avra-m3
Copy link

avra-m3 commented Feb 10, 2022

Hey @chearon, I've played around with your commit and it's solved all my font issues, I initially encountered some weird font spacing issues and found locking pango to 1.48.2-r0 resolved those issues.
It works seamlessly now, really hope we can get this change mainlined :)
Best of luck!

chearon added a commit to chearon/node-canvas that referenced this issue Feb 10, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
@chearon
Copy link
Collaborator

chearon commented Feb 10, 2022

That's great to hear! PR: #1987 should be in a release soon. Again, thanks to @ihuzum for the technique.

@mjgerace
Copy link

@chearon you are the best! If you ever want to meet up for a beer in Columbus, let me know (email in bio)

chearon added a commit to chearon/node-canvas that referenced this issue Feb 14, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
chearon added a commit to chearon/node-canvas that referenced this issue Feb 14, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
zbjornson pushed a commit to chearon/node-canvas that referenced this issue Feb 26, 2022
greatly improves font matching accuracy

Fixes Automattic#1572
zbjornson pushed a commit that referenced this issue Feb 26, 2022
greatly improves font matching accuracy

Fixes #1572
@flohall
Copy link

flohall commented Mar 4, 2022

@chearon which version of pango and cairo has been used to verify that this improvement works? Are there older version not being supported anymore?

The font issues we had have been hard to reproduce and occurred form time to time.

We use cairo 1.15.12-4.amzn2 and pango 1.42.4-4.amzn2, which is newest versions you get on Amazon Linux 2. Do you already now if there might be compatibility issues? Does someone else has experience?

Anyway we will try it out and will update if we experience improvements for our issues. But the testing is complex and will take some time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.