From 71e74949b7189ee7bcb3474f56d5ccbabeb5de6f Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Tue, 26 May 2026 14:57:01 +0200 Subject: [PATCH] Fix global slider reach + UI-scale font math + per-type grid spacing resolvedCardHeight now picks the row's natural base (128 for WIDE / episode aspect, 172 for TALL / poster aspect) via HomeRowViewOptions .aspectRatio and selects the matching scaled global (cardHeightDp or episodeCardHeightDp) from InterfaceCustomization. Previously the function always passed HEIGHT_2X3_DP as the natural base, so every row storing heightDp = HEIGHT_EPISODE = 128 hit the override branch and the global Card slider silently skipped episode-aspect home rows (Favorite-Episode preset, genreDefault rows, etc.). MainContent and DisplaySizePage's preview Density no longer multiply fontScale by the UI-scale factor. sp rendering pipes through density * fontScale, so scaling both produced factor^2 text size - slider 150% rendered fonts at 2.25x while layouts rendered at 1.5x. Scaling density alone makes both linear; the user's system fontScale stays applied on top. ItemGrid, CollectionFolderGrid, CollectionMixedGrid, and HomeRowGrid now honor the per-type viewOptions.spacing base (16/24/16 for the three view-options presets) the same way CollectionFolderList already did - scaled by the global Spacing percent at the consumer site. Before this fix only the list-mode consumer honored the per-type base and grids fell back to a fixed 16dp baseline regardless of preset. Also wires ViewMoreCard's default size into cardHeightDp and QueueRowOverlay's queue-thumbnail height into episodeCardHeightDp so those two surfaces scale too. Adds a Why: comment on the migrateLegacyRowSpacing clamp (50..150 per-row range, intentionally narrower than the global 25..175). Drops a couple of restating // comments in DisplaySizePage and imports java.util.EnumSet at the top of InterfaceHelpersTest. New unit tests cover resolveCardHeight's episode-natural-base path, per-row override precedence over the global, and the linear-density invariant guarding against the fontScale bug regressing. --- .../damontecres/wholphin/MainContent.kt | 2 +- .../wholphin/services/HomeSettingsService.kt | 4 ++ .../wholphin/ui/cards/ViewMoreCard.kt | 6 ++- .../ui/components/CollectionFolderGrid.kt | 2 +- .../wholphin/ui/components/ItemGrid.kt | 2 +- .../wholphin/ui/detail/HomeRowGrid.kt | 2 +- .../detail/collection/CollectionMixedGrid.kt | 2 +- .../ui/playback/overlay/QueueRowOverlay.kt | 3 +- .../displaysize/DisplaySizePage.kt | 8 ++-- .../wholphin/ui/util/InterfaceHelpers.kt | 14 ++++-- .../wholphin/ui/util/InterfaceHelpersTest.kt | 43 ++++++++++++++++++- 11 files changed, 70 insertions(+), 18 deletions(-) diff --git a/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt b/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt index 212f4bba..f58bb5ac 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt @@ -68,7 +68,7 @@ fun MainContent( val factor = interfaceCustomization.uiScalePercent / 100f Density( density = baseDensity.density * factor, - fontScale = baseDensity.fontScale * factor, + fontScale = baseDensity.fontScale, ) } CompositionLocalProvider( diff --git a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt index 997cb7dd..7990a5af 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/services/HomeSettingsService.kt @@ -1191,6 +1191,10 @@ private const val MULTIPLIER_MIN = 50 private const val MULTIPLIER_MAX = 150 private const val MULTIPLIER_STEP = 5 +// Clamped to MULTIPLIER_MIN..MULTIPLIER_MAX (50..150) because that's the +// per-row Spacing multiplier's slider range - intentionally narrower than +// the global Spacing slider's 25..175. A legacy spacing >= 24dp flattens +// to 150% rather than overshooting the per-row UI's bounds. internal fun migrateLegacyRowSpacing( config: HomeRowConfig, legacySpacingDp: Int?, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ViewMoreCard.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ViewMoreCard.kt index 56748473..bd503fb2 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ViewMoreCard.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/cards/ViewMoreCard.kt @@ -40,6 +40,7 @@ import com.github.damontecres.wholphin.ui.Cards import com.github.damontecres.wholphin.ui.PreviewTvSpec import com.github.damontecres.wholphin.ui.enableMarquee import com.github.damontecres.wholphin.ui.theme.WholphinTheme +import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization import kotlinx.coroutines.delay @Composable @@ -49,7 +50,10 @@ fun ViewMoreCard( modifier: Modifier = Modifier, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, aspectRatio: AspectRatio = AspectRatio.TALL, - size: DpSize = DpSize(width = Cards.height2x3 * aspectRatio.ratio, height = Cards.height2x3), + size: DpSize = + LocalInterfaceCustomization.current.cardHeightDp.dp.let { h -> + DpSize(width = h * aspectRatio.ratio, height = h) + }, showTitle: Boolean = true, ) { val focused by interactionSource.collectIsFocusedAsState() diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderGrid.kt index ca731c50..b53b4419 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/CollectionFolderGrid.kt @@ -74,7 +74,7 @@ fun CollectionFolderGrid( ) { val defaultBringIntoViewSpec = LocalBringIntoViewSpec.current val density = LocalDensity.current - val spacingDp = LocalInterfaceCustomization.current.spacingDp + val spacingDp = viewOptions.spacing * LocalInterfaceCustomization.current.spacingPercent / 100 AnimatedVisibility(viewOptions.showDetails) { HomePageHeader( item = focusedItem, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/components/ItemGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/components/ItemGrid.kt index 3936a8f1..16ee2c87 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/components/ItemGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/components/ItemGrid.kt @@ -132,7 +132,7 @@ fun ItemGrid( showJumpButtons = false, showLetterButtons = false, initialPosition = destination.initialPosition, - spacing = LocalInterfaceCustomization.current.spacingDp.dp, + spacing = (destination.viewOptions.spacing * LocalInterfaceCustomization.current.spacingPercent / 100).dp, cardContent = @Composable { (item, index, onClick, onLongClick, widthPx, mod) -> GridCard( item = item, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/HomeRowGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/HomeRowGrid.kt index cc227c8e..98ba6eca 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/HomeRowGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/HomeRowGrid.kt @@ -369,7 +369,7 @@ fun HomeRowGrid( ) }, columns = if (viewOptions.aspectRatio.ratio > 1f) 4 else 6, - spacing = LocalInterfaceCustomization.current.spacingDp.dp, + spacing = (LocalInterfaceCustomization.current.spacingDp * viewOptions.spacingMultiplier / 100).dp, bringIntoViewSpec = LocalBringIntoViewSpec.current, ) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/collection/CollectionMixedGrid.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/collection/CollectionMixedGrid.kt index 9b69d76c..4275e520 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/detail/collection/CollectionMixedGrid.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/detail/collection/CollectionMixedGrid.kt @@ -45,7 +45,7 @@ fun CollectionMixedGrid( val density = LocalDensity.current val cardViewOptions = state.viewOptions.cardViewOptions - val spacingDp = LocalInterfaceCustomization.current.spacingDp + val spacingDp = cardViewOptions.spacing * LocalInterfaceCustomization.current.spacingPercent / 100 CardGrid( pager = state.items, onClickItem = { index: Int, item: BaseItem -> onClickItem.invoke(RowColumn(0, index), item) }, diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/QueueRowOverlay.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/QueueRowOverlay.kt index a44d1fc7..e12a0089 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/QueueRowOverlay.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/playback/overlay/QueueRowOverlay.kt @@ -28,6 +28,7 @@ import com.github.damontecres.wholphin.ui.components.HiddenFocusBox import com.github.damontecres.wholphin.ui.ifElse import com.github.damontecres.wholphin.ui.playback.ControllerViewState import com.github.damontecres.wholphin.ui.tryRequestFocus +import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization @Composable fun QueueRowOverlay( @@ -85,7 +86,7 @@ fun QueueRowOverlay( controllerViewState.hideControls() }, onLongClick = {}, - imageHeight = 140.dp, + imageHeight = LocalInterfaceCustomization.current.episodeCardHeightDp.dp, interactionSource = interactionSource, modifier = Modifier.ifElse( diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/displaysize/DisplaySizePage.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/displaysize/DisplaySizePage.kt index a484df02..85cf3ace 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/displaysize/DisplaySizePage.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/preferences/displaysize/DisplaySizePage.kt @@ -169,13 +169,11 @@ fun DisplaySizePage( val posters by previewViewModel.posters.collectAsState() val episodes by previewViewModel.episodes.collectAsState() - // Snapshot the page's own density on first composition so the slider rail - // and labels stay fixed while the user drags UI scale. + // Freezes the rail width and slider labels so they don't reflow under the user's finger + // while dragging UI scale. val ambientDensity = LocalDensity.current val pageDensity = remember { ambientDensity } - // Resolve back to system base so the preview density is independent of the - // snapshotted page density and reflects the draft uiScale faithfully. val context = LocalContext.current val systemDensity = remember { @@ -190,7 +188,7 @@ fun DisplaySizePage( val factor = uiScale / 100f Density( density = systemDensity.density * factor, - fontScale = systemDensity.fontScale * factor, + fontScale = systemDensity.fontScale, ) } diff --git a/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt b/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt index cf44117a..078a0f14 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpers.kt @@ -5,16 +5,22 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import com.github.damontecres.wholphin.data.model.HomeRowViewOptions import com.github.damontecres.wholphin.preferences.AppSliderPreference +import com.github.damontecres.wholphin.ui.AspectRatio import com.github.damontecres.wholphin.ui.Cards @Composable -internal fun HomeRowViewOptions.resolvedCardHeight(): Dp = - resolveCardHeight( +internal fun HomeRowViewOptions.resolvedCardHeight(): Dp { + val custom = LocalInterfaceCustomization.current + val isWide = aspectRatio == AspectRatio.WIDE + val naturalBase = if (isWide) Cards.HEIGHT_EPISODE else Cards.HEIGHT_2X3_DP + val globalForThisRow = if (isWide) custom.episodeCardHeightDp else custom.cardHeightDp + return resolveCardHeight( heightDp = heightDp, cardSizeMultiplier = cardSizeMultiplier, - globalCardHeightDp = LocalInterfaceCustomization.current.cardHeightDp, - baseCardHeightDp = Cards.HEIGHT_2X3_DP, + globalCardHeightDp = globalForThisRow, + baseCardHeightDp = naturalBase, ).dp +} internal fun resolveCardHeight( heightDp: Int, diff --git a/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt b/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt index 0163509a..f98df7d4 100644 --- a/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt +++ b/app/src/test/java/com/github/damontecres/wholphin/ui/util/InterfaceHelpersTest.kt @@ -9,6 +9,7 @@ import com.github.damontecres.wholphin.ui.Cards import org.jellyfin.sdk.model.UUID import org.junit.Assert.assertEquals import org.junit.Test +import java.util.EnumSet class InterfaceHelpersTest { @Test @@ -76,6 +77,44 @@ class InterfaceHelpersTest { assertEquals(64, resolveCardHeight(128, 50, 172, Cards.HEIGHT_2X3_DP)) } + @Test + fun `resolveCardHeight uses global when heightDp equals episode natural base 128`() { + // For an episode-aspect row, the caller passes baseCardHeightDp = HEIGHT_EPISODE (128) + // and globalCardHeightDp = episodeCardHeightDp. The stored heightDp = 128 then must + // not be treated as an override - the global Card slider should reach this row. + assertEquals(128, resolveCardHeight(Cards.HEIGHT_EPISODE, 100, 128, Cards.HEIGHT_EPISODE)) + // Global at 150%: episodeCardHeightDp = 128 * 150 / 100 = 192. + assertEquals(192, resolveCardHeight(Cards.HEIGHT_EPISODE, 100, 192, Cards.HEIGHT_EPISODE)) + } + + @Test + fun `resolveCardHeight honours per-row override even when row is episode-natural`() { + // A user who manually drags an episode row to heightDp = 200 still wins over the global. + assertEquals(200, resolveCardHeight(200, 100, 192, Cards.HEIGHT_EPISODE)) + // Live-TV-style 96.dp override stays absolute. + assertEquals(96, resolveCardHeight(96, 100, 192, Cards.HEIGHT_EPISODE)) + } + + @Test + fun `UI scale must not double-apply on text`() { + // sp rendering = sp.value * density * fontScale. MainContent's scaled Density + // multiplies density by factor and leaves fontScale alone; at slider 150% text must + // grow by 1.5x, not 1.5*1.5 = 2.25x. Regression guard for the original bug where both + // were multiplied. + val baseDensity = 2.0f + val baseFontScale = 1.0f + val factor = 1.5f + // The pattern MainContent.kt uses after the fix: + val scaledDensity = baseDensity * factor + val scaledFontScale = baseFontScale + val baseTextPx = 14f * baseDensity * baseFontScale + val scaledTextPx = 14f * scaledDensity * scaledFontScale + assertEquals(1.5f * baseTextPx, scaledTextPx, 0.001f) + // Double-application (the bug) would have produced 2.25x. + val buggyTextPx = 14f * (baseDensity * factor) * (baseFontScale * factor) + assertEquals(2.25f * baseTextPx, buggyTextPx, 0.001f) + } + @Test fun `InterfaceCustomization cardHeightDp computes global from slider percent`() { val customization = InterfaceCustomization(prefs = AppPreferences.getDefaultInstance()) @@ -88,7 +127,7 @@ class InterfaceHelpersTest { val customization = InterfaceCustomization( enabledDisplayToggles = - java.util.EnumSet.noneOf( + EnumSet.noneOf( com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, ), cardSizePercent = 150, @@ -102,7 +141,7 @@ class InterfaceHelpersTest { val customization = InterfaceCustomization( enabledDisplayToggles = - java.util.EnumSet.noneOf( + EnumSet.noneOf( com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, ), spacingPercent = 175,