fix(replay): text layouts with center/end alignment return incorrect masking bounding box#5218
Draft
fix(replay): text layouts with center/end alignment return incorrect masking bounding box#5218
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Internal Changes 🔧
🤖 This preview updates automatically when you update the PR. |
Sentry Build Distribution
|
Contributor
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8687935 | 332.52 ms | 362.23 ms | 29.71 ms |
| 27d7cf8 | 309.43 ms | 364.27 ms | 54.85 ms |
| f064536 | 329.00 ms | 395.62 ms | 66.62 ms |
| abfcc92 | 337.38 ms | 427.39 ms | 90.00 ms |
| ae7fed0 | 293.84 ms | 380.22 ms | 86.38 ms |
| d15471f | 361.89 ms | 378.07 ms | 16.18 ms |
| 1df7eb6 | 397.04 ms | 429.64 ms | 32.60 ms |
| 9fbb112 | 401.87 ms | 515.87 ms | 114.00 ms |
| 6405ec5 | 310.88 ms | 354.56 ms | 43.69 ms |
| 62b579c | 318.48 ms | 367.71 ms | 49.24 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 8687935 | 1.58 MiB | 2.19 MiB | 619.17 KiB |
| 27d7cf8 | 1.58 MiB | 2.12 MiB | 549.42 KiB |
| f064536 | 1.58 MiB | 2.20 MiB | 633.90 KiB |
| abfcc92 | 1.58 MiB | 2.13 MiB | 557.31 KiB |
| ae7fed0 | 1.58 MiB | 2.12 MiB | 551.77 KiB |
| d15471f | 1.58 MiB | 2.13 MiB | 559.54 KiB |
| 1df7eb6 | 1.58 MiB | 2.10 MiB | 532.97 KiB |
| 9fbb112 | 1.58 MiB | 2.11 MiB | 539.18 KiB |
| 6405ec5 | 1.58 MiB | 2.12 MiB | 552.23 KiB |
| 62b579c | 0 B | 0 B | 0 B |
… one line of text" This reverts commit cfdcadc.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fix Compose text masking for layouts where text alignment (
TextAlign.Center,TextAlign.End) or weighted layouts produce incorrect masking bounding boxes.Root Cause
Compose's
MultiParagraphalways usesconstraints.maxWidthas the paragraph width, but the Text composable may size its node to a smaller content width. This causesgetLineLeft/getLineRightto return positions in the paragraph coordinate system that don't match the node's actual bounds, resulting in masking rects placed at wrong positions (e.g. off-screen for end-aligned weighted text).Changes
TextLayoutinterface: replacedgetPrimaryHorizontal,getEllipsisCount,getLineVisibleEnd,getLineStartwithgetLineLeft/getLineRight— directly using the layout's line bounds instead of computing them from character offsetslineWidthstarting from x=0, since text alignment has no visible effect in content-wrapped nodeshasFillModifierworkaround: no longer needed since we usegetLineLeft/getLineRightdirectlyfindTextAttributes()→findTextColor(): only the color is needed now, the Fill modifier detection is removedMotivation and Context
The
Fillmodifier detection was a fragile workaround that didn't cover all cases (e.g. weighted text). The new approach correctly handles all text alignment and sizing scenarios by comparing the paragraph width against the actual node width.How did you test it?
Checklist
sendDefaultPIIis enabled.