From bafaee7fe404151d0c6a72a9544f79437005a65b Mon Sep 17 00:00:00 2001 From: Justin Visser Date: Tue, 26 May 2026 15:11:30 +0200 Subject: [PATCH] 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. --- .../damontecres/wholphin/MainContent.kt | 12 ++-- .../displaysize/DisplaySizePage.kt | 9 +-- .../wholphin/ui/util/InterfaceHelpers.kt | 29 ++++++++- .../wholphin/ui/util/InterfaceHelpersTest.kt | 60 +++++++++++++------ 4 files changed, 75 insertions(+), 35 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 f58bb5ac..03f275f0 100644 --- a/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt +++ b/app/src/main/java/com/github/damontecres/wholphin/MainContent.kt @@ -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, 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 85cf3ace..7ca99533 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 @@ -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( 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 078a0f14..897f4d56 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 @@ -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() 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 f98df7d4..9de35728 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 @@ -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