From d97b4bfd2dd952f830d4f0c9376ff88a5c070e14 Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sat, 3 Oct 2026 04:24:55 +0700 Subject: [PATCH] perf(details): load local albums with skeletons and bound artwork decoding --- lib/screens/downloaded_album_screen.dart | 33 +++-- lib/screens/local_album_screen.dart | 65 +++++++- lib/screens/mornye_library_screen.dart | 13 +- lib/screens/queue_tab_navigation.dart | 76 ++++++---- lib/screens/track_metadata_mornye.dart | 7 + lib/screens/track_metadata_screen.dart | 2 +- test/detail_route_loading_test.dart | 181 +++++++++++++++++++++++ 7 files changed, 320 insertions(+), 57 deletions(-) create mode 100644 test/detail_route_loading_test.dart diff --git a/lib/screens/downloaded_album_screen.dart b/lib/screens/downloaded_album_screen.dart index f47116d7..e837a696 100644 --- a/lib/screens/downloaded_album_screen.dart +++ b/lib/screens/downloaded_album_screen.dart @@ -211,17 +211,20 @@ class _DownloadedAlbumScreenState extends ConsumerState required int navigationIndex, }) async { final navigator = Navigator.of(context); - final embeddedCoverPath = - await DownloadedEmbeddedCoverResolver.resolveOrExtract( - item.filePath, - onChanged: _onEmbeddedCoverChanged, - ); + final embeddedCoverPath = context.isMornye + ? DownloadedEmbeddedCoverResolver.resolve(item.filePath) + : await DownloadedEmbeddedCoverResolver.resolveOrExtract( + item.filePath, + onChanged: _onEmbeddedCoverChanged, + ); if (!mounted) return; final artworkSource = embeddedCoverPath ?? item.coverUrl; - if (embeddedCoverPath == null) { + if (embeddedCoverPath == null && !context.isMornye) { precacheCoverImage(context, item.coverUrl); } - final backdropReady = precacheMetadataBackdrop(context, artworkSource); + final backdropReady = context.isMornye + ? Future.value() + : precacheMetadataBackdrop(context, artworkSource); final beforeModTime = await DownloadedEmbeddedCoverResolver.readFileModTimeMillis( item.filePath, @@ -229,15 +232,17 @@ class _DownloadedAlbumScreenState extends ConsumerState await backdropReady; if (!mounted) return; - final result = await navigator.push( - slidePageRoute( - page: TrackMetadataScreen( - item: item, - historyNavigationItems: navigationItems, - navigationIndex: navigationIndex, - ), + final route = slidePageRoute( + page: TrackMetadataScreen( + item: item, + historyNavigationItems: navigationItems, + navigationIndex: navigationIndex, ), ); + final result = await navigator.push(route); + // Refresh the album after the return animation, not while both pages paint. + if (route is TransitionRoute) await route.completed; + if (!mounted) return; await DownloadedEmbeddedCoverResolver.scheduleRefreshForPath( item.filePath, beforeModTime: beforeModTime, diff --git a/lib/screens/local_album_screen.dart b/lib/screens/local_album_screen.dart index c7f06d97..60a65224 100644 --- a/lib/screens/local_album_screen.dart +++ b/lib/screens/local_album_screen.dart @@ -42,13 +42,15 @@ class LocalAlbumScreen extends ConsumerStatefulWidget { final String artistName; final String? coverPath; final List tracks; + final Future> Function()? loadTracks; const LocalAlbumScreen({ super.key, required this.albumName, required this.artistName, this.coverPath, - required this.tracks, + this.tracks = const [], + this.loadTracks, }); @override @@ -61,6 +63,10 @@ class _LocalAlbumScreenState extends ConsumerState CollapsingHeaderScrollMixin { late List _sortedTracksCache; late Map> _discGroupsCache; + List? _loadedTracks; + bool _loadingTracks = false; + Object? _loadError; + int _loadGeneration = 0; void _showCueVirtualTrackSnackBar() { ScaffoldMessenger.of(context).showSnackBar( @@ -77,19 +83,44 @@ class _LocalAlbumScreenState extends ConsumerState void initState() { super.initState(); _rebuildTrackCaches(); + _loadTracks(); + } + + Future _loadTracks() async { + final loader = widget.loadTracks; + if (loader == null) return; + final generation = ++_loadGeneration; + _loadingTracks = true; + _loadError = null; + try { + final tracks = await loader(); + if (!mounted || generation != _loadGeneration) return; + _loadedTracks = tracks; + } catch (error) { + if (!mounted || generation != _loadGeneration) return; + _loadError = error; + } + _loadingTracks = false; + setState(_rebuildTrackCaches); } @override void didUpdateWidget(covariant LocalAlbumScreen oldWidget) { super.didUpdateWidget(oldWidget); if (!identical(oldWidget.tracks, widget.tracks) || - oldWidget.tracks.length != widget.tracks.length) { + oldWidget.tracks.length != widget.tracks.length || + oldWidget.loadTracks != widget.loadTracks) { + _loadGeneration++; + _loadedTracks = null; + _loadingTracks = false; + _loadError = null; _rebuildTrackCaches(); + _loadTracks(); } } List _buildSortedTracks() { - final tracks = List.from(widget.tracks); + final tracks = List.from(_loadedTracks ?? widget.tracks); tracks.sort((a, b) { final aDisc = a.discNumber ?? 1; final bDisc = b.discNumber ?? 1; @@ -196,7 +227,7 @@ class _LocalAlbumScreenState extends ConsumerState : 0.0; final tracks = _sortedTracksCache; - if (tracks.isEmpty) { + if (tracks.isEmpty && !_loadingTracks && _loadError == null) { return Scaffold( appBar: AppBar(title: Text(widget.albumName)), body: Center(child: Text(context.l10n.noTracksFoundForAlbum)), @@ -210,7 +241,26 @@ class _LocalAlbumScreenState extends ConsumerState isSelectionMode: isSelectionMode, onExitSelectionMode: exitSelectionMode, appBar: _buildAppBar(context, colorScheme, qualityLabelMode), - slivers: [_buildTrackList(context, colorScheme, tracks)], + slivers: [ + if (_loadError != null) + SliverToBoxAdapter( + child: Center( + child: TextButton.icon( + onPressed: () => setState(() { + _loadTracks(); + }), + icon: const Icon(Icons.refresh), + label: Text(context.l10n.dialogRetry), + ), + ), + ) + else if (_loadingTracks) + const SliverToBoxAdapter( + child: RepaintBoundary(child: AlbumTrackListSkeleton(itemCount: 8)), + ) + else + _buildTrackList(context, colorScheme, tracks), + ], selectionBar: _buildSelectionBottomBar( context, colorScheme, @@ -231,7 +281,9 @@ class _LocalAlbumScreenState extends ConsumerState .watch( playerMotionArtworkProvider(( album: widget.albumName, - artist: _sortedTracksCache.first.artistName, + artist: + _sortedTracksCache.firstOrNull?.artistName ?? + widget.artistName, )), ) .value @@ -403,6 +455,7 @@ class _LocalAlbumScreenState extends ConsumerState key: ValueKey(track.id), child: StaggeredListItem( index: index, + animate: widget.loadTracks == null, child: _buildTrackItem(context, colorScheme, track), ), ); diff --git a/lib/screens/mornye_library_screen.dart b/lib/screens/mornye_library_screen.dart index 91a238ea..9ff39421 100644 --- a/lib/screens/mornye_library_screen.dart +++ b/lib/screens/mornye_library_screen.dart @@ -119,17 +119,20 @@ class _MornyeLibraryScreenState extends ConsumerState { ); return; } - final rows = await LibraryDatabase.instance.getQueueLocalAlbumTracksByKey( - entry.key, - ); - if (!mounted) return; + Future> loadTracks() async { + final rows = await LibraryDatabase.instance.getQueueLocalAlbumTracksByKey( + entry.key, + ); + return rows.map(LocalLibraryItem.fromJson).toList(growable: false); + } + await Navigator.of(context).push( MaterialPageRoute( builder: (_) => LocalAlbumScreen( albumName: entry.name, artistName: entry.artist, coverPath: entry.cover, - tracks: rows.map(LocalLibraryItem.fromJson).toList(growable: false), + loadTracks: loadTracks, ), ), ); diff --git a/lib/screens/queue_tab_navigation.dart b/lib/screens/queue_tab_navigation.dart index 36ed9745..4c014509 100644 --- a/lib/screens/queue_tab_navigation.dart +++ b/lib/screens/queue_tab_navigation.dart @@ -104,24 +104,30 @@ extension _QueueTabNavigation on _QueueTabState { ); final navigator = Navigator.of(context); - final embeddedCoverPath = - await DownloadedEmbeddedCoverResolver.resolveOrExtract( - historyItem.filePath, - onChanged: _onEmbeddedCoverChanged, - ); + final embeddedCoverPath = context.isMornye + ? DownloadedEmbeddedCoverResolver.resolve(historyItem.filePath) + : await DownloadedEmbeddedCoverResolver.resolveOrExtract( + historyItem.filePath, + onChanged: _onEmbeddedCoverChanged, + ); if (!mounted) return; final artworkSource = embeddedCoverPath ?? historyItem.coverUrl; - if (embeddedCoverPath == null) { + if (embeddedCoverPath == null && !context.isMornye) { precacheCoverImage(context, historyItem.coverUrl); } - final backdropReady = precacheMetadataBackdrop(context, artworkSource); + final backdropReady = context.isMornye + ? Future.value() + : precacheMetadataBackdrop(context, artworkSource); _searchFocusNode.unfocus(); final beforeModTime = await _readFileModTimeMillis(historyItem.filePath); await backdropReady; if (!mounted) return; - final result = await navigator.push( - slidePageRoute(page: TrackMetadataScreen(item: historyItem)), + final route = slidePageRoute( + page: TrackMetadataScreen(item: historyItem), ); + final result = await navigator.push(route); + if (route is TransitionRoute) await route.completed; + if (!mounted) return; _searchFocusNode.unfocus(); if (result == true) { await _scheduleDownloadedEmbeddedCoverRefreshForPath( @@ -143,31 +149,35 @@ extension _QueueTabNavigation on _QueueTabState { int? navigationIndex, }) async { final navigator = Navigator.of(context); - final embeddedCoverPath = - await DownloadedEmbeddedCoverResolver.resolveOrExtract( - item.filePath, - onChanged: _onEmbeddedCoverChanged, - ); + final embeddedCoverPath = context.isMornye + ? DownloadedEmbeddedCoverResolver.resolve(item.filePath) + : await DownloadedEmbeddedCoverResolver.resolveOrExtract( + item.filePath, + onChanged: _onEmbeddedCoverChanged, + ); if (!mounted) return; final artworkSource = embeddedCoverPath ?? item.coverUrl; - if (embeddedCoverPath == null) { + if (embeddedCoverPath == null && !context.isMornye) { precacheCoverImage(context, item.coverUrl); } - final backdropReady = precacheMetadataBackdrop(context, artworkSource); + final backdropReady = context.isMornye + ? Future.value() + : precacheMetadataBackdrop(context, artworkSource); _searchFocusNode.unfocus(); final beforeModTime = await _readFileModTimeMillis(item.filePath); await backdropReady; if (!mounted) return; - final result = await navigator.push( - slidePageRoute( - page: TrackMetadataScreen( - item: item, - historyNavigationItems: navigationItems, - navigationIndex: navigationIndex, - coverHeroTag: 'cover_lib_dl_${item.id}', - ), + final route = slidePageRoute( + page: TrackMetadataScreen( + item: item, + historyNavigationItems: navigationItems, + navigationIndex: navigationIndex, + coverHeroTag: 'cover_lib_dl_${item.id}', ), ); + final result = await navigator.push(route); + if (route is TransitionRoute) await route.completed; + if (!mounted) return; _searchFocusNode.unfocus(); if (result == true) { await _scheduleDownloadedEmbeddedCoverRefreshForPath( @@ -189,7 +199,9 @@ extension _QueueTabNavigation on _QueueTabState { int? navigationIndex, }) async { _searchFocusNode.unfocus(); - await precacheMetadataBackdrop(context, item.coverPath); + if (!context.isMornye) { + await precacheMetadataBackdrop(context, item.coverPath); + } if (!mounted) return; await Navigator.push( context, @@ -222,9 +234,8 @@ extension _QueueTabNavigation on _QueueTabState { ); } - Future _navigateToLocalAlbum(_GroupedLocalAlbum album) async { - var tracks = album.tracks; - if (tracks.isEmpty && album.displayTrackCount > 0) { + void _navigateToLocalAlbum(_GroupedLocalAlbum album) { + Future> loadTracks() async { var rows = album.albumKey.isNotEmpty ? await LibraryDatabase.instance.getQueueLocalAlbumTracksByKey( album.albumKey, @@ -239,16 +250,19 @@ extension _QueueTabNavigation on _QueueTabState { album.artistName, ); } - tracks = rows.map(LocalLibraryItem.fromJson).toList(growable: false); - if (!mounted) return; + return rows.map(LocalLibraryItem.fromJson).toList(growable: false); } + _navigateWithUnfocus( slidePageRoute( page: LocalAlbumScreen( albumName: album.albumName, artistName: album.artistName, coverPath: album.coverPath, - tracks: tracks, + tracks: album.tracks, + loadTracks: album.tracks.isEmpty && album.displayTrackCount > 0 + ? loadTracks + : null, ), ), ); diff --git a/lib/screens/track_metadata_mornye.dart b/lib/screens/track_metadata_mornye.dart index ba24ae97..7379bd4e 100644 --- a/lib/screens/track_metadata_mornye.dart +++ b/lib/screens/track_metadata_mornye.dart @@ -73,6 +73,13 @@ extension _TrackMetadataMornye on _TrackMetadataScreenState { artUri: artwork, colorScheme: scheme, iconSize: 80, + cacheWidth: + (320 * + MediaQuery.devicePixelRatioOf( + context, + )) + .round() + .clamp(320, 1280), ), ), ), diff --git a/lib/screens/track_metadata_screen.dart b/lib/screens/track_metadata_screen.dart index 992c4c23..8f5c4baf 100644 --- a/lib/screens/track_metadata_screen.dart +++ b/lib/screens/track_metadata_screen.dart @@ -191,7 +191,7 @@ class _TrackMetadataScreenState extends ConsumerState _currentDownloadItem = widget.item; _currentLocalLibraryItem = widget.localItem; _currentNavigationIndex = widget.navigationIndex; - _checkFile(); + unawaited(_checkFile()); } @override diff --git a/test/detail_route_loading_test.dart b/test/detail_route_loading_test.dart new file mode 100644 index 00000000..de1d365d --- /dev/null +++ b/test/detail_route_loading_test.dart @@ -0,0 +1,181 @@ +import 'dart:async'; +import 'dart:convert'; + +import 'package:flutter/material.dart'; +import 'package:flutter/services.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; +import 'package:spotiflac_android/l10n/l10n.dart'; +import 'package:spotiflac_android/screens/local_album_screen.dart'; +import 'package:spotiflac_android/screens/track_metadata_screen.dart'; +import 'package:spotiflac_android/services/library_database.dart'; +import 'package:spotiflac_android/theme/mornye_theme.dart'; +import 'package:spotiflac_android/widgets/animation_utils.dart'; + +LocalLibraryItem _track(int number) => LocalLibraryItem( + id: 'track-$number', + trackName: 'Song $number', + artistName: 'Artist', + albumName: 'Album', + filePath: 'content://library/document/song-$number.flac', + trackNumber: number, + scannedAt: DateTime(2026), +); + +void main() { + Future> mount(WidgetTester tester) async { + SharedPreferences.setMockInitialValues({}); + tester.view.physicalSize = const Size(430, 1200); + tester.view.devicePixelRatio = 1; + addTearDown(tester.view.reset); + final navigator = GlobalKey(); + await tester.pumpWidget( + ProviderScope( + child: MaterialApp( + navigatorKey: navigator, + theme: MornyeTheme.build(Brightness.light), + localizationsDelegates: AppLocalizations.localizationsDelegates, + supportedLocales: AppLocalizations.supportedLocales, + home: const Scaffold(body: Text('Library')), + ), + ), + ); + await tester.pumpAndSettle(); + return navigator; + } + + PageRoute route(Widget page) => PageRouteBuilder( + transitionDuration: const Duration(milliseconds: 400), + reverseTransitionDuration: const Duration(milliseconds: 400), + pageBuilder: (_, _, _) => page, + transitionsBuilder: (_, animation, _, child) => SlideTransition( + position: Tween( + begin: const Offset(1, 0), + end: Offset.zero, + ).animate(animation), + child: child, + ), + ); + + for (final popEarly in [false, true]) { + testWidgets( + 'metadata starts reading during entrance, early pop=$popEarly', + (tester) async { + final navigator = await mount(tester); + var statCalls = 0; + var followUpCalls = 0; + final delayedStat = Completer(); + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + messenger.setMockMethodCallHandler(channel, (call) async { + if (call.method == 'safStat') { + statCalls++; + if (popEarly) return delayedStat.future; + return jsonEncode({'exists': false}); + } + followUpCalls++; + return null; + }); + addTearDown(() => messenger.setMockMethodCallHandler(channel, null)); + unawaited( + navigator.currentState!.push( + route(TrackMetadataScreen(localItem: _track(1))), + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 150)); + expect(find.text('Song 1'), findsWidgets); + expect(statCalls, 1); + if (popEarly) navigator.currentState!.pop(); + await tester.pumpAndSettle(); + if (popEarly) { + delayedStat.complete(jsonEncode({'exists': true, 'size': 100})); + await tester.pump(); + expect(followUpCalls, 0); + } + expect(statCalls, 1); + expect(tester.takeException(), isNull); + }, + ); + } + + testWidgets( + 'album shows header immediately and applies rows during entrance', + (tester) async { + final navigator = await mount(tester); + final result = Completer>(); + unawaited( + navigator.currentState!.push( + route( + LocalAlbumScreen( + albumName: 'Album', + artistName: 'Artist', + loadTracks: () => result.future, + ), + ), + ), + ); + await tester.pump(); + await tester.pump(const Duration(milliseconds: 16)); + expect(find.text('Album'), findsWidgets); + expect(find.text('No tracks found for this album'), findsNothing); + expect(find.byType(AlbumTrackListSkeleton), findsOneWidget); + expect(find.byType(ShimmerLoading), findsOneWidget); + expect(find.byType(CircularProgressIndicator), findsNothing); + result.complete([_track(2), _track(1)]); + await tester.pump(const Duration(milliseconds: 150)); + await tester.pump(); + expect(find.text('Song 1'), findsOneWidget); + expect(find.text('Song 2'), findsOneWidget); + expect(find.byType(AlbumTrackListSkeleton), findsNothing); + expect( + ModalRoute.of( + tester.element(find.byType(LocalAlbumScreen)), + )!.animation!.value, + lessThan(1), + ); + await tester.pumpAndSettle(); + expect(find.text('Song 1'), findsOneWidget); + expect(find.text('Song 2'), findsOneWidget); + expect( + tester.getTopLeft(find.text('Song 1')).dy, + lessThan(tester.getTopLeft(find.text('Song 2')).dy), + ); + expect(tester.takeException(), isNull); + }, + ); + + testWidgets('album retries failed loading and ignores completion after pop', ( + tester, + ) async { + final navigator = await mount(tester); + final result = Completer>(); + var attempts = 0; + unawaited( + navigator.currentState!.push( + route( + LocalAlbumScreen( + albumName: 'Album', + artistName: 'Artist', + loadTracks: () async { + if (++attempts == 1) throw StateError('Unavailable'); + return result.future; + }, + ), + ), + ), + ); + await tester.pumpAndSettle(); + await tester.tap(find.text('Retry')); + await tester.pump(); + expect(attempts, 2); + navigator.currentState!.pop(); + await tester.pumpAndSettle(); + result.complete([_track(1)]); + await tester.pump(); + expect(find.byType(LocalAlbumScreen), findsNothing); + expect(tester.takeException(), isNull); + }); +}