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

troika-three-text: First line of text is missing "top" spacing #320

Open
null77 opened this issue Jul 5, 2024 · 10 comments
Open

troika-three-text: First line of text is missing "top" spacing #320

null77 opened this issue Jul 5, 2024 · 10 comments

Comments

@null77
Copy link

null77 commented Jul 5, 2024

This is an issue I found when trying to match up troika-three-text's rendering with pdfkit. trioka-three-text produces output something like this:

image

Here the grey rectangles are the visible and full bounding boxes, and the red dot is the anchor.

Rendering a similar text in pdfkit produces a bounding box like this:

image

Here you can see the bounding box seems to be more aligned with the cap height and baseline than the top of the ascender and bottom of the descender. I'm pretty confident in pdfkit doing things "right" and this aligning with typesetting on the web as well. The situation is pretty noticeable for all-caps.

There could be a way to get this effect using the metrics from getTextRenderInfo, but ideally we could specify we want this kind of behaviour up-front to avoid the async round-trip. I did look at the typesetting code briefly, but didn't come up with a solution.

Could you add this feature to troika-three-text?

Loosely related issue is #193 .

@lojjic
Copy link
Collaborator

lojjic commented Jul 5, 2024

I think you're probably right. This feels like a bug (my intent has always been to match web typesetting where possible), but the question would be is it a bug that's been present long enough where just fixing it would be a breaking change for lots of users? If so it would require some care, maybe even an opt-in flag (yuck).

@null77
Copy link
Author

null77 commented Jul 5, 2024

I think you're probably right. This feels like a bug (my intent has always been to match web typesetting where possible), but the question would be is it a bug that's been present long enough where just fixing it would be a breaking change for lots of users? If so it would require some care, maybe even an opt-in flag (yuck).

Yep, exactly. This kind of thing behaviour change gets annoying when you don't know exactly what people are relying on. You could make it optional, then at some point switch the default setting. It's hard for me to recommend a perfect solution.

Do you know how tricky a fix this would be?

@lojjic
Copy link
Collaborator

lojjic commented Jul 5, 2024

Do you know how tricky a fix this would be?

Should just be an adjustment here

@null77
Copy link
Author

null77 commented Jul 9, 2024

OK I've narrowed this down to a difference in how troika-three-text loads TTF metrics vs how pdfkit uses fontkit to load TTF metrics.

troika-three-text loads "os2" metrics preferentially, then falls back to horizontal header if os2 is not available:

const ascender = firstNum(os2 && os2.sTypoAscender, hhea && hhea.ascender, unitsPerEm)

fontkit uses hhea always for ascender, descender and linegap:

https://github.com/foliojs/fontkit/blob/master/src/TTFFont.js#L168

fontkit uses the same logic as t-t-t for other font properties like xHeight. I'm not sure if hhea is always available given what fontkit does, or if it has some other logic to determine them if they are missing.

For solutions:

  1. match what fontkit does in t-t-t. Given this would change behaviour in real ways, this might not be an appealing solution. Also, given that the choice of metrics is somewhat arbitrary, it might not be the best solution for everyone. See Reference: https://glyphsapp.com/learn/vertical-metrics

  2. add an option to control how the font metrics work, so that my app can match fontkit, and existing apps are unchanged. Would you accept a change like this?

@lojjic
Copy link
Collaborator

lojjic commented Jul 9, 2024

Are you saying just changing it to prefer the hhea metrics makes it render identically to fontkit? I'd be surprised at that -- from your screenshots this looked more like a difference in how the glyphs are vertically centered within the line-height (centering the baseline-to-ascender box vs. centering the descender-to-ascender box.) Also from that glyphsapp document and some actual web fonts I've inspected it seems like the os/2 and hhea metrics are set to the same values.

@null77
Copy link
Author

null77 commented Jul 9, 2024

Are you saying just changing it to prefer the hhea metrics makes it render identically to fontkit?

That's what I'm seeing. It only happens with specific fonts. In your test example, there are few that I think have this mismatch. I've attached two example fonts where I see this.

Fonts.zip

You can drop these into https://fontdrop.info/ and see the metrics there. For Arial Narrow:

os2 sTypoAscender 1491
os2 sTypoDescender -431

hhea ascender 1916
hhea descender -434

For Trirong:

os2 sTypoAscender 750
os2 sTypoDescender -250

hhea ascender 1200
hhea descender -534

When I swapped over t-t-t to use the os2 metrics for ascender/descender I saw the rendered output line up with the PDF output in my app. Let me know what you make of this!

@lojjic
Copy link
Collaborator

lojjic commented Jul 9, 2024

Ahh, right you are! I just hadn't picked the right fonts to check. 😄

I dug a little and found why I originally changed it to prefer the os2 values:

  1. The CSS spec says so
  2. The MS OpenType format docs also say so

I'm even more confused now. 😓

@null77
Copy link
Author

null77 commented Jul 12, 2024

Thanks for finding those references! I filed an issue against fontkit but there's been no response. Given your sources I suspect troika-three-text is doing the right thing, and fontkit is either doing the wrong thing or doing the right thing in a PDF context specifically. Given it's pretty easy for me to monkey patch either fontkit or troika, I'm happy to defer or skip any changes here.

@lojjic
Copy link
Collaborator

lojjic commented Jul 12, 2024

Thanks for the followup, I'm interested to see what the fontkit folks say!

It's also entirely possible that I'm reading the metrics correctly but applying them incorrectly, or that there's some other real-world conditionality that I'm unaware of, or that browsers don't follow those spec recommendations. Given your original statement:

I'm pretty confident in pdfkit doing things "right" and this aligning with typesetting on the web as well.

I'd still like to make it match browser rendering as closely as possible. Since it only affects some fonts I may be more open to a "breaking" change to make it match.

@null77
Copy link
Author

null77 commented Jul 12, 2024

I'm pretty confident in pdfkit doing things "right" and this aligning with typesetting on the web as well.

I'd still like to make it match browser rendering as closely as possible. Since it only affects some fonts I may be more open to a "breaking" change to make it match.

I'm not at all confident anymore that PDFKit is doing the right thing. I'm more inclined to believe I was wrong and that troika is correct and PDFKit is doing something non-standard. Sorry for the mixup!

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

No branches or pull requests

2 participants