From f0719ed565368fe87649efe564229a60c5c8c0d4 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Fri, 21 Dec 2018 11:47:37 +0100 Subject: [PATCH] [api-minor] Change the `getViewport` method, on `PDFPageProxy`, to take a parameter object rather than a bunch of (randomly) ordered parameters If, as PR 10368 suggests, more parameters should be added to `getViewport` I think that it would be a mistake to not change the signature *first* to avoid needlessly unwieldy call-sites. To not break any existing code and third-party use-cases, this is obviously implemented with a deprecation warning *and* with a working fallback[1] for the old method signature. --- [1] This is limited to `GENERIC` builds, which should be sufficient. --- examples/acroforms/acroforms.js | 2 +- examples/browserify/main.js | 2 +- examples/components/pageviewer.js | 2 +- examples/node/getinfo.js | 2 +- examples/node/pdf2png/pdf2png.js | 2 +- examples/node/pdf2svg.js | 2 +- examples/text-only/pdf2svg.js | 2 +- examples/webpack/main.js | 2 +- src/display/api.js | 31 +++++++++++++++++++++++-------- src/display/dom_utils.js | 6 +++--- test/driver.js | 2 +- test/unit/api_spec.js | 17 +++++++++-------- test/unit/custom_spec.js | 4 ++-- web/base_viewer.js | 4 ++-- web/firefox_print_service.js | 2 +- web/pdf_page_view.js | 4 ++-- web/pdf_print_service.js | 2 +- web/pdf_thumbnail_view.js | 2 +- web/pdf_thumbnail_viewer.js | 2 +- 19 files changed, 54 insertions(+), 38 deletions(-) diff --git a/examples/acroforms/acroforms.js b/examples/acroforms/acroforms.js index 1f48233d71764..125c3aa83f85e 100644 --- a/examples/acroforms/acroforms.js +++ b/examples/acroforms/acroforms.js @@ -37,7 +37,7 @@ loadingTask.promise.then(function(doc) { container: container, id: pageNum, scale: DEFAULT_SCALE, - defaultViewport: pdfPage.getViewport(DEFAULT_SCALE), + defaultViewport: pdfPage.getViewport({ scale: DEFAULT_SCALE, }), annotationLayerFactory: new pdfjsViewer.DefaultAnnotationLayerFactory(), renderInteractiveForms: true, diff --git a/examples/browserify/main.js b/examples/browserify/main.js index cb7f5b56359df..6c4e144b1af4d 100644 --- a/examples/browserify/main.js +++ b/examples/browserify/main.js @@ -17,7 +17,7 @@ loadingTask.promise.then(function (pdfDocument) { // Request a first page return pdfDocument.getPage(1).then(function (pdfPage) { // Display page on the existing canvas with 100% scale. - var viewport = pdfPage.getViewport(1.0); + var viewport = pdfPage.getViewport({ scale: 1.0, }); var canvas = document.getElementById('theCanvas'); canvas.width = viewport.width; canvas.height = viewport.height; diff --git a/examples/components/pageviewer.js b/examples/components/pageviewer.js index df69fdad0e774..d58f4ccb41f40 100644 --- a/examples/components/pageviewer.js +++ b/examples/components/pageviewer.js @@ -50,7 +50,7 @@ loadingTask.promise.then(function(pdfDocument) { container: container, id: PAGE_TO_VIEW, scale: SCALE, - defaultViewport: pdfPage.getViewport(SCALE), + defaultViewport: pdfPage.getViewport({ scale: SCALE, }), // We can enable text/annotations layers, if needed textLayerFactory: new pdfjsViewer.DefaultTextLayerFactory(), annotationLayerFactory: new pdfjsViewer.DefaultAnnotationLayerFactory(), diff --git a/examples/node/getinfo.js b/examples/node/getinfo.js index f85e911a14195..3df0b2bac1260 100644 --- a/examples/node/getinfo.js +++ b/examples/node/getinfo.js @@ -38,7 +38,7 @@ loadingTask.promise.then(function(doc) { var loadPage = function (pageNum) { return doc.getPage(pageNum).then(function (page) { console.log('# Page ' + pageNum); - var viewport = page.getViewport(1.0 /* scale */); + var viewport = page.getViewport({ scale: 1.0, }); console.log('Size: ' + viewport.width + 'x' + viewport.height); console.log(); return page.getTextContent().then(function (content) { diff --git a/examples/node/pdf2png/pdf2png.js b/examples/node/pdf2png/pdf2png.js index 2fa7635ce9cff..015851238c81b 100644 --- a/examples/node/pdf2png/pdf2png.js +++ b/examples/node/pdf2png/pdf2png.js @@ -64,7 +64,7 @@ loadingTask.promise.then(function(pdfDocument) { // Get the first page. pdfDocument.getPage(1).then(function (page) { // Render the page on a Node canvas with 100% scale. - var viewport = page.getViewport(1.0); + var viewport = page.getViewport({ scale: 1.0, }); var canvasFactory = new NodeCanvasFactory(); var canvasAndContext = canvasFactory.create(viewport.width, viewport.height); diff --git a/examples/node/pdf2svg.js b/examples/node/pdf2svg.js index 3ac42f80c0b9b..50860d869f824 100644 --- a/examples/node/pdf2svg.js +++ b/examples/node/pdf2svg.js @@ -100,7 +100,7 @@ loadingTask.promise.then(function(doc) { var loadPage = function (pageNum) { return doc.getPage(pageNum).then(function (page) { console.log('# Page ' + pageNum); - var viewport = page.getViewport(1.0 /* scale */); + var viewport = page.getViewport({ scale: 1.0, }); console.log('Size: ' + viewport.width + 'x' + viewport.height); console.log(); diff --git a/examples/text-only/pdf2svg.js b/examples/text-only/pdf2svg.js index 72295b1a65ddf..3ce9d62591867 100644 --- a/examples/text-only/pdf2svg.js +++ b/examples/text-only/pdf2svg.js @@ -52,7 +52,7 @@ function pageLoaded() { var loadingTask = pdfjsLib.getDocument({ url: PDF_PATH, }); loadingTask.promise.then(function(pdfDocument) { pdfDocument.getPage(PAGE_NUMBER).then(function (page) { - var viewport = page.getViewport(PAGE_SCALE); + var viewport = page.getViewport({ scale: PAGE_SCALE, }); page.getTextContent().then(function (textContent) { // building SVG and adding that to the DOM var svg = buildSVG(viewport, textContent); diff --git a/examples/webpack/main.js b/examples/webpack/main.js index 62ff5e4133fa1..f8bfd179bbe1e 100644 --- a/examples/webpack/main.js +++ b/examples/webpack/main.js @@ -17,7 +17,7 @@ loadingTask.promise.then(function (pdfDocument) { // Request a first page return pdfDocument.getPage(1).then(function (pdfPage) { // Display page on the existing canvas with 100% scale. - var viewport = pdfPage.getViewport(1.0); + var viewport = pdfPage.getViewport({ scale: 1.0, }); var canvas = document.getElementById('theCanvas'); canvas.width = viewport.width; canvas.height = viewport.height; diff --git a/src/display/api.js b/src/display/api.js index 6a75d82886b75..a237b03a34392 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -755,6 +755,17 @@ class PDFDocumentProxy { } } +/** + * Page getViewport parameters. + * + * @typedef {Object} GetViewportParameters + * @property {number} scale - The desired scale of the viewport. + * @property {number} rotation - (optional) The desired rotation, in degrees, of + * the viewport. If omitted it defaults to the page rotation. + * @property {boolean} dontFlip - (optional) If true, the y-axis will not be + * flipped. The default value is `false`. + */ + /** * Page getTextContent parameters. * @@ -811,7 +822,7 @@ class PDFDocumentProxy { * @typedef {Object} RenderParameters * @property {Object} canvasContext - A 2D context of a DOM Canvas object. * @property {PageViewport} viewport - Rendering viewport obtained by - * calling of PDFPage.getViewport method. + * calling the `PDFPageProxy.getViewport` method. * @property {string} intent - Rendering intent, can be 'display' or 'print' * (default value is 'display'). * @property {boolean} enableWebGL - (optional) Enables WebGL accelerated @@ -900,18 +911,22 @@ class PDFPageProxy { } /** - * @param {number} scale The desired scale of the viewport. - * @param {number} rotate Degrees to rotate the viewport. If omitted this - * defaults to the page rotation. - * @param {boolean} dontFlip (optional) If true, axis Y will not be flipped. + * @param {GetViewportParameters} params - Viewport parameters. * @return {PageViewport} Contains 'width' and 'height' properties - * along with transforms required for rendering. + * along with transforms required for rendering. */ - getViewport(scale, rotate = this.rotate, dontFlip = false) { + getViewport({ scale, rotation = this.rotate, dontFlip = false, } = {}) { + if ((typeof PDFJSDev !== 'undefined' && PDFJSDev.test('GENERIC')) && + (arguments.length > 1 || typeof arguments[0] === 'number')) { + deprecated('getViewport is called with obsolete arguments.'); + scale = arguments[0]; + rotation = typeof arguments[1] === 'number' ? arguments[1] : this.rotate; + dontFlip = typeof arguments[2] === 'boolean' ? arguments[2] : false; + } return new PageViewport({ viewBox: this.view, scale, - rotation: rotate, + rotation, dontFlip, }); } diff --git a/src/display/dom_utils.js b/src/display/dom_utils.js index 06f7a7b8e98a5..5117405132585 100644 --- a/src/display/dom_utils.js +++ b/src/display/dom_utils.js @@ -140,11 +140,11 @@ class DOMSVGFactory { * @property {Array} viewBox - The xMin, yMin, xMax and yMax coordinates. * @property {number} scale - The scale of the viewport. * @property {number} rotation - The rotation, in degrees, of the viewport. - * @property {number} offsetX - (optional) The vertical, i.e. x-axis, offset. + * @property {number} offsetX - (optional) The horizontal, i.e. x-axis, offset. * The default value is `0`. - * @property {number} offsetY - (optional) The horizontal, i.e. y-axis, offset. + * @property {number} offsetY - (optional) The vertical, i.e. y-axis, offset. * The default value is `0`. - * @property {boolean} dontFlip - (optional) If true, the x-axis will not be + * @property {boolean} dontFlip - (optional) If true, the y-axis will not be * flipped. The default value is `false`. */ diff --git a/test/driver.js b/test/driver.js index ae23dee2c2c19..464f01e15f1db 100644 --- a/test/driver.js +++ b/test/driver.js @@ -471,7 +471,7 @@ var Driver = (function DriverClosure() { // eslint-disable-line no-unused-vars this.canvas.mozOpaque = true; ctx = this.canvas.getContext('2d', { alpha: false, }); task.pdfDoc.getPage(task.pageNum).then(function(page) { - var viewport = page.getViewport(PDF_TO_CSS_UNITS); + var viewport = page.getViewport({ scale: PDF_TO_CSS_UNITS, }); self.canvas.width = viewport.width; self.canvas.height = viewport.height; self._clearCanvas(); diff --git a/test/unit/api_spec.js b/test/unit/api_spec.js index 4ae03fbf0bd6c..c4accfd2800eb 100644 --- a/test/unit/api_spec.js +++ b/test/unit/api_spec.js @@ -1034,7 +1034,7 @@ describe('api', function() { expect(page.view).toEqual([0, 0, 595.28, 841.89]); }); it('gets viewport', function () { - var viewport = page.getViewport(1.5, 90); + var viewport = page.getViewport({ scale: 1.5, rotation: 90, }); expect(viewport.viewBox).toEqual(page.view); expect(viewport.scale).toEqual(1.5); expect(viewport.rotation).toEqual(90); @@ -1045,8 +1045,9 @@ describe('api', function() { it('gets viewport respecting "dontFlip" argument', function () { const scale = 1; const rotation = 135; - let viewport = page.getViewport(scale, rotation); - let dontFlipViewport = page.getViewport(scale, rotation, true); + let viewport = page.getViewport({ scale, rotation, }); + let dontFlipViewport = page.getViewport({ scale, rotation, + dontFlip: true, }); expect(dontFlipViewport).not.toEqual(viewport); expect(dontFlipViewport).toEqual(viewport.clone({ dontFlip: true, })); @@ -1239,7 +1240,7 @@ describe('api', function() { loadingTask.promise.then((pdfDoc) => { return pdfDoc.getPage(1).then((pdfPage) => { - let viewport = pdfPage.getViewport(1); + let viewport = pdfPage.getViewport({ scale: 1, }); canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); let renderTask = pdfPage.render({ @@ -1273,7 +1274,7 @@ describe('api', function() { if (isNodeJS()) { pending('TODO: Support Canvas testing in Node.js.'); } - var viewport = page.getViewport(1); + var viewport = page.getViewport({ scale: 1, }); var canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); var renderTask = page.render({ @@ -1297,7 +1298,7 @@ describe('api', function() { if (isNodeJS()) { pending('TODO: Support Canvas testing in Node.js.'); } - let viewport = page.getViewport(1); + let viewport = page.getViewport({ scale: 1, }); let canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); let renderTask = page.render({ @@ -1326,7 +1327,7 @@ describe('api', function() { if (isNodeJS()) { pending('TODO: Support Canvas testing in Node.js.'); } - var viewport = page.getViewport(1); + var viewport = page.getViewport({ scale: 1, }); var canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); var renderTask1 = page.render({ @@ -1368,7 +1369,7 @@ describe('api', function() { const pdf = await loadingTask.promise; pdfDocuments.push(pdf); const page = await pdf.getPage(1); - const viewport = page.getViewport(1.2); + const viewport = page.getViewport({ scale: 1.2, }); const canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); const renderTask = page.render({ diff --git a/test/unit/custom_spec.js b/test/unit/custom_spec.js index 83bfca55404d5..f0a60a900fae2 100644 --- a/test/unit/custom_spec.js +++ b/test/unit/custom_spec.js @@ -61,7 +61,7 @@ describe('custom canvas rendering', function() { if (isNodeJS()) { pending('TODO: Support Canvas testing in Node.js.'); } - var viewport = page.getViewport(1); + var viewport = page.getViewport({ scale: 1, }); var canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); const renderTask = page.render({ @@ -80,7 +80,7 @@ describe('custom canvas rendering', function() { if (isNodeJS()) { pending('TODO: Support Canvas testing in Node.js.'); } - var viewport = page.getViewport(1); + var viewport = page.getViewport({ scale: 1, }); var canvasAndCtx = CanvasFactory.create(viewport.width, viewport.height); const renderTask = page.render({ diff --git a/web/base_viewer.js b/web/base_viewer.js index f37353a19114f..3888198717903 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -419,7 +419,7 @@ class BaseViewer { // viewport for all pages firstPagePromise.then((pdfPage) => { let scale = this.currentScale; - let viewport = pdfPage.getViewport(scale * CSS_UNITS); + let viewport = pdfPage.getViewport({ scale: scale * CSS_UNITS, }); for (let pageNum = 1; pageNum <= pagesCount; ++pageNum) { let textLayerFactory = null; if (this.textLayerMode !== TextLayerMode.DISABLE) { @@ -1031,7 +1031,7 @@ class BaseViewer { */ getPagesOverview() { let pagesOverview = this._pages.map(function(pageView) { - let viewport = pageView.pdfPage.getViewport(1); + let viewport = pageView.pdfPage.getViewport({ scale: 1, }); return { width: viewport.width, height: viewport.height, diff --git a/web/firefox_print_service.js b/web/firefox_print_service.js index ccf77665dbf26..d69a2d936ff0e 100644 --- a/web/firefox_print_service.js +++ b/web/firefox_print_service.js @@ -48,7 +48,7 @@ function composePage(pdfDocument, pageNumber, size, printContainer) { let renderContext = { canvasContext: ctx, transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0], - viewport: pdfPage.getViewport(1, size.rotation), + viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation, }), intent: 'print', }; return pdfPage.render(renderContext).promise; diff --git a/web/pdf_page_view.js b/web/pdf_page_view.js index a72ecbd7d124e..148d66d47d9d6 100644 --- a/web/pdf_page_view.js +++ b/web/pdf_page_view.js @@ -118,8 +118,8 @@ class PDFPageView { this.pdfPageRotate = pdfPage.rotate; let totalRotation = (this.rotation + this.pdfPageRotate) % 360; - this.viewport = pdfPage.getViewport(this.scale * CSS_UNITS, - totalRotation); + this.viewport = pdfPage.getViewport({ scale: this.scale * CSS_UNITS, + rotation: totalRotation, }); this.stats = pdfPage.stats; this.reset(); } diff --git a/web/pdf_print_service.js b/web/pdf_print_service.js index 4ec3ea6613098..a77d118860a05 100644 --- a/web/pdf_print_service.js +++ b/web/pdf_print_service.js @@ -45,7 +45,7 @@ function renderPage(activeServiceOnEntry, pdfDocument, pageNumber, size) { let renderContext = { canvasContext: ctx, transform: [PRINT_UNITS, 0, 0, PRINT_UNITS, 0, 0], - viewport: pdfPage.getViewport(1, size.rotation), + viewport: pdfPage.getViewport({ scale: 1, rotation: size.rotation, }), intent: 'print', }; return pdfPage.render(renderContext).promise; diff --git a/web/pdf_thumbnail_view.js b/web/pdf_thumbnail_view.js index 6463f44e1a738..0144000fd083c 100644 --- a/web/pdf_thumbnail_view.js +++ b/web/pdf_thumbnail_view.js @@ -146,7 +146,7 @@ class PDFThumbnailView { this.pdfPage = pdfPage; this.pdfPageRotate = pdfPage.rotate; let totalRotation = (this.rotation + this.pdfPageRotate) % 360; - this.viewport = pdfPage.getViewport(1, totalRotation); + this.viewport = pdfPage.getViewport({ scale: 1, rotation: totalRotation, }); this.reset(); } diff --git a/web/pdf_thumbnail_viewer.js b/web/pdf_thumbnail_viewer.js index 1573a0a04d4cf..1fdd89cabcbe2 100644 --- a/web/pdf_thumbnail_viewer.js +++ b/web/pdf_thumbnail_viewer.js @@ -166,7 +166,7 @@ class PDFThumbnailViewer { pdfDocument.getPage(1).then((firstPage) => { let pagesCount = pdfDocument.numPages; - let viewport = firstPage.getViewport(1.0); + let viewport = firstPage.getViewport({ scale: 1, }); for (let pageNum = 1; pageNum <= pagesCount; ++pageNum) { let thumbnail = new PDFThumbnailView({ container: this.container,