Skip to content

Conversation

@yisibl
Copy link
Collaborator

@yisibl yisibl commented Dec 20, 2025

The direction algorithm has been rewritten, most scenarios now align with the browser.

  • Added more comprehensive test cases for direction
  • Also fixed the bug where measureText.width returned different values in LRT and RTL layouts
    The solution is to use getMaxIntrinsicWidth instead of getRectsForRange, which also avoids manually iterating through glyph widths.

Part of #1172


Note

Reworks text direction layout to fix RTL x calculation and unify measureText.width across directions, and adds comprehensive direction/letter-spacing tests.

  • Core text layout (skia-c/skia_c.cpp)
    • Use paragraph->getMaxIntrinsicWidth() for line_width (direction-independent, includes trailing spaces) instead of summing getRectsForRange().
    • Refactor paint_x/alignment logic: separate rtl_offset and letter_spacing_offset; compute center/right/start/end consistently; correct scaling interaction with maxWidth.
    • Adjust metrics outputs (left, right) to include letter-spacing offset; minor float literal and cleanup tweaks.
  • Tests (__test__/text.spec.ts)
    • Expand direction coverage: alignment with/without maxWidth, save/restore, stroke with letterSpacing, negative letterSpacing, MDN example updates.
    • Add direction-measure-text ensuring measureText.width matches in LTR/RTL; refactor alignment test into drawDirectionAlignTest; tweak canvas sizes/fonts where needed.

Written by Cursor Bugbot for commit 6ff4761. This will update automatically on new commits. Configure here.

@yisibl yisibl force-pushed the fix-rtl-maxWidth branch 2 times, most recently from 477120b to a29fbe6 Compare December 20, 2025 10:33
@yisibl yisibl changed the title fix: LTR text fillText(maxWidth) x positioning calculation error fix: RTL text fillText(maxWidth) x positioning calculation error Dec 20, 2025
The `direction` algorithm has been rewritten, most scenarios now align with the browser.

- Added more comprehensive test cases for `direction`
- Also fixed the bug where `measureText.width` returned different values in LRT and RTL layouts
  The solution is to use `getMaxIntrinsicWidth` instead of `getRectsForRange`, which also avoids manually iterating through glyph widths.

Part of #1172
@Brooooooklyn
Copy link
Owner

cursor review

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no bugs!

@Brooooooklyn Brooooooklyn enabled auto-merge (squash) December 21, 2025 14:34
@Brooooooklyn Brooooooklyn merged commit 4db0dac into main Dec 21, 2025
40 checks passed
@Brooooooklyn Brooooooklyn deleted the fix-rtl-maxWidth branch December 21, 2025 14:50
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.

3 participants