From 18b372b4ddfbb7f8c464afe0791e06aa88eb8fcf Mon Sep 17 00:00:00 2001 From: Marvelous Akpotu Date: Sun, 8 Nov 2026 01:22:46 +0100 Subject: [PATCH 1/2] perf: avoid scanning every page when opening PDFs --- .../PDFReaderKit/Models/PDFReaderState.swift | 47 ++++++++++++++++++- Sources/PDFReaderVerify/main.swift | 17 +++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/Sources/PDFReaderKit/Models/PDFReaderState.swift b/Sources/PDFReaderKit/Models/PDFReaderState.swift index 8ccc9a0..44bd47e 100644 --- a/Sources/PDFReaderKit/Models/PDFReaderState.swift +++ b/Sources/PDFReaderKit/Models/PDFReaderState.swift @@ -583,7 +583,7 @@ public final class PDFReaderState: ObservableObject { pdfView?.highlightedSelections = nil pdfView?.currentSelection = nil rebuildOutline() - refreshAnnotations() + refreshAnnotations(onOpenOf: doc) undoStack.removeAll() redoStack.removeAll() let saved = restoreLastPositionIndex() ?? 0 @@ -859,13 +859,32 @@ public final class PDFReaderState: ObservableObject { // MARK: - Annotations public func refreshAnnotations() { + guard let doc = document else { + thumbnailRevision &+= 1 + annotationRows = [] + return + } + refreshAnnotations(in: Array(0 ..< doc.pageCount)) + } + + /// Freshly opened documents are read from disk, so we can ask Core + /// Graphics which pages actually have an `/Annots` array before paying + /// PDFKit's per-page parse cost. Long, annotation-free PDFs would + /// otherwise materialize every page synchronously while opening. + /// Only safe for a just-opened document: in-memory edits may not be on + /// disk yet, so the regular `refreshAnnotations()` must scan every page. + private func refreshAnnotations(onOpenOf doc: PDFDocument) { + refreshAnnotations(in: annotationPageCandidates(for: doc)) + } + + private func refreshAnnotations(in pageIndices: [Int]) { thumbnailRevision &+= 1 guard let doc = document else { annotationRows = [] return } var rows: [PDFAnnotationRow] = [] - for i in 0 ..< doc.pageCount { + for i in pageIndices { guard let page = doc.page(at: i) else { continue } for annotation in page.annotations { rows.append(PDFAnnotationRow(annotation: annotation, pageIndex: i)) @@ -874,6 +893,30 @@ public final class PDFReaderState: ObservableObject { annotationRows = rows } + private func annotationPageCandidates(for doc: PDFDocument) -> [Int] { + guard let url = fileURL, + let cgDoc = CGPDFDocument(url as CFURL), + cgDoc.isUnlocked, + cgDoc.numberOfPages == doc.pageCount + else { + return Array(0 ..< doc.pageCount) + } + var pages: [Int] = [] + for i in 1 ... cgDoc.numberOfPages { + guard let page = cgDoc.page(at: i), let dictionary = page.dictionary else { + return Array(0 ..< doc.pageCount) + } + var annots: CGPDFArrayRef? + if CGPDFDictionaryGetArray(dictionary, "Annots", &annots), + let annots, + CGPDFArrayGetCount(annots) > 0 + { + pages.append(i - 1) + } + } + return pages + } + /// Markup modes that apply automatically once a text selection settles. /// Note mode stays manual (it needs composed content). public var autoAnnotateOnSelection: Bool { diff --git a/Sources/PDFReaderVerify/main.swift b/Sources/PDFReaderVerify/main.swift index 0dd348f..55e3595 100644 --- a/Sources/PDFReaderVerify/main.swift +++ b/Sources/PDFReaderVerify/main.swift @@ -439,11 +439,28 @@ func run() async -> Int { // Printing prepares a job when a document is open and nothing otherwise. check(state.makePrintOperation() != nil, "print operation is available", failures: &failures) check(PDFReaderState().makePrintOperation() == nil, "print without a document yields no operation", failures: &failures) + // The open path only materializes pages that have an `/Annots` + // entry, so make sure an annotation on a later page survives reopen. + if let lastPage = state.document?.page(at: 2) { + let note = PDFAnnotation( + bounds: NSRect(x: 40, y: 40, width: 24, height: 24), + forType: .text, + withProperties: nil + ) + note.contents = "Last page note" + lastPage.addAnnotation(note) + state.refreshAnnotations() + } // Save writes annotations back into the opened file. check(state.save(), "save writes back to file", failures: &failures) let reopenedFile = PDFReaderState() check(reopenedFile.open(url: url), "reopens saved file", failures: &failures) check(!reopenedFile.annotationRows.isEmpty, "saved annotations persist on disk", failures: &failures) + check( + reopenedFile.annotationRows.contains { $0.pageIndex == 2 }, + "annotations on later pages persist on disk", + failures: &failures + ) check( reopenedFile.annotationRows.contains { colorsClose($0.annotation.color, HighlightColor.blue.nsColor) }, "saved annotation color persists", From 5ab8d4f3280f4ff9be79cecbf81c0d6d201208e5 Mon Sep 17 00:00:00 2001 From: Marvelous Akpotu Date: Sun, 8 Nov 2026 01:57:43 +0100 Subject: [PATCH 2/2] fix: restore reading position when returning from library --- Sources/PDFReader/Views/PDFKitView.swift | 8 ++++ .../PDFReaderKit/Models/PDFReaderState.swift | 38 +++++++++++++++++++ Sources/PDFReaderVerify/main.swift | 11 ++++++ 3 files changed, 57 insertions(+) diff --git a/Sources/PDFReader/Views/PDFKitView.swift b/Sources/PDFReader/Views/PDFKitView.swift index abcf294..60bff44 100644 --- a/Sources/PDFReader/Views/PDFKitView.swift +++ b/Sources/PDFReader/Views/PDFKitView.swift @@ -26,6 +26,9 @@ public struct PDFKitView: NSViewRepresentable { // `.preferredColorScheme(.dark)` and would invert back to light). pdfView.backgroundColor = state.usesDarkPages ? .white : .windowBackgroundColor pdfView.document = state.document + // A recreated view (e.g. after visiting the Library) starts at page + // 0; seed the captured section so updateNSView restores it. + state.restoreCapturedLocationIfNeeded() state.pdfView = pdfView NotificationCenter.default.addObserver( @@ -57,6 +60,7 @@ public struct PDFKitView: NSViewRepresentable { } public static func dismantleNSView(_ pdfView: PDFView, coordinator: Coordinator) { + coordinator.captureLocation(from: pdfView) coordinator.tearDown() } @@ -204,6 +208,10 @@ public struct PDFKitView: NSViewRepresentable { } } + func captureLocation(from view: PDFView) { + state?.captureCurrentLocation(from: view) + } + func tearDown() { if let clickMonitor { NSEvent.removeMonitor(clickMonitor) diff --git a/Sources/PDFReaderKit/Models/PDFReaderState.swift b/Sources/PDFReaderKit/Models/PDFReaderState.swift index 44bd47e..98bf7ff 100644 --- a/Sources/PDFReaderKit/Models/PDFReaderState.swift +++ b/Sources/PDFReaderKit/Models/PDFReaderState.swift @@ -255,6 +255,35 @@ public final class PDFReaderState: ObservableObject { applyPendingNavigation(in: view) } + /// Captures the live view's visible section so a PDFView recreated after + /// visiting the Library can return to it instead of page 0. + public func captureCurrentLocation(from view: PDFView) { + guard let doc = document, + view.document === doc, + let url = fileURL, + let dest = view.currentDestination, + let page = dest.page + else { return } + let index = doc.index(for: page) + guard index >= 0, index < doc.pageCount else { return } + savedTabLocations[url.path] = (index, dest.point) + } + + /// Seeds pending navigation from the captured section (or current page) + /// when a PDFView is created. No-op if a document load already requested + /// a page, so opening/switching tabs keeps its saved destination. + public func restoreCapturedLocationIfNeeded() { + guard document != nil, + pendingNavigationPageIndex == nil, + pendingNavigationDestination == nil + else { return } + if let url = fileURL, let loc = savedTabLocations[url.path] { + pendingNavigationDestination = loc + } else { + pendingNavigationPageIndex = currentPageIndex + } + } + /// Executes the pending navigation (exact section first, page top /// otherwise). One `go(to:)` fired around the document commit is not /// enough: PDFView's initial layout pass can reset the scroll position @@ -703,6 +732,15 @@ public final class PDFReaderState: ObservableObject { let page = view.currentPage, let index = Optional(doc.index(for: page)) else { return } + // A recreated PDFView reports page 0 before the pending restore + // jumps to the target. Ignore those intermediate reports so they + // don't overwrite the saved position, but accept the target page + // even when it differs from the last synced index. + if let pendingTarget = pendingNavigationDestination?.pageIndex ?? pendingNavigationPageIndex, + index != pendingTarget + { + return + } if index != currentPageIndex { currentPageIndex = index saveLastPosition() diff --git a/Sources/PDFReaderVerify/main.swift b/Sources/PDFReaderVerify/main.swift index 55e3595..12b8476 100644 --- a/Sources/PDFReaderVerify/main.swift +++ b/Sources/PDFReaderVerify/main.swift @@ -256,6 +256,17 @@ func run() async -> Int { check(state.currentPageIndex == 2, "page navigation clamps high", failures: &failures) state.goToPage(index: -1, viaView: false) check(state.currentPageIndex == 0, "page navigation clamps low", failures: &failures) + // A recreated PDFView (e.g. after visiting the Library) must seed + // navigation from the current page instead of page 0. + state.pendingNavigationPageIndex = nil + state.goToPage(index: 2, viaView: false) + state.restoreCapturedLocationIfNeeded() + check( + state.pendingNavigationPageIndex == 2, + "recreated view restores current page", + failures: &failures + ) + state.pendingNavigationPageIndex = nil state.searchQuery = "Hello PDFReader" state.runSearch() check(!state.searchHits.isEmpty, "search finds generated text", failures: &failures)