fix: text outline should not draw when outline width is 0#1361
Conversation
📝 WalkthroughWalkthroughModified the outline rendering condition in the text item plugin to include an explicit check for outline width greater than zero, preventing strokeText operations when outline width is zero or negative. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/effects-core/src/plugins/text/text-item.ts (1)
406-408: Consider adding the width check here for consistency.For consistency with the fix at line 456 and the pattern at line 510, you could add the
outlineWidth > 0check before callingsetupOutline():-if (style.isOutlined) { +if (style.isOutlined && style.outlineWidth > 0) { this.setupOutline(); }This avoids setting up outline parameters when no outline will be drawn.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/effects-core/src/plugins/text/text-item.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-09T08:44:15.153Z
Learnt from: Fryt1
Repo: galacean/effects-runtime PR: 1305
File: packages/effects-core/src/fallback/migration.ts:325-330
Timestamp: 2025-12-09T08:44:15.153Z
Learning: In `packages/effects-core/src/fallback/migration.ts`, the `textColor` 0-255 to 0-1 conversion in `version35Migration` is intentionally applied only to `TextComponent` and not to `RichTextComponent`, as confirmed by Fryt1.
Applied to files:
packages/effects-core/src/plugins/text/text-item.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/effects-core/src/plugins/text/text-item.ts (1)
456-456: LGTM! Consistent fix for preventing zero-width outlines.The added condition correctly prevents
strokeTextoperations when outline width is 0, which aligns with the same pattern used ingetEffectPadding()at line 510.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.