mirror of
https://github.com/mozilla/pdf.js.git
synced 2025-04-20 07:08:08 +02:00
70d6550002
This method is currently called from `PDFViewer._scrollUpdate` on *every* scroll event in the viewer. However, I cannot see why this code is now necessary (assuming that it once was), since text-selection and searching still works *exactly* the same way with this patch as with the current `master`. When `PDFPageView.updatePosition` is called, the page can be in either of these states: 1. The page hasn't been rendered, in which case the `textLayer` property doesn't exist yet. 2. The page is currently rendering, meaning that the `textLayer` property exists. Given that the `textContent` won't be fetched until the page has been successfully rendered, `TextLayerBuilder.render` will return immediately and effectively be a no-op (since there's nothing to render yet). 3. The has been been rendered, and the `textLayer` is currently rendering. 4. The page, and its `textLayer`, has been completely rendered. In this case, `TextLayerBuilder.render` will return immediately and effectively be a no-op. Here, only the *third* case seem to require any further analysis: When scrolling occurs while the `textLayer` is rendering, `TextLayerBuilder.render` will via a helper method call `TextLayerRenderTask.cancel` (in src/display/text_layer.js) to stop processing. However, due to the run-to-completion nature of JavaScript, once `TextLayerRenderTask._render` has been invoked `appendText` will always run.[1] So even though we cancel rendering of pending `textLayer`s during scrolling, via the repeated `TextLayerBuilder.render` calls from within the `PDFPageView.updatePosition` method, that does *not* prevent us from running the code inside of `TextLayerRenderTask._render` over and over for the *same* page; which all seems *very* inefficient to me.[2] All this will thus have the effect of delaying the *actual* rendering of a `textLayer` ever so slightly while scrolling in the viewer. However, it does so at the expense of potentially hundreds of unnecessary `appendText` calls.[3] Hence it seems to me that it's less resource intensive overall to simply let rendering of the `textLayer` complete, once it has started. Obviously, we still abort all rendering of a page, and its `textLayer`, when it's being destroyed (e.g. by being evicted from the page cache). In case that there's any worry that the patch could affect e.g. highlighting of search results, please note that the existing code in `TextLayerBuilder.render` already calls `updateMatches` when the `TextLayerTask` resolves successfully. *I'm sorry that this became quite long, but to try and summarize:* `PDFPageView.updatePosition` doesn't actually do anything in *most* cases. In the one case where it matters, it seems that it's actually doing more harm than good; which is why I'm proposing that we just remove it. --- [1] Although we may be able to skip the `render` call, provided that it happens *after* a `timeout` (as is the case in the default viewer). [2] With current work being done to support streaming of `TextContent`, we'd also need to add just as many duplicate API calls to `PDFPageView.updatePosition`. [3] The number of duplicate `appendText` calls is directly proportional not only to the scroll speed, but also to the number of pages in the document. |
||
|---|---|---|
| .. | ||
| images | ||
| .gitignore | ||
| annotation_layer_builder.css | ||
| annotation_layer_builder.js | ||
| app.js | ||
| chrome-i18n-allow-access-to-file-urls.json | ||
| chromecom.js | ||
| compatibility.js | ||
| compressed.tracemonkey-pldi-09.pdf | ||
| debugger.js | ||
| default_preferences.json | ||
| dom_events.js | ||
| download_manager.js | ||
| firefox_print_service.js | ||
| firefoxcom.js | ||
| genericcom.js | ||
| genericl10n.js | ||
| grab_to_pan.js | ||
| interfaces.js | ||
| overlay_manager.js | ||
| password_prompt.js | ||
| pdf_attachment_viewer.js | ||
| pdf_cursor_tools.js | ||
| pdf_document_properties.js | ||
| pdf_find_bar.js | ||
| pdf_find_controller.js | ||
| pdf_history.js | ||
| pdf_link_service.js | ||
| pdf_outline_viewer.js | ||
| pdf_page_view.js | ||
| pdf_presentation_mode.js | ||
| pdf_print_service.js | ||
| pdf_rendering_queue.js | ||
| pdf_sidebar.js | ||
| pdf_thumbnail_view.js | ||
| pdf_thumbnail_viewer.js | ||
| pdf_viewer.component.js | ||
| pdf_viewer.css | ||
| pdf_viewer.js | ||
| pdfjs.js | ||
| preferences.js | ||
| secondary_toolbar.js | ||
| text_layer_builder.css | ||
| text_layer_builder.js | ||
| toolbar.js | ||
| ui_utils.js | ||
| view_history.js | ||
| viewer-snippet-chrome-extension.html | ||
| viewer-snippet-chrome-overlays.html | ||
| viewer-snippet-firefox-extension.html | ||
| viewer-snippet-minified.html | ||
| viewer-snippet.html | ||
| viewer.css | ||
| viewer.html | ||
| viewer.js | ||