Skip to content

Convert PDFPageViewBuffer to a standard class, and use a Set internally #14245

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Nov 7, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions test/unit/base_viewer_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,12 @@ describe("BaseViewer", function () {

// Ensure that decreasing the size will evict the correct views,
// while re-ordering the remaining ones correctly.
buffer.resize(3, new Set([1, 2, 3]));
buffer.resize(3, new Set([1, 2, 5]));

expect(buffer._buffer).toEqual([
viewsMap.get(1),
viewsMap.get(2),
viewsMap.get(3),
viewsMap.get(5),
]);
});

Expand Down
41 changes: 0 additions & 41 deletions test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
getVisibleElements,
isPortraitOrientation,
isValidRotation,
moveToEndOfArray,
parseQueryString,
waitOnEventOrTimeout,
WaitOnType,
Expand Down Expand Up @@ -864,44 +863,4 @@ describe("ui_utils", function () {
});
});
});

describe("moveToEndOfArray", function () {
it("works on empty arrays", function () {
const data = [];
moveToEndOfArray(data, function () {});
expect(data).toEqual([]);
});

it("works when moving everything", function () {
const data = [1, 2, 3, 4, 5];
moveToEndOfArray(data, function () {
return true;
});
expect(data).toEqual([1, 2, 3, 4, 5]);
});

it("works when moving some things", function () {
const data = [1, 2, 3, 4, 5];
moveToEndOfArray(data, function (x) {
return x % 2 === 0;
});
expect(data).toEqual([1, 3, 5, 2, 4]);
});

it("works when moving one thing", function () {
const data = [1, 2, 3, 4, 5];
moveToEndOfArray(data, function (x) {
return x === 1;
});
expect(data).toEqual([2, 3, 4, 5, 1]);
});

it("works when moving nothing", function () {
const data = [1, 2, 3, 4, 5];
moveToEndOfArray(data, function (x) {
return x === 0;
});
expect(data).toEqual([1, 2, 3, 4, 5]);
});
});
});
98 changes: 63 additions & 35 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import {
MAX_AUTO_SCALE,
MAX_SCALE,
MIN_SCALE,
moveToEndOfArray,
PresentationModeState,
RendererType,
SCROLLBAR_PADDING,
Expand Down Expand Up @@ -92,18 +91,38 @@ const DEFAULT_CACHE_SIZE = 10;
* @property {IL10n} l10n - Localization service.
*/

function PDFPageViewBuffer(size) {
const data = [];
this.push = function (view) {
const i = data.indexOf(view);
if (i >= 0) {
data.splice(i, 1);
class PDFPageViewBuffer {
// Here we rely on the fact that `Set`s preserve the insertion order.
#buf = new Set();

#size = 0;

constructor(size) {
this.#size = size;

if (
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION || TESTING")
) {
Object.defineProperty(this, "_buffer", {
get() {
return [...this.#buf];
},
});
}
data.push(view);
if (data.length > size) {
data.shift().destroy();
}

push(view) {
const buf = this.#buf;
if (buf.has(view)) {
buf.delete(view); // Move the view to the "end" of the buffer.
}
buf.add(view);

if (buf.size > this.#size) {
this.#destroyFirstView();
}
};
}

/**
* After calling resize, the size of the buffer will be `newSize`.
Expand All @@ -112,31 +131,38 @@ function PDFPageViewBuffer(size) {
* `idsToKeep` has no impact on the final size of the buffer; if `idsToKeep`
* is larger than `newSize`, some of those pages will be destroyed anyway.
*/
this.resize = function (newSize, idsToKeep = null) {
size = newSize;
resize(newSize, idsToKeep = null) {
this.#size = newSize;

const buf = this.#buf;
if (idsToKeep) {
moveToEndOfArray(data, function (page) {
return idsToKeep.has(page.id);
});
const ii = buf.size;
let i = 1;
for (const view of buf) {
if (idsToKeep.has(view.id)) {
buf.delete(view); // Move the view to the "end" of the buffer.
buf.add(view);
}
if (++i > ii) {
break;
}
Comment on lines +146 to +148
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please note: The unit-test was updated to ensure that I didn't mess up this particular condition.

}
}
while (data.length > size) {
data.shift().destroy();

while (buf.size > this.#size) {
this.#destroyFirstView();
}
};
}

this.has = function (view) {
return data.includes(view);
};
has(view) {
return this.#buf.has(view);
}

if (
typeof PDFJSDev === "undefined" ||
PDFJSDev.test("!PRODUCTION || TESTING")
) {
Object.defineProperty(this, "_buffer", {
get() {
return data.slice();
},
});
#destroyFirstView() {
const firstView = this.#buf.keys().next().value;

firstView?.destroy();
this.#buf.delete(firstView);
}
}

Expand All @@ -156,6 +182,8 @@ function isSameScale(oldScale, newScale) {
* Simple viewer control to display PDF content/pages.
*/
class BaseViewer {
#buffer = null;

#scrollModePageState = null;

/**
Expand Down Expand Up @@ -518,7 +546,7 @@ class BaseViewer {
}
// Add the page to the buffer at the start of drawing. That way it can be
// evicted from the buffer and destroyed even if we pause its rendering.
this._buffer.push(pageView);
this.#buffer.push(pageView);
};
this.eventBus._on("pagerender", this._onBeforeDraw);

Expand Down Expand Up @@ -684,7 +712,7 @@ class BaseViewer {
this._currentScale = UNKNOWN_SCALE;
this._currentScaleValue = null;
this._pageLabels = null;
this._buffer = new PDFPageViewBuffer(DEFAULT_CACHE_SIZE);
this.#buffer = new PDFPageViewBuffer(DEFAULT_CACHE_SIZE);
this._location = null;
this._pagesRotation = 0;
this._optionalContentConfigPromise = null;
Expand Down Expand Up @@ -1153,7 +1181,7 @@ class BaseViewer {
return;
}
const newCacheSize = Math.max(DEFAULT_CACHE_SIZE, 2 * numVisiblePages + 1);
this._buffer.resize(newCacheSize, visible.ids);
this.#buffer.resize(newCacheSize, visible.ids);

this.renderingQueue.renderHighestPriority(visible);

Expand Down Expand Up @@ -1304,7 +1332,7 @@ class BaseViewer {
return false;
}
const pageView = this._pages[pageNumber - 1];
return this._buffer.has(pageView);
return this.#buffer.has(pageView);
}

cleanup() {
Expand Down
22 changes: 0 additions & 22 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -906,27 +906,6 @@ class ProgressBar {
}
}

/**
* Moves all elements of an array that satisfy condition to the end of the
* array, preserving the order of the rest.
*/
function moveToEndOfArray(arr, condition) {
const moved = [],
len = arr.length;
let write = 0;
for (let read = 0; read < len; ++read) {
if (condition(arr[read])) {
moved.push(arr[read]);
} else {
arr[write] = arr[read];
++write;
}
}
for (let read = 0; write < len; ++read, ++write) {
arr[write] = moved[read];
}
}

/**
* Get the active or focused element in current DOM.
*
Expand Down Expand Up @@ -1031,7 +1010,6 @@ export {
MAX_AUTO_SCALE,
MAX_SCALE,
MIN_SCALE,
moveToEndOfArray,
noContextMenuHandler,
normalizeWheelEventDelta,
normalizeWheelEventDirection,
Expand Down