Sakurai/pdf.js
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

[api-minor] Remove the manual passing of an AnnotationStorage-instance when calling various API-method

Browse Source 
Note how we purposely don't expose the `AnnotationStorage`-class directly in the official API (see `src/pdf.js`), since trying to use *multiple* ones simultaneously doesn't really make sense (e.g. in the viewer).
Instead we lazily initialize, and cache, just *one* instance via `PDFDocumentProxy.annotationStorage` which should thus be available internally in the API itself without having to be manually passed to various methods.

To support these changes, the `AnnotationStorage`-instance initialization is moved into the `WorkerTransport`-class to allow both `PDFDocumentProxy` and `PDFPageProxy` to access it.
This patch implements the following simplifications:
 - Remove the `annotationStorage`-parameter from `PDFDocumentProxy.saveDocument`, since it's already available internally.
   Furthermore, while it's currently possible to call that method without an `AnnotationStorage`-instance, that really does *not* make any sense at all. In this case you're effectively reducing `PDFDocumentProxy.saveDocument` to a "regular" `PDFDocumentProxy.getData` call, but with *a lot* more overhead, which was obviously not the intention of the `PDFDocumentProxy.saveDocument`-method.

 - Try to discourage third-party users from calling `PDFDocumentProxy.saveDocument` unconditionally, as a replacement for `PDFDocumentProxy.getData` (note the previous point).

 - Replace the `annotationStorage`-parameter, in `PDFPageProxy.render`, with a boolean `includeAnnotationStorage`-parameter which simply indicates if the (internally available) `AnnotationStorage`-instance should be used during rendering (e.g. for printing).

 - By removing the need to *manually* provide `annotationStorage`-parameters to various API-methods, using the API should become simpler (e.g. for third-parties) since you no longer need to worry about manually fetching and passing around this data.
This commit is contained in:
Jonas Jenwald 2021-04-04 16:49:06 +02:00
parent 6429ccc002
commit 72ef183085
5 changed files with 38 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

60
src/display/api.js
View File

@ -675,7 +675,7 @@ class PDFDocumentProxy {
* @type {AnnotationStorage} Storage for annotation data in forms.
*/
get annotationStorage() {
return shadow(this, "annotationStorage", new AnnotationStorage());
return this._transport.annotationStorage;
}
/**
@ -954,13 +954,20 @@ class PDFDocumentProxy {
}
/**
* @param {AnnotationStorage} annotationStorage - Storage for annotation
* data in forms.
* @returns {Promise<Uint8Array>} A promise that is resolved with a
* {Uint8Array} containing the full data of the saved document.
*/
saveDocument(annotationStorage) {
return this._transport.saveDocument(annotationStorage);
saveDocument() {
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("GENERIC")) &&
this._transport.annotationStorage.size <= 0
) {
deprecated(
"saveDocument called while `annotationStorage` is empty, " +
"please use the getData-method instead."
);
}
return this._transport.saveDocument();
}
/**
@ -1068,7 +1075,7 @@ class PDFDocumentProxy {
* some operations. The default value is `false`.
* @property {boolean} [renderInteractiveForms] - Whether or not interactive
* form elements are rendered in the display layer. If so, we do not render
* them on the canvas as well.
* them on the canvas as well. The default value is `false`.
* @property {Array<any>} [transform] - Additional transform, applied just
* before viewport transform.
* @property {Object} [imageLayer] - An object that has `beginLayout`,
@ -1080,8 +1087,9 @@ class PDFDocumentProxy {
* <color> value, a `CanvasGradient` object (a linear or radial gradient) or
* a `CanvasPattern` object (a repetitive image). The default value is
* 'rgb(255,255,255)'.
* @property {AnnotationStorage} [annotationStorage] - Storage for annotation
* data in forms.
* @property {boolean} [includeAnnotationStorage] - Render stored interactive
* form element data, from the {@link AnnotationStorage}-instance, onto the
* canvas itself; useful e.g. for printing. The default value is `false`.
* @property {Promise<OptionalContentConfig>} [optionalContentConfigPromise] -
* A promise that should resolve with an {@link OptionalContentConfig}
* created from `PDFDocumentProxy.getOptionalContentConfig`. If `null`,
@ -1230,7 +1238,7 @@ class PDFPageProxy {
imageLayer = null,
canvasFactory = null,
background = null,
annotationStorage = null,
includeAnnotationStorage = false,
optionalContentConfigPromise = null,
}) {
if (this._stats) {
@ -1264,6 +1272,9 @@ class PDFPageProxy {
const webGLContext = new WebGLContext({
enable: enableWebGL,
});
const annotationStorage = includeAnnotationStorage
? this._transport.annotationStorage.serializable
: null;
// If there's no displayReadyCapability yet, then the operatorList
// was never requested before. Make the request and create the promise.
@ -1282,7 +1293,7 @@ class PDFPageProxy {
pageIndex: this._pageIndex,
intent: renderingIntent,
renderInteractiveForms: renderInteractiveForms === true,
annotationStorage: annotationStorage?.serializable || null,
annotationStorage,
});
}
@ -2192,8 +2203,8 @@ class WorkerTransport {
this.setupMessageHandler();
}
get loadingTaskSettled() {
return this.loadingTask._capability.settled;
get annotationStorage() {
return shadow(this, "annotationStorage", new AnnotationStorage());
}
destroy() {
@ -2220,21 +2231,14 @@ class WorkerTransport {
});
this.pageCache.length = 0;
this.pagePromises.length = 0;
// Allow `AnnotationStorage`-related clean-up when destroying the document.
if (this.hasOwnProperty("annotationStorage")) {
this.annotationStorage.resetModified();
}
// We also need to wait for the worker to finish its long running tasks.
const terminated = this.messageHandler.sendWithPromise("Terminate", null);
waitOn.push(terminated);
// Allow `AnnotationStorage`-related clean-up when destroying the document.
if (this.loadingTaskSettled) {
const annotationStorageResetModified = this.loadingTask.promise
.then(pdfDocument => {
// Avoid initializing the `annotationStorage` if it doesn't exist.
if (pdfDocument.hasOwnProperty("annotationStorage")) {
pdfDocument.annotationStorage.resetModified();
}
})
.catch(() => {});
waitOn.push(annotationStorageResetModified);
}
Promise.all(waitOn).then(() => {
this.commonObjs.clear();
this.fontLoader.clear();
@ -2669,17 +2673,15 @@ class WorkerTransport {
});
}
saveDocument(annotationStorage) {
saveDocument() {
return this.messageHandler
.sendWithPromise("SaveDocument", {
numPages: this._numPages,
annotationStorage: annotationStorage?.serializable || null,
annotationStorage: this.annotationStorage.serializable,
filename: this._fullReader?.filename ?? null,
})
.finally(() => {
if (annotationStorage) {
annotationStorage.resetModified();
}
this.annotationStorage.resetModified();
});
}

9
test/driver.js
View File

@ -635,14 +635,13 @@ var Driver = (function DriverClosure() {
optionalContentConfigPromise: task.optionalContentConfigPromise,
};
if (renderPrint) {
const annotationStorage = task.annotationStorage;
if (annotationStorage) {
const docAnnotationStorage = task.pdfDoc.annotationStorage;
const entries = Object.entries(annotationStorage);
if (task.annotationStorage) {
const entries = Object.entries(task.annotationStorage),
docAnnotationStorage = task.pdfDoc.annotationStorage;
for (const [key, value] of entries) {
docAnnotationStorage.setValue(key, value);
}
renderContext.annotationStorage = docAnnotationStorage;
renderContext.includeAnnotationStorage = true;
}
renderContext.intent = "print";
}

4
web/app.js
View File

@ -998,9 +998,7 @@ const PDFViewerApplication = {
try {
this._ensureDownloadComplete();
const data = await this.pdfDocument.saveDocument(
this.pdfDocument.annotationStorage
);
const data = await this.pdfDocument.saveDocument();
const blob = new Blob([data], { type: "application/pdf" });
await this.downloadManager.download(blob, url, filename, sourceEventType);

2
web/firefox_print_service.js
View File

@ -66,7 +66,7 @@ function composePage(
transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0],
viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation }),
intent: "print",
annotationStorage: pdfDocument.annotationStorage,
includeAnnotationStorage: true,
optionalContentConfigPromise,
};
currentRenderTask = thisRenderTask = pdfPage.render(renderContext);

2
web/pdf_print_service.js
View File

@ -48,7 +48,7 @@ function renderPage(
transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0],
viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation }),
intent: "print",
annotationStorage: pdfDocument.annotationStorage,
includeAnnotationStorage: true,
optionalContentConfigPromise,
};
return pdfPage.render(renderContext).promise;