mirror of
https://github.com/JustinZeus/Wholphin.git
synced 2026-07-08 15:50:16 +02:00
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:
parent
9f90885b4c
commit
71e74949b7
11 changed files with 70 additions and 18 deletions
|
|
@ -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(
|
||||||
|
|
|
||||||
|
|
@ -1191,6 +1191,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?,
|
||||||
|
|
|
||||||
|
|
@ -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()
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
|
|
|
||||||
|
|
@ -132,7 +132,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,
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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) },
|
||||||
|
|
|
||||||
|
|
@ -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(
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
|
|
|
||||||
|
|
@ -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,
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue