From 47f94235abef66e34925b017292675873b22a2dc Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 2 Aug 2021 14:30:08 +0200 Subject: [PATCH] [api-minor] Re-factor the *internal* renderingIntent, and change the default `intent` value in the `PDFPageProxy.getAnnotations` method With the changes made in PR 13746 the *internal* renderingIntent handling became somewhat "messy", since we're now having to do string-matching in various spots in order to handle the "oplist"-intent correctly. Hence this patch, which implements the idea from PR 13746 to convert the `intent`-strings, used in various API-methods, into an *internal* renderingIntent that's implemented using a bit-field instead. *Please note:* This part of the patch, in itself, does *not* change the public API (but see below). This patch is tagged `api-minor` for the following reasons: 1. It changes the *default* value for the `intent` parameter, in the `PDFPageProxy.getAnnotations` method, to "display" in order to be consistent across the API. 2. In order to get *all* annotations, with the `PDFPageProxy.getAnnotations` method, you now need to explicitly set "any" as the `intent` parameter. 3. The `PDFPageProxy.getOperatorList` method will now also support the new "any" intent, to allow accessing the operatorList of all annotations (limited to those types that have one). 4. Finally, for consistency across the API, the `PDFPageProxy.render` method also support the new "any" intent (although I'm not sure how useful that'll be). Points 1 and 2 above are the significant, and thus breaking, changes in *default* behaviour here. However, unfortunately I cannot see a good way to improve the overall API while also keeping `PDFPageProxy.getAnnotations` unchanged. --- src/core/document.js | 32 ++++++++++++++--------- src/core/operator_list.js | 13 +++++++--- src/display/api.js | 53 +++++++++++++++++++++++++++------------ src/shared/util.js | 8 ++++++ test/unit/api_spec.js | 13 +++++++++- 5 files changed, 87 insertions(+), 32 deletions(-) diff --git a/src/core/document.js b/src/core/document.js index 432c6f4e3..45eda09cc 100644 --- a/src/core/document.js +++ b/src/core/document.js @@ -25,6 +25,7 @@ import { isString, OPS, PageActionEventType, + RenderingIntentFlag, shadow, stringToBytes, stringToPDFString, @@ -383,19 +384,18 @@ class Page { pageOpList.flush(true); return { length: pageOpList.totalLength }; } + const intentAny = !!(intent & RenderingIntentFlag.ANY), + intentDisplay = !!(intent & RenderingIntentFlag.DISPLAY), + intentPrint = !!(intent & RenderingIntentFlag.PRINT); // Collect the operator list promises for the annotations. Each promise // is resolved with the complete operator list for a single annotation. - const annotationIntent = intent.startsWith("oplist-") - ? intent.split("-")[1] - : intent; const opListPromises = []; for (const annotation of annotations) { if ( - (annotationIntent === "display" && - annotation.mustBeViewed(annotationStorage)) || - (annotationIntent === "print" && - annotation.mustBePrinted(annotationStorage)) + intentAny || + (intentDisplay && annotation.mustBeViewed(annotationStorage)) || + (intentPrint && annotation.mustBePrinted(annotationStorage)) ) { opListPromises.push( annotation @@ -496,15 +496,23 @@ class Page { getAnnotationsData(intent) { return this._parsedAnnotations.then(function (annotations) { const annotationsData = []; - for (let i = 0, ii = annotations.length; i < ii; i++) { + + if (annotations.length === 0) { + return annotationsData; + } + const intentAny = !!(intent & RenderingIntentFlag.ANY), + intentDisplay = !!(intent & RenderingIntentFlag.DISPLAY), + intentPrint = !!(intent & RenderingIntentFlag.PRINT); + + for (const annotation of annotations) { // Get the annotation even if it's hidden because // JS can change its display. if ( - !intent || - (intent === "display" && annotations[i].viewable) || - (intent === "print" && annotations[i].printable) + intentAny || + (intentDisplay && annotation.viewable) || + (intentPrint && annotation.printable) ) { - annotationsData.push(annotations[i].data); + annotationsData.push(annotation.data); } } return annotationsData; diff --git a/src/core/operator_list.js b/src/core/operator_list.js index e335fdfe4..6f0c3f73d 100644 --- a/src/core/operator_list.js +++ b/src/core/operator_list.js @@ -13,7 +13,14 @@ * limitations under the License. */ -import { assert, ImageKind, OPS, shadow, warn } from "../shared/util.js"; +import { + assert, + ImageKind, + OPS, + RenderingIntentFlag, + shadow, + warn, +} from "../shared/util.js"; function addState(parentState, pattern, checkFn, iterateFn, processFn) { let state = parentState; @@ -597,11 +604,11 @@ class OperatorList { return shadow(this, "CHUNK_SIZE_ABOUT", this.CHUNK_SIZE - 5); } - constructor(intent, streamSink) { + constructor(intent = 0, streamSink) { this._streamSink = streamSink; this.fnArray = []; this.argsArray = []; - if (streamSink && !(intent && intent.startsWith("oplist-"))) { + if (streamSink && !(intent & RenderingIntentFlag.OPLIST)) { this.optimizer = new QueueOptimizer(this); } else { this.optimizer = new NullOptimizer(this); diff --git a/src/display/api.js b/src/display/api.js index ff76f6429..c944a24a6 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -28,6 +28,7 @@ import { isSameOrigin, MissingPDFException, PasswordException, + RenderingIntentFlag, setVerbosityLevel, shadow, stringToBytes, @@ -514,6 +515,26 @@ function _fetchDocument(worker, source, pdfDataRangeTransport, docId) { }); } +function getRenderingIntent(intent, { isOpList = false }) { + let renderingIntent = RenderingIntentFlag.DISPLAY; // Default value. + switch (intent) { + case "any": + renderingIntent = RenderingIntentFlag.ANY; + break; + case "display": + break; + case "print": + renderingIntent = RenderingIntentFlag.PRINT; + break; + default: + warn(`getRenderingIntent - invalid intent: ${intent}`); + } + if (isOpList) { + renderingIntent += RenderingIntentFlag.OPLIST; + } + return renderingIntent; +} + /** * @typedef {Object} OnProgressParameters * @property {number} loaded - Currently loaded number of bytes. @@ -1120,8 +1141,8 @@ class PDFDocumentProxy { * * @typedef {Object} GetAnnotationsParameters * @property {string} [intent] - Determines the annotations that are fetched, - * can be either 'display' (viewable annotations) or 'print' (printable - * annotations). If the parameter is omitted, all annotations are fetched. + * can be 'display' (viewable annotations), 'print' (printable annotations), + * or 'any' (all annotations). The default value is 'display'. */ /** @@ -1131,8 +1152,8 @@ class PDFDocumentProxy { * @property {Object} canvasContext - A 2D context of a DOM Canvas object. * @property {PageViewport} viewport - Rendering viewport obtained by calling * the `PDFPageProxy.getViewport` method. - * @property {string} [intent] - Rendering intent, can be 'display' or 'print'. - * The default value is 'display'. + * @property {string} [intent] - Rendering intent, can be 'display', 'print', + * or 'any'. The default value is 'display'. * @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. The default value is `false`. @@ -1161,8 +1182,8 @@ class PDFDocumentProxy { * Page getOperatorList parameters. * * @typedef {Object} GetOperatorListParameters - * @property {string} [intent] - Rendering intent, can be 'display' or 'print'. - * The default value is 'display'. + * @property {string} [intent] - Rendering intent, can be 'display', 'print', + * or 'any'. The default value is 'display'. */ /** @@ -1276,9 +1297,8 @@ class PDFPageProxy { * @returns {Promise>} A promise that is resolved with an * {Array} of the annotation objects. */ - getAnnotations({ intent = null } = {}) { - const renderingIntent = - intent === "display" || intent === "print" ? intent : null; + getAnnotations({ intent = "display" } = {}) { + const renderingIntent = getRenderingIntent(intent, {}); if ( !this._annotationsPromise || @@ -1336,7 +1356,7 @@ class PDFPageProxy { this._stats.time("Overall"); } - const renderingIntent = intent === "print" ? "print" : "display"; + const renderingIntent = getRenderingIntent(intent, {}); // If there was a pending destroy, cancel it so no cleanup happens during // this call to render. this.pendingCleanup = false; @@ -1390,7 +1410,10 @@ class PDFPageProxy { // Attempt to reduce memory usage during *printing*, by always running // cleanup once rendering has finished (regardless of cleanupAfterRender). - if (this.cleanupAfterRender || renderingIntent === "print") { + if ( + this.cleanupAfterRender || + renderingIntent & RenderingIntentFlag.PRINT + ) { this.pendingCleanup = true; } this._tryCleanup(); @@ -1426,7 +1449,7 @@ class PDFPageProxy { operatorList: intentState.operatorList, pageIndex: this._pageIndex, canvasFactory: canvasFactoryInstance, - useRequestAnimationFrame: renderingIntent !== "print", + useRequestAnimationFrame: !(renderingIntent & RenderingIntentFlag.PRINT), pdfBug: this._pdfBug, }); @@ -1471,9 +1494,7 @@ class PDFPageProxy { } } - const renderingIntent = `oplist-${ - intent === "print" ? "print" : "display" - }`; + const renderingIntent = getRenderingIntent(intent, { isOpList: true }); let intentState = this._intentStates.get(renderingIntent); if (!intentState) { intentState = Object.create(null); @@ -1588,7 +1609,7 @@ class PDFPageProxy { force: true, }); - if (intent.startsWith("oplist-")) { + if (intent & RenderingIntentFlag.OPLIST) { // Avoid errors below, since the renderTasks are just stubs. continue; } diff --git a/src/shared/util.js b/src/shared/util.js index 49970f832..75c3e60f3 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -18,6 +18,13 @@ import "./compatibility.js"; const IDENTITY_MATRIX = [1, 0, 0, 1, 0, 0]; const FONT_IDENTITY_MATRIX = [0.001, 0, 0, 0.001, 0, 0]; +const RenderingIntentFlag = { + ANY: 0x01, + DISPLAY: 0x02, + PRINT: 0x04, + OPLIST: 0x100, +}; + // Permission flags from Table 22, Section 7.6.3.2 of the PDF specification. const PermissionFlag = { PRINT: 0x04, @@ -1033,6 +1040,7 @@ export { PasswordResponses, PermissionFlag, removeNullCharacters, + RenderingIntentFlag, setVerbosityLevel, shadow, StreamType, diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 824d64009..cd8dcc506 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -1443,6 +1443,12 @@ describe("api", function () { expect(data.length).toEqual(4); }); + const anyPromise = page + .getAnnotations({ intent: "any" }) + .then(function (data) { + expect(data.length).toEqual(4); + }); + const displayPromise = page .getAnnotations({ intent: "display" }) .then(function (data) { @@ -1455,7 +1461,12 @@ describe("api", function () { expect(data.length).toEqual(4); }); - await Promise.all([defaultPromise, displayPromise, printPromise]); + await Promise.all([ + defaultPromise, + anyPromise, + displayPromise, + printPromise, + ]); }); it("gets annotations containing relative URLs (bug 766086)", async function () {