Phase 1 (1/5): Package scaffold, CI, and core math layer - #1
Conversation
Bootstrap the PicoManim Swift package (Swift 6.2, macOS 15+/iOS 18+) with the geometry primitives everything else builds on: - Vec2: SIMD2<Double> alias with rotation, lerp, and direction helpers. - Transform2D: scale -> rotate -> translate affine transform kept in decomposed form so animations can interpolate components directly. - ManimColor: RGBA color with the classic Manim palette. - GitHub Actions CI: swift build + swift test on macos-26 and on Linux (swift:6.2 container) to keep the core platform-neutral. The pull_request trigger has no base-branch filter so stacked PRs get CI. Part 1/5 of the Phase 1 stack (shapes -> animation -> ManimView). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGb3ukB5dQy4vgz7Z1hags
There was a problem hiding this comment.
Code Review
This pull request introduces the core math layer for the PicoManim animation library, adding basic structures such as Vec2, Transform2D, and ManimColor along with their corresponding unit tests. The review feedback suggests several improvements to robustness and code reuse: using hypot for safer vector length calculations, clamping color components within the 0...1 range during interpolation to handle overshooting animations, and leveraging the existing lerp helper function in Transform2D to avoid manual interpolation logic.
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.
| public var length: Double { | ||
| (x * x + y * y).squareRoot() | ||
| } |
There was a problem hiding this comment.
Using (x * x + y * y).squareRoot() to calculate the Euclidean length of a vector can lead to overflow or underflow for very large or very small values of x or y. Using hypot(x, y) is more robust as it is specifically designed to avoid intermediate overflow and underflow.
| public var length: Double { | |
| (x * x + y * y).squareRoot() | |
| } | |
| public var length: Double { | |
| hypot(x, y) | |
| } |
There was a problem hiding this comment.
Fixed in d6e9dfa. Vec2.length now uses Foundation.hypot(x, y), avoiding intermediate overflow/underflow. Existing length tests cover the common cases; no behavior change for in-range values. No impact on later PRs in the stack (they consume length unchanged).
Generated by Claude Code
| public static func lerp(_ a: ManimColor, _ b: ManimColor, _ t: Double) -> ManimColor { | ||
| ManimColor( | ||
| red: a.red + (b.red - a.red) * t, | ||
| green: a.green + (b.green - a.green) * t, | ||
| blue: a.blue + (b.blue - a.blue) * t, | ||
| alpha: a.alpha + (b.alpha - a.alpha) * t | ||
| ) | ||
| } |
There was a problem hiding this comment.
The lerp function does not clamp the interpolated color components to the 0...1 range. During animations with overshooting rate functions (such as back or elastic easing), t can go outside 0...1, causing color components to go out of bounds. This violates the documented invariant "components in the 0...1 range" and can cause rendering issues. Using the clamp and lerp helper functions defined in Vec2.swift ensures the components remain valid.
| public static func lerp(_ a: ManimColor, _ b: ManimColor, _ t: Double) -> ManimColor { | |
| ManimColor( | |
| red: a.red + (b.red - a.red) * t, | |
| green: a.green + (b.green - a.green) * t, | |
| blue: a.blue + (b.blue - a.blue) * t, | |
| alpha: a.alpha + (b.alpha - a.alpha) * t | |
| ) | |
| } | |
| public static func lerp(_ a: ManimColor, _ b: ManimColor, _ t: Double) -> ManimColor { | |
| ManimColor( | |
| red: clamp(lerp(a.red, b.red, t), 0...1), | |
| green: clamp(lerp(a.green, b.green, t), 0...1), | |
| blue: clamp(lerp(a.blue, b.blue, t), 0...1), | |
| alpha: clamp(lerp(a.alpha, b.alpha, t), 0...1) | |
| ) | |
| } |
There was a problem hiding this comment.
Fixed in d6e9dfa, with a required correction in 65d2984: the suggestion as written doesn't compile — inside the static lerp member, the unqualified lerp(a.red, ...) call resolves to the enclosing member itself (type scope shadows the module-level scalar helper), which broke CI. The final version module-qualifies the helper (PicoManim.lerp) and clamps every component to 0...1, preserving the documented invariant under overshooting rate functions. A source comment explains the qualification so it doesn't get "simplified" back. Later PRs benefit automatically (PR #4's transform color interpolation goes through this function).
Generated by Claude Code
| public static func lerp(_ a: Transform2D, _ b: Transform2D, _ t: Double) -> Transform2D { | ||
| Transform2D( | ||
| translation: Vec2.lerp(a.translation, b.translation, t), | ||
| rotation: a.rotation + (b.rotation - a.rotation) * t, | ||
| scale: Vec2.lerp(a.scale, b.scale, t) | ||
| ) | ||
| } |
There was a problem hiding this comment.
The lerp function manually computes linear interpolation for the rotation angle, duplicating the logic of the scalar lerp helper function defined in Vec2.swift. Using the helper function improves code reuse and maintainability.
| public static func lerp(_ a: Transform2D, _ b: Transform2D, _ t: Double) -> Transform2D { | |
| Transform2D( | |
| translation: Vec2.lerp(a.translation, b.translation, t), | |
| rotation: a.rotation + (b.rotation - a.rotation) * t, | |
| scale: Vec2.lerp(a.scale, b.scale, t) | |
| ) | |
| } | |
| public static func lerp(_ a: Transform2D, _ b: Transform2D, _ t: Double) -> Transform2D { | |
| Transform2D( | |
| translation: Vec2.lerp(a.translation, b.translation, t), | |
| rotation: lerp(a.rotation, b.rotation, t), | |
| scale: Vec2.lerp(a.scale, b.scale, t) | |
| ) | |
| } |
There was a problem hiding this comment.
Fixed in d6e9dfa. Transform2D.lerp now reuses the shared scalar helper for rotation, module-qualified (PicoManim.lerp) because the unqualified name resolves to the enclosing static member rather than the global helper. No functional change; the existing transformLerpEndpoints test still pins the behavior.
Generated by Claude Code
- Vec2.length uses hypot to avoid intermediate overflow/underflow. - ManimColor.lerp clamps components to 0...1 so overshooting rate functions can't violate the documented component range. - Transform2D.lerp reuses the shared scalar lerp helper. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGb3ukB5dQy4vgz7Z1hags
Inside the static lerp member, unqualified lerp(a.red, ...) resolved to the enclosing member instead of the module-level scalar helper and failed to type-check. Qualify with the module name, matching Transform2D.lerp. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01YGb3ukB5dQy4vgz7Z1hags
|
@codex review Generated by Claude Code |
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Ready for human review/merge. Checklist:
Remaining notes:
Generated by Claude Code |
Summary
Bootstraps the PicoManim Swift package (Swift 6.2, macOS 15+ / iOS 18+) with CI and the geometry primitives every later layer builds on:
Vec2—SIMD2<Double>alias with rotation, lerp, and unit-direction helpers.Transform2D— affine transform kept decomposed (scale → rotate → translate) so animations can interpolate rotation and scale without shearing.ManimColor— RGBA color with the classic Manim palette (.blue,.red,.yellow, …).swift build+swift testonmacos-26and on Linux (swift:6.2container). The Linux job exists to keep the core platform-neutral (SwiftUI code in later PRs is fenced behind#if canImport(SwiftUI)). Thepull_requesttrigger deliberately has no base-branch filter so the stacked PRs in this series get CI too.Stack (Phase 1: shapes + animation + ManimView)
This is PR 1/5, base
main. The stack, in review/merge order:CubicCurve,BezierPath)Mobjectmodel + shape factoriesRateFunction,ManimAnimation,ManimScene)ManimViewSwiftUI preview + demo scene + full docsEach later PR is based on its parent branch; they will be retargeted to
mainas parents merge.Intentionally not included
Paths/shapes, animation, and rendering — they arrive in PRs 2–5 so each diff stays reviewable on its own.
Test strategy
Exact-value math tests (rotation quarter-turn, lerp midpoints, transform composition order, hex color round-trip) — plus this PR proves out the CI pipeline itself for the rest of the stack.
Concurrency
All types are
Sendablevalue types. No@unchecked Sendableanywhere in this stack.🤖 Generated with Claude Code
https://claude.ai/code/session_01YGb3ukB5dQy4vgz7Z1hags
Generated by Claude Code