Text-decoration#4193
Conversation
|
@jsjgdh I have started the AI code review. It will take a few minutes to complete. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for text decorations (underline, overline, and strikethrough) in the text rendering pipeline. It updates TypesettingConfig and the text node to support these properties, implements rendering logic in PathBuilder, and updates the USVG importer to extract decorations and text transforms. Additionally, document migration is added to upgrade existing text nodes. The reviewer suggested a performance optimization in render_decoration_run to return early if no decorations are enabled, avoiding unnecessary font metric lookups.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| pub fn render_decoration_run(&mut self, glyph_run: &GlyphRun<'_, ()>, underline: bool, overline: bool, strikethrough: bool, per_glyph_items: bool) { | ||
| let run = glyph_run.run(); |
There was a problem hiding this comment.
To optimize performance and avoid unnecessary overhead, we can add an early return to render_decoration_run when no text decorations (underline, overline, strikethrough) are enabled. This avoids retrieving font metrics and performing calculations on every glyph run when decorations are not active.
| pub fn render_decoration_run(&mut self, glyph_run: &GlyphRun<'_, ()>, underline: bool, overline: bool, strikethrough: bool, per_glyph_items: bool) { | |
| let run = glyph_run.run(); | |
| pub fn render_decoration_run(&mut self, glyph_run: &GlyphRun<'_, ()>, underline: bool, overline: bool, strikethrough: bool, per_glyph_items: bool) { | |
| if !underline && !overline && !strikethrough { | |
| return; | |
| } | |
| let run = glyph_run.run(); |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 13 files
Confidence score: 4/5
- This PR looks safe to merge from a functionality standpoint; the reported issue is process-related (PR title wording) rather than a code behavior defect.
- The most severe finding targets title style compliance, not runtime logic in
editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, so user-facing regression risk appears low. - Because the issue has high confidence but non-functional impact, it introduces mild merge uncertainty for workflow/policy compliance rather than product stability.
- Pay close attention to
editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs- ensure no actual graph operation behavior changes were missed while addressing the title-enforcement note.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs">
<violation number="1" location="editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs:17">
P2: Custom agent: **PR title enforcement**
PR title is not written in imperative mood with a leading action verb. "Text-decoration" is a noun phrase and does not begin with an action verb as required by the PR title enforcement rule. Suggested: "Add text decoration support for SVG import" or similar imperative form.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| use graph_craft::document::value::TaggedValue; | ||
| use graph_craft::document::{NodeId, NodeInput}; | ||
| use graphene_std::Color; | ||
| use graphene_std::renderer::Quad; |
There was a problem hiding this comment.
P2: Custom agent: PR title enforcement
PR title is not written in imperative mood with a leading action verb. "Text-decoration" is a noun phrase and does not begin with an action verb as required by the PR title enforcement rule. Suggested: "Add text decoration support for SVG import" or similar imperative form.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/graph_operation/graph_operation_message_handler.rs, line 17:
<comment>PR title is not written in imperative mood with a leading action verb. "Text-decoration" is a noun phrase and does not begin with an action verb as required by the PR title enforcement rule. Suggested: "Add text decoration support for SVG import" or similar imperative form.</comment>
<file context>
@@ -14,14 +14,15 @@ use graphene_std::Color;
use graphene_std::renderer::convert_usvg_path::convert_usvg_path;
use graphene_std::table::Table;
-use graphene_std::text::{Font, TypesettingConfig};
+use graphene_std::text::{Font, FontCache, TypesettingConfig};
use graphene_std::vector::style::{Fill, Gradient, GradientSpreadMethod, GradientStop, GradientStops, GradientType, PaintOrder, Stroke, StrokeAlign, StrokeCap, StrokeJoin};
</file context>
There was a problem hiding this comment.
Advice title and description.
1 participant
Text-decoration