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 b080238837
commit bafaee7fe4
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.graphics.RectangleShape
import androidx.compose.ui.platform.LocalDensity
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.dp
import androidx.lifecycle.Lifecycle
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.util.InterfaceCustomization
import com.github.damontecres.wholphin.ui.util.LocalInterfaceCustomization
import com.github.damontecres.wholphin.ui.util.scaledDensity
@Composable
fun MainContent(
@ -63,17 +63,13 @@ fun MainContent(
val interfaceCustomization =
remember(appPreferences) { InterfaceCustomization(appPreferences) }
val baseDensity = LocalDensity.current
val scaledDensity =
val uiScaleDensity =
remember(baseDensity, interfaceCustomization.uiScalePercent) {
val factor = interfaceCustomization.uiScalePercent / 100f
Density(
density = baseDensity.density * factor,
fontScale = baseDensity.fontScale,
)
scaledDensity(baseDensity, interfaceCustomization.uiScalePercent)
}
CompositionLocalProvider(
LocalInterfaceCustomization provides interfaceCustomization,
LocalDensity provides scaledDensity,
LocalDensity provides uiScaleDensity,
) {
NavDisplay(
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.tryRequestFocus
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.util.HomeRowLoadingState
import dagger.hilt.android.lifecycle.HiltViewModel
@ -184,13 +185,7 @@ fun DisplaySizePage(
)
}
val previewDensity =
remember(systemDensity, uiScale) {
val factor = uiScale / 100f
Density(
density = systemDensity.density * factor,
fontScale = systemDensity.fontScale,
)
}
remember(systemDensity, uiScale) { scaledDensity(systemDensity, uiScale) }
CompositionLocalProvider(LocalDensity provides pageDensity) {
Row(

View file

@ -1,6 +1,7 @@
package com.github.damontecres.wholphin.ui.util
import androidx.compose.runtime.Composable
import androidx.compose.ui.unit.Density
import androidx.compose.ui.unit.Dp
import androidx.compose.ui.unit.dp
import com.github.damontecres.wholphin.data.model.HomeRowViewOptions
@ -11,9 +12,9 @@ import com.github.damontecres.wholphin.ui.Cards
@Composable
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
val naturalBase = naturalCardBaseDp(aspectRatio)
val globalForThisRow =
if (naturalBase == Cards.HEIGHT_EPISODE) custom.episodeCardHeightDp else custom.cardHeightDp
return resolveCardHeight(
heightDp = heightDp,
cardSizeMultiplier = cardSizeMultiplier,
@ -22,6 +23,14 @@ internal fun HomeRowViewOptions.resolvedCardHeight(): 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(
heightDp: Int,
cardSizeMultiplier: Int,
@ -33,6 +42,20 @@ internal fun resolveCardHeight(
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 {
val min = preference.min.toInt()
val max = preference.max.toInt()

View file

@ -1,10 +1,12 @@
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.HomeRowViewOptions
import com.github.damontecres.wholphin.preferences.AppPreference
import com.github.damontecres.wholphin.preferences.AppPreferences
import com.github.damontecres.wholphin.services.migrateLegacyRowSpacing
import com.github.damontecres.wholphin.ui.AspectRatio
import com.github.damontecres.wholphin.ui.Cards
import org.jellyfin.sdk.model.UUID
import org.junit.Assert.assertEquals
@ -96,23 +98,47 @@ class InterfaceHelpersTest {
}
@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)
fun `naturalCardBaseDp maps WIDE to episode base`() {
assertEquals(Cards.HEIGHT_EPISODE, naturalCardBaseDp(AspectRatio.WIDE))
}
@Test
fun `naturalCardBaseDp maps TALL, SQUARE, FOUR_THREE to poster base`() {
// SQUARE and FOUR_THREE piggyback on the poster global. Music preset rows (SQUARE)
// store their own absolute heightDp = 100 so they still hit the override branch -
// the natural base just supplies the fallback if a row ever stores no override.
assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.TALL))
assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.SQUARE))
assertEquals(Cards.HEIGHT_2X3_DP, naturalCardBaseDp(AspectRatio.FOUR_THREE))
}
@Test
fun `scaledDensity scales density linearly and leaves fontScale alone`() {
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