Skip to content

Commit e88a565

Browse files
committed
Fix the FontInfo.prototype.clearData method to actually remove the data as intended (PR 20197 follow-up)
The purpose of PR 11844 was to reduce memory usage once fonts have been attached to the DOM, since the font-data can be quite large in many cases. Unfortunately the new `clearData` method added in PR 20197 doesn't actually remove *anything*, it just replaces the font-data with zeros which doesn't help when the underlying `ArrayBuffer` itself isn't modified. The method does include a commented-out `resize` call[1], but uncommenting that just breaks rendering completely. To address this regression, without having to make large or possibly complex changes, this patch simply changes the `clearData` method to replace the internal buffer/view with its contents *before* the font-data. While this does lead to a data copy, the size of this data is usually orders of magnitude smaller than the font-data that we're removing. --- [1] Slightly off-topic, but I don't think that patches should include commented-out code since there's a very real risk that those things never get found/fixed. At the very least such cases should be clearly marked with `// TODO: ...` comments, and should possibly also have an issue filed about fixing the TODO.
1 parent 79df166 commit e88a565

2 files changed

Lines changed: 28 additions & 29 deletions

File tree

src/display/api.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2903,7 +2903,7 @@ class WorkerTransport {
29032903
.bind(font)
29042904
.catch(() => messageHandler.sendWithPromise("FontFallback", { id }))
29052905
.finally(() => {
2906-
if (!font.fontExtraProperties && font.data) {
2906+
if (!font.fontExtraProperties) {
29072907
// Immediately release the `font.data` property once the font
29082908
// has been attached to the DOM, since it's no longer needed,
29092909
// rather than waiting for a `PDFDocumentProxy.cleanup` call.

src/shared/obj-bin-transform.js

Lines changed: 27 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ import { assert, FeatureTest, MeshFigureType } from "./util.js";
1818
class CssFontInfo {
1919
#buffer;
2020

21-
#view;
21+
#decoder = new TextDecoder();
2222

23-
#decoder;
23+
#view;
2424

2525
static strings = ["fontFamily", "fontWeight", "italicAngle"];
2626

@@ -53,7 +53,6 @@ class CssFontInfo {
5353
constructor(buffer) {
5454
this.#buffer = buffer;
5555
this.#view = new DataView(this.#buffer);
56-
this.#decoder = new TextDecoder();
5756
}
5857

5958
#readString(index) {
@@ -84,9 +83,9 @@ class CssFontInfo {
8483
class SystemFontInfo {
8584
#buffer;
8685

87-
#view;
86+
#decoder = new TextDecoder();
8887

89-
#decoder;
88+
#view;
9089

9190
static strings = ["css", "loadedName", "baseFontName", "src"];
9291

@@ -147,7 +146,6 @@ class SystemFontInfo {
147146
constructor(buffer) {
148147
this.#buffer = buffer;
149148
this.#view = new DataView(this.#buffer);
150-
this.#decoder = new TextDecoder();
151149
}
152150

153151
get guessFallback() {
@@ -228,13 +226,12 @@ class FontInfo {
228226

229227
#buffer;
230228

231-
#decoder;
229+
#decoder = new TextDecoder();
232230

233231
#view;
234232

235233
constructor({ data, extra }) {
236234
this.#buffer = data;
237-
this.#decoder = new TextDecoder();
238235
this.#view = new DataView(this.#buffer);
239236
if (extra) {
240237
Object.assign(this, extra);
@@ -379,7 +376,7 @@ class FontInfo {
379376
return this.#readString(3);
380377
}
381378

382-
get data() {
379+
#getDataOffsets() {
383380
let offset = FontInfo.#OFFSET_STRINGS;
384381
const stringsLength = this.#view.getUint32(offset);
385382
offset += 4 + stringsLength;
@@ -388,25 +385,27 @@ class FontInfo {
388385
const cssFontInfoLength = this.#view.getUint32(offset);
389386
offset += 4 + cssFontInfoLength;
390387
const length = this.#view.getUint32(offset);
391-
if (length === 0) {
392-
return undefined;
393-
}
394-
return new Uint8Array(this.#buffer, offset + 4, length);
388+
389+
return { offset, length };
390+
}
391+
392+
get data() {
393+
const { offset, length } = this.#getDataOffsets();
394+
return length === 0
395+
? undefined
396+
: new Uint8Array(this.#buffer, offset + 4, length);
395397
}
396398

397399
clearData() {
398-
let offset = FontInfo.#OFFSET_STRINGS;
399-
const stringsLength = this.#view.getUint32(offset);
400-
offset += 4 + stringsLength;
401-
const systemFontInfoLength = this.#view.getUint32(offset);
402-
offset += 4 + systemFontInfoLength;
403-
const cssFontInfoLength = this.#view.getUint32(offset);
404-
offset += 4 + cssFontInfoLength;
405-
const length = this.#view.getUint32(offset);
406-
const data = new Uint8Array(this.#buffer, offset + 4, length);
407-
data.fill(0);
408-
this.#view.setUint32(offset, 0);
409-
// this.#buffer.resize(offset);
400+
const { offset, length } = this.#getDataOffsets();
401+
if (length === 0) {
402+
return; // The data is either not present, or it was previously cleared.
403+
}
404+
this.#view.setUint32(offset, 0); // Zero the data-length.
405+
406+
// Replace the buffer/view with only its contents *before* the data-block.
407+
this.#buffer = new Uint8Array(this.#buffer, 0, offset + 4).slice().buffer;
408+
this.#view = new DataView(this.#buffer);
410409
}
411410

412411
get cssFontInfo() {
@@ -462,11 +461,11 @@ class FontInfo {
462461
4 +
463462
stringsLength +
464463
4 +
465-
(systemFontInfoBuffer ? systemFontInfoBuffer.byteLength : 0) +
464+
(systemFontInfoBuffer?.byteLength ?? 0) +
466465
4 +
467-
(cssFontInfoBuffer ? cssFontInfoBuffer.byteLength : 0) +
466+
(cssFontInfoBuffer?.byteLength ?? 0) +
468467
4 +
469-
(font.data ? font.data.length : 0);
468+
(font.data?.length ?? 0);
470469

471470
const buffer = new ArrayBuffer(lengthEstimate);
472471
const data = new Uint8Array(buffer);

0 commit comments

Comments
 (0)