From 977a19c5c51c36435b0db7d37314bf6862279466 Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Sun, 27 Sep 2026 14:30:13 +0300 Subject: [PATCH 1/3] perf: render on-screen page previews first and trim the cache once per paint The preview lock serves requests in call order and the paint loop walks pages by index, so with a cache extent the page above the viewport was rendered before the one on screen. Requests are now collected during the loop and issued by distance from the visible rect. removeCacheImagesIfCacheBytesExceedsLimit removed an image before checking the budget, and it ran once per page inside the loop. Out-of-extent previews were evicted even under budget and re-rendered on scroll-back. The budget is now checked first and the trim runs once after the loop. --- .../pdfrx/lib/src/widgets/pdf_viewer.dart | 50 ++++++++++++------- 1 file changed, 33 insertions(+), 17 deletions(-) diff --git a/packages/pdfrx/lib/src/widgets/pdf_viewer.dart b/packages/pdfrx/lib/src/widgets/pdf_viewer.dart index f2acca5f..25b0cecc 100644 --- a/packages/pdfrx/lib/src/widgets/pdf_viewer.dart +++ b/packages/pdfrx/lib/src/widgets/pdf_viewer.dart @@ -1661,6 +1661,7 @@ class _PdfViewerState extends State FilterQuality filterQuality = FilterQuality.high, }) { final unusedPageList = []; + final previewRequests = <({PdfPage page, double scale, Rect rect})>[]; final unmeasuredPageList = []; // Pages inside the extent that are painting as a blank white rectangle // because no preview image has landed for them yet. This is the white-page @@ -1749,7 +1750,7 @@ class _PdfViewerState extends State if (enableLowResolutionPagePreview && (previewImage == null || previewImage.isDirty || previewImage.scale != previewScaleLimit)) { - _requestPagePreviewImageCached(cache, page, previewScaleLimit); + previewRequests.add((page: page, scale: previewScaleLimit, rect: rect)); } final pageScale = page.width > 0 && page.height > 0 @@ -1797,20 +1798,28 @@ class _PdfViewerState extends State callback(canvas, rect, page); } } + } - if (unusedPageList.isNotEmpty) { - final currentPageNumber = _pageNumber; - if (currentPageNumber != null && currentPageNumber > 0) { - final currentPage = _document!.pages[currentPageNumber - 1]; - cache.removeCacheImagesIfCacheBytesExceedsLimit( - unusedPageList, - maxImageCacheBytes, - currentPage, - dist: (pageNumber) => - (_layout!.pageLayouts[pageNumber - 1].center - _layout!.pageLayouts[currentPage.pageNumber - 1].center) - .distanceSquared, - ); - } + // The preview lock serves requests in call order, and the loop above walks pages by index -- so the page above + // the viewport used to render before the one on screen. Ask for the visible pages first. + previewRequests.sort((a, b) => _distanceToRect(a.rect, targetRect).compareTo(_distanceToRect(b.rect, targetRect))); + for (final request in previewRequests) { + _requestPagePreviewImageCached(cache, request.page, request.scale); + } + + // Once per paint, after the loop: inside it the list was still growing and the cache was trimmed once per page. + if (unusedPageList.isNotEmpty) { + final currentPageNumber = _pageNumber; + if (currentPageNumber != null && currentPageNumber > 0) { + final currentPage = _document!.pages[currentPageNumber - 1]; + cache.removeCacheImagesIfCacheBytesExceedsLimit( + unusedPageList, + maxImageCacheBytes, + currentPage, + dist: (pageNumber) => + (_layout!.pageLayouts[pageNumber - 1].center - _layout!.pageLayouts[currentPage.pageNumber - 1].center) + .distanceSquared, + ); } } @@ -2024,6 +2033,13 @@ class _PdfViewerState extends State void _invalidate() => _updateStream.add(_txController.value); + /// 0 for a rect that overlaps [target], otherwise the gap between them. + static double _distanceToRect(Rect rect, Rect target) { + final dx = max(0.0, max(target.left - rect.right, rect.left - target.right)); + final dy = max(0.0, max(target.top - rect.bottom, rect.top - target.bottom)); + return dx * dx + dy * dy; + } + Future _requestPagePreviewImageCached(_PdfPageImageCache cache, PdfPage page, double scale) async { final width = page.width * scale; final height = page.height * scale; @@ -4232,6 +4248,9 @@ class _PdfPageImageCache { pageImages.values.fold(0, (sum, e) => sum + getBytesConsumed(e.image)) + pageImagesPartial.values.fold(0, (sum, e) => sum + getBytesConsumed(e.image)); for (final key in pageNumbers) { + if (bytesConsumed <= acceptableBytes) { + break; + } final removed = pageImages.remove(key); if (removed != null) { bytesConsumed -= getBytesConsumed(removed.image); @@ -4242,9 +4261,6 @@ class _PdfPageImageCache { bytesConsumed -= getBytesConsumed(removedPartial.image); removedPartial.dispose(); } - if (bytesConsumed <= acceptableBytes) { - break; - } } } } From 3ee262726dc2bea7783a25b87fb4133bb5382d3f Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Sun, 27 Sep 2026 14:30:14 +0300 Subject: [PATCH 2/3] fix(PdfTextSearcher): keep a running search when its pattern is re-issued startTextSearch cancelled the running session before search() returned early for an identical pattern, so the matches stayed half collected and isSearching stuck at true. An identical pattern now only drops a pending search for another pattern; _restartSearch clears the condition so it can still rerun the same pattern. goToMatch now notifies listeners, so UI such as a "3/120" counter can follow the current match, and a new pattern clears the position left by the previous one. --- .../lib/src/widgets/pdf_text_searcher.dart | 29 ++-- .../pdfrx/test/pdf_text_searcher_test.dart | 143 ++++++++++++++++++ 2 files changed, 163 insertions(+), 9 deletions(-) create mode 100644 packages/pdfrx/test/pdf_text_searcher_test.dart diff --git a/packages/pdfrx/lib/src/widgets/pdf_text_searcher.dart b/packages/pdfrx/lib/src/widgets/pdf_text_searcher.dart index d486dc5c..bb7319ef 100644 --- a/packages/pdfrx/lib/src/widgets/pdf_text_searcher.dart +++ b/packages/pdfrx/lib/src/widgets/pdf_text_searcher.dart @@ -83,6 +83,13 @@ class PdfTextSearcher extends Listenable { bool goToFirstMatch = true, bool searchImmediately = false, }) { + // Re-issuing the running pattern must not cancel it: search() below would return early for an identical pattern, + // leaving the matches half collected and isSearching stuck. Only drop a pending search for another pattern. + final last = _lastSearchCondition; + if (last != null && last.caseInsensitive == caseInsensitive && _isIdenticalPattern(last.pattern, pattern)) { + _searchTextTimer?.cancel(); + return; + } _cancelTextSearch(); final searchSession = ++_searchSession; @@ -118,7 +125,7 @@ class PdfTextSearcher extends Listenable { _resetTextSearch(notify: false); } - void _resetTextSearch({bool notify = true, bool clearSearchCondition = true}) { + void _resetTextSearch({bool notify = true}) { _cancelTextSearch(); _matches = const []; _matchesPageStartIndices = const []; @@ -126,9 +133,7 @@ class PdfTextSearcher extends Listenable { _currentIndex = null; _currentMatch = null; _isSearching = false; - if (clearSearchCondition) { - _lastSearchCondition = null; - } + _lastSearchCondition = null; if (notify) { notifyListeners(); } @@ -145,6 +150,9 @@ class PdfTextSearcher extends Listenable { final textMatchesPageStartIndex = []; var first = true; _isSearching = true; + // The previous pattern's position does not index into the new matches. + _currentIndex = null; + _currentMatch = null; _totalPageCount = document.pages.length; for (final page in document.pages) { _searchingPageNumber = page.pageNumber; @@ -188,13 +196,15 @@ class PdfTextSearcher extends Listenable { } void _restartSearch() { - _resetTextSearch(clearSearchCondition: false); + final condition = _lastSearchCondition; + // Clearing the condition lets startTextSearch run the same pattern again. + _resetTextSearch(); _cachedText.clear(); - if (_lastSearchCondition != null) { + if (condition != null) { startTextSearch( - _lastSearchCondition!.pattern, - caseInsensitive: _lastSearchCondition!.caseInsensitive, - goToFirstMatch: _lastSearchCondition!.goToFirstMatch, + condition.pattern, + caseInsensitive: condition.caseInsensitive, + goToFirstMatch: condition.goToFirstMatch, ); } } @@ -244,6 +254,7 @@ class PdfTextSearcher extends Listenable { ); controller?.setCurrentPageNumber(match.pageNumber); controller?.invalidate(); + notifyListeners(); } /// Get the matches range for the given page number. diff --git a/packages/pdfrx/test/pdf_text_searcher_test.dart b/packages/pdfrx/test/pdf_text_searcher_test.dart new file mode 100644 index 00000000..130f9ad7 --- /dev/null +++ b/packages/pdfrx/test/pdf_text_searcher_test.dart @@ -0,0 +1,143 @@ +import 'dart:async'; + +import 'package:flutter/widgets.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:pdfrx/pdfrx.dart'; + +class _FakePage extends Fake implements PdfPage { + _FakePage(this.pageNumber); + + @override + final int pageNumber; +} + +class _FakeDocument extends Fake implements PdfDocument { + _FakeDocument(int pageCount) : pages = [for (var i = 1; i <= pageCount; i++) _FakePage(i)]; + + @override + final List pages; + + @override + Stream get events => const Stream.empty(); +} + +class _FakeController extends PdfViewerController { + _FakeController(this.fakeDocument); + + final _FakeDocument fakeDocument; + int currentPageSet = 0; + + @override + bool get isReady => true; + + @override + PdfDocument get document => fakeDocument; + + @override + void invalidate() {} + + @override + Rect calcRectForRectInsidePage({required int pageNumber, required PdfRect rect}) => Rect.zero; + + @override + Future ensureVisible( + Rect rect, { + Duration duration = const Duration(milliseconds: 200), + double margin = 0, + }) async {} + + @override + void setCurrentPageNumber(int pageNumber) => currentPageSet = pageNumber; + + @override + FutureOr useDocument( + FutureOr Function(PdfDocument document) task, { + bool ensureLoaded = true, + Completer? cancelLoading, + }) => task(fakeDocument); +} + +/// Serves page text from [texts]; a page listed in [gates] waits for its completer. +class _GatedSearcher extends PdfTextSearcher { + _GatedSearcher(super.controller, this.texts); + + final Map texts; + final Map> gates = {}; + + @override + Future loadText({required int pageNumber}) async { + await gates[pageNumber]?.future; + final text = texts[pageNumber] ?? ''; + return PdfPageText( + pageNumber: pageNumber, + fullText: text, + charRects: [for (var i = 0; i < text.length; i++) PdfRect(i * 10.0, 10, i * 10.0 + 10, 0)], + fragments: const [], + ); + } +} + +void main() { + test('re-issuing the running pattern does not cut the search short', () async { + final searcher = _GatedSearcher(_FakeController(_FakeDocument(3)), {1: 'word', 2: 'word word', 3: 'word'}); + addTearDown(searcher.dispose); + final gate = searcher.gates[2] = Completer(); + + searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true); + await pumpEventQueue(); + expect(searcher.isSearching, isTrue); + expect(searcher.matches, hasLength(1)); + + // e.g. a search field that fires again with the text unchanged. + searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true); + gate.complete(); + await pumpEventQueue(); + + expect(searcher.matches.map((m) => m.pageNumber), [1, 2, 2, 3]); + expect(searcher.isSearching, isFalse); + }); + + test('a different pattern still replaces the running search', () async { + final searcher = _GatedSearcher(_FakeController(_FakeDocument(2)), {1: 'alpha beta', 2: 'beta'}); + addTearDown(searcher.dispose); + final gate = searcher.gates[2] = Completer(); + + searcher.startTextSearch('alpha', goToFirstMatch: false, searchImmediately: true); + await pumpEventQueue(); + searcher.startTextSearch('beta', goToFirstMatch: false, searchImmediately: true); + gate.complete(); + await pumpEventQueue(); + + expect(searcher.matches.map((m) => m.pageNumber), [1, 2]); + expect(searcher.isSearching, isFalse); + }); + + test('a new pattern clears the position left by the previous one', () async { + final searcher = _GatedSearcher(_FakeController(_FakeDocument(1)), {1: 'alpha alpha beta'}); + addTearDown(searcher.dispose); + searcher.startTextSearch('alpha', goToFirstMatch: false, searchImmediately: true); + await pumpEventQueue(); + await searcher.goToMatchOfIndex(1); + + searcher.startTextSearch('beta', goToFirstMatch: false, searchImmediately: true); + await pumpEventQueue(); + + expect(searcher.matches, hasLength(1)); + expect(searcher.currentIndex, isNull); + }); + + test('moving to a match notifies listeners, so a "3/120" counter can follow', () async { + final searcher = _GatedSearcher(_FakeController(_FakeDocument(1)), {1: 'word word'}); + addTearDown(searcher.dispose); + searcher.startTextSearch('word', goToFirstMatch: false, searchImmediately: true); + await pumpEventQueue(); + + var notified = 0; + searcher.addListener(() => notified++); + await searcher.goToMatchOfIndex(1); + + expect(searcher.currentIndex, 1); + expect(notified, greaterThan(0)); + expect((searcher.controller! as _FakeController).currentPageSet, 1); + }); +} From 36c1bb50313824c8855354db71e9752b3b70f9ea Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Sun, 27 Sep 2026 14:30:14 +0300 Subject: [PATCH 3/3] fix(PdfDocumentListenable): dispose a document whose load finished after eviction When the last listener leaves while a document is loading (e.g. a viewer torn down to retry a slow load), the listenable is evicted and the next resolve builds a new one. The in-flight load still completed into the evicted listenable, where nothing could reach or dispose the document. The load now disposes it instead. --- packages/pdfrx/lib/src/pdf_document_ref.dart | 6 +++ .../pdf_document_ref_abandoned_load_test.dart | 49 +++++++++++++++++++ 2 files changed, 55 insertions(+) create mode 100644 packages/pdfrx/test/pdf_document_ref_abandoned_load_test.dart diff --git a/packages/pdfrx/lib/src/pdf_document_ref.dart b/packages/pdfrx/lib/src/pdf_document_ref.dart index 150b406c..c3097859 100644 --- a/packages/pdfrx/lib/src/pdf_document_ref.dart +++ b/packages/pdfrx/lib/src/pdf_document_ref.dart @@ -440,6 +440,12 @@ class PdfDocumentListenable extends Listenable { setError(err, stackTrace); return report?.copyWith(elapsedTime: stopwatch.elapsed); } + // Every listener left while the document was loading: this listenable was evicted, and the next resolve builds a + // new one. Nothing can reach the document anymore, so keeping it would leak it. + if (!identical(PdfDocumentRef._listenables[ref], this)) { + if (ref.autoDispose) await document.dispose(); + return report?.copyWith(elapsedTime: stopwatch.elapsed); + } setDocument(document); return report?.copyWith(elapsedTime: stopwatch.elapsed); }); diff --git a/packages/pdfrx/test/pdf_document_ref_abandoned_load_test.dart b/packages/pdfrx/test/pdf_document_ref_abandoned_load_test.dart new file mode 100644 index 00000000..bf839604 --- /dev/null +++ b/packages/pdfrx/test/pdf_document_ref_abandoned_load_test.dart @@ -0,0 +1,49 @@ +import 'dart:async'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:pdfrx/pdfrx.dart'; + +class _FakeDocument extends Fake implements PdfDocument { + bool disposed = false; + + @override + Future dispose() async => disposed = true; +} + +void main() { + test('a load that finishes after every listener left disposes its document', () async { + final loading = Completer(); + final ref = PdfDocumentRefByLoader((_) => loading.future, key: PdfDocumentRefKey('abandoned-load-test')); + final listenable = ref.resolveListenable(); + void listener() {} + listenable.addListener(listener); + final load = listenable.load(); + + // e.g. the viewer is torn down to retry a slow load. + listenable.removeListener(listener); + final document = _FakeDocument(); + loading.complete(document); + await load; + + expect(document.disposed, isTrue); + expect(listenable.document, isNull); + }); + + test('a load with a listener still attached keeps its document', () async { + final loading = Completer(); + final ref = PdfDocumentRefByLoader((_) => loading.future, key: PdfDocumentRefKey('kept-load-test')); + final listenable = ref.resolveListenable(); + void listener() {} + listenable.addListener(listener); + final load = listenable.load(); + + final document = _FakeDocument(); + loading.complete(document); + await load; + + expect(document.disposed, isFalse); + expect(listenable.document, same(document)); + listenable.removeListener(listener); + expect(document.disposed, isTrue); + }); +}