From ce45d5a443ce0622a27e1eeaa7f24ec7223a2ac2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 27 May 2026 11:06:57 +0200 Subject: [PATCH] Implement proper serialization of `OptionalContentConfig` I happened to notice that the way the `OptionalContentConfig`-data handled in the PR that implements worker-rendering leaves a lot to be desired: - The way that the optional content state is handled is not correct, since that PR collects the "effective visibility" of the optional content groups rather than their *actual* internal state. - The necessary `OptionalContentConfig`-data is collected piecemeal in the API, which leads to quite frankly very messy code that's hard to read and will be even harder to maintain. The solution to all of these issues seem really simple though, just add a couple of `OptionalContentConfig` methods that serialize/de-serialize the necessary data. In the API calling `optionalContentConfig.serializable` will get *all* of the needed data for transferring to the worker-renderer, and once received there calling `OptionalContentConfig.fromSerializable(/* transferred data here */)` will create an `OptionalContentConfig` instance with the correct internal state. As part of this patch, to avoid increasing bundle-size unnecessarily, a couple of existing methods are stubbed out when the `OptionalContentConfig` class ends up in a worker-file (since they're unused there). This part assumes that the new worker-renderer is built correctly, note how the existing `pdf.worker.mjs` is handled in https://github.com/mozilla/pdf.js/blob/03eda70d7e75267bc58ee4f446a20f7f38792b47/gulpfile.mjs#L539-L545 (*Note:* Submitting a PR was a lot faster than trying to provide review comments, since writing this commit message took longer than writing the patch.) --- src/display/optional_content_config.js | 75 +++++++++++++++++++++----- 1 file changed, 62 insertions(+), 13 deletions(-) diff --git a/src/display/optional_content_config.js b/src/display/optional_content_config.js index 27c59340f..75b437353 100644 --- a/src/display/optional_content_config.js +++ b/src/display/optional_content_config.js @@ -72,6 +72,13 @@ class OptionalContentGroup { this.#userSet = userSet; this.#visible = visible; } + + get serializable() { + return { + userSet: this.#userSet, + visible: this.#visible, + }; + } } class OptionalContentConfig { @@ -83,11 +90,19 @@ class OptionalContentConfig { #order = null; - constructor(data, renderingIntent = RenderingIntentFlag.DISPLAY) { - this.renderingIntent = renderingIntent; + #rawData; - this.name = null; - this.creator = null; + creator = null; + + name = null; + + constructor( + data, + renderingIntent = RenderingIntentFlag.DISPLAY, + groupState = null // Should *only* be used with `fromSerializable`. + ) { + this.#rawData = data; + this.renderingIntent = renderingIntent; if (data === null) { return; @@ -102,18 +117,29 @@ class OptionalContentConfig { ); } - if (data.baseState === "OFF") { - for (const group of this.#groups.values()) { - group._setVisible(INTERNAL, false); + if (groupState) { + if (groupState.size !== this.#groups.size) { + unreachable("Incorrect serialized groupState."); + } + for (const [id, group] of groupState) { + this.#groups + .get(id) + ._setVisible(INTERNAL, group.visible, group.userSet); + } + } else { + if (data.baseState === "OFF") { + for (const group of this.#groups.values()) { + group._setVisible(INTERNAL, false); + } } - } - for (const on of data.on) { - this.#groups.get(on)._setVisible(INTERNAL, true); - } + for (const on of data.on) { + this.#groups.get(on)._setVisible(INTERNAL, true); + } - for (const off of data.off) { - this.#groups.get(off)._setVisible(INTERNAL, false); + for (const off of data.off) { + this.#groups.get(off)._setVisible(INTERNAL, false); + } } // The following code must always run *last* in the constructor. @@ -230,6 +256,9 @@ class OptionalContentConfig { } setVisibility(id, visible = true, preserveRB = true) { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("WORKER_THREAD")) { + throw new Error("Not implemented: setVisibility"); + } const group = this.#groups.get(id); if (!group) { warn(`Optional content group not found: ${id}`); @@ -255,6 +284,9 @@ class OptionalContentConfig { } setOCGState({ state, preserveRB }) { + if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("WORKER_THREAD")) { + throw new Error("Not implemented: setOCGState"); + } let operator; for (const elem of state) { @@ -319,6 +351,23 @@ class OptionalContentConfig { [Symbol.iterator]() { return this.#groups.entries(); } + + get serializable() { + const groupState = new Map(); + for (const [id, group] of this.#groups) { + groupState.set(id, group.serializable); + } + + return { + data: this.#rawData, + renderingIntent: this.renderingIntent, + groupState, + }; + } + + static fromSerializable({ data, renderingIntent, groupState }) { + return new OptionalContentConfig(data, renderingIntent, groupState); + } } export { OptionalContentConfig };