-
-
Notifications
You must be signed in to change notification settings - Fork 307
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support text letter spacing #2280
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe recent updates to the Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant TextRenderer
participant TextUtils
Developer->>TextRenderer: Set letterSpacing(value)
TextRenderer-->>TextRenderer: Update _letterSpacing
TextRenderer->>TextUtils: Calculate text layout with letter spacing
TextUtils-->>TextUtils: Adjust lineWidth and wordWidth
TextUtils->>TextRenderer: Render text with updated spacing
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- packages/core/src/2d/text/TextRenderer.ts (5 hunks)
- packages/core/src/2d/text/TextUtils.ts (8 hunks)
Additional context used
Biome
packages/core/src/2d/text/TextUtils.ts
[error] 238-238: Using this in a static context can be confusing.
this refers to the class.
Unsafe fix: Use the class name instead.(lint/complexity/noThisInStatic)
Additional comments not posted (21)
packages/core/src/2d/text/TextUtils.ts (16)
112-112
: LGTM!The introduction of the
letterSpacing
variable is consistent with the rest of the code.
159-159
: LGTM!The adjustment to subtract
letterSpacing
fromlineWidth
ensures that the rendered text respects the specified spacing between letters.
183-183
: LGTM!The integration of
letterSpacing
into the line width calculation is consistent with the logic of handling letter spacing uniformly.
192-192
: LGTM!Adding
letterSpacing
tocharInfo.xAdvance
ensures that the spacing is uniformly applied throughout the text rendering process.
198-198
: LGTM!Adding
letterSpacing
tocharInfo.xAdvance
in the else block ensures that the spacing is uniformly applied throughout the text rendering process.
205-205
: LGTM!The adjustment to subtract
letterSpacing
fromlineWidth
ensures that the rendered text respects the specified spacing between letters.
220-220
: LGTM!Adding
letterSpacing
tocharInfo.xAdvance
in the word handling logic ensures that the spacing is uniformly applied throughout the text rendering process.
225-225
: LGTM!Adding
letterSpacing
tocharInfo.xAdvance
in the else block of the word handling logic ensures that the spacing is uniformly applied throughout the text rendering process.
237-237
: LGTM!The adjustment to subtract
letterSpacing
fromlineWidth
ensures that the rendered text respects the specified spacing between letters.
245-245
: LGTM!The adjustment to subtract
letterSpacing
fromwordWidth
ensures that the rendered text respects the specified spacing between letters.
259-259
: LGTM!The adjustment to subtract
letterSpacing
fromlineWidth
ensures that the rendered text respects the specified spacing between letters.
291-291
: LGTM!The introduction of the
letterSpacing
variable is consistent with the rest of the code.
297-298
: LGTM!Adding
letterSpacing
tocurWidth
ensures that the spacing is uniformly applied throughout the text rendering process.
302-302
: LGTM!The integration of
letterSpacing
into the line width calculation is consistent with the logic of handling letter spacing uniformly.
Line range hint
573-577
:
LGTM!The integration of
letterSpacing
into the local data update logic ensures that the spacing is uniformly applied throughout the text rendering process.
Line range hint
639-639
:
LGTM!Adding
letterSpacing
tocharInfo.xAdvance
in the local data update logic ensures that the spacing is uniformly applied throughout the text rendering process.packages/core/src/2d/text/TextRenderer.ts (5)
64-64
: LGTM!The introduction of the
_letterSpacing
variable is straightforward and consistent with the rest of the class properties.
176-176
: LGTM!The removal of the reference to "pixels" in the documentation comment for
lineSpacing
is appropriate and makes the comment more concise.
189-191
: LGTM!The new documentation comment for
letterSpacing
is clear and concise, effectively explaining its purpose.
192-201
: LGTM!The getter and setter methods for
letterSpacing
are implemented correctly. The setter's logic to mark the renderer as "dirty" ensures that any necessary updates are triggered in the rendering process.
573-577
: LGTM!The integration of
letterSpacing
into the local data update logic ensures that the spacing is uniformly applied throughout the text rendering process.
Summary by CodeRabbit
New Features
Documentation