Extract naturalCardBaseDp and scaledDensity helpers so tests bind to real code

resolvedCardHeight previously routed through a two-branch if(isWide)
hidden inside the composable extension, so the regression tests for
the global-slider reach bug only exercised the inner resolveCardHeight
(which was never broken). Lift the aspect -> natural base mapping
into a pure naturalCardBaseDp(AspectRatio) function with an exhaustive
when, and test that directly. SQUARE and FOUR_THREE rows piggyback on
the TALL global by design; the override branch still handles their
absolute heightDp values (Music presets at 100dp, etc.).

MainContent and DisplaySizePage's preview now both call a single
scaledDensity(base, percent) helper instead of inlining Density(...)
constructions. The double-fontScale-multiply regression has a real
guard now: tests assert scaledDensity(2.0, 1.0, 150) returns density
3.0 and fontScale 1.0 (not 1.5), and a third test confirms an
accessibility fontScale survives a UI scale change.

Pure refactor + test sharpening; no behaviour change.
This commit is contained in:
Justin Visser 2026-05-26 15:11:30 +02:00
parent 71e74949b7
commit 0b61007b3f
4 changed files with 75 additions and 35 deletions

View file

@ -19,7 +19,6 @@ import androidx.compose.ui.Alignment
import androidx.compose.ui.Modifier import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.RectangleShape import androidx.compose.ui.graphics.RectangleShape
import androidx.compose.ui.platform.LocalDensity import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.dp import androidx.compose.ui.unit.dp
import androidx.lifecycle.Lifecycle import androidx.lifecycle.Lifecycle
import androidx.lifecycle.compose.LifecycleEventEffect import androidx.lifecycle.compose.LifecycleEventEffect
@ -41,6 +40,7 @@ import com.github.damontecres.wholphin.ui.setup.SwitchServerContent
import com.github.damontecres.wholphin.ui.setup.SwitchUserContent import com.github.damontecres.wholphin.ui.setup.SwitchUserContent
import com.github.damontecres.wholphin.ui.util.InterfaceCustomization import com.github.damontecres.wholphin.ui.util.InterfaceCustomization
import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization
import com.github.damontecres.wholphin.ui.util.scaledDensity
@Composable @Composable
fun MainContent( fun MainContent(
@ -63,17 +63,13 @@ fun MainContent(
val interfaceCustomization = val interfaceCustomization =
remember(appPreferences) { InterfaceCustomization(appPreferences) } remember(appPreferences) { InterfaceCustomization(appPreferences) }
val baseDensity = LocalDensity.current val baseDensity = LocalDensity.current
val scaledDensity = val uiScaleDensity =
remember(baseDensity, interfaceCustomization.uiScalePercent) { remember(baseDensity, interfaceCustomization.uiScalePercent) {
val factor = interfaceCustomization.uiScalePercent / 100f scaledDensity(baseDensity, interfaceCustomization.uiScalePercent)
Density(
density = baseDensity.density * factor,
fontScale = baseDensity.fontScale,
)
} }
CompositionLocalProvider( CompositionLocalProvider(
LocalInterfaceCustomization provides interfaceCustomization, LocalInterfaceCustomization provides interfaceCustomization,
LocalDensity provides scaledDensity, LocalDensity provides uiScaleDensity,
) { ) {
NavDisplay( NavDisplay(
backStack = backStack, backStack = backStack,

View file

@ -69,6 +69,7 @@ import com.github.damontecres.wholphin.ui.preferences.PreferencesViewModel
import com.github.damontecres.wholphin.ui.preferences.SliderPreference import com.github.damontecres.wholphin.ui.preferences.SliderPreference
import com.github.damontecres.wholphin.ui.tryRequestFocus import com.github.damontecres.wholphin.ui.tryRequestFocus
import com.github.damontecres.wholphin.ui.util.ResStringProvider import com.github.damontecres.wholphin.ui.util.ResStringProvider
import com.github.damontecres.wholphin.ui.util.scaledDensity
import com.github.damontecres.wholphin.ui.util.withinBoundsOrDefault import com.github.damontecres.wholphin.ui.util.withinBoundsOrDefault
import com.github.damontecres.wholphin.util.HomeRowLoadingState import com.github.damontecres.wholphin.util.HomeRowLoadingState
import dagger.hilt.android.lifecycle.HiltViewModel import dagger.hilt.android.lifecycle.HiltViewModel
@ -184,13 +185,7 @@ fun DisplaySizePage(
) )
} }
val previewDensity = val previewDensity =
remember(systemDensity, uiScale) { remember(systemDensity, uiScale) { scaledDensity(systemDensity, uiScale) }
val factor = uiScale / 100f
Density(
density = systemDensity.density * factor,
fontScale = systemDensity.fontScale,
)
}
CompositionLocalProvider(LocalDensity provides pageDensity) { CompositionLocalProvider(LocalDensity provides pageDensity) {
Row( Row(

View file

@ -1,6 +1,7 @@
package com.github.damontecres.wholphin.ui.util package com.github.damontecres.wholphin.ui.util
import androidx.compose.runtime.Composable import androidx.compose.runtime.Composable
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Dp 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
@ -11,9 +12,9 @@ import com.github.damontecres.wholphin.ui.Cards
@Composable @Composable
internal fun HomeRowViewOptions.resolvedCardHeight(): Dp { internal fun HomeRowViewOptions.resolvedCardHeight(): Dp {
val custom = LocalInterfaceCustomization.current val custom = LocalInterfaceCustomization.current
val isWide = aspectRatio == AspectRatio.WIDE val naturalBase = naturalCardBaseDp(aspectRatio)
val naturalBase = if (isWide) Cards.HEIGHT_EPISODE else Cards.HEIGHT_2X3_DP val globalForThisRow =
val globalForThisRow = if (isWide) custom.episodeCardHeightDp else custom.cardHeightDp if (naturalBase == Cards.HEIGHT_EPISODE) custom.episodeCardHeightDp else custom.cardHeightDp
return resolveCardHeight( return resolveCardHeight(
heightDp = heightDp, heightDp = heightDp,
cardSizeMultiplier = cardSizeMultiplier, cardSizeMultiplier = cardSizeMultiplier,
@ -22,6 +23,14 @@ internal fun HomeRowViewOptions.resolvedCardHeight(): Dp {
).dp ).dp
} }
// SQUARE / FOUR_THREE rows piggyback on the poster (TALL) global; they either store an
// explicit override (Music preset rows at 100dp, etc.) or compose well enough at poster size.
internal fun naturalCardBaseDp(aspectRatio: AspectRatio): Int =
when (aspectRatio) {
AspectRatio.WIDE -> Cards.HEIGHT_EPISODE
AspectRatio.TALL, AspectRatio.SQUARE, AspectRatio.FOUR_THREE -> Cards.HEIGHT_2X3_DP
}
internal fun resolveCardHeight( internal fun resolveCardHeight(
heightDp: Int, heightDp: Int,
cardSizeMultiplier: Int, cardSizeMultiplier: Int,
@ -33,6 +42,20 @@ internal fun resolveCardHeight(
return base * cardSizeMultiplier / 100 return base * cardSizeMultiplier / 100
} }
// Scales density (and therefore both dp layouts and sp text) linearly by uiScalePercent.
// fontScale is left at the user's system value so Android's accessibility setting still
// applies on top - scaling both would render text at factor^2 instead of factor.
internal fun scaledDensity(
base: Density,
uiScalePercent: Int,
): Density {
val factor = uiScalePercent / 100f
return Density(
density = base.density * factor,
fontScale = base.fontScale,
)
}
internal fun Int.withinBoundsOrDefault(preference: AppSliderPreference<*>): Int { internal fun Int.withinBoundsOrDefault(preference: AppSliderPreference<*>): Int {
val min = preference.min.toInt() val min = preference.min.toInt()
val max = preference.max.toInt() val max = preference.max.toInt()

View file

@ -1,10 +1,12 @@
package com.github.damontecres.wholphin.ui.util package com.github.damontecres.wholphin.ui.util
import androidx.compose.ui.unit.Density
import com.github.damontecres.wholphin.data.model.HomeRowConfig import com.github.damontecres.wholphin.data.model.HomeRowConfig
import com.github.damontecres.wholphin.data.model.HomeRowViewOptions import com.github.damontecres.wholphin.data.model.HomeRowViewOptions
import com.github.damontecres.wholphin.preferences.AppPreference import com.github.damontecres.wholphin.preferences.AppPreference
import com.github.damontecres.wholphin.preferences.AppPreferences import com.github.damontecres.wholphin.preferences.AppPreferences
import com.github.damontecres.wholphin.services.migrateLegacyRowSpacing import com.github.damontecres.wholphin.services.migrateLegacyRowSpacing
import com.github.damontecres.wholphin.ui.AspectRatio
import com.github.damontecres.wholphin.ui.Cards 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
@ -96,23 +98,47 @@ class InterfaceHelpersTest {
} }
@Test @Test
fun `UI scale must not double-apply on text`() { fun `naturalCardBaseDp maps WIDE to episode base`() {
// sp rendering = sp.value * density * fontScale. MainContent's scaled Density assertEquals(Cards.HEIGHT_EPISODE, naturalCardBaseDp(AspectRatio.WIDE))
// 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. @Test
val baseDensity = 2.0f fun `naturalCardBaseDp maps TALL, SQUARE, FOUR_THREE to poster base`() {
val baseFontScale = 1.0f // SQUARE and FOUR_THREE piggyback on the poster global. Music preset rows (SQUARE)
val factor = 1.5f // store their own absolute heightDp = 100 so they still hit the override branch -
// The pattern MainContent.kt uses after the fix: // the natural base just supplies the fallback if a row ever stores no override.
val scaledDensity = baseDensity * factor assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.TALL))
val scaledFontScale = baseFontScale assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.SQUARE))
val baseTextPx = 14f * baseDensity * baseFontScale assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.FOUR_THREE))
val scaledTextPx = 14f * scaledDensity * scaledFontScale }
assertEquals(1.5f * baseTextPx, scaledTextPx, 0.001f)
// Double-application (the bug) would have produced 2.25x. @Test
val buggyTextPx = 14f * (baseDensity * factor) * (baseFontScale * factor) fun `scaledDensity scales density linearly and leaves fontScale alone`() {
assertEquals(2.25f * baseTextPx, buggyTextPx, 0.001f) val base = Density(density = 2.0f, fontScale = 1.0f)
val scaled = scaledDensity(base, 150)
// density grows by 1.5 - so dp layouts scale 1.5x.
assertEquals(3.0f, scaled.density, 0.001f)
// fontScale stays put - sp text is (density * fontScale) so it also scales linearly.
// Scaling fontScale too would render text at 1.5^2 = 2.25x.
assertEquals(1.0f, scaled.fontScale, 0.001f)
}
@Test
fun `scaledDensity at 100 percent returns identity`() {
val base = Density(density = 2.0f, fontScale = 1.2f)
val scaled = scaledDensity(base, 100)
assertEquals(base.density, scaled.density, 0.001f)
assertEquals(base.fontScale, scaled.fontScale, 0.001f)
}
@Test
fun `scaledDensity preserves user accessibility fontScale across UI scale changes`() {
// A user with system fontScale 1.3x (accessibility setting) bumping UI scale to 75%
// should keep fontScale = 1.3 - the UI scale slider is independent of accessibility.
val base = Density(density = 2.0f, fontScale = 1.3f)
val scaled = scaledDensity(base, 75)
assertEquals(1.5f, scaled.density, 0.001f)
assertEquals(1.3f, scaled.fontScale, 0.001f)
} }
@Test @Test