Skip to content

Commit

Permalink
Add previous/next-page functionality that takes scroll/spread-modes i…
Browse files Browse the repository at this point in the history
…nto account (issue 11946)

 - For wrapped scrolling, we unfortunately need to do a fair bit of parsing of the *current* page layout. Compared to e.g. the spread-modes, where we can easily tell how the pages are laid out, with wrapped scrolling we cannot tell without actually checking. In particular documents with varying page sizes require some care, since we need to check all pages on the "row" of the current page are visible and that there aren't any "holes" present. Otherwise, in the general case, there's a risk that we'd skip over pages if we'd simply always advance to the previous/next "row" in wrapped scrolling.

 - For horizontal scrolling, this patch simply maintains the current behaviour of advancing *one* page at a time. The reason for this is to prevent inconsistent behaviour for the next and previous cases, since those cannot be handled identically. For the next-case, it'd obviously be simple to advance to the first not completely visible page. However for the previous-case, we'd only be able to go back *one* page since it's not possible to (easily) determine the page layout of non-visible pages (documents with varying page sizes being a particular issue).

 - For vertical scrolling, this patch maintains the current behaviour by default. When spread-modes are being used, we'll now attempt to advance to the next *spread*, rather than just the next page, whenever possible. To prevent skipping over a page, this two-page advance will only apply when both pages of the current spread are visible (to avoid breaking documents with varying page sizes) and when the second page in the current spread is fully visible *horizontally* (to handle larger zoom values).

In order to reduce the performance impact of these changes, note that the previous/next-functionality will only call `getVisibleElements` for the scroll/spread-modes where that's necessary and that "normal" vertical scrolling is thus unaffected by these changes.

To support these changes, the `getVisibleElements` helper function will now also include the `widthPercent` in addition to the existing `percent` property.
The `PDFViewer._updateHelper` method is changed slightly w.r.t. updating the `currentPageNumber` for the non-vertical/non-spread modes, i.e. won't affect "normal" vertical scrolling, since that helped simplify the overall calculation of the page advance.

Finally, these new `BaseViewer` methods also allow (some) simplification of previous/next-page functionality in various viewer components.

*Please note:* There's one thing that this patch does not attempt to change, namely disabling of the previous/next toolbarButtons respectively the firstPage/lastPage secondaryToolbarButtons. The reason for this is that doing so would add quite a bit of complexity in general, and if for some reason `BaseViewer._getPageAdvance` would get things wrong we could end up incorrectly disabling the buttons. Hence it seemed overall safer to *not* touch this, and accept that the buttons won't be `disabled` despite in some edge-cases no further scrolling being possible.
  • Loading branch information
Snuffleupagus committed Jan 21, 2021
1 parent 9d4bad9 commit 1190193
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 56 deletions.
20 changes: 15 additions & 5 deletions test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -650,11 +650,21 @@ describe("ui_utils", function () {
const hiddenWidth =
Math.max(0, scrollLeft - viewLeft) +
Math.max(0, viewRight - scrollRight);
const visibleArea =
(div.clientHeight - hiddenHeight) * (div.clientWidth - hiddenWidth);
const percent =
((visibleArea * 100) / div.clientHeight / div.clientWidth) | 0;
views.push({ id: view.id, x: viewLeft, y: viewTop, view, percent });

const fractionHeight =
(div.clientHeight - hiddenHeight) / div.clientHeight;
const fractionWidth =
(div.clientWidth - hiddenWidth) / div.clientWidth;
const percent = (fractionHeight * fractionWidth * 100) | 0;

views.push({
id: view.id,
x: viewLeft,
y: viewTop,
view,
percent,
widthPercent: (fractionWidth * 100) | 0,
});
}
}
return { first: views[0], last: views[views.length - 1], views };
Expand Down
12 changes: 4 additions & 8 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -2829,10 +2829,10 @@ function webViewerLastPage() {
}
}
function webViewerNextPage() {
PDFViewerApplication.page++;
PDFViewerApplication.pdfViewer.nextPage();
}
function webViewerPreviousPage() {
PDFViewerApplication.page--;
PDFViewerApplication.pdfViewer.previousPage();
}
function webViewerZoomIn() {
PDFViewerApplication.zoomIn();
Expand Down Expand Up @@ -3351,13 +3351,9 @@ function webViewerKeyDown(evt) {
(!turnOnlyIfPageFit || pdfViewer.currentScaleValue === "page-fit")
) {
if (turnPage > 0) {
if (PDFViewerApplication.page < PDFViewerApplication.pagesCount) {
PDFViewerApplication.page++;
}
pdfViewer.nextPage();
} else {
if (PDFViewerApplication.page > 1) {
PDFViewerApplication.page--;
}
pdfViewer.previousPage();
}
handled = true;
}
Expand Down
134 changes: 134 additions & 0 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -1507,6 +1507,140 @@ class BaseViewer {
this.update();
}

/**
* @private
*/
_getPageAdvance(currentPageNumber, previous = false) {
if (this.isInPresentationMode) {
return 1;
}
switch (this._scrollMode) {
case ScrollMode.WRAPPED: {
const { views } = this._getVisiblePages(),
pageLayout = new Map();

// Determine the current (visible) page layout.
for (const { id, y, percent, widthPercent } of views) {
if (percent === 0 || widthPercent < 100) {
continue;
}
let yArray = pageLayout.get(y);
if (!yArray) {
pageLayout.set(y, (yArray ||= []));
}
yArray.push(id);
}
// Find the row of the current page.
for (const yArray of pageLayout.values()) {
const currentIndex = yArray.indexOf(currentPageNumber);
if (currentIndex === -1) {
continue;
}
const numPages = yArray.length;
if (numPages === 1) {
break;
}
// Handle documents with varying page sizes.
if (previous) {
for (let i = currentIndex - 1, ii = 0; i >= ii; i--) {
const currentId = yArray[i],
expectedId = yArray[i + 1] - 1;
if (currentId < expectedId) {
return currentPageNumber - expectedId;
}
}
} else {
for (let i = currentIndex + 1, ii = numPages; i < ii; i++) {
const currentId = yArray[i],
expectedId = yArray[i - 1] + 1;
if (currentId > expectedId) {
return expectedId - currentPageNumber;
}
}
}
// The current row is "complete", advance to the previous/next one.
if (previous) {
const firstId = yArray[0];
if (firstId < currentPageNumber) {
return currentPageNumber - firstId + 1;
}
} else {
const lastId = yArray[numPages - 1];
if (lastId > currentPageNumber) {
return lastId - currentPageNumber + 1;
}
}
break;
}
break;
}
case ScrollMode.HORIZONTAL: {
break;
}
case ScrollMode.VERTICAL: {
if (this._spreadMode === SpreadMode.NONE) {
break; // Normal vertical scrolling.
}
const parity = this._spreadMode - 1;

if (previous && currentPageNumber % 2 !== parity) {
break; // Left-hand side page.
} else if (!previous && currentPageNumber % 2 === parity) {
break; // Right-hand side page.
}
const { views } = this._getVisiblePages(),
expectedId = previous ? currentPageNumber - 1 : currentPageNumber + 1;

for (const { id, percent, widthPercent } of views) {
if (id !== expectedId) {
continue;
}
if (percent > 0 && widthPercent === 100) {
return 2;
}
break;
}
break;
}
}
return 1;
}

/**
* Go to the next page, taking scroll/spread-modes into account.
* @returns {boolean} Whether navigation occured.
*/
nextPage() {
const currentPageNumber = this._currentPageNumber,
pagesCount = this.pagesCount;

if (currentPageNumber >= pagesCount) {
return false;
}
const advance =
this._getPageAdvance(currentPageNumber, /* previous = */ false) || 1;

this.currentPageNumber = Math.min(currentPageNumber + advance, pagesCount);
return true;
}

/**
* Go to the previous page, taking scroll/spread-modes into account.
* @returns {boolean} Whether navigation occured.
*/
previousPage() {
const currentPageNumber = this._currentPageNumber;

if (currentPageNumber <= 1) {
return false;
}
const advance =
this._getPageAdvance(currentPageNumber, /* previous = */ true) || 1;

this.currentPageNumber = Math.max(currentPageNumber - advance, 1);
return true;
}

initializeScriptingEvents() {
if (!this.enableScripting || this._pageOpenPendingSet) {
return;
Expand Down
8 changes: 2 additions & 6 deletions web/pdf_link_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -401,15 +401,11 @@ class PDFLinkService {
break;

case "NextPage":
if (this.page < this.pagesCount) {
this.page++;
}
this.pdfViewer.nextPage();
break;

case "PrevPage":
if (this.page > 1) {
this.page--;
}
this.pdfViewer.previousPage();
break;

case "LastPage":
Expand Down
38 changes: 7 additions & 31 deletions web/pdf_presentation_mode.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,9 @@ class PDFPresentationMode {
const totalDelta = this.mouseScrollDelta;
this._resetMouseScrollState();
const success =
totalDelta > 0 ? this._goToPreviousPage() : this._goToNextPage();
totalDelta > 0
? this.pdfViewer.previousPage()
: this.pdfViewer.nextPage();
if (success) {
this.mouseScrollTimeStamp = currentTime;
}
Expand All @@ -153,32 +155,6 @@ class PDFPresentationMode {
);
}

/**
* @private
*/
_goToPreviousPage() {
const page = this.pdfViewer.currentPageNumber;
// If we're at the first page, we don't need to do anything.
if (page <= 1) {
return false;
}
this.pdfViewer.currentPageNumber = page - 1;
return true;
}

/**
* @private
*/
_goToNextPage() {
const page = this.pdfViewer.currentPageNumber;
// If we're at the last page, we don't need to do anything.
if (page >= this.pdfViewer.pagesCount) {
return false;
}
this.pdfViewer.currentPageNumber = page + 1;
return true;
}

/**
* @private
*/
Expand Down Expand Up @@ -315,9 +291,9 @@ class PDFPresentationMode {
evt.preventDefault();

if (evt.shiftKey) {
this._goToPreviousPage();
this.pdfViewer.previousPage();
} else {
this._goToNextPage();
this.pdfViewer.nextPage();
}
}
}
Expand Down Expand Up @@ -422,9 +398,9 @@ class PDFPresentationMode {
delta = dy;
}
if (delta > 0) {
this._goToPreviousPage();
this.pdfViewer.previousPage();
} else if (delta < 0) {
this._goToNextPage();
this.pdfViewer.nextPage();
}
break;
}
Expand Down
4 changes: 4 additions & 0 deletions web/pdf_single_page_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,10 @@ class PDFSinglePageViewer extends BaseViewer {
_updateScrollMode() {}

_updateSpreadMode() {}

_getPageAdvance() {
return 1;
}
}

export { PDFSinglePageViewer };
7 changes: 6 additions & 1 deletion web/pdf_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* limitations under the License.
*/

import { ScrollMode, SpreadMode } from "./ui_utils.js";
import { BaseViewer } from "./base_viewer.js";
import { shadow } from "pdfjs-lib";

Expand Down Expand Up @@ -57,7 +58,11 @@ class PDFViewer extends BaseViewer {
if (page.percent < 100) {
break;
}
if (page.id === currentId) {
if (
page.id === currentId &&
this._scrollMode === ScrollMode.VERTICAL &&
this._spreadMode === SpreadMode.NONE
) {
stillFullyVisible = true;
break;
}
Expand Down
11 changes: 6 additions & 5 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -563,17 +563,18 @@ function getVisibleElements({
Math.max(0, top - currentHeight) + Math.max(0, viewBottom - bottom);
const hiddenWidth =
Math.max(0, left - currentWidth) + Math.max(0, viewRight - right);
const percent =
(((viewHeight - hiddenHeight) * (viewWidth - hiddenWidth) * 100) /
viewHeight /
viewWidth) |
0;

const fractionHeight = (viewHeight - hiddenHeight) / viewHeight,
fractionWidth = (viewWidth - hiddenWidth) / viewWidth;
const percent = (fractionHeight * fractionWidth * 100) | 0;

visible.push({
id: view.id,
x: currentWidth,
y: currentHeight,
view,
percent,
widthPercent: (fractionWidth * 100) | 0,
});
}

Expand Down

0 comments on commit 1190193

Please sign in to comment.