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
This commit is contained in:
@@ -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<String> 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<String> heroArtCandidates({required double containerAspectRatio}) {
|
||||
final own = resolvedBackdropPaths;
|
||||
final inherited = resolvedGrandparentBackdropPaths;
|
||||
final isNearSquare = containerAspectRatio < _squareHeroAspectRatio;
|
||||
final preferred = switch (kind) {
|
||||
MediaKind.episode when containerAspectRatio < 1.39 => <String?>[backgroundSquarePath, ...inherited, ...own],
|
||||
MediaKind.episode when isNearSquare => <String?>[backgroundSquarePath, ...inherited, ...own],
|
||||
MediaKind.episode => <String?>[...inherited, ...own, backgroundSquarePath],
|
||||
_ when containerAspectRatio < 1.39 => <String?>[backgroundSquarePath, ...own],
|
||||
_ when isNearSquare => <String?>[backgroundSquarePath, ...own],
|
||||
_ => <String?>[...own, backgroundSquarePath],
|
||||
};
|
||||
|
||||
|
||||
@@ -1202,7 +1202,8 @@ class _DiscoverScreenState extends State<DiscoverScreen>
|
||||
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<DiscoverScreen>
|
||||
return blurArtwork(
|
||||
CyclingMediaBackdrop(
|
||||
mediaKey: heroItem.globalKey,
|
||||
imagePaths: heroItem.heroBackdropPaths,
|
||||
imagePaths: heroItem.heroRotationPaths(containerAspectRatio: heroAspectRatio),
|
||||
fallbackImagePaths: heroArtPaths,
|
||||
client: heroClient,
|
||||
active: _isTabVisible,
|
||||
|
||||
@@ -4150,7 +4150,7 @@ class _MediaDetailScreenState extends State<MediaDetailScreen>
|
||||
return blurArtwork(
|
||||
CyclingMediaBackdrop(
|
||||
mediaKey: metadata.globalKey,
|
||||
imagePaths: metadata.heroBackdropPaths,
|
||||
imagePaths: metadata.heroRotationPaths(containerAspectRatio: containerAspect),
|
||||
fallbackImagePaths: heroArtPaths,
|
||||
client: _getArtworkMediaClient(context),
|
||||
localArtworkPathResolver: widget.isOffline
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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<String> 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(
|
||||
|
||||
@@ -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<CyclingMediaBackdrop>(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<String, Object> childrenPageErrors;
|
||||
final Future<List<MediaItem>>? pendingPlayableDescendants;
|
||||
final childrenPageCalls = <({String parentId, int? start, int? size})>[];
|
||||
final thumbnailPaths = <String?>[];
|
||||
|
||||
_FakeMediaServerClient({
|
||||
required this.show,
|
||||
@@ -1247,6 +1275,12 @@ class _FakeMediaServerClient implements MediaServerClient {
|
||||
@override
|
||||
Future<List<MediaHub>> 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() {}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user