Skip to content

Add per-font metric corrections for OpenDyslexic#100

Open
RobinJ1995 wants to merge 2 commits into
masterfrom
claude/opendyslexic-font-spacing-ib5wda
Open

Add per-font metric corrections for OpenDyslexic#100
RobinJ1995 wants to merge 2 commits into
masterfrom
claude/opendyslexic-font-spacing-ib5wda

Conversation

@RobinJ1995

Copy link
Copy Markdown
Owner

Summary

This PR introduces a new FontStyle class that bundles a typeface with per-font metric corrections (letter spacing and line spacing multiplier). This allows fonts like OpenDyslexic, which have oversized glyphs and tall line metrics by design, to be automatically adjusted so text fits properly in UI containers.

Key Changes

  • New FontStyle class: Encapsulates a typeface along with optional letter spacing and line spacing multiplier adjustments. Provides an applyTo(TextView) method to apply both the typeface and spacing corrections.
  • OpenDyslexic spacing corrections: Added constants OPENDYSLEXIC_LETTER_SPACING (-0.05em) and OPENDYSLEXIC_LINE_SPACING_MULTIPLIER (0.8) to tighten OpenDyslexic's oversized metrics while keeping other fonts at neutral defaults.
  • New fontStyle() method: Replaces direct typeface access in FontPreference, returning a FontStyle that includes the appropriate spacing corrections based on the selected font.
  • Updated FontInflaterFactory: Now accepts and applies a FontStyle instead of a raw Typeface, ensuring spacing corrections are applied to all inflated TextViews.
  • Refactored dialog font application: Renamed applyTypeface() to applyFontStyle() and updated it to use the new FontStyle.applyTo() method.
  • Added test coverage: New tests verify that OpenDyslexic gets negative letter spacing and sub-1 line spacing multiplier, while other fonts maintain neutral (identity) spacing.

Implementation Details

  • The spacing corrections are only applied to OpenDyslexic; all other fonts (including system font) use neutral defaults (0 letter spacing, 1.0 line spacing multiplier), making them no-ops.
  • The FontStyle class keeps the existing behavior of preserving each TextView's bold/italic style while swapping the font family.
  • All font application now goes through FontStyle.applyTo(), ensuring spacing corrections always travel with the typeface.

https://claude.ai/code/session_01GosWNcfE3dfMAmHCn4WNoy

OpenDyslexic ships very wide glyph advances and tall line metrics "by
design", which in a UI whose containers are sized for normal fonts means
text no longer fits. Claw the spacing back to something usable with a
negative letter spacing (-0.05em) and a sub-1 line-spacing multiplier
(0.8), applied only when OpenDyslexic is selected.

Introduce a FontStyle holder (typeface + letterSpacing +
lineSpacingMultiplier) that carries these corrections with the typeface
and applies them wherever the font is forced onto TextViews - both the
inflater factory and the dialog decor-view sweep. Other fonts get
neutral defaults, so they are unchanged.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GosWNcfE3dfMAmHCn4WNoy

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a0595c9b62

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +143 to +144
view.letterSpacing = this.letterSpacing
view.setLineSpacing(0f, this.lineSpacingMultiplier)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve existing TextView letter spacing for neutral fonts

For Ubuntu and Oxygen, FontStyle(typeface) still writes 0f here, so the supposedly neutral style is not an identity transform: it removes explicit tracking such as ModernDialogButton's android:letterSpacing="0.01" (app/src/main/res/values/styles.xml:74) and the widget picker header's 0.04 (app/src/main/res/layout/widget_picker_header.xml:26). This affects both normally inflated views and the dialog decor sweep; retain each view's existing spacing when no correction is needed (and apply OpenDyslexic's correction relative to it if appropriate).

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch — fixed in 930d4b4. FontStyle now applies corrections relative to each view's existing spacing: the neutral defaults (delta 0, factor 1) are an exact identity and are skipped entirely, so Ubuntu/Oxygen/System leave a view's explicit letterSpacing (e.g. ModernDialogButton's 0.01, the widget picker header's 0.04) untouched. OpenDyslexic now adds a -0.05em delta and multiplies the line-spacing multiplier by 0.8, tightening relative to that tracking rather than discarding it.


Generated by Claude Code

The previous FontStyle wrote absolute letterSpacing/lineSpacing onto every
TextView, which for neutral fonts (Ubuntu/Oxygen/System) wiped explicit
tracking set in styles and layouts (e.g. ModernDialogButton's 0.01, the
widget picker header's 0.04) instead of leaving it alone.

Make the correction relative: letterSpacingDelta is added to the view's
existing letter spacing and lineSpacingFactor multiplies its existing line
multiplier. The neutral defaults (delta 0, factor 1) are now an exact
identity and are skipped entirely, so non-OpenDyslexic fonts keep each
view's spacing untouched; OpenDyslexic tightens relative to it, preserving
intentional tracking differences.

Co-Authored-By: Claude Opus 4.8 <[email protected]>
Claude-Session: https://claude.ai/code/session_01GosWNcfE3dfMAmHCn4WNoy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants