Repository navigation
Conversation
|
@TupiC is attempting to deploy a commit to the walterlow's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
 [Medium impact] The change affects animated text rendering in both Canvas and GPU paths. Fix the inline-span handoff before merging; styled letters can still jump when motion ends. FindingsFix with agent prompt### Issue 1
src/runtime/renderer/canvas-item-renderer/text.ts:318-323
With `spanLayout: 'inline'` and differently colored `A` and `V` spans, the new formula places `V` at the kerned width of `AV` minus its own width. Settled text draws each run separately and places `V` at the width of `A`, without that pair’s kerning. These two single-character runs matched before this change, but now `V` shifts when motion ends.
Use the same run boundaries and offsets in both paths, and add a split-span handoff test.
### Issue 2
src/runtime/renderer/canvas-item-renderer/text.ts:315-321
The new loop measures every growing prefix again on each animated frame. A line with `N` characters now sends roughly `N² / 2` characters through text measurement instead of advancing through the line once. The GPU loop repeats the same work, even though the text and font usually stay unchanged throughout the animation.
Cache these positions by text and style to avoid repeating this work during playback and export of longer lines.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.SummaryPreserves kerning by measuring whole strings for GPU layout and growing prefixes for letter positions.
Reviews (1) · Last reviewed commit: "fix(text): preserve kerning across anima..." · Reviewed by Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const currentX = | ||
| originX + | ||
| line.startX + | ||
| ctx.measureText(prefix).width - | ||
| advance + | ||
| charIndex * line.letterSpacing |
There was a problem hiding this comment.
With spanLayout: 'inline' and differently colored A and V spans, the new formula places V at the kerned width of AV minus its own width. Settled text draws each run separately and places V at the width of A, without that pair’s kerning. These two single-character runs matched before this change, but now V shifts when motion ends.
Use the same run boundaries and offsets in both paths, and add a split-span handoff test.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/runtime/renderer/canvas-item-renderer/text.ts
Line: 318-323
Comment:
**Styled letters jump after animation**
With `spanLayout: 'inline'` and differently colored `A` and `V` spans, the new formula places `V` at the kerned width of `AV` minus its own width. Settled text draws each run separately and places `V` at the width of `A`, without that pair’s kerning. These two single-character runs matched before this change, but now `V` shifts when motion ends.
Use the same run boundaries and offsets in both paths, and add a split-span handoff test.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| const advance = ctx.measureText(char).width | ||
| prefix += char | ||
| // Include the preceding pair's kerning, as the settled whole-line draw does. | ||
| const currentX = | ||
| originX + | ||
| line.startX + | ||
| ctx.measureText(prefix).width - |
There was a problem hiding this comment.
The new loop measures every growing prefix again on each animated frame. A line with N characters now sends roughly N² / 2 characters through text measurement instead of advancing through the line once. The GPU loop repeats the same work, even though the text and font usually stay unchanged throughout the animation.
Cache these positions by text and style to avoid repeating this work during playback and export of longer lines.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/runtime/renderer/canvas-item-renderer/text.ts
Line: 315-321
Comment:
**Animated lines repeat text measurements**
The new loop measures every growing prefix again on each animated frame. A line with `N` characters now sends roughly `N² / 2` characters through text measurement instead of advancing through the line once. The GPU loop repeats the same work, even though the text and font usually stay unchanged throughout the animation.
Cache these positions by text and style to avoid repeating this work during playback and export of longer lines.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Needed to prevent letters from shifting closer together when an animation ends.