Skip to content
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.

Sign up for GitHub

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

Don't hide the different layers when zooming #19186

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Don't hide the different layers when zooming #19186

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 16 additions & 16 deletions web/pdf_page_view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,8 @@ class PDFPageView {

#scaleRoundY = 1;

#structTreeDOM = null;

#renderError = null;

#renderingState = RenderingStates.INITIAL;
Expand Down Expand Up @@ -482,11 +484,12 @@ class PDFPageView {
* aria-owns to work.
*/
async #renderStructTreeLayer() {
if (!this.textLayer) {
if (!this.textLayer || this.#structTreeDOM) {
return;
}

const treeDom = await this.structTreeLayer?.render();
const treeDom = (this.#structTreeDOM =
await this.structTreeLayer?.render());
if (treeDom) {
this.l10n.pause();
this.structTreeLayer?.addElementsToTextLayer();
Expand Down Expand Up @@ -563,23 +566,9 @@ class PDFPageView {
}
div.removeAttribute("data-loaded");

if (annotationLayerNode) {
// Hide the annotation layer until all elements are resized
// so they are not displayed on the already resized page.
this.annotationLayer.hide();
}
if (annotationEditorLayerNode) {
this.annotationEditorLayer.hide();
}
Comment on lines -566 to -573
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Dec 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What about pages that are still active in the PDFPageViewBuffer buffer, but not currently visible?

These annotation-related layers won't have e.g. their rotation updated until they become visible again, and it's not clear to me if that could cause them to overflow their respective page-containers (similar to the xfaLayer) and thus potentially "mess" with layout?

if (xfaLayerNode) {
// Hide the XFA layer until all elements are resized
// so they are not displayed on the already resized page.
this.xfaLayer.hide();
}
Comment on lines -574 to -578
Copy link
Collaborator

@Snuffleupagus Snuffleupagus Dec 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Given that the xfaLayer is a bit special, is keeping it visible actually correct and safe in general?

Ninja-edit: Testing the preview the answer unfortunately seems to be NO, since scrolling to the end of an XFA document and then rotating it triggers intermittent rendering glitches (since pages that are not visible will not be updated until they are scrolled into view again).

Here's a screen-shot of what this looks like:
xfa

if (textLayerNode) {
this.textLayer.hide();
}
this.structTreeLayer?.hide();

if (!keepCanvasWrapper && this.#canvasWrapper) {
this.#canvasWrapper = null;
Expand Down Expand Up @@ -761,6 +750,8 @@ class PDFPageView {
}
if (this.structTreeLayer && !this.textLayer) {
this.structTreeLayer = null;
this.#structTreeDOM?.remove();
this.#structTreeDOM = null;
}
if (
this.annotationEditorLayer &&
Expand Down Expand Up @@ -984,15 +975,23 @@ class PDFPageView {
// Don't add the canvas until the first draw callback, or until
// drawing is complete when `!this.renderingQueue`, to prevent black
// flickering.
this.l10n.pause();
canvasWrapper.append(canvas);
this.l10n.resume();
showCanvas = null;
return;
}
if (!isLastShow) {
return;
}

this.l10n.pause();
if (prevCanvas) {
if (this.#structTreeDOM) {
// Add the struct tree now in order to minimize the number of reflows.
canvas.append(this.#structTreeDOM);
this.structTreeLayer.show();
}
prevCanvas.replaceWith(canvas);
prevCanvas.width = prevCanvas.height = 0;
} else {
Expand All @@ -1001,6 +1000,7 @@ class PDFPageView {
// have a final flash we just display it once all the drawing is done.
canvasWrapper.append(canvas);
}
this.l10n.resume();

showCanvas = null;
};
Expand Down
Loading