| Summary: | Zoom in/out is slow in Safari with large PDFs | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tim Horton <thorton> | ||||||
| Component: | New Bugs | Assignee: | Tim Horton <thorton> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | bdakin, kling, sam, simon.fraser | ||||||
| Priority: | P2 | ||||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Tim Horton
2017-05-23 02:01:26 PDT
Created attachment 310987 [details]
Patch
Comment on attachment 310987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310987&action=review > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:1156 > + size_t pageCount = [m_pdfDocument pageCount]; > + for (size_t i = 0; i < pageCount; ++i) > + m_pageBoxes.append(enclosingIntRect([[m_pdfDocument pageAtIndex:i] boundsForBox:kPDFDisplayBoxCropBox])); I'm no expert on this code, but shouldn't we clear m_pageBoxes somehow before appending all the boxes to it? Or is this going to only run once? Comment on attachment 310987 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=310987&action=review >> Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:1156 >> + m_pageBoxes.append(enclosingIntRect([[m_pdfDocument pageAtIndex:i] boundsForBox:kPDFDisplayBoxCropBox])); > > I'm no expert on this code, but shouldn't we clear m_pageBoxes somehow before appending all the boxes to it? > Or is this going to only run once? Also, since we know how many things are getting appened, you can reserve capacity and then do uncheckedAppends. (In reply to Andreas Kling from comment #2) > Comment on attachment 310987 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=310987&action=review > > > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:1156 > > + size_t pageCount = [m_pdfDocument pageCount]; > > + for (size_t i = 0; i < pageCount; ++i) > > + m_pageBoxes.append(enclosingIntRect([[m_pdfDocument pageAtIndex:i] boundsForBox:kPDFDisplayBoxCropBox])); > > I'm no expert on this code, but shouldn't we clear m_pageBoxes somehow > before appending all the boxes to it? > Or is this going to only run once? LOL, yes, and this has always been broken, but has never really mattered much because we only use the first box... which means we can simplify this a great deal and do even less work! Created attachment 311035 [details]
Patch
Comment on attachment 311035 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=311035&action=review > Source/WebKit2/WebProcess/Plugins/PDF/PDFPlugin.mm:1160 > + m_firstPageHeight = static_cast<unsigned>(CGCeiling([[m_pdfDocument pageAtIndex:0] boundsForBox:kPDFDisplayBoxCropBox].size.height)); Do we always have a page 0? |