Skip to content

fix(text): raise a clear error instead of IndexError on glyph/char mismatch#4868

Open
SupernovaIa wants to merge 2 commits into
ManimCommunity:mainfrom
SupernovaIa:fix/text-gen-chars-ligature-mismatch
Open

fix(text): raise a clear error instead of IndexError on glyph/char mismatch#4868
SupernovaIa wants to merge 2 commits into
ManimCommunity:mainfrom
SupernovaIa:fix/text-gen-chars-ligature-mismatch

Conversation

@SupernovaIa

@SupernovaIa SupernovaIa commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Text._gen_chars() assumes disable_ligatures=True guarantees exactly one rendered glyph per non-space character. That assumption breaks for fonts that implement some of their ligatures through OpenType features other than liga/dlig/clig/hlig (the ones ManimPango's disable_liga disables) — for example, Fira Code implements its programming ligatures (<=, ->, etc.) exclusively through calt (contextual alternates), which isn't covered. I confirmed this by inspecting Fira Code's own GSUB table with fontTools: it defines calt but none of liga/dlig/clig/hlig.

When that happens, Pango still merges e.g. < and = into a single glyph, and _gen_chars() walks off the end of self.submobjects, raising an opaque IndexError: list index out of range with no indication of what went wrong. This is reproducible today via, for instance:

Code(
    code_string="print(1<=2)",
    language="python",
    paragraph_config={"font": "Fira Code"},
)

This PR does not fix the underlying issue — the actual root cause is in a separate repository, ManimCommunity/ManimPango (its disable_liga feature string would need calt=0 added). That's outside the scope of this PR (would require setting up the Cython/C build for a separate package).

What this PR does do: turn the opaque crash into a clear, actionable error, so that anyone hitting this (there have been a few confused reports in #3237 over time) immediately understands why, instead of debugging a bare IndexError. This is a pure improvement to the failure path — no behavior changes for the cases that already work.

Implementation note

The check is a bounds guard inside the glyph-consuming loop, raised only when there are genuinely fewer glyphs than non-space characters. An earlier version of this PR used a pre-loop count that compared non-space characters against the total number of submobjects; that was wrong, because under disable_ligatures=True a space is rendered as its own glyph, so the two counts legitimately differ for any text containing spaces. It produced a false positive on ordinary spaced text — including the DisableLigature docs example (Text("fl ligature", disable_ligatures=True)), which broke the docs build. The in-loop bounds check fires only on the real glyph shortfall.

Refs #3237

Test plan

  • test_gen_chars_raises_clear_error_on_glyph_mismatch — simulates the glyph shortfall directly (no dependency on any specific font being installed) and asserts the new ValueError is raised
  • test_gen_chars_handles_spaces_with_disable_ligatures — renders Text("fl ligature", disable_ligatures=True) (the case that broke the docs build) and asserts it does not raise
  • uv run pytest tests/module/mobject/text/test_text_mobject.py tests/test_code_mobject.py — all pass
  • Manually reproduced the original crash with Fira Code installed locally, confirmed the new error message is accurate and points at the real cause
  • Rebuilt the DisableLigature docs example locally, confirmed it renders without error
  • uv run pre-commit run --files manim/mobject/text/text_mobject.py tests/module/mobject/text/test_text_mobject.py — ruff, mypy, codespell all pass

…smatch

Text._gen_chars() assumed disable_ligatures=True guarantees one glyph per
non-space character, but that only holds if the font's ligatures are all
implemented via the 'liga'/'dlig'/'clig'/'hlig' OpenType features that
ManimPango disables. Fonts like Fira Code implement programming ligatures
(e.g. '<=', '->') via 'calt' instead, which isn't covered, so Pango still
merges characters into a single glyph and _gen_chars() walks off the end
of self.submobjects with an opaque IndexError (e.g. via Code(...,
paragraph_config={"font": "Fira Code"}), see issue ManimCommunity#3237).

This adds an upfront check that raises a clear ValueError explaining the
likely cause and a workaround, instead of the bare IndexError. Note this
does not fix the underlying issue -- the actual root cause (ManimPango
not disabling 'calt') lives in a separate repo (ManimPango) and would
need to be addressed there for Code/Text to render correctly with fonts
like Fira Code.

Refs ManimCommunity#3237
The previous pre-loop count compared non-space chars against total
submobjects, but spaces render as their own glyph under
disable_ligatures=True, so the counts legitimately differ. This caused
a false-positive ValueError on ordinary spaced text (e.g. the
DisableLigature docs example, breaking the ReadTheDocs build). Move the
check inside the loop as a bounds guard so it only fires on the real
glyph shortfall.
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.

1 participant