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

Let's improve the performance of `CustomFontEmbedder.widthOfTextAtSize`

Open tavoyne opened this issue 3 years ago • 6 comments

Describe your idea

CustomFontEmbedder.widthOfTextAtSize is extremely slow as it relies internally on pdfkit's font.layout method, which itself is very slow because it computes way more metrics than just the advanceWidth we need for this function.

How could this be implemented?

I managed to improve the performance by 5000% in my app by stopping relying on font.layout. It looks like that:

widthOfTextAtSize(text: string, size: number): number {
  const advanceWidth = this.font
    .glyphsForString(text)
    .reduce((acc, glyph) => {
      return acc + glyph.advanceWidth;
    }, 0);
  const scale = size / this.font.unitsPerEm;
  return advanceWidth * scale;
}

What problem are you trying to solve?

I'm creating files with large text sections that I have to break into lines.

Why does this matter to you?

Don't like things to be slow...

Would others find this helpful?

Better performance is better for anyone.

Are you interested in implementing your proposal?

Yes

Why are you submitting a proposal?

If you see no objection, I can prepare a PR.

Additional Notes

No response

tavoyne avatar May 05 '22 14:05 tavoyne

I also noticed that the width calculation seems really slow. Where exactly did you apply this fix? In the CustomFontEmbedder? I would like to try it as well

Simolation avatar May 06 '22 10:05 Simolation

Yeah it is very slow, and it shouldn't be. Computing the width of a string is just of matter of summing up advance widths. It should be nearly instantaneous, even for extremely large strings.

To try it out, just create a fontkit's Font instance (either from fontkit itself of from @Hopding's fork, @pdf-lib/fontkit) and use the snippet in my previous message.

tavoyne avatar May 06 '22 11:05 tavoyne

I'd like to mention calculating font width isn't as simple as summing up advanceWidth if you care about font Kerning, which is something I'd like to see in this library soon. It would need the xOffset attribute too. This library is takes a lot of inspiration from PDFKit, so that's probably why font.layout is used here.

(I have a wip PR upcoming for it, but other things have since come up...)

joewestcott avatar May 08 '22 09:05 joewestcott

Thanks for pointing that out @joewestcott. You're right, we need to care about kerning as well.

What do you mean by xOffset though? Is that the same thing as bearingX?

Let me know if I can be of any help with your PR.

tavoyne avatar May 10 '22 21:05 tavoyne

Hi @tavoyne,

You're right, we need to care about kerning as well.

Thanks.

Currently, pdf-lib doesn't actually write the character kern offsets to the document, but widthOfTextAtSize does include the kern offsets in its calculations (since it uses the more accurate font.layout method under-the-hood). This isn't good - it should return the same width that would be written to the document in order to be useful to the programmer. (although, the difference of kern offsets only makes a tiny difference.)

If this library could write the kern values for each character pair to the PDF, we'd

  1. Have better looking text
  2. widthOfTextAtSize would return the correct width.

This is the eventual path I'd like to see, if kerning is a goal. PDFKit can be used as an example of how to apply it.

What do you mean by xOffset though? Is that the same thing as bearingX?

Sorry, by xOffset I meant xAdvance, see link above. Not sure where you're seeing bearingX?

Let me know if I can be of any help with your PR.

Thanks. My biggest concern at the moment is not having @Hopding as an active maintainer. My prior contributions to this project haven't yet been acknowledged.

Getting back on topic to this issue (improving text layout perf), I think you're right in identifying it as a problem, but I don't think any pdf-lib code is really at fault here. I wouldn't be surprised if some significant improvements could be made to the font.layout method of fontkit, if you feel brave enough to dive into it.

joewestcott avatar May 12 '22 11:05 joewestcott

pdf-lib doesn't actually write the character kern offsets to the document.

Oh I wasn't aware of that. Totally agree it should.

I wouldn't be surprised if some significant improvements could be made to the font.layout method of fontkit.

I'm discovering fontkit's codebase just now, so it's too early for me to say 😅. But I'll keep you posted.

Hope @Hopding will be around soon to give us some insights 🤞.

tavoyne avatar May 16 '22 08:05 tavoyne