fix(reader): use stable page IDs instead of indices for preloading#654
Merged
fix(reader): use stable page IDs instead of indices for preloading#654
Conversation
When segments are prepended/appended in multi-segment readers, page indices shift and become stale. Background preload tasks that captured raw indices would load pages from the wrong book, and cleanup would evict correct pages. Changes: - Replace all index-based preload paths with stable ReaderPageID-based ones - Remove preloadImageForPage(at:) from ReaderViewModel public API - Make ReaderPageLoadScheduler.preloadImage(for:) the single entry point - Track visiblePageIDs in scheduler to protect them from cleanup eviction - Add presentation invalidation observer to PageScrollView (iOS/macOS) so CoverPageView refreshes when images finish loading - Simplify preloadPages to sequential loop (was using task group that serialized on MainActor anyway) - Clean up dead code: unused setPreloadedImage(forPageIndex:) overload and redundant guard block in CoverPageView
everpcpc
force-pushed
the
fix/reader-preload-stale-index
branch
from
1eadfa7 to
6284e15
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When the DIVINA reader opens a book that belongs to a multi-segment series,
preloadNextSegmentIfNeededmay prepend the previous book's pages to thereaderPagesarray. 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
ReaderPageIDlookups 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 →
PageScrollViewUIViewRepresentable), images were loaded but the UI never refreshed because the image cache lives outside@Observable. Fixed by havingPageScrollView's Coordinator register apagePresentationInvalidationObserver— the same pattern the scroll reader already uses viaNativePagedPagePresentationCoordinator.Scope
preloadImageForPage(at:)intopreloadImage(for:)as single entry point; simplifypreloadPagesfrom task-group to sequential loop (tasks were serialising on MainActor anyway); addvisiblePageIDsprotection in cleanup; remove deadsetPreloadedImage(forPageIndex:)overloadpreloadImageForPage(at:)wrapperpreloadVisiblePagesfrom index-based to pageID-based loadingpreloadPresentationWindowasync withprioritizeVisiblePageLoads; collapse redundant guard blockpreloadImage(for:)instead of capturing stale indexValidation