diff --git a/app/src/main/java/be/robinj/distrohopper/preferences/FontInflaterFactory.kt b/app/src/main/java/be/robinj/distrohopper/preferences/FontInflaterFactory.kt index 434e20e8..5324e17a 100644 --- a/app/src/main/java/be/robinj/distrohopper/preferences/FontInflaterFactory.kt +++ b/app/src/main/java/be/robinj/distrohopper/preferences/FontInflaterFactory.kt @@ -1,7 +1,6 @@ package be.robinj.distrohopper.preferences import android.content.Context -import android.graphics.Typeface import android.util.AttributeSet import android.view.LayoutInflater import android.view.View @@ -9,8 +8,9 @@ import android.widget.TextView import androidx.appcompat.app.AppCompatDelegate /** - * A [LayoutInflater.Factory2] that forces a [Typeface] onto every [TextView] as - * it is inflated, so the user's chosen font reaches the whole app. + * A [LayoutInflater.Factory2] that forces a [FontStyle] onto every [TextView] as + * it is inflated, so the user's chosen font (and its spacing corrections) reach + * the whole app. * * Setting a font at the theme level does not cascade to TextViews (the * framework's default text appearances pin fontFamily to sans-serif, which @@ -26,7 +26,7 @@ import androidx.appcompat.app.AppCompatDelegate */ class FontInflaterFactory( private val delegate: AppCompatDelegate, - private val typeface: Typeface, + private val fontStyle: FontStyle, ) : LayoutInflater.Factory2 { override fun onCreateView( @@ -37,9 +37,7 @@ class FontInflaterFactory( ): View? { val view = this.delegate.createView(parent, name, context, attrs) if (view is TextView) { - // Keep the view's own style (bold/italic) while swapping the family. - val style = view.typeface?.style ?: Typeface.NORMAL - view.setTypeface(this.typeface, style) + this.fontStyle.applyTo(view) } return view diff --git a/app/src/main/java/be/robinj/distrohopper/preferences/FontPreference.kt b/app/src/main/java/be/robinj/distrohopper/preferences/FontPreference.kt index 061fcc9d..2ee95f03 100644 --- a/app/src/main/java/be/robinj/distrohopper/preferences/FontPreference.kt +++ b/app/src/main/java/be/robinj/distrohopper/preferences/FontPreference.kt @@ -27,6 +27,19 @@ object FontPreference { const val SYSTEM = "system" + /** + * OpenDyslexic ships very wide glyph advances and tall line metrics "by + * design". In a UI whose containers are sized for normal fonts that means + * text no longer fits, so we claw the spacing back to something usable with a + * negative [FontStyle.letterSpacingDelta] (em units) and a sub-1 + * [FontStyle.lineSpacingFactor]. Both are applied *relative* to each view's + * own spacing (see [FontStyle.applyTo]), so intentional tracking set in + * layouts/styles is tightened, not discarded. These only touch OpenDyslexic; + * every other font keeps the neutral defaults below, which are exact no-ops. + */ + private const val OPENDYSLEXIC_LETTER_SPACING_DELTA = -0.05f + private const val OPENDYSLEXIC_LINE_SPACING_FACTOR = 0.8f + /** Bundled font resource for [value], or null for System / unknown values. */ @FontRes fun fontResFor(value: String?): Int? = when (value) { @@ -42,6 +55,26 @@ object FontPreference { return ResourcesCompat.getFont(context, fontRes) } + /** + * The selected font together with any per-font metric tweaks, or null when + * the system font should be used. This is what actually gets forced onto + * TextViews (see [FontStyle.applyTo]); prefer it over [typeface] so the + * spacing corrections travel with the typeface. + */ + fun fontStyle(context: Context): FontStyle? { + val value = this.current(context) + val typeface = this.typeface(context) ?: return null + return if (value == "opendyslexic") { + FontStyle( + typeface, + OPENDYSLEXIC_LETTER_SPACING_DELTA, + OPENDYSLEXIC_LINE_SPACING_FACTOR, + ) + } else { + FontStyle(typeface) + } + } + private fun current(context: Context): String = Preferences.getSharedPreferences(context) .getString(Preference.FONT.getName(), SYSTEM) ?: SYSTEM @@ -54,7 +87,7 @@ object FontPreference { */ fun applyTo(activity: Activity) { if (activity !is AppCompatActivity) return - val typeface = this.typeface(activity) ?: return + val fontStyle = this.fontStyle(activity) ?: return // We run before AppCompat installs its own factory, so the inflater // should be untouched; guard like AppCompat does to never clobber an @@ -64,7 +97,7 @@ object FontPreference { LayoutInflaterCompat.setFactory2( inflater, - FontInflaterFactory(activity.delegate, typeface), + FontInflaterFactory(activity.delegate, fontStyle), ) } @@ -77,19 +110,79 @@ object FontPreference { * Attach via [Dialog.setOnShowListener] so the views exist when this runs. */ fun applyTo(dialog: Dialog) { - val typeface = this.typeface(dialog.context) ?: return + val fontStyle = this.fontStyle(dialog.context) ?: return val root = dialog.window?.decorView ?: return - this.applyTypeface(root, typeface) + this.applyFontStyle(root, fontStyle) } - private fun applyTypeface(view: View, typeface: Typeface) { + private fun applyFontStyle(view: View, fontStyle: FontStyle) { when (view) { is ViewGroup -> for (i in 0 until view.childCount) { - this.applyTypeface(view.getChildAt(i), typeface) + this.applyFontStyle(view.getChildAt(i), fontStyle) } - // Keep each view's own style (bold/italic) while swapping the family. - is TextView -> view.setTypeface(typeface, view.typeface?.style ?: Typeface.NORMAL) + is TextView -> fontStyle.applyTo(view) } } } + +/** + * A chosen [typeface] plus the per-font metric corrections that must travel with + * it, expressed *relative* to each view's designed spacing: + * [letterSpacingDelta] (em units, added — negative to tighten) and + * [lineSpacingFactor] (multiplies the line-spacing multiplier — < 1 to pull + * lines closer). Applying relatively means a view's intentional tracking (e.g. a + * style's `letterSpacing="0.04"`) is preserved and shifted, not wiped. + * + * "Designed spacing" is the view's spacing the first time it is seen, captured + * and remembered in a tag. Corrections are always recomputed from that baseline, + * never from the current value, so [applyTo] is idempotent: a view reached by + * both the [FontInflaterFactory] and the dialog decor sweep — or swept again on + * every [Dialog] show — lands on the same spacing instead of compounding. + * + * The defaults (delta 0, factor 1) are an exact identity: for fonts that need no + * correction, [applyTo] touches only the typeface and leaves spacing untouched. + */ +class FontStyle( + val typeface: Typeface, + val letterSpacingDelta: Float = 0f, + val lineSpacingFactor: Float = 1f, +) { + + /** True when this style leaves text spacing exactly as the view declared it. */ + private val spacingIsNeutral: Boolean + get() = this.letterSpacingDelta == 0f && this.lineSpacingFactor == 1f + + /** Forces this font (and its spacing) onto [view], keeping its bold/italic. */ + fun applyTo(view: TextView) { + // Keep each view's own style (bold/italic) while swapping the family. + view.setTypeface(this.typeface, view.typeface?.style ?: Typeface.NORMAL) + + // Never touch spacing when there is no correction to make, so fonts that + // need none leave every view exactly as its layout/style declared it. + if (this.spacingIsNeutral) return + + val baseline = this.baselineOf(view) + view.letterSpacing = baseline.letterSpacing + this.letterSpacingDelta + view.setLineSpacing( + baseline.lineSpacingExtra, + baseline.lineSpacingMultiplier * this.lineSpacingFactor, + ) + } + + /** The view's designed spacing, captured on first sight and reused after. */ + private fun baselineOf(view: TextView): SpacingBaseline = + view.getTag(R.id.font_spacing_baseline) as? SpacingBaseline + ?: SpacingBaseline( + view.letterSpacing, + view.lineSpacingExtra, + view.lineSpacingMultiplier, + ).also { view.setTag(R.id.font_spacing_baseline, it) } + + /** A view's text spacing as its layout/style declared it, before correction. */ + private class SpacingBaseline( + val letterSpacing: Float, + val lineSpacingExtra: Float, + val lineSpacingMultiplier: Float, + ) +} diff --git a/app/src/main/res/values/ids.xml b/app/src/main/res/values/ids.xml new file mode 100644 index 00000000..b37f4c2d --- /dev/null +++ b/app/src/main/res/values/ids.xml @@ -0,0 +1,10 @@ + + + + + + + diff --git a/app/src/test/java/be/robinj/distrohopper/FontPreferenceTest.kt b/app/src/test/java/be/robinj/distrohopper/FontPreferenceTest.kt index 77a73c27..a043f47b 100644 --- a/app/src/test/java/be/robinj/distrohopper/FontPreferenceTest.kt +++ b/app/src/test/java/be/robinj/distrohopper/FontPreferenceTest.kt @@ -4,12 +4,15 @@ import android.app.Dialog import android.content.Context import android.graphics.Typeface import android.os.Bundle +import android.view.LayoutInflater import android.widget.LinearLayout import android.widget.TextView import androidx.appcompat.app.AppCompatActivity +import androidx.core.view.LayoutInflaterCompat import androidx.test.core.app.ApplicationProvider import be.robinj.distrohopper.preferences.FontInflaterFactory import be.robinj.distrohopper.preferences.FontPreference +import be.robinj.distrohopper.preferences.FontStyle import be.robinj.distrohopper.preferences.Preference import be.robinj.distrohopper.preferences.Preferences import org.junit.Assert.assertEquals @@ -60,7 +63,7 @@ class FontPreferenceTest { this.setFont("system") val activity = Robolectric.buildActivity(FontTestActivity::class.java).create().get() - val factory = FontInflaterFactory(activity.delegate, Typeface.MONOSPACE) + val factory = FontInflaterFactory(activity.delegate, FontStyle(Typeface.MONOSPACE)) val attrs = Robolectric.buildAttributeSet().build() val view = factory.onCreateView(null, "TextView", activity, attrs) @@ -68,6 +71,121 @@ class FontPreferenceTest { assertEquals(Typeface.MONOSPACE, (view as TextView).typeface) } + /** OpenDyslexic's baked-in spacing is clawed back: a negative letter-spacing + * delta and a sub-1 line-spacing factor so text fits its containers. */ + @Test fun openDyslexicTightensSpacing() { + this.setFont("opendyslexic") + val style = FontPreference.fontStyle(this.context) + + assertNotNull(style) + assertTrue("letter spacing delta should be negative", style!!.letterSpacingDelta < 0f) + assertTrue("line spacing factor should be < 1", style.lineSpacingFactor < 1f) + + val tv = TextView(this.context).apply { + this.letterSpacing = 0.04f // explicit tracking, e.g. from a style + this.setLineSpacing(0f, 1f) + } + style.applyTo(tv) + // Correction is relative: existing 0.04 tracking is tightened, not wiped. + assertEquals(0.04f + style.letterSpacingDelta, tv.letterSpacing, 1e-6f) + assertEquals(style.lineSpacingFactor, tv.lineSpacingMultiplier, 1e-6f) + } + + /** Other bundled fonts get an identity transform: existing letter spacing set + * by a style/layout is preserved, not clobbered to 0. */ + @Test fun otherFontsPreserveExistingSpacing() { + this.setFont("ubuntu") + val style = FontPreference.fontStyle(this.context) + + assertNotNull(style) + assertEquals(0f, style!!.letterSpacingDelta, 0f) + assertEquals(1f, style.lineSpacingFactor, 0f) + + val tv = TextView(this.context).apply { + this.letterSpacing = 0.04f + this.setLineSpacing(8f, 1.1f) + } + style.applyTo(tv) + assertEquals("neutral font must not touch letter spacing", 0.04f, tv.letterSpacing, 0f) + assertEquals("neutral font must not touch line spacing", 8f, tv.lineSpacingExtra, 0f) + assertEquals("neutral font must not touch line spacing", 1.1f, tv.lineSpacingMultiplier, 0f) + } + + /** A view can be reached by both the inflater factory and the dialog decor + * sweep (and swept again on every show), so corrections must be computed + * from the view's designed spacing and never compound. */ + @Test fun spacingCorrectionIsIdempotent() { + this.setFont("opendyslexic") + val style = FontPreference.fontStyle(this.context)!! + + val tv = TextView(this.context).apply { + this.letterSpacing = 0.04f + this.setLineSpacing(0f, 1f) + } + + style.applyTo(tv) + val letterSpacingAfterOnce = tv.letterSpacing + val lineSpacingAfterOnce = tv.lineSpacingMultiplier + + style.applyTo(tv) + style.applyTo(tv) + + assertEquals(letterSpacingAfterOnce, tv.letterSpacing, 1e-6f) + assertEquals(lineSpacingAfterOnce, tv.lineSpacingMultiplier, 1e-6f) + // Explicitly: the third application must not have drifted to 0.04 - 0.15. + assertEquals(0.04f + style.letterSpacingDelta, tv.letterSpacing, 1e-6f) + } + + /** The real compounding path: WidgetPickerDialog's adapter inflates rows with + * an inflater that carries the font factory, and those same rows are then + * caught by the dialog decor sweep. They must be corrected exactly once. */ + @Test fun factoryThenDialogSweepDoesNotCompound() { + this.setFont("opendyslexic") + val style = FontPreference.fontStyle(this.context)!! + val activity = Robolectric.buildActivity(FontTestActivity::class.java).create().get() + + val inflater = LayoutInflater.from(activity).cloneInContext(activity) + LayoutInflaterCompat.setFactory2(inflater, FontInflaterFactory(activity.delegate, style)) + val row = inflater.inflate(R.layout.widget_picker_header, null) + val tvName = row.findViewById(R.id.tvName) + + // The layout declares letterSpacing="0.04"; tighten it exactly once. + val expected = 0.04f + style.letterSpacingDelta + assertEquals(expected, tvName.letterSpacing, 1e-6f) + + // Now the dialog decor sweep reaches the very same row. + val dialog = Dialog(activity) + dialog.setContentView(row) + FontPreference.applyTo(dialog) + + assertEquals(expected, tvName.letterSpacing, 1e-6f) + } + + /** Line spacing is scaled from the view's own multiplier, and its extra + * leading (set in dp by several layouts) is carried through untouched. */ + @Test fun openDyslexicScalesLineSpacingFromTheViewsOwnValues() { + this.setFont("opendyslexic") + val style = FontPreference.fontStyle(this.context)!! + + val tv = TextView(this.context).apply { this.setLineSpacing(4f, 1.1f) } + style.applyTo(tv) + + assertEquals(1.1f * style.lineSpacingFactor, tv.lineSpacingMultiplier, 1e-6f) + assertEquals("extra leading must be preserved", 4f, tv.lineSpacingExtra, 1e-6f) + } + + /** Swapping the family must keep each view's own bold/italic styling. Uses a + * built-in family so this pins our style pass-through rather than how + * faithfully the test runtime derives weights from a bundled OTF. */ + @Test fun applyToPreservesBoldStyle() { + val style = FontStyle(Typeface.MONOSPACE, letterSpacingDelta = -0.05f) + + val tv = TextView(this.context).apply { this.setTypeface(null, Typeface.BOLD) } + style.applyTo(tv) + + assertEquals("bold must survive the family swap", Typeface.BOLD, tv.typeface.style) + } + /** applyTo is a harmless no-op when the system font is selected. */ @Test fun applyToIsNoOpForSystem() { this.setFont("system")