Skip to content

Migrate Text Editing to rust backend#4316

Draft
Annonnymmousss wants to merge 7 commits into
GraphiteEditor:masterfrom
Annonnymmousss:feat/migrate_text_tool
Draft

Migrate Text Editing to rust backend#4316
Annonnymmousss wants to merge 7 commits into
GraphiteEditor:masterfrom
Annonnymmousss:feat/migrate_text_tool

Conversation

@Annonnymmousss

Copy link
Copy Markdown
Contributor

No description provided.

@Annonnymmousss

Copy link
Copy Markdown
Contributor Author

@cubic-dev

@cubic-dev-ai

cubic-dev-ai Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev

@Annonnymmousss I have started the AI code review. It will take a few minutes to complete.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the text tool to handle text editing, cursor movement, selection, and history (undo/redo) directly in the Rust backend instead of relying on a frontend HTML text input overlay. It introduces double-click and triple-click support, cursor blinking, and text selection rendering using the Parley layout library. The review feedback highlights critical areas for improvement, including supporting internationalization (IMEs) by avoiding direct keydown-to-text mapping, handling non-invertible transforms to prevent NaN-induced panics, and safely accessing the global window object to avoid crashes in non-browser environments.

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.

Comment thread frontend/src/utility-functions/input.ts Outdated
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs Outdated
Comment thread node-graph/nodes/text/src/text_context.rs

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

11 issues found across 17 files

Confidence score: 2/5

  • In frontend/src/utility-functions/input.ts, the new keydown path calls preventDefault() but only forwards simple unmodified printable keys, so arrows/delete/shortcuts and IME-like flows can be dropped and text editing can feel broken as-is — only suppress native handling when a full replacement path exists, or forward the missing key classes before merging.
  • In editor/src/messages/input_mapper/input_mappings.rs, the new mappings are inconsistent (document-level vertical selection modifiers don’t mirror non-selection behavior, and Accel+Backspace maps to full-line delete on non-macOS), which can cause unexpected or destructive editing actions for users — align selection/non-selection modifier mappings and restore platform-expected word-delete behavior before merge.
  • In editor/src/messages/tool/tool_messages/text_tool.rs, DoubleClick starts a transaction that is never ended and CommitText can silently exit when get_text_id is None, risking stuck transaction state and silent text-loss/confusing UX — ensure every edit exit path emits EndTransaction and report/recover when the text layer disappears.
  • In editor/src/messages/tool/tool_messages/text_tool.rs and node-graph/nodes/text/src/text_context.rs, non-invertible transforms can propagate NaN positions that bypass clamping, which can lead to invalid hit-test/cursor coordinates and unstable text interactions — add finite-value guards and early fallback behavior when matrix inversion or coordinate validity fails.

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread frontend/src/utility-functions/input.ts Outdated
Comment thread editor/src/messages/input_mapper/input_mappings.rs
Comment thread Cargo.toml
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs Outdated
Comment thread frontend/src/managers/input.ts
Comment thread node-graph/nodes/text/src/text_context.rs
Comment thread editor/src/messages/input_mapper/input_mappings.rs Outdated
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs Outdated
Comment thread editor/src/messages/tool/tool_messages/text_tool.rs
@Annonnymmousss Annonnymmousss changed the title Migrate Text Editing to rust backend based from browser based Migrate Text Editing to rust backend Jul 7, 2026
}

fn spawn_blink_timer(tick: u64, responses: &mut VecDeque<Message>) {
responses.add(FutureMessage::Await {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

you can do this

responses.add(async {});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh okayy. Thanks.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@TrueDoctor We could likely add a general api for doing this, remember when we talked about that when working on the future message handler? This is also kinda bad for tests. And in general a foot gun that easily leads to race conditions (kinda funny in a serial message passing systems).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks like a very wrong approach. I would prefer if we hooked this into animation frame message loop but this can't stay as is

@0HyperCube

0HyperCube commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

A few minor regressions I noted:

  • 1 Compose key does not work. I use this a lot.
  • 2 Pinyin keyboard does not work at all (this has actual users see Simplified Chinese writing support #2868).
  • 3 Graphemes composed of several Unicode codepoints e.g. 'g͆' allow you to position your cursor half way through. This is not possible in any other text editor.
  • 4 When text is clipped vertically, you cannot see what you are typing, nor is there an indication that the text is overflowing (the red line at the bottom only appears if you confirm your change).
  • 5 Double click and drag to select words is very broken.
  • 6 Pressing Ctrl in a word before a line break should position the cursor at the end of the word on the same line. Currently it goes to the start of the word on the new line
  • 7 If Very long text does not forcibly wrap at its max width #4009 is solved, then you will need to support deciding what character the cursor is attached to (cursor affinity). For example the cursor may be positioned after the last character on the first line or before the first character on the second line (both have same index in character array). Currently you only use the downstream affinity.
  • 9 Page up and page down do not alter the text selection like they did before
  • 10 Autoscrolling when typing text off the screen does not work any more.
  • 11 Confirming a new text box by clicking elsewhere on the canvas also makes a new textbox
  • 12 No key repeat support.
  • 13 When editing text, if you click into the text field in the properties panel and type something then the text on the canvas is still in editing mode (clicking elsewhere in canvas reverts the changes).

In regards to IME support, it seems like the typical implementation for web apps is to have an invisible text input element that is positioned at the current cursor position and capture events such as compositionstart. On desktop, you could tell the compositor or winit to make the IME bubble at a particular position. However it is probably easier to reuse the web approach?

@Keavon

Keavon commented Jul 12, 2026

Copy link
Copy Markdown
Member

Thank you for collating that helpful list @0HyperCube! I agree all of these should aim to be fixed, although 1-2 (IME support) will probably have to be deferred since it deserves its own PR for implementing that approach you mentioned. Since the desktop app uses CEF in offscreen rendering mode, it won't be able to know where to place the IME on screen, so dual implementations may be necessary due to that. Also, what is 8 ("Insert mode doesn't work"), are you referring to overtyping with the Insert keyboard key? If that's what you mean, I am not aware of overtyping being functional in modern browsers so this wouldn't be a regression, right?

@Annonnymmousss Annonnymmousss force-pushed the feat/migrate_text_tool branch from f56d87f to f02b849 Compare July 12, 2026 16:19
@Annonnymmousss

Annonnymmousss commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Hii @0HyperCube thanks for the reviews. Point 6 and 9 are working as far I can test or I might be getting the regression wrong, can you please confirm once that what I have attached in the clip is the same what you have mentioned.

Screen.Recording.2026-07-12.at.9.35.03.PM.mov

I have resolved 4,5 and 10. Waiting for your confirmation on point 8 as per Keavon's doubt above. Point 7 is to be discussed after that issue gets resolved. Point 3 is not clear to me, Ill appreciate if you can clarify it a bit.
Thankyou

@0HyperCube

Copy link
Copy Markdown
Contributor

Response to IME criticisms

IME […] implementing that approach you mentioned

The approach I mentioned is basically already implemented as we already have input elements positioned on the canvas in the existing code (before this change). I wouldn't have thought it would be too much work to repurpose this system.

in offscreen rendering mode, it won't be able to know where to place the IME on screen

This is complete nonsense @Keavon. Graphite is in control of rendering and presenting the viewport_texture so it can convert a viewport position to a screen position (it is independent of chromium). Having chromium render to a ui_texture doesn't have any particular bearing on the feasibility of this.

Clarifications on the existing regressions

3 Graphemes composed of several Unicode codepoints e.g. 'g͆' allow you to position your cursor half way through. This is not possible in any other text editor.

Set font to «Noto Sans». Paste the character g͆ into the text box. Use the arrow keys to move the cursor. Note that the cursor can be positioned half way through the character (as seen in video). This is not desired and is not possible in other text editors.

separate_graphmemes.mp4

6 Pressing Ctrl in a word before a line break should position the cursor at the end of the word on the same line. Currently it goes to the start of the word on the new line

@Annonnymmousss to clarify this, say I have the text

hello world
new line

I position my cursor in the middle of the word "world" and then press Ctrl then my cursor ends up at the start of the new line. It other text editors it is at the end of the word world. See video for the current erroneous behaviour:

ctrl+right_end_of_line.mp4

8 overtyping with the Insert keyboard key

My sincere apologies for this error of judgement. My point number 8 was indeed critically flawed. I bear the full responsibility for this grievous oversight. I have removed the point 8 to avoid further confusion.

9 Page up and page down do not alter the text selection like they did before

In your branch, if I press the Page up key, the viewport shifts and nothing happens to the text selection. The previous behaviour was to move the cursor and keep the viewport where it is. I'm not sure what there is to clarify?

Other regressions

Whilst testing this I noticed a few more regressions (added to original list for clarity):

  • Confirming a new text box by clicking elsewhere on the canvas also makes a new textbox
  • No key repeat support.
  • When editing text, if you click into the text field in the properties panel and type something then the text on the canvas is still in editing mode (clicking elsewhere in canvas reverts the changes).

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.

5 participants