From 3b019c8fe254366419774c731aa7044b6301fc13 Mon Sep 17 00:00:00 2001 From: edde746 <86283021+edde746@users.noreply.github.com> Date: Tue, 28 Jul 2026 10:29:14 +0200 Subject: [PATCH] fix(artwork): show square background art on portrait heroes Cycling backdrops reach a fallback path only once every rotating path has failed to load, but every hero passed the rotation-agnostic backdrop list as the rotation set and the aspect-ordered candidates as the fallback. One servable wide backdrop was therefore enough to hide the square background for good, so phone detail and Discover heroes cover-fitted a 16:9 backdrop into a portrait box instead of showing the square image Plex supplies. Give the rotation set the same aspect-aware preference the candidate list already has: near-square containers rotate the square background alone and keep the backdrops behind it as fallbacks. close #1700 --- lib/media/media_item.dart | 28 +++++++++- lib/screens/discover_screen.dart | 5 +- lib/screens/media_detail_screen.dart | 2 +- lib/widgets/tv_spotlight_background.dart | 2 +- test/media/media_item_test.dart | 61 ++++++++++++++++++++++ test/screens/media_detail_screen_test.dart | 34 ++++++++++++ 6 files changed, 126 insertions(+), 6 deletions(-) diff --git a/lib/media/media_item.dart b/lib/media/media_item.dart index 5a53f86a..4ff0ba92 100644 --- a/lib/media/media_item.dart +++ b/lib/media/media_item.dart @@ -14,6 +14,11 @@ import 'media_version.dart'; part 'media_item.freezed.dart'; part 'media_item.g.dart'; +/// Container aspect ratio below which a hero prefers square background art. +/// A 16:9 backdrop only cover-fits a taller box by discarding most of the +/// frame, so portrait phone/tablet heroes read better with the square image. +const double _squareHeroAspectRatio = 1.39; + /// Backend-neutral media item shape used by UI, providers, persistence, and /// playback. Concrete variants retain backend-only fields without forcing the /// rest of the app to traffic in Plex/Jellyfin DTOs. @@ -676,14 +681,33 @@ sealed class MediaItem with _$MediaItem { return resolvedBackdropPaths; } + /// The backdrops a hero may rotate through in a container of + /// [containerAspectRatio]. + /// + /// `CyclingMediaBackdrop` cycles its rotation set indefinitely and reaches a + /// fallback path only once every rotating path has failed to load, so the + /// rotation set must hold whatever [heroArtCandidates] prefers — otherwise + /// one servable wide backdrop hides the square background for good and a + /// near-square hero is stuck with a cropped 16:9 frame. Such containers + /// therefore rotate the square background alone, which is to say they hold + /// still. + List heroRotationPaths({required double containerAspectRatio}) { + if (containerAspectRatio < _squareHeroAspectRatio) { + final square = backgroundSquarePath; + if (square != null && square.isNotEmpty) return [square]; + } + return heroBackdropPaths; + } + /// Returns hero art candidates in display-preference order. List heroArtCandidates({required double containerAspectRatio}) { final own = resolvedBackdropPaths; final inherited = resolvedGrandparentBackdropPaths; + final isNearSquare = containerAspectRatio < _squareHeroAspectRatio; final preferred = switch (kind) { - MediaKind.episode when containerAspectRatio < 1.39 => [backgroundSquarePath, ...inherited, ...own], + MediaKind.episode when isNearSquare => [backgroundSquarePath, ...inherited, ...own], MediaKind.episode => [...inherited, ...own, backgroundSquarePath], - _ when containerAspectRatio < 1.39 => [backgroundSquarePath, ...own], + _ when isNearSquare => [backgroundSquarePath, ...own], _ => [...own, backgroundSquarePath], }; diff --git a/lib/screens/discover_screen.dart b/lib/screens/discover_screen.dart index 4b4f0d9b..9bf75a3c 100644 --- a/lib/screens/discover_screen.dart +++ b/lib/screens/discover_screen.dart @@ -1202,7 +1202,8 @@ class _DiscoverScreenState extends State final isEpisode = heroItem.isEpisode; final showName = heroItem.grandparentTitle ?? heroItem.displayTitle; final screenWidth = MediaQuery.sizeOf(context).width; - final heroArtPaths = heroItem.heroArtCandidates(containerAspectRatio: screenWidth / heroHeight); + final heroAspectRatio = screenWidth / heroHeight; + final heroArtPaths = heroItem.heroArtCandidates(containerAspectRatio: heroAspectRatio); final isLargeScreen = ScreenBreakpoints.isWideTabletOrLarger(screenWidth); final isTv = PlatformDetector.isTV(); final alignLeft = isTv || isLargeScreen; @@ -1268,7 +1269,7 @@ class _DiscoverScreenState extends State return blurArtwork( CyclingMediaBackdrop( mediaKey: heroItem.globalKey, - imagePaths: heroItem.heroBackdropPaths, + imagePaths: heroItem.heroRotationPaths(containerAspectRatio: heroAspectRatio), fallbackImagePaths: heroArtPaths, client: heroClient, active: _isTabVisible, diff --git a/lib/screens/media_detail_screen.dart b/lib/screens/media_detail_screen.dart index 365f72b1..424f8a8c 100644 --- a/lib/screens/media_detail_screen.dart +++ b/lib/screens/media_detail_screen.dart @@ -4150,7 +4150,7 @@ class _MediaDetailScreenState extends State return blurArtwork( CyclingMediaBackdrop( mediaKey: metadata.globalKey, - imagePaths: metadata.heroBackdropPaths, + imagePaths: metadata.heroRotationPaths(containerAspectRatio: containerAspect), fallbackImagePaths: heroArtPaths, client: _getArtworkMediaClient(context), localArtworkPathResolver: widget.isOffline diff --git a/lib/widgets/tv_spotlight_background.dart b/lib/widgets/tv_spotlight_background.dart index 5e18908d..c7124c5e 100644 --- a/lib/widgets/tv_spotlight_background.dart +++ b/lib/widgets/tv_spotlight_background.dart @@ -75,7 +75,7 @@ class TvSpotlightBackground extends StatelessWidget { final backdropSize = cornerBackdrop ? Size(size.width * 0.68, size.height * 0.72) : size; final backdrop = CyclingMediaBackdrop( mediaKey: media?.globalKey, - imagePaths: media?.heroBackdropPaths ?? const [], + imagePaths: media?.heroRotationPaths(containerAspectRatio: containerAspect) ?? const [], fallbackImagePaths: fallbackPaths, client: client, localArtworkPathResolver: localArtworkPathResolver == null ? null : (path) => localArtworkPathResolver!(path), diff --git a/test/media/media_item_test.dart b/test/media/media_item_test.dart index 218318bf..1d87c24b 100644 --- a/test/media/media_item_test.dart +++ b/test/media/media_item_test.dart @@ -228,6 +228,67 @@ void main() { }); }); + group('MediaItem.heroRotationPaths', () { + /// The order `CyclingMediaBackdrop` attempts paths as each one fails: + /// every rotating path, then the fallbacks it is not already rotating. + /// Only the head of this list is ever displayed by a healthy server. + List displayOrder(MediaItem item, double aspect) { + final rotation = item.heroRotationPaths(containerAspectRatio: aspect); + final candidates = item.heroArtCandidates(containerAspectRatio: aspect); + return [...rotation, ...candidates.where((path) => !rotation.contains(path))]; + } + + test('near-square containers hold on square art instead of rotating backdrops', () { + final movie = _movie( + backend: MediaBackend.jellyfin, + artPath: '/art-0', + backdropPaths: ['/art-0', '/art-1'], + backgroundSquarePath: '/square', + ); + + expect(movie.heroRotationPaths(containerAspectRatio: 1.0), ['/square']); + }); + + test('near-square containers rotate backdrops when there is no square art', () { + final movie = _movie(backend: MediaBackend.jellyfin, artPath: '/art-0', backdropPaths: ['/art-0', '/art-1']); + + expect(movie.heroRotationPaths(containerAspectRatio: 1.0), ['/art-0', '/art-1']); + }); + + test('wide containers rotate backdrops and leave square art behind them', () { + final movie = _movie( + backend: MediaBackend.jellyfin, + artPath: '/art-0', + backdropPaths: ['/art-0', '/art-1'], + backgroundSquarePath: '/square', + ); + + expect(movie.heroRotationPaths(containerAspectRatio: 16 / 9), ['/art-0', '/art-1']); + }); + + test('rotation before fallback reproduces the candidate order at every aspect', () { + final episode = testMediaItem( + id: 'e-order', + backend: MediaBackend.jellyfin, + kind: MediaKind.episode, + artPath: '/episode-0', + backdropPaths: ['/episode-0', '/episode-1'], + grandparentArtPath: '/show-0', + grandparentBackdropPaths: ['/show-0', '/show-1'], + backgroundSquarePath: '/square', + serverId: 's1', + ); + + for (final aspect in [0.75, 1.0, 1.38, 1.39, 16 / 9, 2.4]) { + expect( + displayOrder(episode, aspect), + episode.heroArtCandidates(containerAspectRatio: aspect), + reason: 'aspect $aspect', + ); + } + }); + }); + group('MediaItem.isPartiallyWatched', () { test('show with some leaves watched is partially watched', () { final show = testMediaItem( diff --git a/test/screens/media_detail_screen_test.dart b/test/screens/media_detail_screen_test.dart index 911793e8..b35aa8a9 100644 --- a/test/screens/media_detail_screen_test.dart +++ b/test/screens/media_detail_screen_test.dart @@ -38,6 +38,7 @@ import 'package:plezy/utils/platform_detector.dart'; import 'package:plezy/utils/watch_state_notifier.dart'; import 'package:plezy/utils/video_player_navigation.dart'; import 'package:plezy/widgets/collapsible_text.dart'; +import 'package:plezy/widgets/cycling_media_backdrop.dart'; import 'package:plezy/widgets/episode_card.dart'; import 'package:plezy/widgets/tv_browse_rail.dart'; import 'package:provider/provider.dart'; @@ -941,6 +942,32 @@ void main() { expect(find.text('Director'), findsNothing); }); + testWidgets('portrait phone hero shows square art instead of the cropped backdrop', (tester) async { + final movie = testMediaItem( + id: 'square_hero', + backend: MediaBackend.plex, + kind: MediaKind.movie, + title: 'Square hero', + artPath: '/library/metadata/square_hero/art', + backgroundSquarePath: '/library/metadata/square_hero/squareBg', + serverId: 'server_1', + serverName: 'Server', + ); + final client = _FakeMediaServerClient(show: movie, childrenByParent: const {}); + + await pumpPhoneDetail(tester, client, movie); + + final backdrop = find.byType(CyclingMediaBackdrop); + expect(backdrop, findsOneWidget); + // A fallback is reached only once every rotating path has failed, so the + // square background has to be in the rotation set. Listed behind a + // servable wide backdrop it would never be shown at all. + final widget = tester.widget(backdrop); + expect(widget.imagePaths, ['/library/metadata/square_hero/squareBg']); + expect(widget.fallbackImagePaths, contains('/library/metadata/square_hero/art')); + expect(client.thumbnailPaths.first, '/library/metadata/square_hero/squareBg'); + }); + FocusNode overviewFocusNode(WidgetTester tester) { final overviewFocus = find.byWidgetPredicate( (widget) => widget is Focus && widget.focusNode?.debugLabel == 'overview', @@ -1176,6 +1203,7 @@ class _FakeMediaServerClient implements MediaServerClient { final Map childrenPageErrors; final Future>? pendingPlayableDescendants; final childrenPageCalls = <({String parentId, int? start, int? size})>[]; + final thumbnailPaths = []; _FakeMediaServerClient({ required this.show, @@ -1247,6 +1275,12 @@ class _FakeMediaServerClient implements MediaServerClient { @override Future> fetchRelatedHubs(String id, {int count = 10}) async => const []; + @override + String thumbnailUrl(String? path, {int? width, int? height, bool cover = true}) { + thumbnailPaths.add(path); + return ''; + } + @override void close() {}