Skip to content

Commit

Permalink
🐛 Fix error when reusing fonts or images
Browse files Browse the repository at this point in the history
The internal `Font` and `Image` objects are kept in global stores in the
PdfMaker context. During the generation of a document, these objects got
PDFRefs attached that outlived the document and caused errors during the
generation of subsequent documents.

The solution is to attach these references on the PDFDocument instead.
  • Loading branch information
ralfstx committed Jan 19, 2025
1 parent eec2a03 commit 2d4ad75
Show file tree
Hide file tree
Showing 9 changed files with 65 additions and 32 deletions.
26 changes: 24 additions & 2 deletions src/api/PdfMaker.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,22 @@
import { readFile } from 'node:fs/promises';
import { join } from 'node:path';
import { before } from 'node:test';

import { describe, expect, it } from 'vitest';
import { describe, expect, it, vi } from 'vitest';

import { image, text } from './layout.ts';
import { PdfMaker } from './PdfMaker.ts';

describe('makePdf', () => {
let pdfMaker: PdfMaker;

before(() => {
before(async () => {
pdfMaker = new PdfMaker();
pdfMaker.setResourceRoot(join(__dirname, '../test/resources'));
const fontData = await readFile(
join(__dirname, '../test/resources/fonts/roboto/Roboto-Regular.ttf'),
);
pdfMaker.registerFont(fontData);
});

it('creates data that starts with a PDF 1.7 header', async () => {
Expand All @@ -31,4 +39,18 @@ describe('makePdf', () => {
const string = Buffer.from(pdf.buffer).toString();
expect(string).toMatch(/\/ID \[ <[0-9A-F]{64}> <[0-9A-F]{64}> \]/);
});

it('creates consistent results across runs', async () => {
// ensure same timestamps in generated PDF
vi.useFakeTimers();
// include fonts and images to ensure they can be reused
const content = [text('Test'), image('file:/torus.png')];

const pdf1 = await pdfMaker.makePdf({ content });
const pdf2 = await pdfMaker.makePdf({ content });

const pdfStr1 = Buffer.from(pdf1.buffer).toString();
const pdfStr2 = Buffer.from(pdf2.buffer).toString();
expect(pdfStr1).toEqual(pdfStr2);
});
});
1 change: 1 addition & 0 deletions src/font-store.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ describe('FontStore', () => {
const font = await store.selectFont({ fontFamily: 'Test' });

expect(font).toEqual({
key: 'Test:normal:normal',
name: 'Test',
style: 'normal',
weight: 400,
Expand Down
5 changes: 3 additions & 2 deletions src/font-store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,21 @@ export class FontStore {
selector.fontWeight ?? 'normal',
].join(':');
try {
return await (this.#fontCache[cacheKey] ??= this._loadFont(selector));
return await (this.#fontCache[cacheKey] ??= this._loadFont(selector, cacheKey));
} catch (error) {
const { fontFamily: family, fontStyle: style, fontWeight: weight } = selector;
const selectorStr = `'${family}', style=${style ?? 'normal'}, weight=${weight ?? 'normal'}`;
throw new Error(`Could not load font for ${selectorStr}`, { cause: error });

Check failure on line 38 in src/font-store.ts

View workflow job for this annotation

GitHub Actions / build

src/api/PdfMaker.test.ts > makePdf > creates consistent results across runs

Error: Could not load font for 'undefined', style=normal, weight=normal ❯ FontStore.selectFont src/font-store.ts:38:13 ❯ src/text.ts:45:36 ❯ Module.extractTextSegments src/text.ts:32:15 ❯ layoutText src/layout/layout-text.ts:37:26 ❯ Module.layoutTextContent src/layout/layout-text.ts:17:22 ❯ layoutBlockContent src/layout/layout.ts:162:18 ❯ Module.layoutBlock src/layout/layout.ts:144:24 ❯ Module.layoutRowsContent src/layout/layout-rows.ts:36:40 ❯ layoutBlockContent src/layout/layout.ts:171:18 Caused by: Caused by: Error: No fonts defined ❯ selectFont src/font-store.ts:61:11 ❯ FontStore._loadFont src/font-store.ts:43:26 ❯ FontStore.selectFont src/font-store.ts:34:56 ❯ src/text.ts:45:36 ❯ Module.extractTextSegments src/text.ts:32:15 ❯ layoutText src/layout/layout-text.ts:37:26 ❯ Module.layoutTextContent src/layout/layout-text.ts:17:22 ❯ layoutBlockContent src/layout/layout.ts:162:18 ❯ Module.layoutBlock src/layout/layout.ts:144:24
}
}

_loadFont(selector: FontSelector): Promise<Font> {
_loadFont(selector: FontSelector, key: string): Promise<Font> {
const selectedFont = selectFont(this.#fontDefs, selector);
const data = parseBinaryData(selectedFont.data);
const fkFont = selectedFont.fkFont ?? fontkit.create(data);
return Promise.resolve(
pickDefined({
key,
name: fkFont.fullName ?? fkFont.postscriptName ?? selectedFont.family,
data,
style: selector.fontStyle ?? 'normal',
Expand Down
15 changes: 13 additions & 2 deletions src/fonts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,12 @@ export type FontDef = {
};

export type Font = {
key: string;
name: string;
style: FontStyle;
weight: number;
data: Uint8Array;
fkFont: fontkit.Font;
pdfRef?: PDFRef;
};

export type FontSelector = {
Expand Down Expand Up @@ -54,14 +54,25 @@ export function readFont(input: unknown): Partial<FontDef> {
} as FontDef;
}

export function registerFont(font: Font, pdfDoc: PDFDocument) {
export function registerFont(font: Font, pdfDoc: PDFDocument): PDFRef {
const registeredFonts = ((pdfDoc as any)._pdfmkr_registeredFonts ??= {});
if (font.key in registeredFonts) return registeredFonts[font.key];
const ref = pdfDoc.context.nextRef();
const embedder = new (CustomFontSubsetEmbedder as any)(font.fkFont, font.data);
const pdfFont = PDFFont.of(ref, pdfDoc, embedder);
(pdfDoc as any).fonts.push(pdfFont);
registeredFonts[font.key] = ref;
return ref;
}

export function findRegisteredFont(font: Font, pdfDoc: PDFDocument): PDFFont | undefined {
const registeredFonts = ((pdfDoc as any)._pdfmkr_registeredFonts ??= {});
const ref = registeredFonts[font.key];
if (ref) {
return (pdfDoc as any).fonts?.find((font: PDFFont) => font.ref === ref);
}
}

export function weightToNumber(weight: FontWeight): number {
if (weight === 'normal') {
return 400;
Expand Down
6 changes: 4 additions & 2 deletions src/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ export type Image = {
height: number;
data: Uint8Array;
format: ImageFormat;
pdfRef?: PDFRef;
};

export function readImages(input: unknown): ImageDef[] {
Expand All @@ -36,7 +35,9 @@ function readImage(input: unknown) {
}) as { data: Uint8Array; format?: ImageFormat };
}

export function registerImage(image: Image, pdfDoc: PDFDocument) {
export function registerImage(image: Image, pdfDoc: PDFDocument): PDFRef {
const registeredImages = ((pdfDoc as any)._pdfmkr_registeredImages ??= {});
if (image.url in registeredImages) return registeredImages[image.url];
const ref = pdfDoc.context.nextRef();
(pdfDoc as any).images.push({
async embed() {
Expand All @@ -50,5 +51,6 @@ export function registerImage(image: Image, pdfDoc: PDFDocument) {
}
},
});
registeredImages[image.url] = ref;
return ref;
}
22 changes: 8 additions & 14 deletions src/page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,28 +29,22 @@ export type Page = {

export function addPageFont(page: Page, font: Font): PDFName {
if (!page.pdfPage) throw new Error('Page not initialized');
if (!font.pdfRef) {
font.pdfRef = registerFont(font, page.pdfPage.doc);
}
page.fonts ??= {};
const key = font.pdfRef.toString();
if (!(key in page.fonts)) {
page.fonts[key] = (page.pdfPage as any).node.newFontDictionary(font.name, font.pdfRef);
if (!(font.key in page.fonts)) {
const pdfRef = registerFont(font, page.pdfPage.doc);
page.fonts[font.key] = (page.pdfPage as any).node.newFontDictionary(font.name, pdfRef);
}
return page.fonts[key];
return page.fonts[font.key];
}

export function addPageImage(page: Page, image: Image): PDFName {
if (!page.pdfPage) throw new Error('Page not initialized');
if (!image.pdfRef) {
image.pdfRef = registerImage(image, page.pdfPage.doc);
}
page.images ??= {};
const key = image.pdfRef.toString();
if (!(key in page.images)) {
page.images[key] = (page.pdfPage as any).node.newXObject('Image', image.pdfRef);
if (!(image.url in page.images)) {
const pdfRef = registerImage(image, page.pdfPage.doc);
page.images[image.url] = (page.pdfPage as any).node.newXObject('Image', pdfRef);
}
return page.images[key];
return page.images[image.url];
}

type ExtGraphicsParams = { ca: number; CA: number };
Expand Down
6 changes: 3 additions & 3 deletions src/render/render-image.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ describe('renderImage', () => {
size = { width: 500, height: 800 };
const pdfPage = fakePDFPage();
page = { size, pdfPage } as Page;
image = { pdfRef: 23 } as unknown as Image;
image = { url: 'test-url' } as unknown as Image;
});

it('renders single text object', () => {
it('renders single image object', () => {
const obj: ImageObject = { type: 'image', image, x: 1, y: 2, width: 30, height: 40 };

renderImage(obj, page, pos);
Expand All @@ -29,7 +29,7 @@ describe('renderImage', () => {
'q',
'1 0 0 1 11 738 cm',
'30 0 0 40 0 0 cm',
'/Image-23-1 Do',
'/Image-1-0-1 Do',
'Q',
]);
});
Expand Down
7 changes: 3 additions & 4 deletions src/render/render-text.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { Color, PDFContentStream, PDFFont, PDFName, PDFOperator } from 'pdf-lib';
import type { Color, PDFContentStream, PDFName, PDFOperator } from 'pdf-lib';
import {
beginText,
endText,
Expand All @@ -10,6 +10,7 @@ import {
setTextRise,
showText,
} from 'pdf-lib';
import { findRegisteredFont } from 'src/fonts.ts';

import type { Pos } from '../box.ts';
import type { TextObject } from '../frame.ts';
Expand All @@ -27,9 +28,7 @@ export function renderText(object: TextObject, page: Page, base: Pos) {
contentStream.push(setTextMatrix(1, 0, 0, 1, x + row.x, y - row.y - row.baseline));
row.segments?.forEach((seg) => {
const fontKey = addPageFont(page, seg.font);
const pdfFont = (page.pdfPage as any)?.doc?.fonts?.find(
(font: PDFFont) => font.ref === seg.font.pdfRef,
);
const pdfFont = findRegisteredFont(seg.font, page.pdfPage!.doc)!;
const encodedText = pdfFont.encodeText(seg.text);
const operators = compact([
setTextColorOp(state, seg.color),
Expand Down
9 changes: 6 additions & 3 deletions src/test/test-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export function fakeFont(
): Font {
const key = `${name}-${opts?.style ?? 'normal'}-${opts?.weight ?? 400}`;
const font: Font = {
key,
name,
style: opts?.style ?? 'normal',
weight: weightToNumber(opts?.weight ?? 'normal'),
Expand All @@ -23,7 +24,8 @@ export function fakeFont(
if (opts.doc) {
const pdfFont = fakePdfFont(name, font.fkFont);
(opts.doc as any).fonts.push(pdfFont);
font.pdfRef = pdfFont.ref;
(opts.doc as any)._pdfmkr_registeredFonts ??= {};
(opts.doc as any)._pdfmkr_registeredFonts[font.key] = pdfFont.ref;
}
return font;
}
Expand Down Expand Up @@ -79,8 +81,9 @@ export function fakePDFPage(document?: PDFDocument): PDFPage {
const contentStream: any[] = [];
let counter = 1;
(node as any).newFontDictionary = (name: string) => PDFName.of(`${name}-${counter++}`);
(node as any).newXObject = (type: string, ref: string) =>
PDFName.of(`${type}-${ref}-${counter++}`);
let xObjectCounter = 1;
(node as any).newXObject = (tag: string, ref: PDFRef) =>
PDFName.of(`${tag}-${ref.objectNumber}-${ref.generationNumber}-${xObjectCounter++}`);
(node as any).newExtGState = (type: string) => PDFName.of(`${type}-${counter++}`);
return {
doc,
Expand Down

0 comments on commit 2d4ad75

Please sign in to comment.