Reduce the number of calls to PDFView.getVisiblePages from updateViewarea #4772
Conversation
|
It's somewhat unexpected for renderHighestPriority to return result of getVisiblePages. Can we change renderHighestPriority to accept visiblePages as a parameter? We can make it optional and default it to getVisiblePages call results, if needed. |
That was actually my first idea, which I abandoned in favour of the current implementation because I felt that it was more error prone (there's no stopping anyone from calling If you think that's an unwarranted concern, I'll gladly change this! |
|
I understand the concern, that's why I recommend and optional parameter for renderHighestPriority. I people not sure what to provide there, they will omit it. I'm afraid of reverse effect, people will start using renderHighestPriority to get visiblePages :) |
|
I've addressed the comment; so now we instead pass the visiblePages to /botio-linux preview |
From: Bot.io (Linux)ReceivedCommand cmd_preview from @Snuffleupagus received. Current queue size: 0 Live output at: http://107.21.233.14:8877/6f01ace1d4111ff/output.txt |
From: Bot.io (Linux)SuccessFull output at http://107.21.233.14:8877/6f01ace1d4111ff/output.txt Total script time: 0.73 mins Published
|
|
Looks good to me. Codewise it's a cleaner solution. |
Reduce the number of calls to PDFView.getVisiblePages from updateViewarea
Currently for every call to
updateViewareawe callPDFView.getVisiblePages(at viewer.js#L1922) before callingPDFView.renderHighestPriority(at viewer.js#L1928); in that function we again callgetVisiblePages(at viewer.js#L1215).This means that every time when the user scrolls, we will essentially do two back-to-back calls to
getVisiblePages. Given that this requires traversing the DOM to check which pages are actually visible on screen, this seem like the kind of thing you really want to avoid doing if possible (especially in long documents).To avoid the above, I've refactoredrenderHighestPriorityto always return the result ofgetVisiblePages.