Extract Lottie text layout module and align rendering with Lottie spec

.github/skills/code-review/SKILL.md

@@ -74,17 +74,18 @@ Record the output of this pass in your response, not just in internal reasoning.
 For the branch as a whole, and then for each non-trivial new or changed function:
 
 1. **Identify the stated intent.** Read the commit messages, PR description, any task/issue reference, and the function's name and doc comment. Write down in one sentence what the code claims to do. If no commit message, PR description, or doc comment explains the intent, derive it from the function name and surrounding call sites, and **flag the missing context as a Warning** — a reviewer should not have to guess what the code is for.
-2. **Enumerate representative inputs.** List concrete input shapes the function must handle. Typical shapes to consider: empty, single element, two elements, boundary values at each end of a range, the symmetric counterpart of the obvious case (if the author handled A, did they handle not-A?), and any input that would take the code through a different branch or early return.
+2. **Enumerate representative inputs.** List concrete input shapes the function must handle. Typical shapes to consider: empty, single element, two elements, boundary values at each end of a range, the symmetric counterpart of the obvious case (if the author handled A, did they handle not-A?), and any input that would take the code through a different branch or early return. For union-typed inputs (e.g. `number[] | string`), walk through one concrete value per variant so every branch sees a realistic value — a single input that happens to satisfy a shared guard (like `.length`) will hide bugs where the guard means different things across variants.
 3. **Trace the output for each input.** Walk each input through the implementation and compare the result against the stated intent. If any input produces a result that doesn't match the intent, that is a Critical or Warning issue — even if the code compiles, lints, and every existing test passes.
 4. **Check test coverage against the enumerated inputs.** If a particular input shape matters to the stated intent and no test exercises it, flag that as a Warning. Passing tests only prove the cases the author thought to test.
 5. **For every branch, cache, or shortcut: state the precondition.** When the code takes a fast path, reads a cached value, or returns early for a subset of inputs, write down the precondition under which that path produces the same result as the general path, then check the surrounding code actually guarantees it. Common failure shapes: a cached value computed under different assumptions than when it is read; a memoization keyed on a subset of the real inputs; a length-based shortcut that skips a step the general path would have applied.
+6. **For every field or variable the PR assigns a new value to: re-read its declaration and doc comment.** If the new value no longer fits the declared name or documented semantics, that is a Warning — either rename the field, update the doc comment, or remove the field if nothing consumes it. Doc-comment drift is one of the most common refactor regressions, and values with no downstream readers are especially prone to it because nothing else forces them back into agreement.
 
 ### Step 4: Mechanical checklist
 
 Apply each item to every changed line that is not excluded under Step 2.
 
 1. **Repository conventions.** Apply every rule from the instruction files in [instructions/index.md](../../instructions/index.md) that matches the changed files — coding conventions, prohibited APIs, performance rules for render-loop code, side-effect imports, backward-compatibility rules, documentation standards, test patterns, and any domain-specific rules (Inspector, glTF extensions, playgrounds, etc.). If an instruction file's content is already in your system prompt context, apply it directly; only read from disk when it is not.
-2. **Correctness.** Logic errors, off-by-one, null/undefined access, race conditions, unhandled edge cases, incorrect operator precedence, wrong loop bounds. Verify that doc comments accurately describe the implementation's actual behavior.
+2. **Correctness.** Logic errors, off-by-one, null/undefined access, race conditions, unhandled edge cases, incorrect operator precedence, wrong loop bounds. Verify that doc comments accurately describe the implementation's actual behavior. When a refactor changes the value assigned to a named field or variable, verify its name and doc comment still describe the new value — renames are the most common regression vector in data-shape refactors.
 3. **Error handling.** When code detects an error or invalid state (exceeding limits, missing data, unsupported configuration), it must handle it appropriately — bail out, fall back to a safe alternative, or properly resolve the condition. Flag cases that merely log a warning or swallow the error while continuing as if nothing happened.
 4. **Security.** Prototype pollution, unsafe `eval` / `Function()`, unsafe deserialization of untrusted input (e.g. parsed scene files, glTF extensions, user-supplied JSON).
 5. **General quality.** Dead code, unreachable branches, duplicated logic, overly complex control flow, poor or misleading naming.
@@ -116,9 +117,9 @@ Capture any failures and include them as issues in the review. If a command does
 
 Compile every issue found into a single markdown table, sorted by severity (Critical → Warning → Nit). This is the table you will present in Step 8, so record issues in their final format now.
 
-| #   | File                             | Line(s) | Severity | Issue                                                 | Fix Applied                                          |
-| --- | -------------------------------- | ------- | -------- | ----------------------------------------------------- | ---------------------------------------------------- |
-| 1   | [path/file.ts](path/file.ts#L42) | 42      | Critical | Clear description of the problem and how to fix it    | Filled in during Step 7 ("Skipped" / "N/A" allowed)  |
+| #   | File                             | Line(s) | Severity | Issue                                              | Fix Applied                                         |
+| --- | -------------------------------- | ------- | -------- | -------------------------------------------------- | --------------------------------------------------- |
+| 1   | [path/file.ts](path/file.ts#L42) | 42      | Critical | Clear description of the problem and how to fix it | Filled in during Step 7 ("Skipped" / "N/A" allowed) |
 
 File paths should be markdown links. The **Fix Applied** column is left blank in Step 6 and filled in during Step 7 as each issue is resolved (or marked `Skipped` / `Needs confirmation` / `N/A`).
 

packages/dev/lottiePlayer/src/maths/boundingBox.ts

@@ -1,14 +1,6 @@
-import {
-    type RawElement,
-    type RawFont,
-    type RawEllipseShape,
-    type RawPathShape,
-    type RawRectangleShape,
-    type RawStrokeShape,
-    type RawTextData,
-    type RawTextDocument,
-} from "../parsing/rawTypes";
+import { type RawElement, type RawFont, type RawEllipseShape, type RawPathShape, type RawRectangleShape, type RawStrokeShape, type RawTextData } from "../parsing/rawTypes";
 import { GetInitialVectorValues, GetInitialBezierData } from "../parsing/rawPropertyHelpers";
+import { ApplyLottieTextContext, MeasureLottieText, ResolveLottieText } from "../parsing/textLayout";
 
 /**
  * Represents a bounding box for a shape in the animation.
@@ -28,10 +20,10 @@ export type BoundingBox = {
     offsetY: number;
     /** Inset for the stroke, if applicable. */
     strokeInset: number;
-    /** Optional: Canvas2D text metrics for precise vertical alignment */
-    actualBoundingBoxAscent?: number;
-    /** Optional: Canvas2D text metrics for precise vertical alignment */
-    actualBoundingBoxDescent?: number;
+    /** Optional: Distance from the top of the text texture to the first baseline. Only populated for text bounding boxes. */
+    baselineOffsetY?: number;
+    /** Optional: Descent (in pixels) of the last text line below its baseline. Only populated for text bounding boxes. */
+    descent?: number;
 };
 
 // Corners of the bounding box
@@ -114,55 +106,29 @@ export function GetTextBoundingBox(
     variables: Map<string, string>
 ): BoundingBox | undefined {
     spritesCanvasContext.save();
-    let textInfo: RawTextDocument | undefined = undefined;
-    if (textData.d && textData.d.k && textData.d.k.length > 0) {
-        textInfo = textData.d.k[0].s as RawTextDocument;
-    }
-
-    if (!textInfo) {
-        spritesCanvasContext.restore();
-        return undefined;
-    }
 
-    const fontSize = textInfo.s;
-    const fontFamily = textInfo.f;
-    const finalFont = rawFonts.get(fontFamily);
-    if (!finalFont) {
+    const resolvedText = ResolveLottieText(textData, rawFonts, variables);
+    if (!resolvedText) {
         spritesCanvasContext.restore();
         return undefined;
     }
 
-    const weight = finalFont.fWeight || "400"; // Default to normal weight if not specified
-    spritesCanvasContext.font = `${weight} ${fontSize}px ${finalFont.fFamily}`;
-
-    if (textInfo.sc !== undefined && textInfo.sc.length >= 3 && textInfo.sw !== undefined && textInfo.sw > 0) {
-        spritesCanvasContext.lineWidth = textInfo.sw;
-    }
-
-    // Text is supported as a possible variable (for localization for example)
-    // Check if the text is a variable and replace it if it is
-    let text = textInfo.t;
-    const variableText = variables.get(text);
-    if (variableText !== undefined) {
-        text = variableText;
-    }
-    const metrics = spritesCanvasContext.measureText(text);
+    ApplyLottieTextContext(spritesCanvasContext, resolvedText);
 
-    const widthPx = Math.ceil(metrics.width);
-    const heightPx = Math.ceil(metrics.actualBoundingBoxAscent) + Math.ceil(metrics.actualBoundingBoxDescent);
+    const layout = MeasureLottieText(resolvedText, (text) => spritesCanvasContext.measureText(text));
 
     spritesCanvasContext.restore();
 
     return {
-        width: widthPx,
-        height: heightPx,
-        centerX: widthPx / 2,
-        centerY: heightPx / 2,
-        offsetX: 0, // The bounding box calculated by the canvas for the text is always centered in (0, 0)
-        offsetY: 0, // The bounding box calculated by the canvas for the text is always centered in (0, 0)
+        width: layout.width,
+        height: layout.height,
+        centerX: layout.width / 2,
+        centerY: layout.height / 2,
+        offsetX: layout.offsetX,
+        offsetY: layout.offsetY,
         strokeInset: 0, // Text bounding box ignores stroke padding here
-        actualBoundingBoxAscent: metrics.actualBoundingBoxAscent,
-        actualBoundingBoxDescent: metrics.actualBoundingBoxDescent,
+        baselineOffsetY: layout.baselineOffsetY,
+        descent: layout.descent,
     };
 }
 

packages/dev/lottiePlayer/src/parsing/rawTypes.ts

@@ -257,6 +257,8 @@ export type RawTextDocumentKeyframe = {
 };
 
 export type RawTextDocument = {
+    sz?: number[]; // Paragraph box size [width, height]
+    ps?: number[]; // Paragraph box top-left position [x, y] relative to the text layer origin
     f: string; // Font family
     s: number; // Font size
     lh: number; // Line height

packages/dev/lottiePlayer/src/parsing/spritePacker.ts

@@ -15,9 +15,9 @@ import {
     type RawRectangleShape,
     type RawStrokeShape,
     type RawTextData,
-    type RawTextDocument,
 } from "./rawTypes";
 import { GetInitialScalarValue, GetInitialVectorValues, GetInitialBezierData } from "./rawPropertyHelpers";
+import { ApplyLottieTextContext, DrawLottieText, MeasureLottieText, ResolveLottieText } from "./textLayout";
 
 import { type BoundingBox, GetShapesBoundingBox, GetTextBoundingBox } from "../maths/boundingBox";
 
@@ -243,7 +243,7 @@ export class SpritePacker {
         const page = this._getPageWithRoom(this._spriteAtlasInfo.cellWidth, this._spriteAtlasInfo.cellHeight);
 
         // Draw the text in the canvas
-        this._drawText(textData, boundingBox, scalingFactor, page);
+        this._drawText(textData, scalingFactor, page);
         this._extrudeSpriteEdges(page, page.currentX, page.currentY, this._spriteAtlasInfo.cellWidth, this._spriteAtlasInfo.cellHeight);
         page.isDirty = true;
 
@@ -473,66 +473,51 @@ export class SpritePacker {
         page.context.restore();
     }
 
-    private _drawText(textData: RawTextData, boundingBox: BoundingBox, scalingFactor: IVector2Like, page: AtlasPage): void {
+    private _drawText(textData: RawTextData, scalingFactor: IVector2Like, page: AtlasPage): void {
         if (this._rawFonts === undefined) {
             return;
         }
 
-        const textInfo = textData.d.k[0].s as RawTextDocument;
-
-        const fontFamily = textInfo.f;
-        const finalFont = this._rawFonts.get(fontFamily);
-        if (!finalFont) {
+        const resolvedText = ResolveLottieText(textData, this._rawFonts, this._variables);
+        if (!resolvedText) {
             return;
         }
 
         page.context.save();
         page.context.translate(page.currentX, page.currentY);
         page.context.scale(scalingFactor.x, scalingFactor.y);
 
-        // Set up font (same setup as GetTextBoundingBox for measurement consistency)
-        const weight = finalFont.fWeight || "400";
-        page.context.font = `${weight} ${textInfo.s}px ${finalFont.fFamily}`;
-
-        if (textInfo.sc !== undefined && textInfo.sc.length >= 3 && textInfo.sw !== undefined && textInfo.sw > 0) {
-            page.context.lineWidth = textInfo.sw;
-        }
-
-        // Clip to cell bounds to prevent text overdraw into adjacent cells
-        page.context.beginPath();
-        page.context.rect(0, 0, boundingBox.width, boundingBox.height);
-        page.context.clip();
-
-        if (textInfo.fc !== undefined && textInfo.fc.length >= 3) {
-            const rawFillStyle = textInfo.fc;
+        // Resolve fill color. fc is either an RGB array or a variable name string; the two shapes need different guards
+        // (arrays need at least 3 components; strings just need a non-undefined variable lookup).
+        if (resolvedText.textInfo.fc !== undefined) {
+            const rawFillStyle = resolvedText.textInfo.fc;
             if (Array.isArray(rawFillStyle)) {
-                // If the fill style is an array, we assume it's a color array
-                page.context.fillStyle = this._lottieColorToCSSColor(rawFillStyle, 1);
+                if (rawFillStyle.length >= 3) {
+                    page.context.fillStyle = this._lottieColorToCSSColor(rawFillStyle, 1);
+                }
             } else {
-                // If it's a string, we need to get the value from the variables map
                 const variableFillStyle = this._variables.get(rawFillStyle);
                 if (variableFillStyle !== undefined) {
                     page.context.fillStyle = variableFillStyle;
                 }
             }
         }
 
-        if (textInfo.sc !== undefined && textInfo.sc.length >= 3 && textInfo.sw !== undefined && textInfo.sw > 0) {
-            page.context.strokeStyle = this._lottieColorToCSSColor(textInfo.sc, 1);
+        if (resolvedText.hasStroke) {
+            // ResolveLottieText only sets hasStroke when sc is present and well-formed, so the non-null assertion here is safe.
+            page.context.strokeStyle = this._lottieColorToCSSColor(resolvedText.textInfo.sc!, 1);
         }
 
-        // Text is supported as a possible variable (for localization for example)
-        // Check if the text is a variable and replace it if it is
-        let text = textInfo.t;
-        const variableText = this._variables.get(text);
-        if (variableText !== undefined) {
-            text = variableText;
-        }
+        ApplyLottieTextContext(page.context, resolvedText);
 
-        page.context.fillText(text, 0, boundingBox.actualBoundingBoxAscent!);
-        if (textInfo.sc !== undefined && textInfo.sc.length >= 3 && textInfo.sw !== undefined && textInfo.sw > 0 && textInfo.of === true) {
-            page.context.strokeText(text, 0, boundingBox.actualBoundingBoxAscent!);
-        }
+        const layout = MeasureLottieText(resolvedText, (text) => page.context.measureText(text));
+
+        // Clip to cell bounds to prevent text overdraw into adjacent cells
+        page.context.beginPath();
+        page.context.rect(0, 0, layout.width, layout.height);
+        page.context.clip();
+
+        DrawLottieText(page.context, resolvedText, layout);
 
         page.context.restore();
     }