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.
This commit is contained in:
Justin Visser 2026-05-26 14:57:01 +02:00
parent 908b08bd73
commit b080238837
11 changed files with 70 additions and 18 deletions

View file

@ -68,7 +68,7 @@ fun MainContent(
val factor = interfaceCustomization.uiScalePercent / 100f val factor = interfaceCustomization.uiScalePercent / 100f
Density( Density(
density = baseDensity.density * factor, density = baseDensity.density * factor,
fontScale = baseDensity.fontScale * factor, fontScale = baseDensity.fontScale,
) )
} }
CompositionLocalProvider( CompositionLocalProvider(

View file

@ -1184,6 +1184,10 @@ private const val MULTIPLIER_MIN = 50
private const val MULTIPLIER_MAX = 150 private const val MULTIPLIER_MAX = 150
private const val MULTIPLIER_STEP = 5 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( internal fun migrateLegacyRowSpacing(
config: HomeRowConfig, config: HomeRowConfig,
legacySpacingDp: Int?, legacySpacingDp: Int?,

View file

@ -40,6 +40,7 @@ import com.github.damontecres.wholphin.ui.Cards
import com.github.damontecres.wholphin.ui.PreviewTvSpec import com.github.damontecres.wholphin.ui.PreviewTvSpec
import com.github.damontecres.wholphin.ui.enableMarquee import com.github.damontecres.wholphin.ui.enableMarquee
import com.github.damontecres.wholphin.ui.theme.WholphinTheme import com.github.damontecres.wholphin.ui.theme.WholphinTheme
import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization
import kotlinx.coroutines.delay import kotlinx.coroutines.delay
@Composable @Composable
@ -49,7 +50,10 @@ fun ViewMoreCard(
modifier: Modifier = Modifier, modifier: Modifier = Modifier,
interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }, interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
aspectRatio: AspectRatio = AspectRatio.TALL, 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, showTitle: Boolean = true,
) { ) {
val focused by interactionSource.collectIsFocusedAsState() val focused by interactionSource.collectIsFocusedAsState()

View file

@ -74,7 +74,7 @@ fun CollectionFolderGrid(
) { ) {
val defaultBringIntoViewSpec = LocalBringIntoViewSpec.current val defaultBringIntoViewSpec = LocalBringIntoViewSpec.current
val density = LocalDensity.current val density = LocalDensity.current
val spacingDp = LocalInterfaceCustomization.current.spacingDp val spacingDp = viewOptions.spacing * LocalInterfaceCustomization.current.spacingPercent / 100
AnimatedVisibility(viewOptions.showDetails) { AnimatedVisibility(viewOptions.showDetails) {
HomePageHeader( HomePageHeader(
item = focusedItem, item = focusedItem,

View file

@ -141,7 +141,7 @@ fun ItemGrid(
showJumpButtons = false, showJumpButtons = false,
showLetterButtons = false, showLetterButtons = false,
initialPosition = destination.initialPosition, 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) -> cardContent = @Composable { (item, index, onClick, onLongClick, widthPx, mod) ->
GridCard( GridCard(
item = item, item = item,

View file

@ -369,7 +369,7 @@ fun HomeRowGrid(
) )
}, },
columns = if (viewOptions.aspectRatio.ratio > 1f) 4 else 6, 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, bringIntoViewSpec = LocalBringIntoViewSpec.current,
) )
} }

View file

@ -45,7 +45,7 @@ fun CollectionMixedGrid(
val density = LocalDensity.current val density = LocalDensity.current
val cardViewOptions = state.viewOptions.cardViewOptions val cardViewOptions = state.viewOptions.cardViewOptions
val spacingDp = LocalInterfaceCustomization.current.spacingDp val spacingDp = cardViewOptions.spacing * LocalInterfaceCustomization.current.spacingPercent / 100
CardGrid( CardGrid(
pager = state.items, pager = state.items,
onClickItem = { index: Int, item: BaseItem -> onClickItem.invoke(RowColumn(0, index), item) }, onClickItem = { index: Int, item: BaseItem -> onClickItem.invoke(RowColumn(0, index), item) },

View file

@ -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.ifElse
import com.github.damontecres.wholphin.ui.playback.ControllerViewState import com.github.damontecres.wholphin.ui.playback.ControllerViewState
import com.github.damontecres.wholphin.ui.tryRequestFocus import com.github.damontecres.wholphin.ui.tryRequestFocus
import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization
@Composable @Composable
fun QueueRowOverlay( fun QueueRowOverlay(
@ -85,7 +86,7 @@ fun QueueRowOverlay(
controllerViewState.hideControls() controllerViewState.hideControls()
}, },
onLongClick = {}, onLongClick = {},
imageHeight = 140.dp, imageHeight = LocalInterfaceCustomization.current.episodeCardHeightDp.dp,
interactionSource = interactionSource, interactionSource = interactionSource,
modifier = modifier =
Modifier.ifElse( Modifier.ifElse(

View file

@ -169,13 +169,11 @@ fun DisplaySizePage(
val posters by previewViewModel.posters.collectAsState() val posters by previewViewModel.posters.collectAsState()
val episodes by previewViewModel.episodes.collectAsState() val episodes by previewViewModel.episodes.collectAsState()
// Snapshot the page's own density on first composition so the slider rail // Freezes the rail width and slider labels so they don't reflow under the user's finger
// and labels stay fixed while the user drags UI scale. // while dragging UI scale.
val ambientDensity = LocalDensity.current val ambientDensity = LocalDensity.current
val pageDensity = remember { ambientDensity } 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 context = LocalContext.current
val systemDensity = val systemDensity =
remember { remember {
@ -190,7 +188,7 @@ fun DisplaySizePage(
val factor = uiScale / 100f val factor = uiScale / 100f
Density( Density(
density = systemDensity.density * factor, density = systemDensity.density * factor,
fontScale = systemDensity.fontScale * factor, fontScale = systemDensity.fontScale,
) )
} }

View file

@ -5,16 +5,22 @@ import androidx.compose.ui.unit.Dp
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.data.model.HomeRowViewOptions
import com.github.damontecres.wholphin.preferences.AppSliderPreference import com.github.damontecres.wholphin.preferences.AppSliderPreference
import com.github.damontecres.wholphin.ui.AspectRatio
import com.github.damontecres.wholphin.ui.Cards import com.github.damontecres.wholphin.ui.Cards
@Composable @Composable
internal fun HomeRowViewOptions.resolvedCardHeight(): Dp = internal fun HomeRowViewOptions.resolvedCardHeight(): Dp {
resolveCardHeight( 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, heightDp = heightDp,
cardSizeMultiplier = cardSizeMultiplier, cardSizeMultiplier = cardSizeMultiplier,
globalCardHeightDp = LocalInterfaceCustomization.current.cardHeightDp, globalCardHeightDp = globalForThisRow,
baseCardHeightDp = Cards.HEIGHT_2X3_DP, baseCardHeightDp = naturalBase,
).dp ).dp
}
internal fun resolveCardHeight( internal fun resolveCardHeight(
heightDp: Int, heightDp: Int,

View file

@ -9,6 +9,7 @@ import com.github.damontecres.wholphin.ui.Cards
import org.jellyfin.sdk.model.UUID import org.jellyfin.sdk.model.UUID
import org.junit.Assert.assertEquals import org.junit.Assert.assertEquals
import org.junit.Test import org.junit.Test
import java.util.EnumSet
class InterfaceHelpersTest { class InterfaceHelpersTest {
@Test @Test
@ -76,6 +77,44 @@ class InterfaceHelpersTest {
assertEquals(64, resolveCardHeight(128, 50, 172, Cards.HEIGHT_2X3_DP)) 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 @Test
fun `InterfaceCustomization cardHeightDp computes global from slider percent`() { fun `InterfaceCustomization cardHeightDp computes global from slider percent`() {
val customization = InterfaceCustomization(prefs = AppPreferences.getDefaultInstance()) val customization = InterfaceCustomization(prefs = AppPreferences.getDefaultInstance())
@ -88,7 +127,7 @@ class InterfaceHelpersTest {
val customization = val customization =
InterfaceCustomization( InterfaceCustomization(
enabledDisplayToggles = enabledDisplayToggles =
java.util.EnumSet.noneOf( EnumSet.noneOf(
com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, com.github.damontecres.wholphin.preferences.DisplayToggle::class.java,
), ),
cardSizePercent = 150, cardSizePercent = 150,
@ -102,7 +141,7 @@ class InterfaceHelpersTest {
val customization = val customization =
InterfaceCustomization( InterfaceCustomization(
enabledDisplayToggles = enabledDisplayToggles =
java.util.EnumSet.noneOf( EnumSet.noneOf(
com.github.damontecres.wholphin.preferences.DisplayToggle::class.java, com.github.damontecres.wholphin.preferences.DisplayToggle::class.java,
), ),
spacingPercent = 175, spacingPercent = 175,