Add (basic) support for Spread modes in PresentationMode (issue 14749) #14877
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ethod private In PR 14112 usage of this *internal* method was reduced, and it thus can't hurt to clean-up things a little bit more. Note in particular that we can simplify the call-sites by directly passing in the already available `PDFPageView`-instance, since the `id`-property those contain can replace the previous `pageNumber`-parameter[1]. Given that the method name has always been prefixed with an underscore it was thus never intended to be "public", hence we can now enforce that with modern ECMAScript features. --- [1] There's already a bunch of other spots, throughout the viewer-code, where we assume that the `PDFPageView.id`-property contains proper page *numbers* (and not e.g. indices); note how we initialize the `PDFPageView`-instances in the `BaseViewer.setDocument`-method.
After recent changes, adding *basic* Spread mode support in PresentationMode has now become reasonably straightforward. However, documents with *varying* page sizes are non-trivial to handle and would require re-writing (or at least re-factoring) a bunch of the zooming-code. Hence this PR *purposely* only allow Spread modes to be used, in PresentationMode, for documents where all pages have exactly the same size. While this obviously isn't a fully complete solution, it will however cover the vast majority of all documents and should hopefully be deemed good enough for now.
|
/botio-linux preview |
From: Bot.io (Linux m4)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://54.241.84.105:8877/e28a3bf6328fbd7/output.txt |
From: Bot.io (Linux m4)SuccessFull output at http://54.241.84.105:8877/e28a3bf6328fbd7/output.txt Total script time: 2.77 mins Published |
timvandermeij
approved these changes
May 7, 2022
|
Looks good! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simplify the signature of
BaseViewer._scrollIntoViewand make the method privateIn PR 14112 usage of this internal method was reduced, and it thus can't hurt to clean-up things a little bit more.
Note in particular that we can simplify the call-sites by directly passing in the already available
PDFPageView-instance, since theid-property those contain can replace the previouspageNumber-parameter[1].Given that the method name has always been prefixed with an underscore it was thus never intended to be "public", hence we can now enforce that with modern ECMAScript features.
[1] There's already a bunch of other spots, throughout the viewer-code, where we assume that the
PDFPageView.id-property contains proper page numbers (and not e.g. indices); note how we initialize thePDFPageView-instances in theBaseViewer.setDocument-method.Add (basic) support for Spread modes in PresentationMode (issue 14749)
After recent changes, adding basic Spread mode support in PresentationMode has now become reasonably straightforward.
However, documents with varying page sizes are non-trivial to handle and would require re-writing (or at least re-factoring) a bunch of the zooming-code.
Hence this PR purposely only allow Spread modes to be used, in PresentationMode, for documents where all pages have exactly the same size. While this obviously isn't a fully complete solution, it will however cover the vast majority of all documents and should hopefully be deemed good enough for now.