Skip to content

Phase 2-C: Text mobjects via CoreText outlines - #13

Open
ronaldmannak wants to merge 4 commits into
mainfrom
claude/picomanim-phase2-03-text
Open

Phase 2-C: Text mobjects via CoreText outlines#13
ronaldmannak wants to merge 4 commits into
mainfrom
claude/picomanim-phase2-03-text

Conversation

@ronaldmannak

@ronaldmannak ronaldmannak commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Phase 2-C: Text mobjects via CoreText outlines

Scope

Mobject.text("Hello") — real vector text that morphs, fades, and transforms like any other mobject, by extracting glyph outlines with CoreText and converting them to the existing cubic-Bézier representation. create on stroke-less filled mobjects (like text) now draws a temporary borrowed-fill-color outline (Manim's Write behavior) so the draw-in phase is visible.

Public API

#if canImport(CoreText)
extension Mobject {
    /// A filled, strokeless text mobject centered at `position`.
    /// fontSize is Manim-style: 48 (the default) spans one scene unit
    /// per em, so default text is a bit under one unit tall.
    public static func text(
        _ string: String,
        fontName: String? = nil,     // nil → system UI font
        fontSize: Double = 48,       // Manim-style; 48 → 1 unit per em
        at position: Vec2 = .zero,
        color: ManimColor = .white
    ) -> Mobject
}

extension BezierPath {
    public static func text(_ string: String, fontName: String?, fontSize: Double) -> BezierPath
}
#endif

Design notes

  • Platform gating: the whole file is #if canImport(CoreText), so the package still builds on Linux CI — text is simply unavailable there. Tests are gated the same way (they run on the macOS job, no-op on Linux).
  • Pipeline: CTLine → runs → glyphs + positions → CTFontCreatePathForGlyphCGPath.applyWithBlock → subpaths. Quadratic segments are elevated to cubics, close-path elements emit explicit closing edges so the morph system sees closed subpaths, and the element switch has an @unknown default.
  • Sizing (Manim parity): glyphs are laid out at 48 pt and scaled so fontSize 48 = one scene unit per em (unitsPerPoint = (fontSize / pointSize) / pointSize); fontSize scales linearly from there.
  • Centering: the mobject recenters on its visual bounding box, so at: places the text's optical center, consistent with shapes.
  • Fill-only by default (stroke width 0) — matching how Manim renders text. The create animation borrows the fill color as a temporary outline during draw-in and fades it out as the fill arrives, ending exactly at the authored state.
  • The run-font lookup type-checks via CFGetTypeID before casting — Swift 6 rejects as? CTFont ("conditional downcast to CoreFoundation type will always succeed") so a conditional cast is not an option here.

Test strategy

TextMobjectTests.swift (CoreText-gated): glyph outlines are non-empty and closed, em sizing, linear scaling between font sizes, visual centering, whitespace advances, align/morph round-trip against a shape, .create on text, and the temporary draw-in outline (createShowsTemporaryOutlineForStrokelessText).

Concurrency

CoreText objects never escape the factory — everything returned is the existing Sendable value types. No @unchecked Sendable.

Stack

Independent — based on main.

claude added 2 commits July 7, 2026 01:49
Mobject.text(_:fontName:fontSize:at:color:) lays out a single line with
CoreText and converts each glyph's outline (CGPath elements, with
quadratics elevated to cubics and closing edges made explicit) into
ordinary Bezier subpaths of one mobject. Text therefore draws in with
create, morphs with transform, and styles like any other shape.

Sizing contract: a font size of 48 spans one scene unit per em, so
default text lands a bit under one unit tall, matching Manim's
proportions. Text is filled (no stroke) and recentered on its visual
bounding box so rotation and scaling act about the text center.

The file is fenced behind #if canImport(CoreText): Apple platforms get
text, the Linux build and its CI job are untouched. Tests (also gated)
cover outline extraction, em sizing, linear font-size scaling,
centering, whitespace advances, alignment/morph interop, and create.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YGb3ukB5dQy4vgz7Z1hags
Mobject.boundingBox and Mobject.center are introduced by the groups
branch and don't exist on main, which this branch is based on. Use
worldPath.boundingBox() / worldPath.boundingBoxCenter instead so the
branch stays independent of the groups PR.

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces text mobjects to the PicoManim library by extracting glyph outlines using CoreText and storing them as Bézier subpaths, enabling text to be drawn, morphed, and styled like other mobjects. Comprehensive unit tests are also added to verify layout, scaling, and animations. The review feedback suggests improving safety by avoiding force-casting when extracting font attributes from CTRun, and enhancing maintainability by replacing magic numbers with the defined pointSize constant in the scaling calculation.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +82 to +84
let attributes = CTRunGetAttributes(run) as? [NSAttributedString.Key: Any]
let runFont = attributes?[NSAttributedString.Key(kCTFontAttributeName as String)]
.map { $0 as! CTFont } ?? font

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Casting the run attributes to [NSAttributedString.Key: Any] and then force-casting the font with as! is verbose and potentially unsafe. We can cast to [AnyHashable: Any] and safely cast the font using as? CTFont with the CoreText key kCTFontAttributeName directly.

Suggested change
let attributes = CTRunGetAttributes(run) as? [NSAttributedString.Key: Any]
let runFont = attributes?[NSAttributedString.Key(kCTFontAttributeName as String)]
.map { $0 as! CTFont } ?? font
let attributes = CTRunGetAttributes(run) as? [AnyHashable: Any]
let runFont = attributes?[kCTFontAttributeName] as? CTFont ?? font

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Partially fixed in 6d83c0a + a16396c. The force cast is gone, but the suggestion as written does not compile: Swift 6 rejects as? CTFont outright — the macOS CI job failed with "error: conditional downcast to CoreFoundation type 'CTFont' will always succeed" (run for 6d83c0a, which tried the conditional-cast form). a16396c replaces it with a CFGetTypeID(value as CFTypeRef) == CTFontGetTypeID() check followed by the (now type-verified) unconditional cast, with a source comment explaining why the conditional cast can't be used.

(Source: comment r3533121355 on PR #13 — the as? CTFont form is a compile error in Swift 6, so please don't re-suggest it.)


Generated by Claude Code

Comment on lines +56 to +57
let pointSize: CGFloat = 48
let unitsPerPoint = (fontSize / 48) / 48

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Avoid duplicating the magic number 48 for the font size and scaling. Using the defined pointSize constant makes the scaling formula more maintainable and self-documenting.

Suggested change
let pointSize: CGFloat = 48
let unitsPerPoint = (fontSize / 48) / 48
let pointSize: CGFloat = 48
let unitsPerPoint = (fontSize / Double(pointSize)) / Double(pointSize)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6d83c0a — the scale formula now uses the pointSize constant ((fontSize / Double(pointSize)) / Double(pointSize)) instead of repeating the literal.


Generated by Claude Code

@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: f699af7cba

ℹ️ 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 +38 to +39
strokeColor: color.withOpacity(0),
strokeWidth: 0,

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 Make text visible during create draw-in

When callers animate default text with .create, the first half of the animation only advances strokeEnd before the fill fade starts; checked ManimScene.apply's .create case, and it doesn't reveal fill until progress exceeds 0.5. Because text is initialized here with both zero stroke alpha and zero stroke width, Mobject.text("Hi") remains completely invisible during that draw-in phase and then just fades in, so the new text mobject does not actually draw in like the other path mobjects.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6d83c0aManimScene.apply's .create case now borrows the fill color as a temporary outline whenever the target has no visible stroke and a visible fill (Manim's Write behavior): the borrowed stroke is fully opaque during the draw-in phase, fades out exactly as the fill fades in, and is gone at progress 1 so the end state matches the target bit-for-bit. This benefits any stroke-less filled mobject, not just text. Test added: createShowsTemporaryOutlineForStrokelessText (visible stroke at t = 0.25, no stroke and full fill at the end).


Generated by Claude Code

) -> BezierPath {
// Lay out in font points, then scale to scene units: 48 pt = 1 unit.
let pointSize: CGFloat = 48
let unitsPerPoint = (fontSize / 48) / 48

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 Scale fontSize as scene units

When callers use the advertised API where fontSize is the em height in scene units, this applies an extra / 48: Mobject.text("A", fontSize: 1) lays out a 48-point glyph and then scales it to roughly 1/48 of a scene unit instead of one unit, making the documented default/non-default sizing effectively tiny. The fixed CoreText point size should only determine outline extraction; the scene scale needs to be based on fontSize / pointSize.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not a code bug — the PR description was wrong, and I've corrected it. The code and its doc comments consistently implement Manim-style font sizes: fontSize defaults to 48, and 48 → one scene unit per em (unitsPerPoint = (fontSize / 48) / 48), so Mobject.text("A") is about one unit tall, matching Manim's proportions. The "fontSize is the em height in scene units / default 1" wording in the PR description was a drafting error on my side; the description now states the Manim-style contract the code actually implements. The tests (defaultSizeSpansAboutOneUnitPerEm, fontSizeScalesLinearly) pin that behavior.

(Source: comment r3533141965 on PR #13 — resolved by fixing the PR description, not the code.)


Generated by Claude Code

claude added 2 commits July 7, 2026 02:20
… cast

- create now borrows the fill color as a temporary outline while drawing
  in a stroke-less filled mobject (like text), fading it out as the fill
  arrives, so Mobject.text draws in visibly instead of staying invisible
  for the first half (Codex). Pinned by
  createShowsTemporaryOutlineForStrokelessText.
- Replace the force-cast when reading the run font with a conditional
  cast (Gemini).
- Use the pointSize constant instead of repeating 48 in the scale
  formula (Gemini).
Swift 6 rejects a conditional downcast to a CoreFoundation type
(error: conditional downcast to CoreFoundation type 'CTFont' will
always succeed), which broke the previous commit's cast on the macOS
CI job. Type-check with CFGetTypeID before the unconditional cast.

Copy link
Copy Markdown
Contributor Author

@codex review


Generated by Claude Code

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: a16396c562

ℹ️ 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".

Copy link
Copy Markdown
Contributor Author

Ready for human review/merge

Phase 2-C (text mobjects via CoreText outlines) is merge-ready.

  • Focused diff — CoreText glyph-outline extraction (Mobject.text / BezierPath.text) plus the Write-style temporary outline for stroke-less create, all gated behind #if canImport(CoreText).
  • CI greentest-macos and test-linux both pass on the head commit (a16396c); the platform gate keeps the Linux build clean while the text tests run on macOS.
  • Tests — glyph outlines, em sizing, linear scaling, visual centering, whitespace advances, align/morph round-trip, .create, and createShowsTemporaryOutlineForStrokelessText.
  • Automated review resolved — Codex reviewed the head commit clean; the earlier Gemini/Codex comments were each answered (draw-in visibility fixed, pointSize constant, font cast reworked to a CFGetTypeID check after the as? CTFont form failed to compile, sizing clarified in the description).
  • PR description updated to state the Manim-style sizing contract the code implements.

Independent — based on main. Not merging; awaiting your go-ahead.


Generated by Claude Code

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