Skip to content

Commit 21470f4

Browse files
authored
fix(reader): use stable page IDs instead of indices for preloading (#654)
## Problem When the DIVINA reader opens a book that belongs to a multi-segment series, `preloadNextSegmentIfNeeded` may prepend the previous book's pages to the `readerPages` array. This shifts all page indices, causing background preload tasks that captured raw indices to load pages from the wrong book. The cleanup pass then evicts the correctly-loaded visible pages, leaving the UI stuck on a loading spinner — most noticeable in dual-page and cover modes. ## Approach Replace every external index-based preload path with stable `ReaderPageID` lookups that resolve the index at call time, so stale captures are impossible. Additionally, track the set of currently visible page IDs in the scheduler so cleanup never evicts pages the user is looking at. For CoverPageView (pure SwiftUI → `PageScrollView` UIViewRepresentable), images were loaded but the UI never refreshed because the image cache lives outside `@Observable`. Fixed by having `PageScrollView`'s Coordinator register a `pagePresentationInvalidationObserver` — the same pattern the scroll reader already uses via `NativePagedPagePresentationCoordinator`. ## Scope - **ReaderPageLoadScheduler**: merge `preloadImageForPage(at:)` into `preloadImage(for:)` as single entry point; simplify `preloadPages` from task-group to sequential loop (tasks were serialising on MainActor anyway); add `visiblePageIDs` protection in cleanup; remove dead `setPreloadedImage(forPageIndex:)` overload - **ReaderViewModel**: remove public `preloadImageForPage(at:)` wrapper - **ScrollPageView (iOS/macOS)**: switch `preloadVisiblePages` from index-based to pageID-based loading - **CoverPageView**: make `preloadPresentationWindow` async with `prioritizeVisiblePageLoads`; collapse redundant guard block - **NativeImagePageViewController**: use `preloadImage(for:)` instead of capturing stale index - **PageScrollView (iOS/macOS)**: register presentation invalidation observer for automatic UIKit refresh
1 parent ea783ed commit 21470f4

9 files changed

Lines changed: 69 additions & 97 deletions

File tree

KMReader.xcodeproj/project.pbxproj

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,7 @@
438438
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = "KMReader/KMReader-macOS.entitlements";
439439
CODE_SIGN_IDENTITY = "Apple Development";
440440
CODE_SIGN_STYLE = Automatic;
441-
CURRENT_PROJECT_VERSION = 362;
441+
CURRENT_PROJECT_VERSION = 363;
442442
DEVELOPMENT_TEAM = M777UHWZA4;
443443
ENABLE_APP_SANDBOX = YES;
444444
ENABLE_HARDENED_RUNTIME = YES;
@@ -496,7 +496,7 @@
496496
"CODE_SIGN_ENTITLEMENTS[sdk=macosx*]" = "KMReader/KMReader-macOS.entitlements";
497497
CODE_SIGN_IDENTITY = "Apple Distribution";
498498
CODE_SIGN_STYLE = Manual;
499-
CURRENT_PROJECT_VERSION = 362;
499+
CURRENT_PROJECT_VERSION = 363;
500500
DEVELOPMENT_TEAM = "";
501501
"DEVELOPMENT_TEAM[sdk=appletvos*]" = M777UHWZA4;
502502
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = M777UHWZA4;
@@ -556,7 +556,7 @@
556556
CODE_SIGN_ENTITLEMENTS = KMReaderWidgets/KMReaderWidgets.entitlements;
557557
CODE_SIGN_IDENTITY = "Apple Development";
558558
CODE_SIGN_STYLE = Automatic;
559-
CURRENT_PROJECT_VERSION = 362;
559+
CURRENT_PROJECT_VERSION = 363;
560560
DEVELOPMENT_TEAM = M777UHWZA4;
561561
GENERATE_INFOPLIST_FILE = YES;
562562
INFOPLIST_FILE = KMReaderWidgets/Info.plist;
@@ -590,7 +590,7 @@
590590
CODE_SIGN_IDENTITY = "Apple Distribution";
591591
"CODE_SIGN_IDENTITY[sdk=iphoneos*]" = "iPhone Distribution";
592592
CODE_SIGN_STYLE = Manual;
593-
CURRENT_PROJECT_VERSION = 362;
593+
CURRENT_PROJECT_VERSION = 363;
594594
DEVELOPMENT_TEAM = "";
595595
"DEVELOPMENT_TEAM[sdk=iphoneos*]" = M777UHWZA4;
596596
GENERATE_INFOPLIST_FILE = YES;

KMReader/Features/Reader/Services/ReaderPageLoadScheduler.swift

Lines changed: 18 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ final class ReaderPageLoadScheduler {
4242
private var preloadingImageTasks: [ReaderPageID: ImageLoadTaskRecord] = [:]
4343
private var lastPreloadRequestTime: Date?
4444
private var preloadTask: Task<Void, Never>?
45+
private var visiblePageIDs: [ReaderPageID] = []
4546

4647
init(
4748
preloadBefore: Int = ReaderConstants.preloadBefore,
@@ -97,6 +98,8 @@ final class ReaderPageLoadScheduler {
9798
}
9899

99100
func prioritizeVisiblePageLoads(for pageIDs: [ReaderPageID]) {
101+
visiblePageIDs = pageIDs
102+
100103
let keepPageIDs = prioritizedPageIDs(around: pageIDs)
101104
guard !keepPageIDs.isEmpty else { return }
102105

@@ -127,57 +130,31 @@ final class ReaderPageLoadScheduler {
127130
guard !pagesToPreload.isEmpty else { return }
128131

129132
preloadTask?.cancel()
133+
let pageIDsToPreload = pagesToPreload.map(\.pageID)
130134
preloadTask = Task { [weak self] in
131135
guard let self else { return }
132136

133-
let results = await withTaskGroup(of: (Int, ReaderPageID, PlatformImage?, URL?).self) {
134-
group -> [(Int, ReaderPageID, PlatformImage?, URL?)] in
135-
for (index, pageID) in pagesToPreload {
136-
if Task.isCancelled { break }
137-
if self.preloadedImage(for: pageID) != nil {
138-
continue
139-
}
140-
group.addTask {
141-
if Task.isCancelled { return (index, pageID, nil, nil) }
142-
let (image, animatedSourceFileURL) = await self.preloadDecodedPageImage(pageIndex: index)
143-
return (index, pageID, image, animatedSourceFileURL)
144-
}
145-
}
146-
147-
var collected: [(Int, ReaderPageID, PlatformImage?, URL?)] = []
148-
for await result in group {
149-
if !Task.isCancelled {
150-
collected.append(result)
151-
}
152-
}
153-
return collected
137+
for pageID in pageIDsToPreload {
138+
guard !Task.isCancelled else { return }
139+
if self.preloadedImage(for: pageID) != nil { continue }
140+
_ = await self.preloadImage(for: pageID)
154141
}
155142

156-
if Task.isCancelled { return }
157-
158-
for (pageIndex, pageID, image, animatedSourceFileURL) in results {
159-
self.updateAnimatedPresentation(
160-
knownAnimatedState: animatedSourceFileURL != nil,
161-
sourceFileURL: animatedSourceFileURL,
162-
for: pageID
163-
)
164-
if let image {
165-
self.setPreloadedImage(image, forPageIndex: pageIndex)
166-
}
143+
if !Task.isCancelled {
144+
self.cleanupDistantImagesAroundCurrentPage()
167145
}
168-
169-
self.cleanupDistantImagesAroundCurrentPage()
170146
}
171147
}
172148

173149
func cleanupDistantImagesAroundCurrentPage() {
174-
let keepPageIDs = Set(
150+
var keepPageIDs = Set(
175151
pageWindowEntries(
176152
around: currentPageID,
177153
before: keepRangeBefore,
178154
after: keepRangeAfter
179155
).map(\.pageID)
180156
)
157+
keepPageIDs.formUnion(visiblePageIDs)
181158
guard !keepPageIDs.isEmpty else { return }
182159

183160
let imageKeysToRemove = preloadedImagesByID.keys.filter { !keepPageIDs.contains($0) }
@@ -192,8 +169,6 @@ final class ReaderPageLoadScheduler {
192169
for key in animatedKeysToRemove {
193170
updateAnimatedPresentation(knownAnimatedState: nil, sourceFileURL: nil, for: key)
194171
}
195-
196-
logger.debug("🖼️ Memory Cache: \(preloadedImagesByID.count) images")
197172
}
198173

199174
func isAnimatedPage(for pageID: ReaderPageID) -> Bool {
@@ -216,12 +191,15 @@ final class ReaderPageLoadScheduler {
216191
await prepareAnimatedPagePlaybackURL(pageIndex: pageIndex)
217192
}
218193

219-
func preloadImageForPage(at pageIndex: Int) async -> PlatformImage? {
220-
guard let pageID = readerPageID(forPageIndex: pageIndex) else { return nil }
194+
func preloadImage(for pageID: ReaderPageID) async -> PlatformImage? {
221195
if let cached = preloadedImagesByID[pageID] {
222196
return cached
223197
}
224198

199+
guard let pageIndex = pageIndex(for: pageID) else {
200+
return nil
201+
}
202+
225203
if let existingTask = preloadingImageTasks[pageID] {
226204
return await existingTask.task.value
227205
}
@@ -249,13 +227,6 @@ final class ReaderPageLoadScheduler {
249227
return image
250228
}
251229

252-
func preloadImage(for pageID: ReaderPageID) async -> PlatformImage? {
253-
guard let pageIndex = pageIndex(for: pageID) else {
254-
return preloadedImagesByID[pageID]
255-
}
256-
return await preloadImageForPage(at: pageIndex)
257-
}
258-
259230
func clearPreloadedImages() {
260231
preloadTask?.cancel()
261232
preloadTask = nil
@@ -288,6 +259,7 @@ final class ReaderPageLoadScheduler {
288259
readerPages.removeAll()
289260
readerPageIndexByID.removeAll()
290261
currentPageID = nil
262+
visiblePageIDs.removeAll()
291263
}
292264

293265
private func pageIndex(for pageID: ReaderPageID) -> Int? {
@@ -331,11 +303,6 @@ final class ReaderPageLoadScheduler {
331303
return keepPageIDs
332304
}
333305

334-
private func setPreloadedImage(_ image: PlatformImage, forPageIndex pageIndex: Int) {
335-
guard let pageID = readerPageID(forPageIndex: pageIndex) else { return }
336-
setPreloadedImage(image, for: pageID)
337-
}
338-
339306
private func setPreloadedImage(_ image: PlatformImage, for pageID: ReaderPageID) {
340307
preloadedImagesByID[pageID] = image
341308
invalidatePresentation(.pages([pageID]))

KMReader/Features/Reader/ViewModels/ReaderViewModel.swift

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -823,10 +823,6 @@ class ReaderViewModel {
823823
await pageLoadScheduler.prepareAnimatedPagePlaybackURL(pageID: pageID)
824824
}
825825

826-
func preloadImageForPage(at pageIndex: Int) async -> PlatformImage? {
827-
await pageLoadScheduler.preloadImageForPage(at: pageIndex)
828-
}
829-
830826
func preloadImage(for pageID: ReaderPageID) async -> PlatformImage? {
831827
await pageLoadScheduler.preloadImage(for: pageID)
832828
}

KMReader/Features/Reader/Views/CoverPageView.swift

Lines changed: 12 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -653,53 +653,41 @@ struct CoverPageView: View {
653653
postTransitionTask = Task(priority: .utility) {
654654
guard !Task.isCancelled else { return }
655655

656-
let shouldSyncPosition = await MainActor.run { () -> Bool in
656+
let shouldContinue = await MainActor.run { () -> Bool in
657657
guard token == transitionToken else { return false }
658658
guard currentItem == item else { return false }
659659
viewModel.updateCurrentPosition(viewItem: item)
660660
return true
661661
}
662-
guard shouldSyncPosition else { return }
663-
664-
let shouldPreload = await MainActor.run { () -> Bool in
665-
guard token == transitionToken else { return false }
666-
guard currentItem == item else { return false }
667-
preloadPresentationWindow(around: item)
668-
return true
669-
}
670-
guard shouldPreload else { return }
662+
guard shouldContinue else { return }
671663

664+
await preloadPresentationWindow(around: item)
672665
await viewModel.preloadPages()
673666
}
674667
}
675668

676-
private func preloadPresentationWindow(around item: ReaderViewItem) {
669+
private func preloadPresentationWindow(around item: ReaderViewItem) async {
677670
let candidateItems = [
678671
item,
679672
viewModel.adjacentViewItem(from: item, offset: 1),
680673
viewModel.adjacentViewItem(from: item, offset: -1),
681674
].compactMap { $0 }
682675

683-
var pageIndices: [Int] = []
676+
var pageIDs: [ReaderPageID] = []
684677
var seenPageIDs: Set<ReaderPageID> = []
685678
for candidateItem in candidateItems {
686679
for pageID in candidateItem.pageIDs where seenPageIDs.insert(pageID).inserted {
687-
if let pageIndex = viewModel.pageIndex(for: pageID) {
688-
pageIndices.append(pageIndex)
689-
}
680+
pageIDs.append(pageID)
690681
}
691682
}
692683

693-
guard !pageIndices.isEmpty else { return }
684+
guard !pageIDs.isEmpty else { return }
694685

695-
Task(priority: .userInitiated) {
696-
await withTaskGroup(of: Void.self) { group in
697-
for pageIndex in pageIndices {
698-
group.addTask {
699-
_ = await viewModel.preloadImageForPage(at: pageIndex)
700-
}
701-
}
702-
}
686+
viewModel.prioritizeVisiblePageLoads(for: item.pageIDs)
687+
688+
for pageID in pageIDs {
689+
guard !Task.isCancelled else { return }
690+
_ = await viewModel.preloadImage(for: pageID)
703691
}
704692
}
705693

KMReader/Features/Reader/Views/NativeImagePageViewController.swift

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,13 +189,12 @@
189189
private func startLoadingImageIfNeeded() {
190190
guard loadTask == nil else { return }
191191
guard let viewModel else { return }
192-
guard let requestedPageIndex = viewModel.pageIndex(for: pageID) else { return }
193192

194193
let requestedPageID = pageID
195194

196195
loadTask = Task { [weak self] in
197196
guard let self else { return }
198-
let image = await viewModel.preloadImageForPage(at: requestedPageIndex)
197+
let image = await viewModel.preloadImage(for: requestedPageID)
199198
guard !Task.isCancelled else { return }
200199
guard self.pageID == requestedPageID else { return }
201200

KMReader/Features/Reader/Views/PageImage/PageScrollView_iOS.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
context.coordinator.parent = self
8686
context.coordinator.applyDisplayModeIfNeeded(in: scrollView)
8787
context.coordinator.setupNativeInteractions(on: scrollView)
88+
context.coordinator.registerInvalidationObserver()
8889
return scrollView
8990
}
9091

@@ -127,8 +128,21 @@
127128
weak var contentStack: UIStackView?
128129
var contentHeightConstraint: NSLayoutConstraint?
129130
private var pageViews: [NativePageItem] = []
131+
private var invalidationObserverToken: UUID?
132+
133+
func registerInvalidationObserver() {
134+
guard invalidationObserverToken == nil else { return }
135+
invalidationObserverToken = parent.viewModel.addPagePresentationInvalidationObserver {
136+
[weak self] _ in
137+
self?.updatePages()
138+
}
139+
}
130140

131141
func prepareForDismantle() {
142+
if let token = invalidationObserverToken {
143+
parent?.viewModel.removePagePresentationInvalidationObserver(token)
144+
invalidationObserverToken = nil
145+
}
132146
pageViews.forEach { view in
133147
view.prepareForDismantle()
134148
view.removeFromSuperview()

KMReader/Features/Reader/Views/PageImage/PageScrollView_macOS.swift

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
context.coordinator.scrollView = scrollView
9393
context.coordinator.applyDisplayModeIfNeeded(in: scrollView)
9494
context.coordinator.setupNativeInteractions(on: scrollView)
95+
context.coordinator.registerInvalidationObserver()
9596

9697
return scrollView
9798
}
@@ -162,8 +163,21 @@
162163
weak var contentStack: NSStackView?
163164
var contentHeightConstraint: NSLayoutConstraint?
164165
private var pageViews: [NativePageItem] = []
166+
private var invalidationObserverToken: UUID?
167+
168+
func registerInvalidationObserver() {
169+
guard invalidationObserverToken == nil else { return }
170+
invalidationObserverToken = parent.viewModel.addPagePresentationInvalidationObserver {
171+
[weak self] _ in
172+
self?.updatePages()
173+
}
174+
}
165175

166176
func prepareForDismantle() {
177+
if let token = invalidationObserverToken {
178+
parent?.viewModel.removePagePresentationInvalidationObserver(token)
179+
invalidationObserverToken = nil
180+
}
167181
if let scrollView = scrollView {
168182
NotificationCenter.default.removeObserver(
169183
self, name: NSScrollView.didEndLiveScrollNotification, object: scrollView)

KMReader/Features/Reader/Views/ScrollPageView_iOS.swift

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -652,7 +652,7 @@
652652
private func preloadVisiblePages(for item: ReaderViewItem) {
653653
let visiblePageIDs = item.pageIDs
654654
if visiblePreloadItem == item,
655-
visiblePageIDs.allSatisfy({
655+
visiblePreloadTask != nil || visiblePageIDs.allSatisfy({
656656
parent.viewModel.preloadedImage(for: $0) != nil
657657
|| parent.viewModel.hasPendingImageLoad(for: $0)
658658
})
@@ -662,9 +662,6 @@
662662

663663
parent.viewModel.prioritizeVisiblePageLoads(for: visiblePageIDs)
664664

665-
let visiblePageIndices = visiblePageIDs.compactMap { parent.viewModel.pageIndex(for: $0) }
666-
guard !visiblePageIndices.isEmpty else { return }
667-
668665
visiblePreloadTask?.cancel()
669666
visiblePreloadItem = item
670667
let viewModel = parent.viewModel
@@ -675,9 +672,9 @@
675672
}
676673
}
677674

678-
for pageIndex in visiblePageIndices {
675+
for pageID in visiblePageIDs {
679676
guard !Task.isCancelled else { return }
680-
_ = await viewModel.preloadImageForPage(at: pageIndex)
677+
_ = await viewModel.preloadImage(for: pageID)
681678
}
682679
}
683680
}

KMReader/Features/Reader/Views/ScrollPageView_macOS.swift

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -757,7 +757,7 @@
757757
private func preloadVisiblePages(for item: ReaderViewItem) {
758758
let visiblePageIDs = item.pageIDs
759759
if visiblePreloadItem == item,
760-
visiblePageIDs.allSatisfy({
760+
visiblePreloadTask != nil || visiblePageIDs.allSatisfy({
761761
parent.viewModel.preloadedImage(for: $0) != nil
762762
|| parent.viewModel.hasPendingImageLoad(for: $0)
763763
})
@@ -767,9 +767,6 @@
767767

768768
parent.viewModel.prioritizeVisiblePageLoads(for: visiblePageIDs)
769769

770-
let visiblePageIndices = visiblePageIDs.compactMap { parent.viewModel.pageIndex(for: $0) }
771-
guard !visiblePageIndices.isEmpty else { return }
772-
773770
visiblePreloadTask?.cancel()
774771
visiblePreloadItem = item
775772
let viewModel = parent.viewModel
@@ -780,9 +777,9 @@
780777
}
781778
}
782779

783-
for pageIndex in visiblePageIndices {
780+
for pageID in visiblePageIDs {
784781
guard !Task.isCancelled else { return }
785-
_ = await viewModel.preloadImageForPage(at: pageIndex)
782+
_ = await viewModel.preloadImage(for: pageID)
786783
}
787784
}
788785
}

0 commit comments

Comments
 (0)