Skip to content

Embed images from buffers with the same bytes only once - #1851

Open
kroplabeskidu wants to merge 1 commit into
foliojs:masterfrom
kroplabeskidu:dedupe-buffer-images
Open

kroplabeskidu wants to merge 1 commit into
foliojs:masterfrom
kroplabeskidu:dedupe-buffer-images

Conversation

@kroplabeskidu

Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Bug fix. Fixes #1361

An image from a file path is embedded once however often it is used, but one from a Buffer was embedded every time: the same 33 KB JPEG three times gave a 97 KB file. openImage now also finds Buffer, Uint8Array and ArrayBuffer images in the registry, by the MD5 of their bytes, which PDFKit already has for attachments.

I used the bytes rather than the object, because a buffer can be changed between calls. It also catches separate buffers with the same image. The cost is hashing each buffer once per image() call.

Checklist:

  • Unit Tests
  • Update CHANGELOG.md
  • Ready to be merged

Comment thread lib/mixins/images.js
key = src;
} else if (src instanceof Uint8Array || src instanceof ArrayBuffer) {
// by content, as a buffer can be changed or come from a pool
key = `md5:${md5Hex(src instanceof ArrayBuffer ? new Uint8Array(src) : src)}`;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is out of scope handling image data mutation. Add a weak map to the instance like was done in font

This branch has not been deployed

No deployments
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

Successfully merging this pull request may close these issues.

Multiple images from Buffer gives huge PDF files

2 participants